diff --git a/src/chain/electrum.rs b/src/chain/electrum.rs index 23c930d98..e255158ca 100644 --- a/src/chain/electrum.rs +++ b/src/chain/electrum.rs @@ -25,14 +25,17 @@ use lightning::util::ser::Writeable; use lightning_transaction_sync::ElectrumSyncClient; use super::WalletSyncStatus; -use crate::config::{Config, ElectrumSyncConfig, BDK_CLIENT_STOP_GAP}; +use crate::config::{ + clamp_full_scan_stop_gap, Config, ElectrumSyncConfig, MAX_FULL_SCAN_STOP_GAP, + MIN_FULL_SCAN_STOP_GAP, +}; use crate::error::Error; use crate::fee_estimator::{ apply_post_estimation_adjustments, get_all_conf_targets, get_num_block_defaults_for_target, ConfirmationTarget, OnchainFeeEstimator, }; use crate::io::utils::update_and_persist_node_metrics; -use crate::logger::{log_bytes, log_debug, log_error, log_trace, LdkLogger, Logger}; +use crate::logger::{log_bytes, log_debug, log_error, log_trace, log_warn, LdkLogger, Logger}; use crate::runtime::Runtime; use crate::types::{ChainMonitor, ChannelManager, DynStore, Sweeper, Wallet}; use crate::PersistedNodeMetrics; @@ -496,11 +499,12 @@ impl ElectrumRuntimeClient { ) -> Result, Error> { let bdk_electrum_client = Arc::clone(&self.bdk_electrum_client); bdk_electrum_client.populate_tx_cache(cached_txs); + let full_scan_stop_gap = self.bounded_full_scan_stop_gap(); let spawn_fut = self.runtime.spawn_blocking(move || { bdk_electrum_client.full_scan( request, - BDK_CLIENT_STOP_GAP, + full_scan_stop_gap, BDK_ELECTRUM_CLIENT_BATCH_SIZE, true, ) @@ -526,6 +530,22 @@ impl ElectrumRuntimeClient { }) } + fn bounded_full_scan_stop_gap(&self) -> usize { + let configured = self.sync_config.full_scan_stop_gap; + let bounded = clamp_full_scan_stop_gap(configured); + if bounded != configured { + log_warn!( + self.logger, + "Configured Electrum on-chain wallet full-scan stop gap {} is outside the allowed range {}..={}; using {}.", + configured, + MIN_FULL_SCAN_STOP_GAP, + MAX_FULL_SCAN_STOP_GAP, + bounded + ); + } + bounded as usize + } + async fn get_incremental_sync_wallet_update( &self, request: BdkSyncRequest<(BdkKeyChainKind, u32)>, cached_txs: impl IntoIterator>>, diff --git a/src/chain/esplora.rs b/src/chain/esplora.rs index 0754986e8..b46fe183c 100644 --- a/src/chain/esplora.rs +++ b/src/chain/esplora.rs @@ -18,13 +18,16 @@ use lightning::util::ser::Writeable; use lightning_transaction_sync::EsploraSyncClient; use super::WalletSyncStatus; -use crate::config::{Config, EsploraSyncConfig, BDK_CLIENT_CONCURRENCY, BDK_CLIENT_STOP_GAP}; +use crate::config::{ + clamp_full_scan_stop_gap, Config, EsploraSyncConfig, BDK_CLIENT_CONCURRENCY, + MAX_FULL_SCAN_STOP_GAP, MIN_FULL_SCAN_STOP_GAP, +}; use crate::fee_estimator::{ apply_post_estimation_adjustments, get_all_conf_targets, get_num_block_defaults_for_target, OnchainFeeEstimator, }; use crate::io::utils::update_and_persist_node_metrics; -use crate::logger::{log_bytes, log_debug, log_error, log_trace, LdkLogger, Logger}; +use crate::logger::{log_bytes, log_debug, log_error, log_trace, log_warn, LdkLogger, Logger}; use crate::types::{ChainMonitor, ChannelManager, DynStore, Sweeper, Wallet}; use crate::{Error, PersistedNodeMetrics}; @@ -194,13 +197,14 @@ impl EsploraChainSource { get_and_apply_wallet_update!(wallet_sync_timeout_fut) } else { let full_scan_request = onchain_wallet.get_full_scan_request(); + let full_scan_stop_gap = self.bounded_full_scan_stop_gap(); let wallet_sync_timeout_fut = tokio::time::timeout( Duration::from_secs( self.sync_config.timeouts_config.onchain_wallet_sync_timeout_secs, ), self.esplora_client.full_scan( full_scan_request, - BDK_CLIENT_STOP_GAP, + full_scan_stop_gap, BDK_CLIENT_CONCURRENCY, ), ); @@ -212,6 +216,22 @@ impl EsploraChainSource { res } + fn bounded_full_scan_stop_gap(&self) -> usize { + let configured = self.sync_config.full_scan_stop_gap; + let bounded = clamp_full_scan_stop_gap(configured); + if bounded != configured { + log_warn!( + self.logger, + "Configured Esplora on-chain wallet full-scan stop gap {} is outside the allowed range {}..={}; using {}.", + configured, + MIN_FULL_SCAN_STOP_GAP, + MAX_FULL_SCAN_STOP_GAP, + bounded + ); + } + bounded as usize + } + pub(super) async fn sync_lightning_wallet( &self, channel_manager: Arc, chain_monitor: Arc, output_sweeper: Arc, diff --git a/src/config.rs b/src/config.rs index ad1b91181..f83cf3d43 100644 --- a/src/config.rs +++ b/src/config.rs @@ -57,9 +57,20 @@ pub const DEFAULT_STORAGE_DIR_PATH: &str = "/tmp/ldk_node"; // The default Esplora server we're using. pub(crate) const DEFAULT_ESPLORA_SERVER_URL: &str = "https://blockstream.info/api"; -// The 'stop gap' parameter used by BDK's wallet sync. This seems to configure the threshold -// number of derivation indexes after which BDK stops looking for new scripts belonging to the wallet. -pub(crate) const BDK_CLIENT_STOP_GAP: usize = 20; +/// The default stop gap used for BDK full scans of the on-chain wallet. +/// +/// The current default is 20. +pub const DEFAULT_FULL_SCAN_STOP_GAP: u32 = 20; + +/// The minimum allowed stop gap used for BDK full scans of the on-chain wallet. +/// +/// Values below 1 are clamped to 1 when a full scan runs. +pub const MIN_FULL_SCAN_STOP_GAP: u32 = 1; + +/// The maximum allowed stop gap used for BDK full scans of the on-chain wallet. +/// +/// Values above 1000 are clamped to 1000 when a full scan runs. +pub const MAX_FULL_SCAN_STOP_GAP: u32 = 1000; // The number of concurrent requests made against the API provider. pub(crate) const BDK_CLIENT_CONCURRENCY: usize = 4; @@ -506,6 +517,23 @@ pub struct EsploraSyncConfig { pub background_sync_config: Option, /// Sync timeouts configuration. pub timeouts_config: SyncTimeoutsConfig, + /// The stop gap used for BDK full scans of the on-chain wallet. + /// + /// A full scan for each keychain stops after this many consecutive script pubkeys + /// with no associated transactions. This value is only used for BDK `full_scan` + /// calls, which ldk-node performs on the first on-chain wallet sync or when + /// [`Self::force_wallet_full_scan`] is set. Incremental BDK `sync` calls do not use it. + /// + /// **Default:** 20 ([`DEFAULT_FULL_SCAN_STOP_GAP`]) + /// + /// **Allowed values:** 1 ([`MIN_FULL_SCAN_STOP_GAP`]) to 1000 + /// ([`MAX_FULL_SCAN_STOP_GAP`]), inclusive. Values outside this range will be clamped to the + /// nearest bound and a warning will be logged when the full scan runs. + /// + /// **Note:** Large values can cause many Esplora requests, hit server rate limits, + /// take a long time to complete, or cause syncs to fail with + /// [`SyncTimeoutsConfig::onchain_wallet_sync_timeout_secs`]. + pub full_scan_stop_gap: u32, /// Whether to force BDK full scans until one succeeds. /// /// This can be useful when restoring a wallet from seed on a node that has already synced @@ -518,6 +546,7 @@ impl Default for EsploraSyncConfig { Self { background_sync_config: Some(BackgroundSyncConfig::default()), timeouts_config: SyncTimeoutsConfig::default(), + full_scan_stop_gap: DEFAULT_FULL_SCAN_STOP_GAP, force_wallet_full_scan: false, } } @@ -539,6 +568,23 @@ pub struct ElectrumSyncConfig { pub background_sync_config: Option, /// Sync timeouts configuration. pub timeouts_config: SyncTimeoutsConfig, + /// The stop gap used for BDK full scans of the on-chain wallet. + /// + /// A full scan for each keychain stops after this many consecutive script pubkeys + /// with no associated transactions. This value is only used for BDK `full_scan` + /// calls, which ldk-node performs on the first on-chain wallet sync or when + /// [`Self::force_wallet_full_scan`] is set. Incremental BDK `sync` calls do not use it. + /// + /// **Default:** 20 ([`DEFAULT_FULL_SCAN_STOP_GAP`]) + /// + /// **Allowed values:** 1 ([`MIN_FULL_SCAN_STOP_GAP`]) to 1000 + /// ([`MAX_FULL_SCAN_STOP_GAP`]), inclusive. Values outside this range will be clamped to the + /// nearest bound and a warning will be logged when the full scan runs. + /// + /// **Note:** Large values can cause many Electrum requests, hit server rate limits, + /// take a long time to complete, or cause syncs to fail with + /// [`SyncTimeoutsConfig::onchain_wallet_sync_timeout_secs`]. + pub full_scan_stop_gap: u32, /// Whether to force BDK full scans until one succeeds. /// /// This can be useful when restoring a wallet from seed on a node that has already synced @@ -551,11 +597,16 @@ impl Default for ElectrumSyncConfig { Self { background_sync_config: Some(BackgroundSyncConfig::default()), timeouts_config: SyncTimeoutsConfig::default(), + full_scan_stop_gap: DEFAULT_FULL_SCAN_STOP_GAP, force_wallet_full_scan: false, } } } +pub(crate) fn clamp_full_scan_stop_gap(full_scan_stop_gap: u32) -> u32 { + full_scan_stop_gap.clamp(MIN_FULL_SCAN_STOP_GAP, MAX_FULL_SCAN_STOP_GAP) +} + /// Configuration for syncing with Bitcoin Core backend via REST. #[derive(Debug, Clone)] pub struct BitcoindRestClientConfig { @@ -711,7 +762,11 @@ pub enum AsyncPaymentsRole { mod tests { use std::str::FromStr; - use super::{may_announce_channel, AnnounceError, Config, NodeAlias, SocketAddress}; + use super::{ + clamp_full_scan_stop_gap, may_announce_channel, AnnounceError, Config, ElectrumSyncConfig, + EsploraSyncConfig, NodeAlias, SocketAddress, DEFAULT_FULL_SCAN_STOP_GAP, + MAX_FULL_SCAN_STOP_GAP, MIN_FULL_SCAN_STOP_GAP, + }; #[test] fn node_announce_channel() { @@ -758,4 +813,22 @@ mod tests { } assert!(may_announce_channel(&node_config).is_ok()); } + + #[test] + fn full_scan_stop_gap_defaults() { + assert_eq!(EsploraSyncConfig::default().full_scan_stop_gap, DEFAULT_FULL_SCAN_STOP_GAP); + assert_eq!(ElectrumSyncConfig::default().full_scan_stop_gap, DEFAULT_FULL_SCAN_STOP_GAP); + } + + #[test] + fn full_scan_stop_gap_is_clamped_to_valid_range() { + assert_eq!(clamp_full_scan_stop_gap(MIN_FULL_SCAN_STOP_GAP), MIN_FULL_SCAN_STOP_GAP); + assert_eq!( + clamp_full_scan_stop_gap(DEFAULT_FULL_SCAN_STOP_GAP), + DEFAULT_FULL_SCAN_STOP_GAP + ); + assert_eq!(clamp_full_scan_stop_gap(MAX_FULL_SCAN_STOP_GAP), MAX_FULL_SCAN_STOP_GAP); + assert_eq!(clamp_full_scan_stop_gap(0), MIN_FULL_SCAN_STOP_GAP); + assert_eq!(clamp_full_scan_stop_gap(MAX_FULL_SCAN_STOP_GAP + 1), MAX_FULL_SCAN_STOP_GAP); + } } diff --git a/src/logger.rs b/src/logger.rs index 3ef939b6d..c5a4584a1 100644 --- a/src/logger.rs +++ b/src/logger.rs @@ -19,7 +19,7 @@ use lightning::ln::types::ChannelId; use lightning::types::payment::PaymentHash; pub use lightning::util::logger::Level as LogLevel; pub(crate) use lightning::util::logger::{Logger as LdkLogger, Record as LdkRecord}; -pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace}; +pub(crate) use lightning::{log_bytes, log_debug, log_error, log_info, log_trace, log_warn}; use log::{Level as LogFacadeLevel, Record as LogFacadeRecord}; /// A unit of logging output with metadata to enable filtering `module_path`, diff --git a/tests/common/mod.rs b/tests/common/mod.rs index a56d46e05..f0148da8a 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -437,6 +437,7 @@ pub(crate) struct TestConfig { pub async_payments_role: Option, pub wallet_rescan_from_height: Option, pub force_wallet_full_scan: bool, + pub full_scan_stop_gap: Option, } impl Default for TestConfig { @@ -450,6 +451,7 @@ impl Default for TestConfig { let async_payments_role = None; let wallet_rescan_from_height = None; let force_wallet_full_scan = false; + let full_scan_stop_gap = None; TestConfig { node_config, log_writer, @@ -458,6 +460,7 @@ impl Default for TestConfig { async_payments_role, wallet_rescan_from_height, force_wallet_full_scan, + full_scan_stop_gap, } } } @@ -541,6 +544,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) -> let mut sync_config = EsploraSyncConfig::default(); sync_config.background_sync_config = None; sync_config.force_wallet_full_scan = config.force_wallet_full_scan; + if let Some(full_scan_stop_gap) = config.full_scan_stop_gap { + sync_config.full_scan_stop_gap = full_scan_stop_gap; + } builder.set_chain_source_esplora(esplora_url.clone(), Some(sync_config)); }, TestChainSource::Electrum(electrsd) => { @@ -548,6 +554,9 @@ pub(crate) fn setup_node(chain_source: &TestChainSource, config: TestConfig) -> let mut sync_config = ElectrumSyncConfig::default(); sync_config.background_sync_config = None; sync_config.force_wallet_full_scan = config.force_wallet_full_scan; + if let Some(full_scan_stop_gap) = config.full_scan_stop_gap { + sync_config.full_scan_stop_gap = full_scan_stop_gap; + } builder.set_chain_source_electrum(electrum_url.clone(), Some(sync_config)); }, TestChainSource::BitcoindRpcSync(bitcoind) => { diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index c3c2f4262..eedd62ebe 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -29,7 +29,7 @@ use common::{ }; use electrsd::corepc_node::{self, Node as BitcoinD}; use electrsd::ElectrsD; -use ldk_node::config::{AsyncPaymentsRole, EsploraSyncConfig}; +use ldk_node::config::{AsyncPaymentsRole, EsploraSyncConfig, DEFAULT_FULL_SCAN_STOP_GAP}; use ldk_node::entropy::NodeEntropy; use ldk_node::liquidity::LSPS2ServiceConfig; use ldk_node::payment::{ @@ -999,6 +999,76 @@ async fn onchain_wallet_force_full_scan_rediscovers_esplora_funds() { ); } +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn onchain_wallet_full_scan_stop_gap_recovers_far_esplora_and_electrum_funds() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + premine_blocks(&bitcoind.client, &electrsd.client).await; + + do_onchain_wallet_full_scan_stop_gap_recovers_far_funds( + TestChainSource::Esplora(&electrsd), + &bitcoind, + &electrsd, + ) + .await; + do_onchain_wallet_full_scan_stop_gap_recovers_far_funds( + TestChainSource::Electrum(&electrsd), + &bitcoind, + &electrsd, + ) + .await; +} + +async fn do_onchain_wallet_full_scan_stop_gap_recovers_far_funds( + chain_source: TestChainSource<'_>, bitcoind: &BitcoinD, electrsd: &ElectrsD, +) { + let configured_stop_gap = DEFAULT_FULL_SCAN_STOP_GAP + 5; + + let address_source_config = random_config(true); + let node_entropy = address_source_config.node_entropy; + let address_source_node = setup_node(&chain_source, address_source_config); + let mut far_address = None; + for _ in 0..configured_stop_gap { + far_address = Some(address_source_node.onchain_payment().new_address().unwrap()); + } + address_source_node.stop().unwrap(); + drop(address_source_node); + let far_address = far_address.unwrap(); + + let premine_amount_sat = 100_000; + let txid = bitcoind + .client + .send_to_address(&far_address, Amount::from_sat(premine_amount_sat)) + .unwrap() + .0 + .parse() + .unwrap(); + wait_for_tx(&electrsd.client, txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 1).await; + + let mut default_gap_config = random_config(true); + default_gap_config.node_entropy = node_entropy.clone(); + let default_gap_node = setup_node(&chain_source, default_gap_config); + default_gap_node.sync_wallets().unwrap(); + assert_eq!( + default_gap_node.list_balances().spendable_onchain_balance_sats, + 0, + "default full-scan stop gap should not recover funds past its address gap" + ); + default_gap_node.stop().unwrap(); + drop(default_gap_node); + + let mut configured_gap_config = random_config(true); + configured_gap_config.node_entropy = node_entropy; + configured_gap_config.full_scan_stop_gap = Some(configured_stop_gap); + let configured_gap_node = setup_node(&chain_source, configured_gap_config); + configured_gap_node.sync_wallets().unwrap(); + assert_eq!( + configured_gap_node.list_balances().spendable_onchain_balance_sats, + premine_amount_sat, + "configured full-scan stop gap should recover funds past the default address gap" + ); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn onchain_wallet_recovery_rescans_from_birthday_height() { // End-to-end test for `wallet_rescan_from_height` against a bitcoind chain source. The