From d3082a63050a9ee7fdc2f50153f161432e9b2989 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 11 Sep 2024 07:50:28 +0200 Subject: [PATCH 1/4] electrum: make sure chain updates connect on rescan The `from_chain_tip` command takes a `CheckPoint` of our current chain tip for purposes of providing a chain update which connects to it, including in case of a reorg. This is not a point from which to rescan the chain. The current code made it so in case a rescan was performed while there was too much progress on the backend (>8 blocks), or when there was a deep enough reorg, the chain update could not unambiguously connect to our local chain. Here is an instance (logs were adapted to debug this issue): 2024-09-10T15:19:48.258728Z DEBUG liana::bitcoin::electrum:196: Full local chain: LocalChain { tip: CheckPoint(CPInner { block: BlockId { height: 618, hash: 0x6c6f30d76e1ce1bee2157d271300b93e968c501685c20e8466635310a4622efe }, prev: Some(CPInner { block: BlockId { height: 617, hash: 0x3ed1127ea206b672f47c6dfc984c63c229264f618015865057de9a75e7a6f8b0 }, prev: Some(CPInner { block: BlockId { height: 616, hash: 0x6ffe4d528 b5689802c46c86a1bd0acc9ecb35abf0bf73d2ccce72935b333c29e }, prev: Some(CPInner { block: BlockId { height: 615, hash: 0x433f6d69da936c29fdf02d43aeca9e0cfce0ce877f628743ada38e566dff1d73 }, prev: Some(CPInner { block: BlockId { height: 614, hash: 0x12a64d6416e196618154c34ad58f5c7342b59a752dbc4686af668e987f38dc28 }, prev: Some(CPInner { block: BlockId { height: 613, hash: 0x539942c4bb38f94e9ae35f0d120c8e250edd7a56930e130fe079 59d179dd2c0a }, prev: Some(CPInner { block: BlockId { height: 612, hash: 0x12ab45ad9c2dc246e450f98831cf2e6c5b3c9c5a0ab4af46c8a8cd915680d728 }, prev: Some(CPInner { block: BlockId { height: 611, hash: 0x21c890e0885f4672e22dbb6d17b110863bdd6b0464e0f5e731072d63f45a12d7 }, prev: Some(CPInner { block: BlockId { height: 515, hash: 0x021e3c76fbe92630214b1b23b85d44a9b443c6b1d01b336b7d1994a18944ebb1 }, prev: Some(CPInner { block: BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) }) }) } 2024-09-10T15:19:48.259058Z DEBUG liana::bitcoin::electrum:197: Full chain update: CheckPoint(CPInner { block: BlockId { height: 633, hash: 0x41ce59abb80de83d165501a1f9ba7acc98ed0ce4af7fde512fd2491905e777bd }, prev: Some(CPInner { block: BlockId { height: 632, hash: 0x4e812e2a98150c5da85efa44d792feed64e76ffbd0bf31ac5925a1b798bad959 }, prev: Some(CPInner { block: BlockId { height: 631, hash: 0x1379b6c609ce9adb0e2123dd36 629e71a5011dc0e9f6aa3935f2f6c5c9ed28b2 }, prev: Some(CPInner { block: BlockId { height: 630, hash: 0x4bb4f27e5cefd3ef2d9707af6ed933c6bb049a48081250d365fdb1bd3a10da4d }, prev: Some(CPInner { block: BlockId { height: 629, hash: 0x64bf474cde129b6b9a45c7f697d57c9514ef33de02cda3df1f65543f0b111366 }, prev: Some(CPInner { block: BlockId { height: 628, hash: 0x60c205d56d5a244da352867c7db48a696fe9ecfe04d0fd4f105c7663afdf5717 }, p rev: Some(CPInner { block: BlockId { height: 627, hash: 0x70b8d4091481295e39582d82f5c72ca5cb62505861a6d804afc1b1a0caa25c2b }, prev: Some(CPInner { block: BlockId { height: 626, hash: 0x15ad17258b32d87b8719e6ee911e4b54e8095a3429d05617d391f262a4e8b8c1 }, prev: Some(CPInner { block: BlockId { height: 0, hash: 0x0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206 }, prev: None }) }) }) }) }) }) }) }) }) --- src/bitcoin/electrum/mod.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/bitcoin/electrum/mod.rs b/src/bitcoin/electrum/mod.rs index 42f067eb1..f02a27cdf 100644 --- a/src/bitcoin/electrum/mod.rs +++ b/src/bitcoin/electrum/mod.rs @@ -162,9 +162,7 @@ impl Electrum { } else { log::info!("Performing full scan."); // Either local_chain has height 0 or we want to trigger a full scan. - // In both cases, the scan should be from the genesis block. - let genesis_block = local_chain_tip.get(0).expect("must contain genesis block"); - let mut request = FullScanRequest::from_chain_tip(genesis_block) + let mut request = FullScanRequest::from_chain_tip(local_chain_tip.clone()) .cache_graph_txs(self.bdk_wallet.graph()); for (k, spks) in self.bdk_wallet.index().all_unbounded_spk_iters() { From 59ba0ad8ef3f3a7203b1d0587ebf07e8642767f2 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 11 Sep 2024 08:20:34 +0200 Subject: [PATCH 2/4] electrum: log local and updated chains after polling Electrum --- src/bitcoin/electrum/mod.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/bitcoin/electrum/mod.rs b/src/bitcoin/electrum/mod.rs index f02a27cdf..f9c32dd20 100644 --- a/src/bitcoin/electrum/mod.rs +++ b/src/bitcoin/electrum/mod.rs @@ -191,6 +191,9 @@ impl Electrum { chain_update.height() ); + log::debug!("Full local chain: {:?}", self.local_chain()); + log::debug!("Full chain update: {:?}", chain_update); + // Increment the sync count and apply changes. self.sync_count = self.sync_count.checked_add(1).expect("must fit"); if let Some(keychain_update) = keychain_update { From 193f38a7c7258978e590f54aa55e048adb45e9ef Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 11 Sep 2024 08:27:28 +0200 Subject: [PATCH 3/4] ci: lower overall timeout 2 minutes should be more than enough, this more than halves the time to detect something's wrong! --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 384b67347..1aa5751d7 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -8,7 +8,7 @@ task: EXECUTOR_WORKERS: 3 VERBOSE: 0 LOG_LEVEL: debug - TIMEOUT: 300 + TIMEOUT: 120 matrix: - name: 'Misc functional tests' env: From 3d7b3e44734ec859e37d1ac4005f25678f0cbc94 Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Wed, 11 Sep 2024 09:57:31 +0200 Subject: [PATCH 4/4] qa: correct reorg_shit function to use given `height` It was using the passed height for all other calculation, except for the invalidation itself where it was using the wider-scope-initial-tip! This is probably a mistake due to moving it to its own, inner, function. --- tests/test_chain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_chain.py b/tests/test_chain.py index b4bf5960c..30027bbbc 100644 --- a/tests/test_chain.py +++ b/tests/test_chain.py @@ -208,7 +208,7 @@ def reorg_shift(height, txs): """Remine the chain from given height, shifting the txs by one block.""" delta = bitcoind.rpc.getblockcount() - height + 1 assert delta > 2 - h = bitcoind.rpc.getblockhash(initial_tip["height"]) + h = bitcoind.rpc.getblockhash(height) bitcoind.rpc.invalidateblock(h) bitcoind.generate_block(1) for tx in txs: