Repository navigation
tests: wait for nodes to reach the chain tip after mining - #1163
randomlogin wants to merge 1 commit into
Conversation
This is a prerequisite PR for compact block filter node. Contrary to all other chain sources, CBF chain source in tests is different from the node who mines blocks. That means we need to ensure the tx from chain source has been broadcast. In the same fashion we ensure that after the block has mined we received it from the mining node. Tests assumed that a node observes a freshly mined block as soon as `sync_wallets` returns. Make `generate_blocks_and_wait` and `premine_and_distribute_funds` return the new chain height, add `wait_for_node_tip` to poll until a node reports at least that height (syncing on every try, for pull-based sources), and use it after mining in the tests. Other changes: - Let `exponential_backoff_poll` return `None` instead of panicking when it runs out of tries, and `expect` at the call sites. - Return a `Result` from the on-chain tx type assertions, so a failure is reported at the call site. - Wait for the sweep transaction to reach the mempool before mining it, and wait for the transaction before mining in `unified_send_receive_bip21_uri`, so the blocks are guaranteed to include it. - In `lsps2_client_trusts_lsp`, wait for the client's tip, which the assertions inspect, rather than the payer's. Co-authored-by: febyeji <yeji.han@sf.snu.ac.kr>
|
I've assigned @tnull as a reviewer! |
tnull
left a comment
tnull
left a comment
There was a problem hiding this comment.
Tbh. a bit confused why we need these changes, can you expand?
CBF chain source in tests is different from the node who mines blocks
How is 'different'? Why would we ever be synced to a new tip before we synced..?
| let new_height = | ||
| generate_blocks_and_wait(bitcoind, electrsd, blocks_to_go as usize).await; | ||
| wait_for_node_tip(node, new_height).await.expect("node hasn't synced to the tip"); | ||
| wait_for_node_tip(peer_node, new_height) |
There was a problem hiding this comment.
I'm confused: why would we ever reach the new tip before we synced our wallets?
There was a problem hiding this comment.
Chain source might have not received the block propagation. We wait for the chain source tip to be the latest block we've just mined.
There was a problem hiding this comment.
I mean that the wallet might not know about the new block despite we've just mined it. It might have not propagated to us.
| } | ||
| assert!(tries < 20, "Reached max tries."); | ||
| if tries >= 20 { | ||
| return None; |
There was a problem hiding this comment.
What does this None value represent? If we didn't succeed in the given number of tries, we should give up, no? That's what this tries value is for?
There was a problem hiding this comment.
The idea was to change the location of occurred error from exponential_backoff_poll to the real caller, when we do unwrap.
None does not carry any other meaning except did not succeed in waiting.
Motivation is that when you run tests you want the exact location where something went wrong.
Mining node and the chain source node can be different nodes (I am speaking about CBF node). We need to wait for propagation of both tx when we broadcast tx from the chain source to the mining node and of a block from a mining node to the chain source. That means that after we mine something we need to wait for the tip of our chain source tip to be the latest block. |
3 participants
This is a prerequisite PR for compact block filter node chain source.
Contrary to all other chain sources, CBF chain source in tests is different from the node who mines blocks. That means we need to ensure the tx from chain source has been broadcast. In the same fashion we ensure that after the block has mined we received it from the mining node.
Tests assumed that a node observes a freshly mined block as soon as
sync_walletsreturns.Make
generate_blocks_and_waitandpremine_and_distribute_fundsreturn the new chain height, addwait_for_node_tipto poll until a node reports at least that height (syncing on every try, for pull-based sources), and use it after mining in the tests.Other changes:
exponential_backoff_pollreturnNoneinstead of panicking when it runs out of tries, andexpectat the call sites.Resultfrom the on-chain tx type assertions, so a failure is reported at the call site.unified_send_receive_bip21_uri, so the blocks are guaranteed to include it.lsps2_client_trusts_lsp, wait for the client's tip, which the assertions inspect, rather than the payer's.