From c3d02be536ac3f35c10efa03653186a17ebbfc12 Mon Sep 17 00:00:00 2001 From: Gloria Zhao Date: Wed, 3 Jan 2024 16:44:12 +0000 Subject: [PATCH 1/2] [test] rescan legacy wallet with reorged parent + IsFromMe child in mempool Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs. --- test/functional/wallet_import_rescan.py | 56 +++++++++++++++++++++---- 1 file changed, 47 insertions(+), 9 deletions(-) diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py index 0ac67607e19..7f01d23941c 100755 --- a/test/functional/wallet_import_rescan.py +++ b/test/functional/wallet_import_rescan.py @@ -20,7 +20,10 @@ happened previously. """ from test_framework.test_framework import BitcoinTestFramework -from test_framework.address import AddressType +from test_framework.address import ( + AddressType, + ADDRESS_BCRT1_UNSPENDABLE, +) from test_framework.util import ( assert_equal, set_node_times, @@ -109,7 +112,7 @@ class Variant(collections.namedtuple("Variant", "call data address_type rescan p 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["amount"], self.amount_received) assert_equal(address["confirmations"], confirmations) # Verify the transaction is correctly marked watchonly depending on # whether the transaction pays to an imported public key or @@ -223,11 +226,11 @@ class ImportRescanTest(BitcoinTestFramework): 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.amount_received = variant.initial_amount variant.expected_txs = 1 variant.check(variant.initial_txid, variant.initial_amount, variant.confirmation_height) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() @@ -247,7 +250,7 @@ class ImportRescanTest(BitcoinTestFramework): # Check the latest results from getbalance and listtransactions. for variant in IMPORT_VARIANTS: self.log.info('Run check for variant {}'.format(variant)) - variant.expected_balance += variant.sent_amount + variant.amount_received += variant.sent_amount variant.expected_txs += 1 variant.check(variant.sent_txid, variant.sent_amount, variant.confirmation_height) @@ -267,13 +270,44 @@ class ImportRescanTest(BitcoinTestFramework): address_type=variant.address_type.value, )) variant.key = self.nodes[1].dumpprivkey(variant.address["address"]) - variant.initial_amount = get_rand_amount() + variant.initial_amount = get_rand_amount() * 2 variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) variant.confirmation_height = 0 variant.timestamp = timestamp + # Mine a block so these parents are confirmed assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) self.sync_mempools() + block_to_disconnect = self.generate(self.nodes[0], 1)[0] + assert_equal(len(self.nodes[0].getrawmempool()), 0) + + # For each variant, create an unconfirmed child transaction from initial_txid, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + # An equivalent test for descriptors exists in wallet_rescan_unconfirmed.py. + unspent_txid_map = {txin["txid"] : txin for txin in self.nodes[1].listunspent()} + for variant in mempool_variants: + # Send full amount, subtracting fee from outputs, to ensure no change is created. + child = self.nodes[1].send( + add_to_wallet=False, + inputs=[unspent_txid_map[variant.initial_txid]], + outputs=[{ADDRESS_BCRT1_UNSPENDABLE : variant.initial_amount}], + subtract_fee_from_outputs=[0] + ) + variant.child_txid = child["txid"] + variant.amount_received = 0 + self.nodes[0].sendrawtransaction(child["hex"]) + + # Mempools should contain the child transactions for each variant. + assert_equal(len(self.nodes[0].getrawmempool()), len(mempool_variants)) + self.sync_mempools() + + # Mock a reorg so the parent transactions are added back to the mempool + for node in self.nodes: + node.invalidateblock(block_to_disconnect) + # Mempools should now contain the parent and child for each variant. + assert_equal(len(node.getrawmempool()), 2 * len(mempool_variants)) # For each variation of wallet key import, invoke the import RPC and # check the results from getbalance and listtransactions. @@ -283,11 +317,15 @@ class ImportRescanTest(BitcoinTestFramework): 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 + # Ensure both transactions were rescanned. This would raise a JSONRPCError if the + # transactions were not identified as belonging to the wallet. + assert_equal(variant.node.gettransaction(variant.initial_txid)['confirmations'], 0) + assert_equal(variant.node.gettransaction(variant.child_txid)['confirmations'], 0) + variant.amount_received = variant.initial_amount variant.expected_txs = 1 - variant.check(variant.initial_txid, variant.initial_amount) + variant.check(variant.initial_txid, variant.initial_amount, 0) else: - variant.expected_balance = 0 + variant.amount_received = 0 variant.expected_txs = 0 variant.check() From df30247705940c50c5eaafd74e2abbeb8b0cec07 Mon Sep 17 00:00:00 2001 From: glozow Date: Tue, 9 Jan 2024 10:03:20 +0000 Subject: [PATCH 2/2] [test] import descriptor wallet with reorged parent + IsFromMe child in mempool Test that wallet rescans process transactions topologically, even if a parent's entry into the mempool is later than that of its child. This behavior is important because IsFromMe requires the ability to look up a transaction's inputs. Co-authored-by: furszy --- test/functional/test_runner.py | 1 + test/functional/wallet_rescan_unconfirmed.py | 83 ++++++++++++++++++++ 2 files changed, 84 insertions(+) create mode 100755 test/functional/wallet_rescan_unconfirmed.py diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index 2460b2e3e65..c6ccff1847e 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -335,6 +335,7 @@ BASE_SCRIPTS = [ 'wallet_create_tx.py --descriptors', 'wallet_inactive_hdchains.py --legacy-wallet', 'wallet_spend_unconfirmed.py', + 'wallet_rescan_unconfirmed.py --descriptors', 'p2p_fingerprint.py', 'feature_uacomment.py', 'feature_init.py', diff --git a/test/functional/wallet_rescan_unconfirmed.py b/test/functional/wallet_rescan_unconfirmed.py new file mode 100755 index 00000000000..ad9fa081c21 --- /dev/null +++ b/test/functional/wallet_rescan_unconfirmed.py @@ -0,0 +1,83 @@ +#!/usr/bin/env python3 +# Copyright (c) 2024 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Test that descriptor wallets rescan mempool transactions properly when importing.""" + +from test_framework.address import ( + address_to_scriptpubkey, + ADDRESS_BCRT1_UNSPENDABLE, +) +from test_framework.messages import COIN +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal +from test_framework.wallet import MiniWallet +from test_framework.wallet_util import test_address + + +class WalletRescanUnconfirmed(BitcoinTestFramework): + def add_options(self, parser): + self.add_wallet_options(parser, legacy=False) + + def set_test_params(self): + self.num_nodes = 1 + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + self.skip_if_no_sqlite() + + def run_test(self): + self.log.info("Create wallets and mine initial chain") + node = self.nodes[0] + tester_wallet = MiniWallet(node) + + node.createwallet(wallet_name='w0', disable_private_keys=False) + w0 = node.get_wallet_rpc('w0') + + self.log.info("Create a parent tx and mine it in a block that will later be disconnected") + parent_address = w0.getnewaddress() + tx_parent_to_reorg = tester_wallet.send_to( + from_node=node, + scriptPubKey=address_to_scriptpubkey(parent_address), + amount=COIN, + ) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + block_to_reorg = self.generate(tester_wallet, 1)[0] + assert_equal(len(node.getrawmempool()), 0) + node.syncwithvalidationinterfacequeue() + assert_equal(w0.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 1) + + # Create an unconfirmed child transaction from the parent tx, sending all + # the funds to an unspendable address. Importantly, no change output is created so the + # transaction can't be recognized using its outputs. The wallet rescan needs to know the + # inputs of the transaction to detect it, so the parent must be processed before the child. + w0_utxos = w0.listunspent() + + self.log.info("Create a child tx and wait for it to propagate to all mempools") + # The only UTXO available to spend is tx_parent_to_reorg. + assert_equal(len(w0_utxos), 1) + assert_equal(w0_utxos[0]["txid"], tx_parent_to_reorg["txid"]) + tx_child_unconfirmed_sweep = w0.sendall([ADDRESS_BCRT1_UNSPENDABLE]) + assert tx_child_unconfirmed_sweep["txid"] in node.getrawmempool() + node.syncwithvalidationinterfacequeue() + + self.log.info("Mock a reorg, causing parent to re-enter mempools after its child") + node.invalidateblock(block_to_reorg) + assert tx_parent_to_reorg["txid"] in node.getrawmempool() + + self.log.info("Import descriptor wallet on another node") + descriptors_to_import = [{"desc": w0.getaddressinfo(parent_address)['parent_desc'], "timestamp": 0, "label": "w0 import"}] + + node.createwallet(wallet_name="w1", disable_private_keys=True) + w1 = node.get_wallet_rpc("w1") + w1.importdescriptors(descriptors_to_import) + + self.log.info("Check that the importing node has properly rescanned mempool transactions") + # Check that parent address is correctly determined as ismine + test_address(w1, parent_address, solvable=True, ismine=True) + # This would raise a JSONRPCError if the transactions were not identified as belonging to the wallet. + assert_equal(w1.gettransaction(tx_parent_to_reorg["txid"])["confirmations"], 0) + assert_equal(w1.gettransaction(tx_child_unconfirmed_sweep["txid"])["confirmations"], 0) + +if __name__ == '__main__': + WalletRescanUnconfirmed().main()