diff --git a/src/interfaces/chain.h b/src/interfaces/chain.h index 4e858d1f898..c9ef46243c4 100644 --- a/src/interfaces/chain.h +++ b/src/interfaces/chain.h @@ -289,6 +289,9 @@ public: //! Check if any block has been pruned. virtual bool havePruned() = 0; + //! Get the current prune height. + virtual std::optional getPruneHeight() = 0; + //! Check if the node is ready to broadcast transactions. virtual bool isReadyToBroadcast() = 0; diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 7ae2ff64536..73ce927f712 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -46,6 +46,7 @@ #include #include #include +#include #include #include #include @@ -770,6 +771,11 @@ public: LOCK(::cs_main); return chainman().m_blockman.m_have_pruned; } + std::optional getPruneHeight() override + { + LOCK(chainman().GetMutex()); + return GetPruneHeight(chainman().m_blockman, chainman().ActiveChain()); + } bool isReadyToBroadcast() override { return !chainman().m_blockman.LoadingBlocks() && !isInitialBlockDownload(); } bool isInitialBlockDownload() override { diff --git a/src/validation.cpp b/src/validation.cpp index 64588e802d7..0384018bc36 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -5623,9 +5623,8 @@ double ChainstateManager::GuessVerificationProgress(const CBlockIndex* pindex) c return 0.0; } - if (!Assume(pindex->m_chain_tx_count > 0)) { - LogWarning("Internal bug detected: block %d has unset m_chain_tx_count (%s %s). Please report this issue here: %s\n", - pindex->nHeight, CLIENT_NAME, FormatFullVersion(), CLIENT_BUGREPORT); + if (pindex->m_chain_tx_count == 0) { + LogDebug(BCLog::VALIDATION, "Block %d has unset m_chain_tx_count. Unable to estimate verification progress.\n", pindex->nHeight); return 0.0; } diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 79f1a40ec5a..ac23b092d40 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1745,20 +1745,27 @@ RPCHelpMan importdescriptors() if (scanned_time <= GetImportTimestamp(request, now) || results.at(i).exists("error")) { response.push_back(results.at(i)); } else { + std::string error_msg{strprintf("Rescan failed for descriptor with timestamp %d. There " + "was an error reading a block from time %d, which is after or within %d seconds " + "of key creation, and could contain transactions pertaining to the desc. As a " + "result, transactions and coins using this desc may not appear in the wallet.", + GetImportTimestamp(request, now), scanned_time - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW)}; + if (pwallet->chain().havePruned()) { + error_msg += strprintf(" This error could be caused by pruning or data corruption " + "(see bitcoind log for details) and could be dealt with by downloading and " + "rescanning the relevant blocks (see -reindex option and rescanblockchain RPC)."); + } else if (pwallet->chain().hasAssumedValidChain()) { + error_msg += strprintf(" This error is likely caused by an in-progress assumeutxo " + "background sync. Check logs or getchainstates RPC for assumeutxo background " + "sync progress and try again later."); + } else { + error_msg += strprintf(" This error could potentially caused by data corruption. If " + "the issue persists you may want to reindex (see -reindex option)."); + } + UniValue result = UniValue(UniValue::VOBJ); result.pushKV("success", UniValue(false)); - result.pushKV( - "error", - JSONRPCError( - RPC_MISC_ERROR, - strprintf("Rescan failed for descriptor with timestamp %d. There was an error reading a " - "block from time %d, which is after or within %d seconds of key creation, and " - "could contain transactions pertaining to the desc. As a result, transactions " - "and coins using this desc may not appear in the wallet. This error could be " - "caused by pruning or data corruption (see bitcoind log for details) and could " - "be dealt with by downloading and rescanning the relevant blocks (see -reindex " - "option and rescanblockchain RPC).", - GetImportTimestamp(request, now), scanned_time - TIMESTAMP_WINDOW - 1, TIMESTAMP_WINDOW))); + result.pushKV("error", JSONRPCError(RPC_MISC_ERROR, error_msg)); response.push_back(std::move(result)); } } diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 61cf36a6c10..f3cfdfcc831 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -909,9 +910,15 @@ RPCHelpMan rescanblockchain() } } - // We can't rescan beyond non-pruned blocks, stop and throw an error + // We can't rescan unavailable blocks, stop and throw an error if (!pwallet->chain().hasBlocks(pwallet->GetLastBlockHash(), start_height, stop_height)) { - throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height."); + if (pwallet->chain().havePruned() && pwallet->chain().getPruneHeight() >= start_height) { + throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height."); + } + if (pwallet->chain().hasAssumedValidChain()) { + throw JSONRPCError(RPC_MISC_ERROR, "Failed to rescan unavailable blocks likely due to an in-progress assumeutxo background sync. Check logs or getchainstates RPC for assumeutxo background sync progress and try again later."); + } + throw JSONRPCError(RPC_MISC_ERROR, "Failed to rescan unavailable blocks, potentially caused by data corruption. If the issue persists you may want to reindex (see -reindex option)."); } CHECK_NONFATAL(pwallet->chain().findAncestorByHeight(pwallet->GetLastBlockHash(), start_height, FoundBlock().hash(start_block))); diff --git a/test/functional/wallet_assumeutxo.py b/test/functional/wallet_assumeutxo.py index ac904d6ce4a..d817d33f1c8 100755 --- a/test/functional/wallet_assumeutxo.py +++ b/test/functional/wallet_assumeutxo.py @@ -7,11 +7,11 @@ See feature_assumeutxo.py for background. ## Possible test improvements -- TODO: test import descriptors while background sync is in progress - TODO: test loading a wallet (backup) on a pruned node """ from test_framework.address import address_to_scriptpubkey +from test_framework.descriptors import descsum_create from test_framework.test_framework import BitcoinTestFramework from test_framework.messages import COIN from test_framework.util import ( @@ -20,6 +20,7 @@ from test_framework.util import ( ensure_for, ) from test_framework.wallet import MiniWallet +from test_framework.wallet_util import get_generate_key START_HEIGHT = 199 SNAPSHOT_BASE_HEIGHT = 299 @@ -49,6 +50,13 @@ class AssumeutxoTest(BitcoinTestFramework): self.add_nodes(3) self.start_nodes(extra_args=self.extra_args) + def import_descriptor(self, node, wallet_name, key, timestamp): + import_request = [{"desc": descsum_create("pkh(" + key.pubkey + ")"), + "timestamp": timestamp, + "label": "Descriptor import test"}] + wrpc = node.get_wallet_rpc(wallet_name) + return wrpc.importdescriptors(import_request) + def run_test(self): """ Bring up two (disconnected) nodes, mine some new blocks on the first, @@ -157,6 +165,21 @@ class AssumeutxoTest(BitcoinTestFramework): self.log.info("Backup from before the snapshot height can't be loaded during background sync") assert_raises_rpc_error(-4, "Wallet loading failed. Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order when using assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height 299", n1.restorewallet, "w2", "backup_w2.dat") + self.log.info("Test loading descriptors during background sync") + wallet_name = "w1" + n1.createwallet(wallet_name, disable_private_keys=True) + key = get_generate_key() + time = n1.getblockchaininfo()['time'] + timestamp = 0 + expected_error_message = f"Rescan failed for descriptor with timestamp {timestamp}. There was an error reading a block from time {time}, which is after or within 7200 seconds of key creation, and could contain transactions pertaining to the desc. As a result, transactions and coins using this desc may not appear in the wallet. This error is likely caused by an in-progress assumeutxo background sync. Check logs or getchainstates RPC for assumeutxo background sync progress and try again later." + result = self.import_descriptor(n1, wallet_name, key, timestamp) + assert_equal(result[0]['error']['code'], -1) + assert_equal(result[0]['error']['message'], expected_error_message) + + self.log.info("Test that rescanning blocks from before the snapshot fails when blocks are not available from the background sync yet") + w1 = n1.get_wallet_rpc(wallet_name) + assert_raises_rpc_error(-1, "Failed to rescan unavailable blocks likely due to an in-progress assumeutxo background sync. Check logs or getchainstates RPC for assumeutxo background sync progress and try again later.", w1.rescanblockchain, 100) + PAUSE_HEIGHT = FINAL_HEIGHT - 40 self.log.info("Restarting node to stop at height %d", PAUSE_HEIGHT) @@ -204,6 +227,11 @@ class AssumeutxoTest(BitcoinTestFramework): self.wait_until(lambda: len(n2.getchainstates()['chainstates']) == 1) ensure_for(duration=1, f=lambda: (n2.getbalance() == 34)) + self.log.info("Ensuring descriptors can be loaded after background sync") + n1.loadwallet(wallet_name) + result = self.import_descriptor(n1, wallet_name, key, timestamp) + assert_equal(result[0]['success'], True) + if __name__ == '__main__': AssumeutxoTest(__file__).main()