diff --git a/src/builder.rs b/src/builder.rs index ab641cf86..b85c71703 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -427,9 +427,9 @@ impl NodeBuilder { /// ## Parameters: /// * `rpc_host`, `rpc_port`, `rpc_user`, `rpc_password` - Required parameters for the Bitcoin Core RPC /// connection. - /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from on first - /// startup, before wallet state exists. Existing wallets are not rewound. The height must - /// be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` + /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from (inclusive) on + /// first startup, before wallet state exists. Existing wallets are not rewound. The height + /// must be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` /// checkpoints at the current tip. #[cfg(feature = "chain-bitcoind")] pub fn set_chain_source_bitcoind_rpc( @@ -456,9 +456,9 @@ impl NodeBuilder { /// * `rest_host`, `rest_port` - Required parameters for the Bitcoin Core REST connection. /// * `rpc_host`, `rpc_port`, `rpc_user`, `rpc_password` - Required parameters for the Bitcoin Core RPC /// connection - /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from on first - /// startup, before wallet state exists. Existing wallets are not rewound. The height must - /// be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` + /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from (inclusive) on + /// first startup, before wallet state exists. Existing wallets are not rewound. The height + /// must be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` /// checkpoints at the current tip. #[cfg(feature = "chain-bitcoind")] pub fn set_chain_source_bitcoind_rest( @@ -1075,9 +1075,9 @@ impl Builder { /// ## Parameters: /// * `rpc_host`, `rpc_port`, `rpc_user`, `rpc_password` - Required parameters for the Bitcoin Core RPC /// connection. - /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from on first - /// startup, before wallet state exists. Existing wallets are not rewound. The height must - /// be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` + /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from (inclusive) on + /// first startup, before wallet state exists. Existing wallets are not rewound. The height + /// must be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` /// checkpoints at the current tip. pub fn set_chain_source_bitcoind_rpc( &self, rpc_host: String, rpc_port: u16, rpc_user: String, rpc_password: String, @@ -1101,9 +1101,9 @@ impl Builder { /// * `rest_host`, `rest_port` - Required parameters for the Bitcoin Core REST connection. /// * `rpc_host`, `rpc_port`, `rpc_user`, `rpc_password` - Required parameters for the Bitcoin Core RPC /// connection - /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from on first - /// startup, before wallet state exists. Existing wallets are not rewound. The height must - /// be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` + /// * `wallet_rescan_from_height` - Optional wallet birthday height to rescan from (inclusive) on + /// first startup, before wallet state exists. Existing wallets are not rewound. The height + /// must be at or below the current tip. Passing `Some(0)` rescans from genesis; passing `None` /// checkpoints at the current tip. pub fn set_chain_source_bitcoind_rest( &self, rest_host: String, rest_port: u16, rpc_host: String, rpc_port: u16, @@ -1771,8 +1771,9 @@ fn build_with_store_internal( })?; // Decide which block (if any) to insert as the initial BDK checkpoint. If the - // bitcoind config provides a wallet rescan height, resolve that block and use it as - // the checkpoint. Otherwise, use the current chain tip to avoid any rescan. + // bitcoind config provides a wallet rescan height, use the preceding block as the + // checkpoint so synchronization includes the requested height. Otherwise, use the + // current chain tip to avoid any rescan. let checkpoint_block = match wallet_rescan_from_height { None => chain_tip_opt, #[cfg(feature = "chain-bitcoind")] @@ -1788,6 +1789,10 @@ fn build_with_store_internal( return Err(BuildError::WalletRescanHeightTooHigh); } } + // `synchronize_listeners` connects blocks strictly above each listener's + // checkpoint. The genesis block is already BDK's initial checkpoint, so + // saturating subtraction also handles a requested height of zero. + let checkpoint_height = height.saturating_sub(1); let utxo_source = chain_source.as_utxo_source().ok_or_else(|| { log_error!( @@ -1799,16 +1804,17 @@ fn build_with_store_internal( let hash_res = runtime.block_on(async { lightning_block_sync::gossip::UtxoSource::get_block_hash_by_height( &utxo_source, - height, + checkpoint_height, ) .await }); match hash_res { - Ok(hash) => Some(BlockLocator::new(hash, height)), + Ok(hash) => Some(BlockLocator::new(hash, checkpoint_height)), Err(e) => { log_error!( logger, - "Failed to resolve block hash at height {} for wallet rescan: {:?}", + "Failed to resolve checkpoint block hash at height {} for wallet rescan from height {}: {:?}", + checkpoint_height, height, e, ); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 0dad32d6a..b1c71f084 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -1557,19 +1557,19 @@ async fn do_onchain_wallet_full_scan_stop_gap_recovers_far_funds( #[cfg(feature = "chain-bitcoind")] #[tokio::test(flavor = "multi_thread", worker_threads = 1)] -async fn onchain_wallet_recovery_rescans_from_birthday_height() { +async fn onchain_wallet_recovery_includes_rescan_height() { // End-to-end test for `wallet_rescan_from_height` against a bitcoind chain source. The // scenario: // - // 1. Create a node at some "birthday" height and generate two receive addresses. + // 1. Create a node and generate two receive addresses. // 2. Shut the node down and drop all persisted state except the seed. - // 3. Advance the chain past the birthday. - // 4. Send funds to the addresses generated at the birthday height and confirm them. + // 3. Advance the chain and send funds to both addresses. + // 4. Confirm both transactions in the block used as the rescan height. // 5. Restart a fresh node with just the seed and no rescan height. Its wallet birthday - // is pinned at the current tip, which is above the blocks containing the funding - // transactions — so the node must not see the funds. - // 6. Restart again with `wallet_rescan_from_height: Some(birthday)`. Now the wallet must - // find and report both funding transactions. + // is pinned at the current tip, which is the block containing the funding transactions, + // so the node must not see the funds. + // 6. Restart again with the funding block as `wallet_rescan_from_height`. The wallet must + // include that block and find both funding transactions. let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); // We specifically exercise the bitcoind RPC backend because that's where // `rescan_from_height` is honored precisely (via `get_block_hash_by_height`). @@ -1578,7 +1578,7 @@ async fn onchain_wallet_recovery_rescans_from_birthday_height() { // Mine the initial 101 blocks so bitcoind's wallet can fund our later sends. premine_blocks(&bitcoind.client, &electrsd.client).await; - // Step 1: bring up an "original" node at the birthday height and generate addresses. + // Step 1: bring up an "original" node and generate addresses. let original_config = random_config(); let original_node_entropy = original_config.node_entropy; let original_node = setup_node(&chain_source, original_config); @@ -1588,23 +1588,15 @@ async fn onchain_wallet_recovery_rescans_from_birthday_height() { let addr_1 = original_node.onchain_payment().new_address().unwrap(); let addr_2 = original_node.onchain_payment().new_address().unwrap(); - let birthday_height: u32 = bitcoind - .client - .get_blockchain_info() - .expect("failed to get blockchain info") - .blocks - .try_into() - .unwrap(); - // Step 2: shut the node down and drop its state. original_node.stop().unwrap(); drop(original_node); - // Step 3: advance the chain past the birthday, so a fresh node would otherwise pin its - // wallet birthday at a height above the funding transactions in step 4. + // Step 3: advance the chain so a fresh node can later pin its wallet birthday at the funding + // block without having seen the transactions. generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 10).await; - // Step 4: fund both addresses and confirm them. + // Step 4: fund both addresses and confirm them in the configured rescan block. let txid_1 = bitcoind .client .send_to_address(&addr_1, Amount::from_sat(premine_amount_sat)) @@ -1621,10 +1613,14 @@ async fn onchain_wallet_recovery_rescans_from_birthday_height() { .parse() .unwrap(); wait_for_tx(&electrsd.client, txid_2).await; + let rescan_height: u32 = + (bitcoind.client.get_blockchain_info().expect("failed to get blockchain info").blocks + 1) + .try_into() + .unwrap(); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 1).await; - // Step 5: restart a fresh node with only the seed and no rescan height. It must NOT see - // the funds, because its wallet birthday sits above the funding transactions. + // Step 5: restart a fresh node with only the seed and no rescan height. It must NOT see the + // funds, because its wallet birthday is the block containing the funding transactions. let mut pinned_config = random_config(); pinned_config.node_entropy = original_node_entropy; let pinned_node = setup_node(&chain_source, pinned_config); @@ -1632,16 +1628,16 @@ async fn onchain_wallet_recovery_rescans_from_birthday_height() { assert_eq!( pinned_node.list_balances().spendable_onchain_balance_sats, 0, - "fresh node without rescan height should not find funds below its wallet birthday" + "fresh node without rescan height should not scan its wallet birthday block" ); pinned_node.stop().unwrap(); drop(pinned_node); - // Step 6: restart with a rescan height set to the birthday height. Funds must be - // re-discovered. + // Step 6: restart from the exact block containing both transactions. Funds must be + // re-discovered, proving that `wallet_rescan_from_height` is inclusive. let mut recovered_config = random_config(); recovered_config.node_entropy = original_node_entropy; - recovered_config.wallet_rescan_from_height = Some(birthday_height); + recovered_config.wallet_rescan_from_height = Some(rescan_height); let recovered_node = setup_node(&chain_source, recovered_config); recovered_node.sync_wallets().unwrap(); assert_eq!(