diff --git a/doc/release-notes-pr13381.md b/doc/release-notes-pr13381.md new file mode 100644 index 0000000000000..75faad99060f5 --- /dev/null +++ b/doc/release-notes-pr13381.md @@ -0,0 +1,29 @@ +RPC importprivkey: new label behavior +------------------------------------- + +Previously, `importprivkey` automatically added the default empty label +("") to all addresses associated with the imported private key. Now it +defaults to using any existing label for those addresses. For example: + +- Old behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label is changed from "cold wallet" + to "". + +- New behavior: you import a watch-only address with the label "cold + wallet". Later, you import the corresponding private key using the + default settings. The address's label remains "cold wallet". + +In both the previous and current case, if you directly specify a label +during the import, that label will override whatever previous label the +addresses may have had. Also in both cases, if none of the addresses +previously had a label, they will still receive the default empty label +(""). Examples: + +- You import a watch-only address with the label "temporary". Later you + import the corresponding private key with the label "final". The + address's label will be changed to "final". + +- You use the default settings to import a private key for an address that + was not previously in the wallet. Its addresses will receive the default + empty label (""). diff --git a/src/chainparams.cpp b/src/chainparams.cpp index e4276ee49768b..38c9355b030c9 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -814,10 +814,10 @@ class CRegTestParams : public CChainParams { consensus.nGovernanceMinQuorum = 1; consensus.nGovernanceFilterElements = 100; consensus.nMasternodeMinimumConfirmations = 1; - consensus.BIP34Height = 100000000; // BIP34 has not activated on regtest (far in the future so block v1 are not rejected in tests) + consensus.BIP34Height = 500; // BIP34 activated on regtest (Used in functional tests) consensus.BIP34Hash = uint256(); - consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in rpc activation tests) - consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in rpc activation tests) + consensus.BIP65Height = 1351; // BIP65 activated on regtest (Used in functional tests) + consensus.BIP66Height = 1251; // BIP66 activated on regtest (Used in functional tests) consensus.DIP0001Height = 2000; consensus.DIP0003Height = 432; consensus.DIP0003EnforcementHeight = 500; diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 4c72fa4f69607..08ba78b0b955a 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1856,7 +1856,7 @@ static UniValue reconsiderblock(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( RPCHelpMan{"reconsiderblock", - "\nRemoves invalidity status of a block and its descendants, reconsider them for activation.\n" + "\nRemoves invalidity status of a block, its ancestors and its descendants, reconsider them for activation.\n" "This can be used to undo the effects of invalidateblock.\n", { {"blockhash", RPCArg::Type::STR_HEX, RPCArg::Optional::NO, "the hash of the block to reconsider"}, diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index bf97ddf247f3a..6b15fc1801b3a 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -1,5 +1,5 @@ // Copyright (c) 2010 Satoshi Nakamoto -// Copyright (c) 2009-2018 The Bitcoin Core developers +// Copyright (c) 2009-2019 The Bitcoin Core developers // Copyright (c) 2014-2021 The Dash Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -54,7 +54,7 @@ struct RPCCommandExecution explicit RPCCommandExecution(const std::string& method) { LOCK(g_rpc_server_info.mutex); - it = g_rpc_server_info.active_commands.insert(g_rpc_server_info.active_commands.cend(), {method, GetTimeMicros()}); + it = g_rpc_server_info.active_commands.insert(g_rpc_server_info.active_commands.end(), {method, GetTimeMicros()}); } ~RPCCommandExecution() { diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index c73bfb9d521b4..6a4b49005b6bd 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -157,7 +157,10 @@ UniValue importprivkey(const JSONRPCRequest& request) CKeyID vchAddress = pubkey.GetID(); { pwallet->MarkDirty(); - pwallet->SetAddressBook(vchAddress, strLabel, "receive"); + + if (!request.params[1].isNull() || pwallet->mapAddressBook.count(vchAddress) == 0) { + pwallet->SetAddressBook(vchAddress, strLabel, "receive"); + } // Don't throw error in case a key is already there if (pwallet->HaveKey(vchAddress)) { @@ -1493,7 +1496,9 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) if (mainRequest.fHelp || mainRequest.params.size() < 1 || mainRequest.params.size() > 2) throw std::runtime_error( RPCHelpMan{"importmulti", - "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), rescanning all addresses in one-shot-only (rescan can be disabled via options). Requires a new wallet backup.\n" + "\nImport addresses/scripts (with private or public keys, redeem script (P2SH)), optionally rescanning the blockchain from the earliest creation time of the imported scripts. Requires a new wallet backup.\n" + "If an address/script is imported without all of the private keys required to spend from that address, it will be watchonly. The 'watchonly' option must be set to true in this case or a warning will be returned.\n" + "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 exists but related transactions are still missing.\n", { @@ -1527,7 +1532,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) }, {"range", RPCArg::Type::RANGE, RPCArg::Optional::OMITTED, "If a ranged descriptor is used, this specifies the end or the range (in the form [begin,end]) to import"}, {"internal", RPCArg::Type::BOOL, /* default */ "false", "Stating whether matching outputs should be treated as not incoming payments (also known as change)"}, - {"watchonly", RPCArg::Type::BOOL, /* default */ "false", "Stating whether matching outputs should be considered watched even when not all private keys are provided."}, + {"watchonly", RPCArg::Type::BOOL, /* default */ "false", "Stating whether matching outputs should be considered watchonly."}, {"label", RPCArg::Type::STR, /* default */ "''", "Label to assign to the address, only allowed with internal=false"}, {"keypool", RPCArg::Type::BOOL, /* default */ "false", "Stating whether imported public keys should be added to the keypool for when users request new addresses. Only allowed when wallet private keys are disabled"}, }, diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 09a355b1913c5..2578148503da3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -3719,8 +3719,7 @@ UniValue getaddressinfo(const JSONRPCRequest& request) " ,...\n" " ]\n" " \"sigsrequired\" : xxxxx (numeric, optional) Number of signatures required to spend multisig output (only if \"script\" is \"multisig\")\n" - " \"pubkey\" : \"publickeyhex\", (string, optional) The hex value of the raw public key, for single-key addresses (possibly embedded in P2SH)\n" - " \"embedded\" : {...}, (object, optional) Information about the address embedded in P2SH, if relevant and known. It includes all getaddressinfo output fields for the embedded address, excluding metadata (\"timestamp\", \"hdkeypath\") and relation to the wallet (\"ismine\", \"iswatchonly\").\n" + " \"pubkey\" : \"publickeyhex\", (string, optional) The hex value of the raw public key, for single-key addresses\n" " \"iscompressed\" : true|false, (boolean, optional) If the pubkey is compressed\n" " \"label\" : \"label\" (string) The label associated with the address, \"\" is the default label\n" " \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n" @@ -4023,7 +4022,7 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request) return NullUniValue; } - if (request.fHelp || request.params.size() < 2 || request.params.size() > 6) + if (request.fHelp || request.params.size() < 2 || request.params.size() > 5) throw std::runtime_error( RPCHelpMan{"walletcreatefundedpsbt", "\nCreates and funds a transaction in the Partially Signed Transaction format. Inputs will be added if supplied inputs are not enough\n" diff --git a/test/functional/feature_block.py b/test/functional/feature_block.py index 238acc4492083..cc854cad7ca14 100755 --- a/test/functional/feature_block.py +++ b/test/functional/feature_block.py @@ -1250,7 +1250,7 @@ def run_test(self): blocks = [] spend = out[32] for i in range(89, LARGE_REORG_SIZE + 89): - b = self.next_block(i, spend) + b = self.next_block(i, spend, version=4) tx = CTransaction() script_length = MAX_BLOCK_SIZE - len(b.serialize()) - 69 script_output = CScript([b'\x00' * script_length]) @@ -1269,20 +1269,32 @@ def run_test(self): self.move_tip(88) blocks2 = [] for i in range(89, LARGE_REORG_SIZE + 89): - blocks2.append(self.next_block("alt" + str(i))) + blocks2.append(self.next_block("alt" + str(i), version=4)) self.send_blocks(blocks2, False, force_send=True) # extend alt chain to trigger re-org - block = self.next_block("alt" + str(chain1_tip + 1)) + block = self.next_block("alt" + str(chain1_tip + 1), version=4) self.send_blocks([block], True, timeout=960) # ... and re-org back to the first chain self.move_tip(chain1_tip) - block = self.next_block(chain1_tip + 1) + block = self.next_block(chain1_tip + 1, version=4) self.send_blocks([block], False, force_send=True) - block = self.next_block(chain1_tip + 2) + block = self.next_block(chain1_tip + 2, version=4) self.send_blocks([block], True, timeout=960) + self.log.info("Reject a block with an invalid block header version") + b_v1 = self.next_block('b_v1', version=1) + self.send_blocks([b_v1], success=False, force_send=True, reject_reason='bad-version(0x00000001)') + + self.move_tip(chain1_tip + 2) + b_cb34 = self.next_block('b_cb34', version=4) + b_cb34.vtx[0].vin[0].scriptSig = b_cb34.vtx[0].vin[0].scriptSig[:-1] + b_cb34.vtx[0].rehash() + b_cb34.hashMerkleRoot = b_cb34.calc_merkle_root() + b_cb34.solve() + self.send_blocks([b_cb34], success=False, reject_reason='bad-cb-height', reconnect=True) + # Helper methods ################ @@ -1310,7 +1322,7 @@ def create_and_sign_transaction(self, spend_tx, value, script=CScript([OP_TRUE]) tx.rehash() return tx - def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), solve=True): + def next_block(self, number, spend=None, additional_coinbase_value=0, script=CScript([OP_TRUE]), solve=True, *, version=1): if self.tip is None: base_block_hash = self.genesis_hash block_time = self.mocktime + 1 @@ -1323,11 +1335,11 @@ def next_block(self, number, spend=None, additional_coinbase_value=0, script=CSc coinbase.vout[0].nValue += additional_coinbase_value coinbase.rehash() if spend is None: - block = create_block(base_block_hash, coinbase, block_time) + block = create_block(base_block_hash, coinbase, block_time, version=version) else: coinbase.vout[0].nValue += spend.vout[0].nValue - 1 # all but one satoshi to fees coinbase.rehash() - block = create_block(base_block_hash, coinbase, block_time) + block = create_block(base_block_hash, coinbase, block_time, version=version) tx = self.create_tx(spend, 0, 1, script) # spend 1 satoshi self.sign_tx(tx, spend) self.add_transactions_to_block(block, [tx]) diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index 4066f041d3c9c..6188da0df763b 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -4,10 +4,14 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Test the invalidateblock RPC.""" -import time - from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, connect_nodes, wait_until +from test_framework.address import ADDRESS_BCRT1_UNSPENDABLE +from test_framework.util import ( + assert_equal, + connect_nodes, + wait_until, +) + class InvalidateTest(BitcoinTestFramework): def set_test_params(self): @@ -21,46 +25,41 @@ def run_test(self): self.log.info("Make sure we repopulate setBlockIndexCandidates after InvalidateBlock:") self.log.info("Mine 4 blocks on Node 0") self.nodes[0].generatetoaddress(4, self.nodes[0].get_deterministic_priv_key().address) - assert self.nodes[0].getblockcount() == 4 - besthash = self.nodes[0].getbestblockhash() + assert_equal(self.nodes[0].getblockcount(), 4) + besthash_n0 = self.nodes[0].getbestblockhash() self.log.info("Mine competing 6 blocks on Node 1") self.nodes[1].generatetoaddress(6, self.nodes[1].get_deterministic_priv_key().address) - assert self.nodes[1].getblockcount() == 6 + assert_equal(self.nodes[1].getblockcount(), 6) self.log.info("Connect nodes to force a reorg") connect_nodes(self.nodes[0], 1) self.sync_blocks(self.nodes[0:2]) - assert self.nodes[0].getblockcount() == 6 + assert_equal(self.nodes[0].getblockcount(), 6) badhash = self.nodes[1].getblockhash(2) self.log.info("Invalidate block 2 on node 0 and verify we reorg to node 0's original chain") self.nodes[0].invalidateblock(badhash) - newheight = self.nodes[0].getblockcount() - newhash = self.nodes[0].getbestblockhash() - if (newheight != 4 or newhash != besthash): - raise AssertionError("Wrong tip for node0, hash %s, height %d"%(newhash,newheight)) + assert_equal(self.nodes[0].getblockcount(), 4) + assert_equal(self.nodes[0].getbestblockhash(), besthash_n0) self.log.info("Make sure we won't reorg to a lower work chain:") - connect_nodes(self.nodes[1], 2) + connect_nodes(self.nodes[ 1], 2) self.log.info("Sync node 2 to node 1 so both have 6 blocks") self.sync_blocks(self.nodes[1:3]) - assert self.nodes[2].getblockcount() == 6 + assert_equal(self.nodes[2].getblockcount(), 6) self.log.info("Invalidate block 5 on node 1 so its tip is now at 4") self.nodes[1].invalidateblock(self.nodes[1].getblockhash(5)) - assert self.nodes[1].getblockcount() == 4 + assert_equal(self.nodes[1].getblockcount(), 4) self.log.info("Invalidate block 3 on node 2, so its tip is now 2") self.nodes[2].invalidateblock(self.nodes[2].getblockhash(3)) - assert self.nodes[2].getblockcount() == 2 + assert_equal(self.nodes[2].getblockcount(), 2) self.log.info("..and then mine a block") self.nodes[2].generatetoaddress(1, self.nodes[2].get_deterministic_priv_key().address) self.log.info("Verify all nodes are at the right height") - time.sleep(5) - assert_equal(self.nodes[2].getblockcount(), 3) - assert_equal(self.nodes[0].getblockcount(), 4) - node1height = self.nodes[1].getblockcount() - if node1height < 4: - raise AssertionError("Node 1 reorged to a lower height: %d"%node1height) + wait_until(lambda: self.nodes[2].getblockcount() == 3, timeout=5) + wait_until(lambda: self.nodes[0].getblockcount() == 4, timeout=5) + wait_until(lambda: self.nodes[1].getblockcount() == 4, timeout=5) self.log.info("Make sure ResetBlockFailureFlags does the job correctly") self.restart_node(0, extra_args=["-checkblocks=5"]) @@ -88,6 +87,30 @@ def run_test(self): wait_until(lambda: self.nodes[1].getblockcount() == newheight + 20) assert_equal(tip, self.nodes[1].getbestblockhash()) + self.log.info("Verify that we reconsider all ancestors as well") + blocks = self.nodes[1].generatetoaddress(10, ADDRESS_BCRT1_UNSPENDABLE) + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + # Invalidate the two blocks at the tip + self.nodes[1].invalidateblock(blocks[-1]) + self.nodes[1].invalidateblock(blocks[-2]) + assert_equal(self.nodes[1].getbestblockhash(), blocks[-3]) + # Reconsider only the previous tip + self.nodes[1].reconsiderblock(blocks[-1]) + # Should be back at the tip by now + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + + self.log.info("Verify that we reconsider all descendants") + blocks = self.nodes[1].generatetoaddress(10, ADDRESS_BCRT1_UNSPENDABLE) + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + # Invalidate the two blocks at the tip + self.nodes[1].invalidateblock(blocks[-2]) + self.nodes[1].invalidateblock(blocks[-4]) + assert_equal(self.nodes[1].getbestblockhash(), blocks[-5]) + # Reconsider only the previous tip + self.nodes[1].reconsiderblock(blocks[-4]) + # Should be back at the tip by now + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + if __name__ == '__main__': InvalidateTest().main() diff --git a/test/functional/test_framework/blocktools.py b/test/functional/test_framework/blocktools.py index 694775aac1573..3877e508adf18 100644 --- a/test/functional/test_framework/blocktools.py +++ b/test/functional/test_framework/blocktools.py @@ -22,9 +22,10 @@ # Genesis block time (regtest) TIME_GENESIS_BLOCK = 1417713337 -def create_block(hashprev, coinbase, ntime=None): +def create_block(hashprev, coinbase, ntime=None, *, version=1): """Create a block (with regtest difficulty).""" block = CBlock() + block.nVersion = version if ntime is None: import time block.nTime = int(time.time() + 600) diff --git a/test/functional/test_framework/wallet_util.py b/test/functional/test_framework/wallet_util.py new file mode 100755 index 0000000000000..49e6af0dbad34 --- /dev/null +++ b/test/functional/test_framework/wallet_util.py @@ -0,0 +1,74 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. +"""Useful util functions for testing the wallet""" +from collections import namedtuple + +from test_framework.address import ( + key_to_p2pkh, + script_to_p2sh, +) +from test_framework.script import ( + CScript, + OP_2, + OP_3, + OP_CHECKMULTISIG, + OP_CHECKSIG, + OP_DUP, + OP_EQUAL, + OP_EQUALVERIFY, + OP_HASH160, + hash160, +) +from test_framework.util import hex_str_to_bytes + +Key = namedtuple('Key', ['privkey', + 'pubkey', + 'p2pkh_script', + 'p2pkh_addr']) + +Multisig = namedtuple('Multisig', ['privkeys', + 'pubkeys', + 'p2sh_script', + 'p2sh_addr', + 'redeem_script']) + +def get_key(node): + """Generate a fresh key on node + + Returns a named tuple of privkey, pubkey and all address and scripts.""" + addr = node.getnewaddress() + pubkey = node.getaddressinfo(addr)['pubkey'] + pkh = hash160(hex_str_to_bytes(pubkey)) + return Key(privkey=node.dumpprivkey(addr), + pubkey=pubkey, + p2pkh_script=CScript([OP_DUP, OP_HASH160, pkh, OP_EQUALVERIFY, OP_CHECKSIG]).hex(), + p2pkh_addr=key_to_p2pkh(pubkey)) + +def get_multisig(node): + """Generate a fresh 2-of-3 multisig on node + + Returns a named tuple of privkeys, pubkeys and all address and scripts.""" + addrs = [] + pubkeys = [] + for _ in range(3): + addr = node.getaddressinfo(node.getnewaddress()) + addrs.append(addr['address']) + pubkeys.append(addr['pubkey']) + script_code = CScript([OP_2] + [hex_str_to_bytes(pubkey) for pubkey in pubkeys] + [OP_3, OP_CHECKMULTISIG]) + return Multisig(privkeys=[node.dumpprivkey(addr) for addr in addrs], + pubkeys=pubkeys, + p2sh_script=CScript([OP_HASH160, hash160(script_code), OP_EQUAL]).hex(), + p2sh_addr=script_to_p2sh(script_code), + redeem_script=script_code.hex()) + +def test_address(node, address, **kwargs): + """Get address info for `address` and test whether the returned values are as expected.""" + addr_info = node.getaddressinfo(address) + for key, value in kwargs.items(): + if value is None: + if key in addr_info.keys(): + raise AssertionError("key {} unexpectedly returned in getaddressinfo.".format(key)) + elif addr_info[key] != value: + raise AssertionError("key {} value {} did not match expected value {}".format(key, addr_info[key], value)) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index bcf8d2884425e..8e35753bc7850 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -185,6 +185,7 @@ 'mempool_accept.py', 'mempool_expiry.py', 'wallet_import_rescan.py', + 'wallet_import_with_label.py', 'rpc_bind.py --ipv4', 'rpc_bind.py --ipv6', 'rpc_bind.py --nonloopback', diff --git a/test/functional/wallet_import_with_label.py b/test/functional/wallet_import_with_label.py new file mode 100755 index 0000000000000..5ec3912387b40 --- /dev/null +++ b/test/functional/wallet_import_with_label.py @@ -0,0 +1,129 @@ +#!/usr/bin/env python3 +# Copyright (c) 2018 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 the behavior of RPC importprivkey on set and unset labels of +addresses. + +It tests different cases in which an address is imported with importaddress +with or without a label and then its private key is imported with importprivkey +with and without a label. +""" + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.wallet_util import test_address + + +class ImportWithLabel(BitcoinTestFramework): + def set_test_params(self): + self.num_nodes = 2 + self.setup_clean_chain = True + + def skip_test_if_missing_module(self): + self.skip_if_no_wallet() + + def run_test(self): + """Main test logic""" + + self.log.info( + "Test importaddress with label and importprivkey without label." + ) + self.log.info("Import a watch-only address with a label.") + address = self.nodes[0].getnewaddress() + label = "Test Label" + self.nodes[1].importaddress(address, label) + test_address(self.nodes[1], + address, + iswatchonly=True, + ismine=False, + label=label) + + self.log.info( + "Import the watch-only address's private key without a " + "label and the address should keep its label." + ) + priv_key = self.nodes[0].dumpprivkey(address) + self.nodes[1].importprivkey(priv_key) + + test_address(self.nodes[1], + address, + label=label) + + self.log.info( + "Test importaddress without label and importprivkey with label." + ) + self.log.info("Import a watch-only address without a label.") + address2 = self.nodes[0].getnewaddress() + self.nodes[1].importaddress(address2) + test_address(self.nodes[1], + address2, + iswatchonly=True, + ismine=False, + label="") + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key2 = self.nodes[0].dumpprivkey(address2) + label2 = "Test Label 2" + self.nodes[1].importprivkey(priv_key2, label2) + + test_address(self.nodes[1], + address2, + label=label2) + + self.log.info("Test importaddress with label and importprivkey with label.") + self.log.info("Import a watch-only address with a label.") + address3 = self.nodes[0].getnewaddress() + label3_addr = "Test Label 3 for importaddress" + self.nodes[1].importaddress(address3, label3_addr) + test_address(self.nodes[1], + address3, + iswatchonly=True, + ismine=False, + label=label3_addr) + + self.log.info( + "Import the watch-only address's private key with a " + "label and the address should have its label updated." + ) + priv_key3 = self.nodes[0].dumpprivkey(address3) + label3_priv = "Test Label 3 for importprivkey" + self.nodes[1].importprivkey(priv_key3, label3_priv) + + test_address(self.nodes[1], + address3, + label=label3_priv) + + self.log.info( + "Test importprivkey won't label new dests with the same " + "label as others labeled dests for the same key." + ) + self.log.info("Import a watch-only legacy address with a label.") + address4 = self.nodes[0].getnewaddress() + label4_addr = "Test Label 4 for importaddress" + self.nodes[1].importaddress(address4, label4_addr) + test_address(self.nodes[1], + address4, + iswatchonly=True, + ismine=False, + label=label4_addr) + + self.log.info( + "Import the watch-only address's private key without a " + "label and new destinations for the key should have an " + "empty label while the 'old' destination should keep " + "its label." + ) + priv_key4 = self.nodes[0].dumpprivkey(address4) + self.nodes[1].importprivkey(priv_key4) + test_address(self.nodes[1], + address4, + label=label4_addr) + + self.stop_nodes() + + +if __name__ == "__main__": + ImportWithLabel().main() diff --git a/test/functional/wallet_importmulti.py b/test/functional/wallet_importmulti.py index 9371c0f70e229..b325cdf3435ff 100755 --- a/test/functional/wallet_importmulti.py +++ b/test/functional/wallet_importmulti.py @@ -14,24 +14,10 @@ success, and (if unsuccessful) test the error code and error message returned. - `test_address()` is called to call getaddressinfo for an address on node1 and test the values returned.""" -from collections import namedtuple -from test_framework.address import ( - key_to_p2pkh, - script_to_p2sh, -) from test_framework.script import ( CScript, - OP_2, - OP_3, - OP_CHECKMULTISIG, - OP_CHECKSIG, - OP_DUP, - OP_EQUAL, - OP_EQUALVERIFY, - OP_HASH160, OP_NOP, - hash160, ) from test_framework.test_framework import BitcoinTestFramework from test_framework.descriptors import descsum_create @@ -40,18 +26,11 @@ assert_greater_than, assert_raises_rpc_error, ) - -Key = namedtuple('Key', ['privkey', - 'pubkey', - 'p2pkh_script', - 'p2pkh_addr']) - -Multisig = namedtuple('Multisig', ['privkeys', - 'pubkeys', - 'p2sh_script', - 'p2sh_addr', - 'redeem_script']) - +from test_framework.wallet_util import ( + get_key, + get_multisig, + test_address, +) class ImportMultiTest(BitcoinTestFramework): def set_test_params(self): @@ -65,35 +44,6 @@ def skip_test_if_missing_module(self): def setup_network(self): self.setup_nodes() - def get_key(self): - """Generate a fresh key on node0 - - Returns a named tuple of privkey, pubkey and all address and scripts.""" - addr = self.nodes[0].getnewaddress() - pubkey = self.nodes[0].getaddressinfo(addr)['pubkey'] - pkh = hash160(bytes.fromhex(pubkey)) - return Key(self.nodes[0].dumpprivkey(addr), - pubkey, - CScript([OP_DUP, OP_HASH160, pkh, OP_EQUALVERIFY, OP_CHECKSIG]).hex(), # p2pkh - key_to_p2pkh(pubkey)) # p2pkh addr - - def get_multisig(self): - """Generate a fresh multisig on node0 - - Returns a named tuple of privkeys, pubkeys and all address and scripts.""" - addrs = [] - pubkeys = [] - for _ in range(3): - addr = self.nodes[0].getaddressinfo(self.nodes[0].getnewaddress()) - addrs.append(addr['address']) - pubkeys.append(addr['pubkey']) - script_code = CScript([OP_2] + [bytes.fromhex(pubkey) for pubkey in pubkeys] + [OP_3, OP_CHECKMULTISIG]) - return Multisig([self.nodes[0].dumpprivkey(addr) for addr in addrs], - pubkeys, - CScript([OP_HASH160, hash160(script_code), OP_EQUAL]).hex(), # p2sh - script_to_p2sh(script_code), # p2sh addr - script_code.hex()) # redeem script - def test_importmulti(self, req, success, error_code=None, error_message=None, warnings=[]): """Run importmulti and assert success""" result = self.nodes[1].importmulti([req]) @@ -106,16 +56,6 @@ def test_importmulti(self, req, success, error_code=None, error_message=None, wa assert_equal(result[0]['error']['code'], error_code) assert_equal(result[0]['error']['message'], error_message) - def test_address(self, address, **kwargs): - """Get address info for `address` and test whether the returned values are as expected.""" - addr_info = self.nodes[1].getaddressinfo(address) - for key, value in kwargs.items(): - if value is None: - if key in addr_info.keys(): - raise AssertionError("key {} unexpectedly returned in getaddressinfo.".format(key)) - elif addr_info[key] != value: - raise AssertionError("key {} value {} did not match expected value {}".format(key, addr_info[key], value)) - def run_test(self): self.log.info("Mining blocks...") self.nodes[0].generate(1) @@ -140,177 +80,178 @@ def run_test(self): # Bitcoin Address (implicit non-internal) self.log.info("Should import an address") - key = self.get_key() - address = key.p2pkh_addr - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now"}, - True) - self.test_address(address, - iswatchonly=True, - ismine=False, - timestamp=timestamp, - ischange=False) - watchonly_address = address + success=True) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + timestamp=timestamp, + ischange=False) + watchonly_address = key.p2pkh_addr watchonly_timestamp = timestamp self.log.info("Should not import an invalid address") self.test_importmulti({"scriptPubKey": {"address": "not valid address"}, "timestamp": "now"}, - False, + success=False, error_code=-5, error_message='Invalid address \"not valid address\"') # ScriptPubKey + internal self.log.info("Should import a scriptPubKey with internal flag") - key = self.get_key() + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "internal": True}, - True) - self.test_address(key.p2pkh_addr, - iswatchonly=True, - ismine=False, - timestamp=timestamp, - ischange=True) + success=True) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + timestamp=timestamp, + ischange=True) # ScriptPubKey + internal + label self.log.info("Should not allow a label to be specified when internal is true") - key = self.get_key() + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "internal": True, "label": "Example label"}, - False, + success=False, error_code=-8, error_message='Internal addresses should not have a label') # Nonstandard scriptPubKey + !internal self.log.info("Should not import a nonstandard scriptPubKey without internal flag") nonstandardScriptPubKey = key.p2pkh_script + CScript([OP_NOP]).hex() - key = self.get_key() - address = key.p2pkh_addr + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": nonstandardScriptPubKey, "timestamp": "now"}, - False, + success=False, error_code=-8, error_message='Internal must be set to true for nonstandard scriptPubKey imports.') - self.test_address(address, - iswatchonly=False, - ismine=False, - timestamp=None) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=False, + timestamp=None) # Address + Public key + !Internal(explicit) self.log.info("Should import an address with public key") - key = self.get_key() - address = key.p2pkh_addr - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "pubkeys": [key.pubkey], "internal": False}, - True, + success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + timestamp=timestamp) # ScriptPubKey + Public key + internal self.log.info("Should import a scriptPubKey with internal and with public key") - key = self.get_key() - address = key.p2pkh_addr + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "pubkeys": [key.pubkey], "internal": True}, - True, + success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + timestamp=timestamp) # Nonstandard scriptPubKey + Public key + !internal self.log.info("Should not import a nonstandard scriptPubKey without internal and with public key") - key = self.get_key() - address = key.p2pkh_addr + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": nonstandardScriptPubKey, "timestamp": "now", "pubkeys": [key.pubkey]}, - False, + success=False, error_code=-8, error_message='Internal must be set to true for nonstandard scriptPubKey imports.') - self.test_address(address, - iswatchonly=False, - ismine=False, - timestamp=None) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=False, + timestamp=None) # Address + Private key + !watchonly self.log.info("Should import an address with private key") - key = self.get_key() - address = key.p2pkh_addr - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "keys": [key.privkey]}, - True) - self.test_address(address, - iswatchonly=False, - ismine=True, - timestamp=timestamp) + success=True) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=True, + timestamp=timestamp) self.log.info("Should not import an address with private key if is already imported") - self.test_importmulti({"scriptPubKey": {"address": address}, + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "keys": [key.privkey]}, - False, + success=False, error_code=-4, error_message='The wallet already contains the private key for this address or script ("' + key.p2pkh_script + '")') # Address + Private key + watchonly self.log.info("Should import an address with private key and with watchonly") - key = self.get_key() - address = key.p2pkh_addr - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "keys": [key.privkey], "watchonly": True}, - True, + success=True, warnings=["All private keys are provided, outputs will be considered spendable. If this is intentional, do not specify the watchonly flag."]) - self.test_address(address, - iswatchonly=False, - ismine=True, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=True, + timestamp=timestamp) # ScriptPubKey + Private key + internal self.log.info("Should import a scriptPubKey with internal and with private key") - key = self.get_key() - address = key.p2pkh_addr + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "keys": [key.privkey], "internal": True}, - True) - self.test_address(address, - iswatchonly=False, - ismine=True, - timestamp=timestamp) + success=True) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=True, + timestamp=timestamp) # Nonstandard scriptPubKey + Private key + !internal self.log.info("Should not import a nonstandard scriptPubKey without internal and with private key") - key = self.get_key() - address = key.p2pkh_addr + key = get_key(self.nodes[0]) self.test_importmulti({"scriptPubKey": nonstandardScriptPubKey, "timestamp": "now", "keys": [key.privkey]}, - False, + success=False, error_code=-8, error_message='Internal must be set to true for nonstandard scriptPubKey imports.') - self.test_address(address, - iswatchonly=False, - ismine=False, - timestamp=None) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=False, + ismine=False, + timestamp=None) # P2SH address - multisig = self.get_multisig() + multisig = get_multisig(self.nodes[0]) self.nodes[1].generate(100) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) self.nodes[1].generate(1) @@ -320,17 +261,18 @@ def run_test(self): self.log.info("Should import a p2sh") self.test_importmulti({"scriptPubKey": {"address": multisig.p2sh_addr}, "timestamp": "now"}, - True) - self.test_address(multisig.p2sh_addr, - isscript=True, - iswatchonly=True, - timestamp=timestamp) + success=True) + test_address(self.nodes[1], + multisig.p2sh_addr, + isscript=True, + iswatchonly=True, + timestamp=timestamp) p2shunspent = self.nodes[1].listunspent(0, 999999, [multisig.p2sh_addr])[0] assert_equal(p2shunspent['spendable'], False) assert_equal(p2shunspent['solvable'], False) # P2SH + Redeem script - multisig = self.get_multisig() + multisig = get_multisig(self.nodes[0]) self.nodes[1].generate(100) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) self.nodes[1].generate(1) @@ -341,16 +283,17 @@ def run_test(self): self.test_importmulti({"scriptPubKey": {"address": multisig.p2sh_addr}, "timestamp": "now", "redeemscript": multisig.redeem_script}, - True, + success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(multisig.p2sh_addr, timestamp=timestamp, iswatchonly=True, ismine=False, solvable=True) + test_address(self.nodes[1], + multisig.p2sh_addr, timestamp=timestamp, iswatchonly=True, ismine=False, solvable=True) p2shunspent = self.nodes[1].listunspent(0, 999999, [multisig.p2sh_addr])[0] assert_equal(p2shunspent['spendable'], False) assert_equal(p2shunspent['solvable'], True) # P2SH + Redeem script + Private Keys + !Watchonly - multisig = self.get_multisig() + multisig = get_multisig(self.nodes[0]) self.nodes[1].generate(100) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) self.nodes[1].generate(1) @@ -362,20 +305,21 @@ def run_test(self): "timestamp": "now", "redeemscript": multisig.redeem_script, "keys": multisig.privkeys[0:2]}, - True, + success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(multisig.p2sh_addr, - timestamp=timestamp, - ismine=False, - iswatchonly=True, - solvable=True) + test_address(self.nodes[1], + multisig.p2sh_addr, + timestamp=timestamp, + ismine=False, + iswatchonly=True, + solvable=True) p2shunspent = self.nodes[1].listunspent(0, 999999, [multisig.p2sh_addr])[0] assert_equal(p2shunspent['spendable'], False) assert_equal(p2shunspent['solvable'], True) # P2SH + Redeem script + Private Keys + Watchonly - multisig = self.get_multisig() + multisig = get_multisig(self.nodes[0]) self.nodes[1].generate(100) self.nodes[1].sendtoaddress(multisig.p2sh_addr, 10.00) self.nodes[1].generate(1) @@ -388,98 +332,101 @@ def run_test(self): "redeemscript": multisig.redeem_script, "keys": multisig.privkeys[0:2], "watchonly": True}, - True) - self.test_address(multisig.p2sh_addr, - iswatchonly=True, - ismine=False, - solvable=True, - timestamp=timestamp) + success=True) + test_address(self.nodes[1], + multisig.p2sh_addr, + iswatchonly=True, + ismine=False, + solvable=True, + timestamp=timestamp) # Address + Public key + !Internal + Wrong pubkey self.log.info("Should not import an address with the wrong public key as non-solvable") - key = self.get_key() - address = key.p2pkh_addr - wrong_key = self.get_key().pubkey - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + wrong_key = get_key(self.nodes[0]).pubkey + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "pubkeys": [wrong_key]}, - True, + success=True, warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - solvable=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + solvable=False, + timestamp=timestamp) # ScriptPubKey + Public key + internal + Wrong pubkey self.log.info("Should import a scriptPubKey with internal and with a wrong public key as non-solvable") - key = self.get_key() - address = key.p2pkh_addr - wrong_key = self.get_key().pubkey + key = get_key(self.nodes[0]) + wrong_key = get_key(self.nodes[0]).pubkey self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "pubkeys": [wrong_key], "internal": True}, - True, + success=True, warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - solvable=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + solvable=False, + timestamp=timestamp) # Address + Private key + !watchonly + Wrong private key self.log.info("Should import an address with a wrong private key as non-solvable") - key = self.get_key() - address = key.p2pkh_addr - wrong_privkey = self.get_key().privkey - self.test_importmulti({"scriptPubKey": {"address": address}, + key = get_key(self.nodes[0]) + wrong_privkey = get_key(self.nodes[0]).privkey + self.test_importmulti({"scriptPubKey": {"address": key.p2pkh_addr}, "timestamp": "now", "keys": [wrong_privkey]}, - True, + success=True, warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - solvable=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + solvable=False, + timestamp=timestamp) # ScriptPubKey + Private key + internal + Wrong private key self.log.info("Should import a scriptPubKey with internal and with a wrong private key as non-solvable") - key = self.get_key() - address = key.p2pkh_addr - wrong_privkey = self.get_key().privkey + key = get_key(self.nodes[0]) + wrong_privkey = get_key(self.nodes[0]).privkey self.test_importmulti({"scriptPubKey": key.p2pkh_script, "timestamp": "now", "keys": [wrong_privkey], "internal": True}, - True, + success=True, warnings=["Importing as non-solvable: some required keys are missing. If this is intentional, don't provide any keys, pubkeys, or redeemscript.", "Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(address, - iswatchonly=True, - ismine=False, - solvable=False, - timestamp=timestamp) + test_address(self.nodes[1], + key.p2pkh_addr, + iswatchonly=True, + ismine=False, + solvable=False, + timestamp=timestamp) # Importing existing watch only address with new timestamp should replace saved timestamp. assert_greater_than(timestamp, watchonly_timestamp) self.log.info("Should replace previously saved watch only timestamp.") self.test_importmulti({"scriptPubKey": {"address": watchonly_address}, "timestamp": "now"}, - True) - self.test_address(watchonly_address, - iswatchonly=True, - ismine=False, - timestamp=timestamp) + success=True) + test_address(self.nodes[1], + watchonly_address, + iswatchonly=True, + ismine=False, + timestamp=timestamp) watchonly_timestamp = timestamp # restart nodes to check for proper serialization/deserialization of watch only address self.stop_nodes() self.start_nodes() - self.test_address(watchonly_address, - iswatchonly=True, - ismine=False, - timestamp=watchonly_timestamp) + test_address(self.nodes[1], + watchonly_address, + iswatchonly=True, + ismine=False, + timestamp=watchonly_timestamp) # Bad or missing timestamps self.log.info("Should throw on invalid or missing timestamp values") @@ -525,20 +472,21 @@ def run_test(self): success=False, error_code=-8, error_message='Range is too large') # Test importing of a P2PKH address via descriptor - key = self.get_key() + key = get_key(self.nodes[0]) self.log.info("Should import a p2pkh address from descriptor") self.test_importmulti({"desc": descsum_create("pkh(" + key.pubkey + ")"), "timestamp": "now", "label": "Descriptor import test"}, - True, + success=True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) - self.test_address(key.p2pkh_addr, + test_address(self.nodes[1], + key.p2pkh_addr, solvable=True, ismine=False, label="Descriptor import test") # Test import fails if both desc and scriptPubKey are provided - key = self.get_key() + key = get_key(self.nodes[0]) self.log.info("Import should fail if both scriptPubKey and desc are provided") self.test_importmulti({"desc": descsum_create("pkh(" + key.pubkey + ")"), "scriptPubKey": {"address": key.p2pkh_addr}, @@ -548,7 +496,7 @@ def run_test(self): error_message='Both a descriptor and a scriptPubKey should not be provided.') # Test import fails if neither desc nor scriptPubKey are present - key = self.get_key() + key = get_key(self.nodes[0]) self.log.info("Import should fail if neither a descriptor nor a scriptPubKey are provided") self.test_importmulti({"timestamp": "now"}, success=False, @@ -556,15 +504,16 @@ def run_test(self): error_message='Either a descriptor or scriptPubKey must be provided.') # Test importing of a multisig via descriptor - key1 = self.get_key() - key2 = self.get_key() + key1 = get_key(self.nodes[0]) + key2 = get_key(self.nodes[0]) self.log.info("Should import a 1-of-2 bare multisig from descriptor") self.test_importmulti({"desc": descsum_create("multi(1," + key1.pubkey + "," + key2.pubkey + ")"), "timestamp": "now"}, True, warnings=["Some private keys are missing, outputs will be considered watchonly. If this is intentional, specify the watchonly flag."]) self.log.info("Should not treat individual keys from the imported bare multisig as watchonly") - self.test_address(key1.p2pkh_addr, + test_address(self.nodes[1], + key1.p2pkh_addr, ismine=False, iswatchonly=False)