Merge bitcoin/bitcoin#25351: rpc, wallet: Scan mempool after import* - Second attempt

1be7964189 test, wallet: Add mempool rescan test for import RPCs (Fabian Jahr)
833ce76df7 rpc, wallet: Document mempool rescan after importdescriptor, importwallet (Fabian Jahr)
0e396d1ba7 rpc, wallet: Document mempool scan after importmulti (Fabian Jahr)
e6d3ef8586 rpc, wallet: Document mempool scan after importpubkey (Fabian Jahr)
6d3db52e66 rpc, wallet: Document and test mempool scan after importprivkey (João Barbosa)
3abdbbb90a rpc, wallet: Document and test mempool scan after importaddress (João Barbosa)
236239bd40 wallet: Rescan mempool for transactions as well (Fabian Jahr)

Pull request description:

  This PR picks up the work from #18964 and closes #18954.

  It should incorporate all the unaddressed feedback from the PR:
  - Mempool rescan now expanded to all relevant import* RPCs
  - Added documentation in the help of each RPC
  - More tests

ACKs for top commit:
  Sjors:
    re-utACK 1be7964189 (only a test change)
  achow101:
    ACK 1be7964189
  w0xlt:
    reACK 1be7964189

Tree-SHA512: b62fed5f97c6c242b2af417b41c9696a1f18878483d9e1c9429791f9c05257f57a00540a9a84df23c49faf6a61c3109c22972de81540083f38b506217804fcc5
This commit is contained in:
Andrew Chow 2022-07-18 14:15:06 -04:00
commit 4aaa3b5200
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
6 changed files with 99 additions and 17 deletions

View file

@ -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{
@ -201,6 +203,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"
@ -210,7 +214,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, "", ""},
@ -401,11 +405,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{
@ -484,7 +490,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"},
},
@ -1250,6 +1256,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",
@ -1291,7 +1299,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\""},
},
@ -1593,7 +1601,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)"},

View file

@ -53,9 +53,6 @@ static const std::shared_ptr<CWallet> 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;
}
@ -768,6 +765,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);
@ -780,6 +778,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);
@ -803,7 +803,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);

View file

@ -1678,7 +1678,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.
@ -1799,6 +1800,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);

View file

@ -273,6 +273,26 @@ 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')
# 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__':
WalletTest().main()

View file

@ -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()

View file

@ -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