From 236239bd40ae1175537fc932df5af27902326329 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 1 May 2022 14:28:14 +0200 Subject: [PATCH 1/7] wallet: Rescan mempool for transactions as well --- src/wallet/test/wallet_tests.cpp | 8 ++++---- src/wallet/wallet.cpp | 7 ++++++- test/functional/wallet_importdescriptors.py | 4 +++- 3 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 70863f5464a..0ba39b52d53 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -55,9 +55,6 @@ static const std::shared_ptr TestLoadWallet(WalletContext& context) auto database = MakeWalletDatabase("", options, status, error); auto wallet = CWallet::Create(context, "", std::move(database), options.create_flags, error, warnings); NotifyWalletLoaded(context, wallet); - if (context.chain) { - wallet->postInitProcess(); - } return wallet; } @@ -765,6 +762,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // being blocked wallet = TestLoadWallet(context); BOOST_CHECK(rescan_completed); + // AddToWallet events for block_tx and mempool_tx BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); @@ -777,6 +775,8 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) // transactionAddedToMempool events are processed promise.set_value(); SyncWithValidationInterfaceQueue(); + // AddToWallet events for block_tx and mempool_tx events are counted a + // second time as the notificaiton queue is processed BOOST_CHECK_EQUAL(addtx_count, 4); @@ -800,7 +800,7 @@ BOOST_FIXTURE_TEST_CASE(CreateWallet, TestChain100Setup) SyncWithValidationInterfaceQueue(); }); wallet = TestLoadWallet(context); - BOOST_CHECK_EQUAL(addtx_count, 4); + BOOST_CHECK_EQUAL(addtx_count, 2); { LOCK(wallet->cs_wallet); BOOST_CHECK_EQUAL(wallet->mapWallet.count(block_tx.GetHash()), 1U); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 910562e6690..e3aebd4f817 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1687,7 +1687,8 @@ int64_t CWallet::RescanFromTime(int64_t startTime, const WalletRescanReserver& r /** * Scan the block chain (starting in start_block) for transactions * from or to us. If fUpdate is true, found transactions that already - * exist in the wallet will be updated. + * exist in the wallet will be updated. If max_height is not set, the + * mempool will be scanned as well. * * @param[in] start_block Scan starting block. If block is not on the active * chain, the scan will return SUCCESS immediately. @@ -1797,6 +1798,10 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc } } } + if (!max_height) { + WalletLogPrintf("Scanning current mempool transactions.\n"); + WITH_LOCK(cs_wallet, chain().requestMempoolTransactions(*this)); + } ShowProgress(strprintf("%s " + _("Rescanning…").translated, GetDisplayName()), 100); // hide progress dialog in GUI if (block_height && fAbortRescan) { WalletLogPrintf("Rescan aborted at block %d. Progress=%f\n", block_height, progress_current); diff --git a/test/functional/wallet_importdescriptors.py b/test/functional/wallet_importdescriptors.py index ac74ff24844..b74a041539e 100755 --- a/test/functional/wallet_importdescriptors.py +++ b/test/functional/wallet_importdescriptors.py @@ -480,7 +480,9 @@ class ImportDescriptorsTest(BitcoinTestFramework): addr = wmulti_pub.getnewaddress('', 'bech32') assert_equal(addr, 'bcrt1qp8s25ckjl7gr6x2q3dx3tn2pytwp05upkjztk6ey857tt50r5aeqn6mvr9') # Derived at m/84'/0'/0'/1 change_addr = wmulti_pub.getrawchangeaddress('bech32') - assert_equal(change_addr, 'bcrt1qt9uhe3a9hnq7vajl7a094z4s3crm9ttf8zw3f5v9gr2nyd7e3lnsy44n8e') + assert_equal(change_addr, 'bcrt1qzxl0qz2t88kljdnkzg4n4gapr6kte26390gttrg79x66nt4p04fssj53nl') + assert(send_txid in self.nodes[0].getrawmempool(True)) + assert(send_txid in (x['txid'] for x in wmulti_pub.listunspent(0))) assert_equal(wmulti_pub.getwalletinfo()['keypoolsize'], 999) # generate some utxos for next tests From 3abdbbb90a4a8f2041fec37506268e66a0b3eb31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 12 May 2020 23:33:43 +0100 Subject: [PATCH 2/7] rpc, wallet: Document and test mempool scan after importaddress co-authored-by: Fabian Jahr --- src/wallet/rpc/backup.cpp | 4 +++- test/functional/wallet_balance.py | 16 ++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index aa9f23c8866..0b73d444f85 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -201,6 +201,8 @@ RPCHelpMan importaddress() "\nAdds an address or script (in hex) that can be watched as if it were in your wallet but cannot be used to spend. Requires a new wallet backup.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported address exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "If you have the full public key, you should call importpubkey instead of this.\n" "Hint: use importmulti to import more than one address.\n" "\nNote: If you import a non-standard raw script in hex form, outputs sending to it will be treated\n" @@ -209,7 +211,7 @@ RPCHelpMan importaddress() { {"address", RPCArg::Type::STR, RPCArg::Optional::NO, "The Bitcoin address (or hex-encoded script)"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, {"p2sh", RPCArg::Type::BOOL, RPCArg::Default{false}, "Add the P2SH version of the script as well"}, }, RPCResult{RPCResult::Type::NONE, "", ""}, diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 0c93821e7f4..94b74c0696d 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -273,6 +273,22 @@ class WalletTest(BitcoinTestFramework): self.generatetoaddress(self.nodes[1], 1, ADDRESS_WATCHONLY) assert_equal(self.nodes[0].getbalance(minconf=0), total_amount + 1) # The reorg recovered our fee of 1 coin + if not self.options.descriptors: + self.log.info('Check if mempool is taken into account after import*') + address = self.nodes[0].getnewaddress() + privkey = self.nodes[0].dumpprivkey(address) + self.nodes[0].sendtoaddress(address, 0.1) + self.nodes[0].unloadwallet('') + # check importaddress on fresh wallet + self.nodes[0].createwallet('w1', False, True) + self.nodes[0].importaddress(address) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], 0) + assert_equal(self.nodes[0].getbalances()['watchonly']['untrusted_pending'], Decimal('0.1')) + self.nodes[0].importprivkey(privkey) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('0.1')) + assert_equal(self.nodes[0].getbalances()['watchonly']['untrusted_pending'], 0) + self.nodes[0].unloadwallet('w1') + if __name__ == '__main__': WalletTest().main() From 6d3db52e667474b6c0c2e4eeb9fb5b3ba4063205 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Barbosa?= Date: Tue, 12 May 2020 23:34:04 +0100 Subject: [PATCH 3/7] rpc, wallet: Document and test mempool scan after importprivkey co-authored-by: Fabian Jahr --- src/wallet/rpc/backup.cpp | 4 +++- test/functional/wallet_balance.py | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 0b73d444f85..386c0e90508 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -100,11 +100,13 @@ RPCHelpMan importprivkey() "Hint: use importmulti to import more than one private key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported key exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"privkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The private key (see dumpprivkey)"}, {"label", RPCArg::Type::STR, RPCArg::DefaultHint{"current label if address exists, otherwise \"\""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ diff --git a/test/functional/wallet_balance.py b/test/functional/wallet_balance.py index 94b74c0696d..d49bca68557 100755 --- a/test/functional/wallet_balance.py +++ b/test/functional/wallet_balance.py @@ -288,6 +288,10 @@ class WalletTest(BitcoinTestFramework): assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('0.1')) assert_equal(self.nodes[0].getbalances()['watchonly']['untrusted_pending'], 0) self.nodes[0].unloadwallet('w1') + # check importprivkey on fresh wallet + self.nodes[0].createwallet('w2', False, True) + self.nodes[0].importprivkey(privkey) + assert_equal(self.nodes[0].getbalances()['mine']['untrusted_pending'], Decimal('0.1')) if __name__ == '__main__': From e6d3ef85867545a5a66a211e35e818e8a1b166fa Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 1 May 2022 19:45:32 +0200 Subject: [PATCH 4/7] rpc, wallet: Document mempool scan after importpubkey --- src/wallet/rpc/backup.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 386c0e90508..0f058e45edf 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -404,11 +404,13 @@ RPCHelpMan importpubkey() "Hint: use importmulti to import more than one public key.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported pubkey exists but related transactions are still missing, leading to temporarily incorrect/bogus balances and unspent outputs until rescan completes.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"pubkey", RPCArg::Type::STR, RPCArg::Optional::NO, "The hex-encoded public key"}, {"label", RPCArg::Type::STR, RPCArg::Default{""}, "An optional label"}, - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Rescan the wallet for transactions"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions."}, }, RPCResult{RPCResult::Type::NONE, "", ""}, RPCExamples{ From 0e396d1ba701c9ac6280a98bf37f53352167e724 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 1 May 2022 19:53:09 +0200 Subject: [PATCH 5/7] rpc, wallet: Document mempool scan after importmulti --- src/wallet/rpc/backup.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 0f058e45edf..e8a39239eb7 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1255,6 +1255,8 @@ RPCHelpMan importmulti() "Conversely, if all the private keys are provided and the address/script is spendable, the watchonly option must be set to false, or a warning will be returned.\n" "\nNote: This call can take over an hour to complete if rescan is true, during that time, other rpc calls\n" "may report that the imported keys, addresses or scripts exist but related transactions are still missing.\n" + "The rescan parameter can be set to false if the key was never used to create transactions. If it is set to false,\n" + "but the key was used to create transactions, rescanwallet needs to be called with the appropriate block range.\n" "Note: Use \"getwalletinfo\" to query the scanning progress.\n", { {"requests", RPCArg::Type::ARR, RPCArg::Optional::NO, "Data to be imported", @@ -1296,7 +1298,7 @@ RPCHelpMan importmulti() "\"requests\""}, {"options", RPCArg::Type::OBJ, RPCArg::Optional::OMITTED_NAMED_ARG, "", { - {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Stating if should rescan the blockchain after all imports"}, + {"rescan", RPCArg::Type::BOOL, RPCArg::Default{true}, "Scan the chain and mempool for wallet transactions after all imports."}, }, "\"options\""}, }, From 833ce76df712932c19e99737e87b5569e2bca34b Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Sun, 1 May 2022 20:00:10 +0200 Subject: [PATCH 6/7] rpc, wallet: Document mempool rescan after importdescriptor, importwallet --- src/wallet/rpc/backup.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index e8a39239eb7..26c2cccb2f6 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -489,7 +489,7 @@ RPCHelpMan importwallet() { return RPCHelpMan{"importwallet", "\nImports keys from a wallet dump file (see dumpwallet). Requires a new wallet backup to include imported keys.\n" - "Note: Use \"getwalletinfo\" to query the scanning progress.\n", + "Note: Blockchain and Mempool will be rescanned after a successful import. Use \"getwalletinfo\" to query the scanning progress.\n", { {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet file"}, }, @@ -1600,7 +1600,7 @@ RPCHelpMan importdescriptors() " Use the string \"now\" to substitute the current synced blockchain time.\n" " \"now\" can be specified to bypass scanning, for outputs which are known to never have been used, and\n" " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest timestamp\n" - " of all descriptors being imported will be scanned.", + "of all descriptors being imported will be scanned as well as the mempool.", /*oneline_description=*/"", {"timestamp | \"now\"", "integer / string"} }, {"internal", RPCArg::Type::BOOL, RPCArg::Default{false}, "Whether matching outputs should be treated as not incoming payments (e.g. change)"}, From 1be796418934ae7370cb0ed501877db59e738106 Mon Sep 17 00:00:00 2001 From: Fabian Jahr Date: Wed, 1 Jun 2022 01:11:50 +0200 Subject: [PATCH 7/7] test, wallet: Add mempool rescan test for import RPCs --- test/functional/wallet_import_rescan.py | 57 ++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 5 deletions(-) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index d9acc8cea5b..085ad51c799 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -87,6 +87,7 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p assert_equal(len(txs), self.expected_txs) addresses = self.node.listreceivedbyaddress(minconf=0, include_watchonly=True, address_filter=self.address['address']) + if self.expected_txs: assert_equal(len(addresses[0]["txids"]), self.expected_txs) @@ -98,13 +99,18 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p assert_equal(tx["category"], "receive") assert_equal(tx["label"], self.label) assert_equal(tx["txid"], txid) - assert_equal(tx["confirmations"], 1 + current_height - confirmation_height) - assert "trusted" not in tx + + # If no confirmation height is given, the tx is still in the + # mempool. + confirmations = (1 + current_height - confirmation_height) if confirmation_height else 0 + assert_equal(tx["confirmations"], confirmations) + if confirmations: + assert "trusted" not in tx address, = [ad for ad in addresses if txid in ad["txids"]] assert_equal(address["address"], self.address["address"]) assert_equal(address["amount"], self.expected_balance) - assert_equal(address["confirmations"], 1 + current_height - confirmation_height) + assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or # imported private key. The test setup ensures that transaction @@ -162,11 +168,12 @@ class ImportRescanTest(BitcoinTestFramework): self.import_deterministic_coinbase_privkeys() self.stop_nodes() - self.start_nodes() + self.start_nodes(extra_args=[["-whitelist=noban@127.0.0.1"]] * self.num_nodes) for i in range(1, self.num_nodes): self.connect_nodes(i, 0) def run_test(self): + # Create one transaction on node 0 with a unique amount for # each possible type of wallet import RPC. for i, variant in enumerate(IMPORT_VARIANTS): @@ -207,7 +214,7 @@ class ImportRescanTest(BitcoinTestFramework): variant.check() # Create new transactions sending to each address. - for i, variant in enumerate(IMPORT_VARIANTS): + for variant in IMPORT_VARIANTS: variant.sent_amount = get_rand_amount() variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount) self.generate(self.nodes[0], 1) # Generate one block for each send @@ -223,6 +230,46 @@ class ImportRescanTest(BitcoinTestFramework): variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) + self.log.info('Test that the mempool is rescanned as well if the rescan parameter is set to true') + + # The late timestamp and pruned variants are not necessary when testing mempool rescan + mempool_variants = [variant for variant in IMPORT_VARIANTS if variant.rescan != Rescan.late_timestamp and not variant.prune] + # No further blocks are mined so the timestamp will stay the same + timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] + + # Create one transaction on node 0 with a unique amount for + # each possible type of wallet import RPC. + for i, variant in enumerate(mempool_variants): + variant.label = "mempool label {} {}".format(i, variant) + variant.address = self.nodes[1].getaddressinfo(self.nodes[1].getnewaddress( + label=variant.label, + address_type=variant.address_type.value, + )) + variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) + variant.initial_amount = get_rand_amount() + variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) + variant.confirmation_height = 0 + variant.timestamp = timestamp + + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + + # For each variation of wallet key import, invoke the import RPC and + # check the results from getbalance and listtransactions. + for variant in mempool_variants: + self.log.info('Run import for mempool variant {}'.format(variant)) + expect_rescan = variant.rescan == Rescan.yes + variant.node = self.nodes[2 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] + variant.do_import(variant.timestamp) + if expect_rescan: + variant.expected_balance = variant.initial_amount + variant.expected_txs = 1 + variant.check(variant.initial_txid, variant.initial_amount) + else: + variant.expected_balance = 0 + variant.expected_txs = 0 + variant.check() + if __name__ == "__main__": ImportRescanTest().main()