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 3f774fd0a1e63..372a0aad48c1f 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 76cd2097a3a19..ac904d6ce4af0 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -17,6 +17,7 @@ from test_framework.util import ( assert_equal, assert_raises_rpc_error, + ensure_for, ) from test_framework.wallet import MiniWallet @@ -34,17 +35,18 @@ def add_options(self, parser): 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 @@ def run_test(self): """ n0 = self.nodes[0] n1 = self.nodes[1] + n2 = self.nodes[2] self.mini_wallet = MiniWallet(n0) @@ -88,6 +91,7 @@ def run_test(self): # 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 @@ def run_test(self): 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 @@ def run_test(self): 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()