From 226d03dd610dd65938554bcf0abfe79f7ca7fb4d Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 23 Dec 2024 13:51:36 -0500 Subject: [PATCH 1/2] validation: Send correct notification during snapshot completion If AssumeUtxo background sync is completed in this ActivateBestChain() call, the GetRole() function returns "normal" instead of "background" for this chainstate. This would make the wallet (which ignores BlockConnected notifcation for the background chainstate) process it, change m_last_block_processed_height, and display an incorrect balance. --- src/validation.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index 3f774fd0a1..372a0aad48 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -3526,6 +3526,10 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< bool fInvalidFound = false; std::shared_ptr nullBlockPtr; + // BlockConnected signals must be sent for the original role; + // in case snapshot validation is completed during ActivateBestChainStep, the + // result of GetRole() changes from BACKGROUND to NORMAL. + const ChainstateRole chainstate_role{this->GetRole()}; if (!ActivateBestChainStep(state, pindexMostWork, pblock && pblock->GetHash() == pindexMostWork->GetBlockHash() ? pblock : nullBlockPtr, fInvalidFound, connectTrace)) { // A system error occurred return false; @@ -3541,7 +3545,7 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< for (const PerBlockConnectTrace& trace : connectTrace.GetBlocksConnected()) { assert(trace.pblock && trace.pindex); if (m_chainman.m_options.signals) { - m_chainman.m_options.signals->BlockConnected(this->GetRole(), trace.pblock, trace.pindex); + m_chainman.m_options.signals->BlockConnected(chainstate_role, trace.pblock, trace.pindex); } } From bc43ecaf6dc0830a27296d3a29428814fed07bb1 Mon Sep 17 00:00:00 2001 From: Martin Zumsande Date: Mon, 23 Dec 2024 14:52:56 -0500 Subject: [PATCH 2/2] test: add functional test for balance after snapshot completion Use a third node for this, which doesn't get restarted like the second node. This test would fail without the previous commit. --- test/functional/wallet_assumeutxo.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py index 76cd2097a3..ac904d6ce4 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -17,6 +17,7 @@ from test_framework.messages import COIN from test_framework.util import ( assert_equal, assert_raises_rpc_error, + ensure_for, ) from test_framework.wallet import MiniWallet @@ -34,17 +35,18 @@ class AssumeutxoTest(BitcoinTestFramework): def set_test_params(self): """Use the pregenerated, deterministic chain up to height 199.""" - self.num_nodes = 2 + self.num_nodes = 3 self.rpc_timeout = 120 self.extra_args = [ [], [], + [], ] def setup_network(self): """Start with the nodes disconnected so that one can generate a snapshot including blocks the other hasn't yet seen.""" - self.add_nodes(2) + self.add_nodes(3) self.start_nodes(extra_args=self.extra_args) def run_test(self): @@ -57,6 +59,7 @@ class AssumeutxoTest(BitcoinTestFramework): """ n0 = self.nodes[0] n1 = self.nodes[1] + n2 = self.nodes[2] self.mini_wallet = MiniWallet(n0) @@ -88,6 +91,7 @@ class AssumeutxoTest(BitcoinTestFramework): # make n1 aware of the new header, but don't give it the block. n1.submitheader(newblock) + n2.submitheader(newblock) # Ensure everyone is seeing the same headers. for n in self.nodes: @@ -125,6 +129,7 @@ class AssumeutxoTest(BitcoinTestFramework): assert_equal(n0.getblockcount(), FINAL_HEIGHT) assert_equal(n1.getblockcount(), START_HEIGHT) + assert_equal(n2.getblockcount(), START_HEIGHT) assert_equal(n0.getblockchaininfo()["blocks"], FINAL_HEIGHT) @@ -192,6 +197,13 @@ class AssumeutxoTest(BitcoinTestFramework): w = n1.get_wallet_rpc("w") assert_equal(w.getbalance(), 34) + self.log.info("Check balance of a wallet that is active during snapshot completion") + n2.restorewallet("w", "backup_w.dat") + loaded = n2.loadtxoutset(dump_output['path']) + self.connect_nodes(0, 2) + self.wait_until(lambda: len(n2.getchainstates()['chainstates']) == 1) + ensure_for(duration=1, f=lambda: (n2.getbalance() == 34)) + if __name__ == '__main__': AssumeutxoTest(__file__).main()