Backports 0.18 pr20

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 ("").

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;

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"},

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

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"},
                                 },

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"

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])

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

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)