From 9a37add21f1f320ac6accedc493ddf858ac4d48b Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 29 Jul 2016 17:37:57 +0200 Subject: [PATCH 01/12] Merge #8417: [QA] Add walletdump RPC test (including HD- & encryption-tests) 54af51d [QA] Add walletdump RPC test (including HD- & encryption-tests) (Jonas Schnelli) --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/wallet-dump.py | 120 ++++++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100755 qa/rpc-tests/wallet-dump.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 2c430ac18f..b978f7d5c7 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -118,6 +118,7 @@ testScripts = [ # vv Tests less than 2m vv 'wallet.py', 'wallet-accounts.py', + 'wallet-dump.py', 'listtransactions.py', # vv Tests less than 60s vv 'sendheaders.py', # NOTE: needs dash_hash to pass diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py new file mode 100755 index 0000000000..dd675f57fc --- /dev/null +++ b/qa/rpc-tests/wallet-dump.py @@ -0,0 +1,120 @@ +#!/usr/bin/env python3 +# Copyright (c) 2016 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import * +import os +import shutil + + +class WalletDumpTest(BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.setup_clean_chain = False + self.num_nodes = 1 + + def setup_network(self, split=False): + extra_args = [["-keypool=100"]] + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args) + + def run_test (self): + tmpdir = self.options.tmpdir + + #generate 20 addresses to compare against the dump + test_addr_count = 20 + addrs = [] + for i in range(0,test_addr_count): + addr = self.nodes[0].getnewaddress() + vaddr= self.nodes[0].validateaddress(addr) #required to get hd keypath + addrs.append(vaddr) + + # dump unencrypted wallet + self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.unencrypted.dump") + + #open file + inputfile = open(tmpdir + "/node0/wallet.unencrypted.dump") + found_addr = 0 + found_addr_chg = 0 + found_addr_rsv = 0 + hdmasteraddr = "" + for line in inputfile: + #only read non comment lines + if line[0] != "#" and len(line) > 10: + #split out some data + keyLabel, comment = line.split("#") + key = keyLabel.split(" ")[0] + keytype = keyLabel.split(" ")[2] + if len(comment) > 1: + addrKeypath = comment.split(" addr=")[1] + addr = addrKeypath.split(" ")[0] + keypath = "" + if keytype != "hdmaster=1": + keypath = addrKeypath.rstrip().split("hdkeypath=")[1] + else: + #keep hd master for later comp. + hdmasteraddr = addr + + #count key types + for addrObj in addrs: + if (addrObj['address'] == addr and addrObj['hdkeypath'] == keypath and keytype == "label="): + found_addr+=1 + break + elif (keytype == "change=1"): + found_addr_chg+=1 + break + elif (keytype == "reserve=1"): + found_addr_rsv+=1 + break + assert(found_addr == test_addr_count) #all keys must be in the dump + assert(found_addr_chg == 50) #50 blocks where mined + assert(found_addr_rsv == 100) #100 reserve keys (keypool) + + #encrypt wallet, restart, unlock and dump + self.nodes[0].encryptwallet('test') + bitcoind_processes[0].wait() + self.nodes[0] = start_node(0, self.options.tmpdir) + self.nodes[0].walletpassphrase('test', 10) + self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") + + #open dump done with an encrypted wallet + inputfile = open(tmpdir + "/node0/wallet.encrypted.dump") + found_addr = 0 + found_addr_chg = 0 + found_addr_rsv = 0 + for line in inputfile: + if line[0] != "#" and len(line) > 10: + keyLabel, comment = line.split("#") + key = keyLabel.split(" ")[0] + keytype = keyLabel.split(" ")[2] + if len(comment) > 1: + addrKeypath = comment.split(" addr=")[1] + addr = addrKeypath.split(" ")[0] + keypath = "" + if keytype != "hdmaster=1": + keypath = addrKeypath.rstrip().split("hdkeypath=")[1] + else: + #ensure we have generated a new hd master key + assert(hdmasteraddr != addr) + if keytype == "inactivehdmaster=1": + #ensure the old master is still available + assert(hdmasteraddr == addr) + for addrObj in addrs: + if (addrObj['address'] == addr and addrObj['hdkeypath'] == keypath and keytype == "label="): + found_addr+=1 + break + elif (keytype == "change=1"): + found_addr_chg+=1 + break + elif (keytype == "reserve=1"): + found_addr_rsv+=1 + break + + assert(found_addr == test_addr_count) + assert(found_addr_chg == 150) #old reserve keys are marked as change now + assert(found_addr_rsv == 100) #keypool size + +if __name__ == '__main__': + WalletDumpTest().main () From 8d90f295e86dd6dbf13f364e2b68237baa09f03f Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 3 Aug 2016 10:58:25 +0200 Subject: [PATCH 02/12] Merge #8442: [qa] Rework hd wallet dump test fa4439d [qa] Rework hd wallet dump test (MarcoFalke) --- qa/rpc-tests/wallet-dump.py | 144 ++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 80 deletions(-) diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py index dd675f57fc..6028d2c20b 100755 --- a/qa/rpc-tests/wallet-dump.py +++ b/qa/rpc-tests/wallet-dump.py @@ -4,9 +4,52 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import * -import os -import shutil +from test_framework.util import (start_nodes, start_node, assert_equal, bitcoind_processes) + + +def read_dump(file_name, addrs, hd_master_addr_old): + """ + Read the given dump, count the addrs that match, count change and reserve. + Also check that the old hd_master is inactive + """ + with open(file_name) as inputfile: + found_addr = 0 + found_addr_chg = 0 + found_addr_rsv = 0 + hd_master_addr_ret = None + for line in inputfile: + # only read non comment lines + if line[0] != "#" and len(line) > 10: + # split out some data + key_label, comment = line.split("#") + # key = key_label.split(" ")[0] + keytype = key_label.split(" ")[2] + if len(comment) > 1: + addr_keypath = comment.split(" addr=")[1] + addr = addr_keypath.split(" ")[0] + keypath = None + if keytype == "inactivehdmaster=1": + # ensure the old master is still available + assert(hd_master_addr_old == addr) + elif keytype == "hdmaster=1": + # ensure we have generated a new hd master key + assert(hd_master_addr_old != addr) + hd_master_addr_ret = addr + else: + keypath = addr_keypath.rstrip().split("hdkeypath=")[1] + + # count key types + for addrObj in addrs: + if addrObj['address'] == addr and addrObj['hdkeypath'] == keypath and keytype == "label=": + found_addr += 1 + break + elif keytype == "change=1": + found_addr_chg += 1 + break + elif keytype == "reserve=1": + found_addr_rsv += 1 + break + return found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_ret class WalletDumpTest(BitcoinTestFramework): @@ -15,106 +58,47 @@ class WalletDumpTest(BitcoinTestFramework): super().__init__() self.setup_clean_chain = False self.num_nodes = 1 + self.extra_args = [["-keypool=90"]] def setup_network(self, split=False): - extra_args = [["-keypool=100"]] - self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args) + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args) def run_test (self): tmpdir = self.options.tmpdir - #generate 20 addresses to compare against the dump + # generate 20 addresses to compare against the dump test_addr_count = 20 addrs = [] for i in range(0,test_addr_count): addr = self.nodes[0].getnewaddress() vaddr= self.nodes[0].validateaddress(addr) #required to get hd keypath addrs.append(vaddr) + # Should be a no-op: + self.nodes[0].keypoolrefill() # dump unencrypted wallet self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.unencrypted.dump") - #open file - inputfile = open(tmpdir + "/node0/wallet.unencrypted.dump") - found_addr = 0 - found_addr_chg = 0 - found_addr_rsv = 0 - hdmasteraddr = "" - for line in inputfile: - #only read non comment lines - if line[0] != "#" and len(line) > 10: - #split out some data - keyLabel, comment = line.split("#") - key = keyLabel.split(" ")[0] - keytype = keyLabel.split(" ")[2] - if len(comment) > 1: - addrKeypath = comment.split(" addr=")[1] - addr = addrKeypath.split(" ")[0] - keypath = "" - if keytype != "hdmaster=1": - keypath = addrKeypath.rstrip().split("hdkeypath=")[1] - else: - #keep hd master for later comp. - hdmasteraddr = addr - - #count key types - for addrObj in addrs: - if (addrObj['address'] == addr and addrObj['hdkeypath'] == keypath and keytype == "label="): - found_addr+=1 - break - elif (keytype == "change=1"): - found_addr_chg+=1 - break - elif (keytype == "reserve=1"): - found_addr_rsv+=1 - break - assert(found_addr == test_addr_count) #all keys must be in the dump - assert(found_addr_chg == 50) #50 blocks where mined - assert(found_addr_rsv == 100) #100 reserve keys (keypool) + found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_unenc = \ + read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, None) + assert_equal(found_addr, test_addr_count) # all keys must be in the dump + assert_equal(found_addr_chg, 50) # 50 blocks where mined + assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) #encrypt wallet, restart, unlock and dump self.nodes[0].encryptwallet('test') bitcoind_processes[0].wait() - self.nodes[0] = start_node(0, self.options.tmpdir) + self.nodes[0] = start_node(0, self.options.tmpdir, self.extra_args[0]) self.nodes[0].walletpassphrase('test', 10) + # Should be a no-op: + self.nodes[0].keypoolrefill() self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.encrypted.dump") - #open dump done with an encrypted wallet - inputfile = open(tmpdir + "/node0/wallet.encrypted.dump") - found_addr = 0 - found_addr_chg = 0 - found_addr_rsv = 0 - for line in inputfile: - if line[0] != "#" and len(line) > 10: - keyLabel, comment = line.split("#") - key = keyLabel.split(" ")[0] - keytype = keyLabel.split(" ")[2] - if len(comment) > 1: - addrKeypath = comment.split(" addr=")[1] - addr = addrKeypath.split(" ")[0] - keypath = "" - if keytype != "hdmaster=1": - keypath = addrKeypath.rstrip().split("hdkeypath=")[1] - else: - #ensure we have generated a new hd master key - assert(hdmasteraddr != addr) - if keytype == "inactivehdmaster=1": - #ensure the old master is still available - assert(hdmasteraddr == addr) - for addrObj in addrs: - if (addrObj['address'] == addr and addrObj['hdkeypath'] == keypath and keytype == "label="): - found_addr+=1 - break - elif (keytype == "change=1"): - found_addr_chg+=1 - break - elif (keytype == "reserve=1"): - found_addr_rsv+=1 - break - - assert(found_addr == test_addr_count) - assert(found_addr_chg == 150) #old reserve keys are marked as change now - assert(found_addr_rsv == 100) #keypool size + found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_enc = \ + read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, hd_master_addr_unenc) + assert_equal(found_addr, test_addr_count) + assert_equal(found_addr_chg, 90 + 1 + 50) # old reserve keys are marked as change now + assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) if __name__ == '__main__': WalletDumpTest().main () From 9bb9e9ea7267818c5a6163a751cb497067a5c842 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 7 Nov 2016 18:31:06 +0100 Subject: [PATCH 03/12] Merge #9077: [qa] Increase wallet-dump RPC timeout e89614b [qa] Add more helpful RPC timeout message (Russell Yanofsky) 8463aaa [qa] Increase wallet-dump RPC timeout (Russell Yanofsky) --- qa/rpc-tests/wallet-dump.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py index 6028d2c20b..7da3e2d9e3 100755 --- a/qa/rpc-tests/wallet-dump.py +++ b/qa/rpc-tests/wallet-dump.py @@ -61,7 +61,11 @@ class WalletDumpTest(BitcoinTestFramework): self.extra_args = [["-keypool=90"]] def setup_network(self, split=False): - self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args) + # Use 1 minute timeout because the initial getnewaddress RPC can take + # longer than the default 30 seconds due to an expensive + # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in + # the test often takes even longer. + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args, timewait=60) def run_test (self): tmpdir = self.options.tmpdir From a92b7b2ede5bb1c00e4e9a83dadbc6a892792699 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 31 Jan 2018 10:37:15 +0100 Subject: [PATCH 04/12] Add missed change from previous backport to wallet-dump.py #8840: test: Explicitly set encoding to utf8 when opening text files --- qa/rpc-tests/wallet-dump.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py index 7da3e2d9e3..c6dc2e3d10 100755 --- a/qa/rpc-tests/wallet-dump.py +++ b/qa/rpc-tests/wallet-dump.py @@ -12,7 +12,7 @@ def read_dump(file_name, addrs, hd_master_addr_old): Read the given dump, count the addrs that match, count change and reserve. Also check that the old hd_master is inactive """ - with open(file_name) as inputfile: + with open(file_name, encoding='utf8') as inputfile: found_addr = 0 found_addr_chg = 0 found_addr_rsv = 0 From 9c5032c540107b8afb04e49836bdb0c83da69a56 Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Wed, 31 Jan 2018 10:40:27 +0100 Subject: [PATCH 05/12] Explicitly start nodes with -usehd=1 in wallet-dump.py We currently have usehd defaulted to 0, so we need to explicitly start it with usehd=1 in wallet-dump.py. This requires setting up a new data dirs cache for hd as the wallets won't be compatible otherwise. At the same time we need the initial state of the wallet to test the dump functionality. Also use redirect_stderr=True to fix failure when run from pull-tester --- qa/rpc-tests/test_framework/util.py | 4 +++- qa/rpc-tests/wallet-dump.py | 19 +++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index 5d07a01efd..32f508ea25 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -240,7 +240,7 @@ def wait_for_bitcoind_start(process, url, i): raise # unknown JSON RPC exception time.sleep(0.25) -def initialize_chain(test_dir, num_nodes, cachedir): +def initialize_chain(test_dir, num_nodes, cachedir, extra_args=None): """ Create a cache of a 200-block-long chain (with wallet) for MAX_NODES Afterward, create num_nodes copies from the cache @@ -266,6 +266,8 @@ def initialize_chain(test_dir, num_nodes, cachedir): args = [ os.getenv("DASHD", "dashd"), "-server", "-keypool=1", "-datadir="+datadir, "-discover=0" ] if i > 0: args.append("-connect=127.0.0.1:"+str(p2p_port(0))) + if extra_args is not None: + args += extra_args bitcoind_processes[i] = subprocess.Popen(args) if os.getenv("PYTHON_DEBUG", ""): print("initialize_chain: dashd started, waiting for RPC to come up") diff --git a/qa/rpc-tests/wallet-dump.py b/qa/rpc-tests/wallet-dump.py index c6dc2e3d10..adef593621 100755 --- a/qa/rpc-tests/wallet-dump.py +++ b/qa/rpc-tests/wallet-dump.py @@ -4,7 +4,7 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import (start_nodes, start_node, assert_equal, bitcoind_processes) +from test_framework.util import * def read_dump(file_name, addrs, hd_master_addr_old): @@ -57,15 +57,21 @@ class WalletDumpTest(BitcoinTestFramework): def __init__(self): super().__init__() self.setup_clean_chain = False + self.redirect_stderr = True self.num_nodes = 1 - self.extra_args = [["-keypool=90"]] + self.extra_args = [["-keypool=90", "-usehd=1"]] + + def setup_chain(self): + # TODO remove this when usehd=1 becomes the default + # use our own cache and -usehd=1 as extra arg as the default cache is run with -usehd=0 + initialize_chain(self.options.tmpdir, self.num_nodes, self.options.cachedir + "/hd", ["-usehd=1"]) def setup_network(self, split=False): # Use 1 minute timeout because the initial getnewaddress RPC can take # longer than the default 30 seconds due to an expensive # CWallet::TopUpKeyPool call, and the encryptwallet RPC made later in # the test often takes even longer. - self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args, timewait=60) + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, self.extra_args, timewait=60, redirect_stderr=True) def run_test (self): tmpdir = self.options.tmpdir @@ -87,7 +93,7 @@ class WalletDumpTest(BitcoinTestFramework): read_dump(tmpdir + "/node0/wallet.unencrypted.dump", addrs, None) assert_equal(found_addr, test_addr_count) # all keys must be in the dump assert_equal(found_addr_chg, 50) # 50 blocks where mined - assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) + assert_equal(found_addr_rsv, 180) # keypool size (external+internal) #encrypt wallet, restart, unlock and dump self.nodes[0].encryptwallet('test') @@ -101,8 +107,9 @@ class WalletDumpTest(BitcoinTestFramework): found_addr, found_addr_chg, found_addr_rsv, hd_master_addr_enc = \ read_dump(tmpdir + "/node0/wallet.encrypted.dump", addrs, hd_master_addr_unenc) assert_equal(found_addr, test_addr_count) - assert_equal(found_addr_chg, 90 + 1 + 50) # old reserve keys are marked as change now - assert_equal(found_addr_rsv, 90 + 1) # keypool size (TODO: fix off-by-one) + # TODO clarify if we want the behavior that is tested below in Dash (only when HD seed was generated and not user-provided) + # assert_equal(found_addr_chg, 180 + 50) # old reserve keys are marked as change now + assert_equal(found_addr_rsv, 180) # keypool size (TODO: fix off-by-one) if __name__ == '__main__': WalletDumpTest().main () From 6f86725d006b3a20c696f410634cfc11c1d83e77 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Tue, 14 Feb 2017 14:24:27 +0100 Subject: [PATCH 06/12] Merge #9682: Require timestamps for importmulti keys 266a811 Use MTP for importmulti "now" timestamps (Russell Yanofsky) 3cf9917 Add test to check new importmulti "now" value (Russell Yanofsky) 442887f Require timestamps for importmulti keys (Russell Yanofsky) --- qa/rpc-tests/importmulti.py | 2 ++ src/rpc/misc.cpp | 18 +++++++++++++----- src/wallet/rpcdump.cpp | 2 +- 3 files changed, 16 insertions(+), 6 deletions(-) diff --git a/qa/rpc-tests/importmulti.py b/qa/rpc-tests/importmulti.py index bd6e1a520d..68549c1575 100755 --- a/qa/rpc-tests/importmulti.py +++ b/qa/rpc-tests/importmulti.py @@ -149,6 +149,7 @@ class ImportMultiTest (BitcoinTestFramework): # Address + Private key + !watchonly print("Should import an address with private key") address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) + timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] result = self.nodes[1].importmulti([{ "scriptPubKey": { "address": address['address'] @@ -160,6 +161,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], True) + assert_equal(address_assert['timestamp'], timestamp) # Address + Private key + watchonly print("Should not import an address with private key and with watchonly") diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index c42d67ae9d..0ef0ade8f1 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -296,6 +296,7 @@ UniValue validateaddress(const JSONRPCRequest& request) " \"pubkey\" : \"publickeyhex\", (string) The hex value of the raw public key\n" " \"iscompressed\" : true|false, (boolean) If the address is compressed\n" " \"account\" : \"account\" (string) DEPRECATED. The account associated with the address, \"\" is the default account\n" + " \"timestamp\" : timestamp, (number, optional) The creation time of the key if available in seconds since epoch (Jan 1 1970 GMT)\n" " \"hdkeypath\" : \"keypath\" (string, optional) The HD keypath if the key is HD and available\n" " \"hdchainid\" : \"\" (string, optional) The ID of the HD chain\n" "}\n" @@ -333,11 +334,18 @@ UniValue validateaddress(const JSONRPCRequest& request) if (pwalletMain && pwalletMain->mapAddressBook.count(dest)) ret.push_back(Pair("account", pwalletMain->mapAddressBook[dest].name)); CKeyID keyID; - CHDChain hdChainCurrent; - if (pwalletMain && address.GetKeyID(keyID) && pwalletMain->mapHdPubKeys.count(keyID) && pwalletMain->GetHDChain(hdChainCurrent)) - { - ret.push_back(Pair("hdkeypath", pwalletMain->mapHdPubKeys[keyID].GetKeyPath())); - ret.push_back(Pair("hdchainid", hdChainCurrent.GetID().GetHex())); + if (pwalletMain) { + const auto& meta = pwalletMain->mapKeyMetadata; + auto it = address.GetKeyID(keyID) ? meta.find(keyID) : meta.end(); + if (it != meta.end()) { + ret.push_back(Pair("timestamp", it->second.nCreateTime)); + } + + CHDChain hdChainCurrent; + if (!keyID.IsNull() && pwalletMain->mapHdPubKeys.count(keyID) && pwalletMain->GetHDChain(hdChainCurrent)) { + ret.push_back(Pair("hdkeypath", pwalletMain->mapHdPubKeys[keyID].GetKeyPath())); + ret.push_back(Pair("hdchainid", hdChainCurrent.GetID().GetHex())); + } } #endif } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index ac1c64c963..18e4bf0d9f 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1240,7 +1240,7 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) EnsureWalletIsUnlocked(); // Verify all timestamps are present before importing any keys. - const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetBlockTime() : 0; + const int64_t now = chainActive.Tip() ? chainActive.Tip()->GetMedianTimePast() : 0; for (const UniValue& data : requests.getValues()) { GetImportTimestamp(data, now); } From 43f697866e4c83f803e55e5bc9ba7d4c1a896ea9 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Wed, 15 Feb 2017 11:12:00 +0100 Subject: [PATCH 07/12] Merge #9108: Use importmulti timestamp when importing watch only keys (on top of #9682) a80f98b Use importmulti timestamp when importing watch only keys (Russell Yanofsky) a58370e Dedup nTimeFirstKey update logic (Russell Yanofsky) --- qa/rpc-tests/importmulti.py | 33 ++++++++++++++++++++++++- src/rpc/misc.cpp | 3 +++ src/wallet/rpcdump.cpp | 37 +++++++++++++--------------- src/wallet/wallet.cpp | 49 +++++++++++++++++++++++++------------ src/wallet/wallet.h | 26 ++++++++++++++++---- src/wallet/walletdb.cpp | 43 +++++++++++++++++++------------- src/wallet/walletdb.h | 2 +- 7 files changed, 133 insertions(+), 60 deletions(-) diff --git a/qa/rpc-tests/importmulti.py b/qa/rpc-tests/importmulti.py index 68549c1575..1aa4ba2e18 100755 --- a/qa/rpc-tests/importmulti.py +++ b/qa/rpc-tests/importmulti.py @@ -20,6 +20,7 @@ class ImportMultiTest (BitcoinTestFramework): print ("Mining blocks...") self.nodes[0].generate(1) self.nodes[1].generate(1) + timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] # keyword definition PRIV_KEY = 'privkey' @@ -59,6 +60,9 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) + assert_equal(address_assert['timestamp'], timestamp) + watchonly_address = address['address'] + watchonly_timestamp = timestamp print("Should not import an invalid address") result = self.nodes[1].importmulti([{ @@ -83,6 +87,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) + assert_equal(address_assert['timestamp'], timestamp) # ScriptPubKey + !internal print("Should not import a scriptPubKey without internal flag") @@ -97,6 +102,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # Address + Public key + !Internal @@ -113,6 +119,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) + assert_equal(address_assert['timestamp'], timestamp) # ScriptPubKey + Public key + internal @@ -129,6 +136,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], True) assert_equal(address_assert['ismine'], False) + assert_equal(address_assert['timestamp'], timestamp) # ScriptPubKey + Public key + !internal print("Should not import a scriptPubKey without internal and with public key") @@ -145,11 +153,11 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # Address + Private key + !watchonly print("Should import an address with private key") address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress()) - timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] result = self.nodes[1].importmulti([{ "scriptPubKey": { "address": address['address'] @@ -180,6 +188,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # ScriptPubKey + Private key + internal print("Should import a scriptPubKey with internal and with private key") @@ -194,6 +203,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], True) + assert_equal(address_assert['timestamp'], timestamp) # ScriptPubKey + Private key + !internal print("Should not import a scriptPubKey without internal and with private key") @@ -209,6 +219,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # P2SH address @@ -219,6 +230,7 @@ class ImportMultiTest (BitcoinTestFramework): self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) + timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] transaction = self.nodes[1].gettransaction(transactionid) print("Should import a p2sh") @@ -232,6 +244,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(multi_sig_script['address']) assert_equal(address_assert['isscript'], True) assert_equal(address_assert['iswatchonly'], True) + assert_equal(address_assert['timestamp'], timestamp) p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0] assert_equal(p2shunspent['spendable'], False) assert_equal(p2shunspent['solvable'], False) @@ -245,6 +258,7 @@ class ImportMultiTest (BitcoinTestFramework): self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) + timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] transaction = self.nodes[1].gettransaction(transactionid) print("Should import a p2sh with respective redeem script") @@ -256,6 +270,8 @@ class ImportMultiTest (BitcoinTestFramework): "redeemscript": multi_sig_script['redeemScript'] }]) assert_equal(result[0]['success'], True) + address_assert = self.nodes[1].validateaddress(multi_sig_script['address']) + assert_equal(address_assert['timestamp'], timestamp) p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0] assert_equal(p2shunspent['spendable'], False) @@ -270,6 +286,7 @@ class ImportMultiTest (BitcoinTestFramework): self.nodes[1].generate(100) transactionid = self.nodes[1].sendtoaddress(multi_sig_script['address'], 10.00) self.nodes[1].generate(1) + timestamp = self.nodes[1].getblock(self.nodes[1].getbestblockhash())['mediantime'] transaction = self.nodes[1].gettransaction(transactionid) print("Should import a p2sh with respective redeem script and private keys") @@ -282,6 +299,8 @@ class ImportMultiTest (BitcoinTestFramework): "keys": [ self.nodes[0].dumpprivkey(sig_address_1['address']), self.nodes[0].dumpprivkey(sig_address_2['address'])] }]) assert_equal(result[0]['success'], True) + address_assert = self.nodes[1].validateaddress(multi_sig_script['address']) + assert_equal(address_assert['timestamp'], timestamp) p2shunspent = self.nodes[1].listunspent(0,999999, [multi_sig_script['address']])[0] assert_equal(p2shunspent['spendable'], False) @@ -329,6 +348,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # ScriptPubKey + Public key + internal + Wrong pubkey @@ -348,6 +368,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # Address + Private key + !watchonly + Wrong private key @@ -367,6 +388,7 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) # ScriptPubKey + Private key + internal + Wrong private key @@ -385,6 +407,15 @@ class ImportMultiTest (BitcoinTestFramework): address_assert = self.nodes[1].validateaddress(address['address']) assert_equal(address_assert['iswatchonly'], False) assert_equal(address_assert['ismine'], False) + assert_equal('timestamp' in address_assert, False) + + # restart nodes to check for proper serialization/deserialization of watch only address + stop_nodes(self.nodes) + self.nodes = start_nodes(2, self.options.tmpdir) + address_assert = self.nodes[1].validateaddress(watchonly_address) + assert_equal(address_assert['iswatchonly'], True) + assert_equal(address_assert['ismine'], False) + assert_equal(address_assert['timestamp'], watchonly_timestamp); # Bad or missing timestamps print("Should throw on invalid or missing timestamp values") diff --git a/src/rpc/misc.cpp b/src/rpc/misc.cpp index 0ef0ade8f1..3dca2516f1 100644 --- a/src/rpc/misc.cpp +++ b/src/rpc/misc.cpp @@ -337,6 +337,9 @@ UniValue validateaddress(const JSONRPCRequest& request) if (pwalletMain) { const auto& meta = pwalletMain->mapKeyMetadata; auto it = address.GetKeyID(keyID) ? meta.find(keyID) : meta.end(); + if (it == meta.end()) { + it = meta.find(CScriptID(scriptPubKey)); + } if (it != meta.end()) { ret.push_back(Pair("timestamp", it->second.nCreateTime)); } diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 18e4bf0d9f..72d3e12393 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -146,7 +146,7 @@ UniValue importprivkey(const JSONRPCRequest& request) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); // whenever a key is imported, we need to scan the whole chain - pwalletMain->nTimeFirstKey = 1; // 0 would be considered 'no value' + pwalletMain->UpdateTimeFirstKey(1); if (fRescan) { pwalletMain->ScanForWalletTransactions(chainActive.Genesis(), true); @@ -164,7 +164,7 @@ void ImportScript(const CScript& script, const string& strLabel, bool isRedeemSc pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script)) + if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script, 0 /* nCreateTime */)) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); if (isRedeemScript) { @@ -498,8 +498,8 @@ UniValue importwallet(const JSONRPCRequest& request) } file.close(); pwalletMain->ShowProgress("", 100); // hide progress dialog in GUI - if (!pwalletMain->nTimeFirstKey || nTimeBegin < pwalletMain->nTimeFirstKey) - pwalletMain->nTimeFirstKey = nTimeBegin; + + pwalletMain->UpdateTimeFirstKey(nTimeBegin); CBlockIndex *pindex = chainActive.FindEarliestAtLeast(nTimeBegin - 7200); @@ -635,8 +635,7 @@ UniValue importelectrumwallet(const JSONRPCRequest& request) // Assume that electrum wallet was created at that block int nTimeBegin = chainActive[nStartHeight]->GetBlockTime(); - if (!pwalletMain->nTimeFirstKey || nTimeBegin < pwalletMain->nTimeFirstKey) - pwalletMain->nTimeFirstKey = nTimeBegin; + pwalletMain->UpdateTimeFirstKey(nTimeBegin); LogPrintf("Rescanning %i blocks\n", chainActive.Height() - nStartHeight + 1); pwalletMain->ScanForWalletTransactions(chainActive[nStartHeight], true); @@ -755,15 +754,17 @@ UniValue dumpwallet(const JSONRPCRequest& request) if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); - std::map mapKeyBirth; + std::map mapKeyBirth; std::set setKeyPool; pwalletMain->GetKeyBirthTimes(mapKeyBirth); pwalletMain->GetAllReserveKeys(setKeyPool); // sort time/key pairs std::vector > vKeyBirth; - for (std::map::const_iterator it = mapKeyBirth.begin(); it != mapKeyBirth.end(); it++) { - vKeyBirth.push_back(std::make_pair(it->second, it->first)); + for (const auto& entry : mapKeyBirth) { + if (const CKeyID* keyID = boost::get(&entry.first)) { // set and test + vKeyBirth.push_back(std::make_pair(entry.second, *keyID)); + } } mapKeyBirth.clear(); std::sort(vKeyBirth.begin(), vKeyBirth.end()); @@ -927,7 +928,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(redeemScript) && !pwalletMain->AddWatchOnly(redeemScript)) { + if (!pwalletMain->HaveWatchOnly(redeemScript) && !pwalletMain->AddWatchOnly(redeemScript, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } @@ -944,7 +945,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(redeemDestination) && !pwalletMain->AddWatchOnly(redeemDestination)) { + if (!pwalletMain->HaveWatchOnly(redeemDestination) && !pwalletMain->AddWatchOnly(redeemDestination, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } @@ -988,9 +989,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } - if (timestamp < pwalletMain->nTimeFirstKey) { - pwalletMain->nTimeFirstKey = timestamp; - } + pwalletMain->UpdateTimeFirstKey(timestamp); } } @@ -1039,7 +1038,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(pubKeyScript) && !pwalletMain->AddWatchOnly(pubKeyScript)) { + if (!pwalletMain->HaveWatchOnly(pubKeyScript) && !pwalletMain->AddWatchOnly(pubKeyScript, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } @@ -1057,7 +1056,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(scriptRawPubKey) && !pwalletMain->AddWatchOnly(scriptRawPubKey)) { + if (!pwalletMain->HaveWatchOnly(scriptRawPubKey) && !pwalletMain->AddWatchOnly(scriptRawPubKey, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } @@ -1118,9 +1117,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) throw JSONRPCError(RPC_WALLET_ERROR, "Error adding key to wallet"); } - if (timestamp < pwalletMain->nTimeFirstKey) { - pwalletMain->nTimeFirstKey = timestamp; - } + pwalletMain->UpdateTimeFirstKey(timestamp); success = true; } @@ -1133,7 +1130,7 @@ UniValue ProcessImport(const UniValue& data, const int64_t timestamp) pwalletMain->MarkDirty(); - if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script)) { + if (!pwalletMain->HaveWatchOnly(script) && !pwalletMain->AddWatchOnly(script, timestamp)) { throw JSONRPCError(RPC_WALLET_ERROR, "Error adding address to wallet"); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 2430385e65..5d88cf15e4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -130,8 +130,7 @@ CPubKey CWallet::GenerateNewKey(uint32_t nAccountIndex, bool fInternal) // Create new metadata mapKeyMetadata[pubkey.GetID()] = metadata; - if (!nTimeFirstKey || nCreationTime < nTimeFirstKey) - nTimeFirstKey = nCreationTime; + UpdateTimeFirstKey(nCreationTime); if (!AddKeyPubKey(secret, pubkey)) throw std::runtime_error(std::string(__func__) + ": AddKey failed"); @@ -171,8 +170,7 @@ void CWallet::DeriveNewChildKey(const CKeyMetadata& metadata, CKey& secretRet, u // store metadata mapKeyMetadata[pubkey.GetID()] = metadata; - if (!nTimeFirstKey || metadata.nCreateTime < nTimeFirstKey) - nTimeFirstKey = metadata.nCreateTime; + UpdateTimeFirstKey(metadata.nCreateTime); // update the chain model in the database CHDChain hdChainCurrent; @@ -333,13 +331,11 @@ bool CWallet::AddCryptedKey(const CPubKey &vchPubKey, return false; } -bool CWallet::LoadKeyMetadata(const CPubKey &pubkey, const CKeyMetadata &meta) +bool CWallet::LoadKeyMetadata(const CTxDestination& keyID, const CKeyMetadata &meta) { AssertLockHeld(cs_wallet); // mapKeyMetadata - if (meta.nCreateTime && (!nTimeFirstKey || meta.nCreateTime < nTimeFirstKey)) - nTimeFirstKey = meta.nCreateTime; - - mapKeyMetadata[pubkey.GetID()] = meta; + UpdateTimeFirstKey(meta.nCreateTime); + mapKeyMetadata[keyID] = meta; return true; } @@ -348,6 +344,18 @@ bool CWallet::LoadCryptedKey(const CPubKey &vchPubKey, const std::vector &mapKeyBirth) const { +void CWallet::GetKeyBirthTimes(std::map &mapKeyBirth) const { AssertLockHeld(cs_wallet); // mapKeyMetadata mapKeyBirth.clear(); // get birth times for keys with metadata - for (std::map::const_iterator it = mapKeyMetadata.begin(); it != mapKeyMetadata.end(); it++) - if (it->second.nCreateTime) - mapKeyBirth[it->first] = it->second.nCreateTime; + for (const auto& entry : mapKeyMetadata) { + if (entry.second.nCreateTime) { + mapKeyBirth[entry.first] = entry.second.nCreateTime; + } + } // map in which we'll infer heights of other keys CBlockIndex *pindexMax = chainActive[std::max(0, chainActive.Height() - 144)]; // the tip can be reorganized; use a 144-block safety margin diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index a66eaa7e0b..89bdae7a59 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -679,6 +679,20 @@ private: std::set setInternalKeyPool; std::set setExternalKeyPool; + + int64_t nTimeFirstKey; + + /** + * Private version of AddWatchOnly method which does not accept a + * timestamp, and which will reset the wallet's nTimeFirstKey value to 1 if + * the watch key did not previously have a timestamp associated with it. + * Because this is an inherited virtual method, it is accessible despite + * being marked private, but it is marked private anyway to encourage use + * of the other AddWatchOnly which accepts a timestamp and sets + * nTimeFirstKey more intelligently for more efficient rescans. + */ + bool AddWatchOnly(const CScript& dest) override; + public: /* * Main wallet lock. @@ -707,7 +721,9 @@ public: mapKeyMetadata[keyid] = CKeyMetadata(keypool.nTime); } - std::map mapKeyMetadata; + // Map from Key ID (for regular keys) or Script ID (for watch-only keys) to + // key metadata. + std::map mapKeyMetadata; typedef std::map MasterKeyMap; MasterKeyMap mapMasterKeys; @@ -766,7 +782,6 @@ public: std::set setLockedCoins; - int64_t nTimeFirstKey; int64_t nKeysLeftSinceAutoBackup; std::map mapHdPubKeys; // &vchCryptedSecret); @@ -860,7 +876,7 @@ public: bool GetDestData(const CTxDestination &dest, const std::string &key, std::string *value) const; //! Adds a watch-only address to the store, and saves it to disk. - bool AddWatchOnly(const CScript &dest); + bool AddWatchOnly(const CScript& dest, int64_t nCreateTime); bool RemoveWatchOnly(const CScript &dest); //! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet) bool LoadWatchOnly(const CScript &dest); @@ -869,7 +885,7 @@ public: bool ChangeWalletPassphrase(const SecureString& strOldWalletPassphrase, const SecureString& strNewWalletPassphrase); bool EncryptWallet(const SecureString& strWalletPassphrase); - void GetKeyBirthTimes(std::map &mapKeyBirth) const; + void GetKeyBirthTimes(std::map &mapKeyBirth) const; /** * Increment the next transaction order id diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 47f0584c65..be32e94f04 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -120,15 +120,19 @@ bool CWalletDB::WriteCScript(const uint160& hash, const CScript& redeemScript) return Write(std::make_pair(std::string("cscript"), hash), *(const CScriptBase*)(&redeemScript), false); } -bool CWalletDB::WriteWatchOnly(const CScript &dest) +bool CWalletDB::WriteWatchOnly(const CScript &dest, const CKeyMetadata& keyMeta) { nWalletDBUpdateCounter++; + if (!Write(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)), keyMeta)) + return false; return Write(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest)), '1'); } bool CWalletDB::EraseWatchOnly(const CScript &dest) { nWalletDBUpdateCounter++; + if (!Erase(std::make_pair(std::string("watchmeta"), *(const CScriptBase*)(&dest)))) + return false; return Erase(std::make_pair(std::string("watchs"), *(const CScriptBase*)(&dest))); } @@ -259,6 +263,7 @@ class CWalletScanState { public: unsigned int nKeys; unsigned int nCKeys; + unsigned int nWatchKeys; unsigned int nKeyMeta; bool fIsEncrypted; bool fAnyUnordered; @@ -266,7 +271,7 @@ public: vector vWalletUpgrade; CWalletScanState() { - nKeys = nCKeys = nKeyMeta = 0; + nKeys = nCKeys = nWatchKeys = nKeyMeta = 0; fIsEncrypted = false; fAnyUnordered = false; nFileVersion = 0; @@ -348,16 +353,13 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } else if (strType == "watchs") { + wss.nWatchKeys++; CScript script; ssKey >> *(CScriptBase*)(&script); char fYes; ssValue >> fYes; if (fYes == '1') pwallet->LoadWatchOnly(script); - - // Watch-only addresses have no birthday information for now, - // so set the wallet birthday to the beginning of time. - pwallet->nTimeFirstKey = 1; } else if (strType == "key" || strType == "wkey") { @@ -458,20 +460,27 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, } wss.fIsEncrypted = true; } - else if (strType == "keymeta") + else if (strType == "keymeta" || strType == "watchmeta") { - CPubKey vchPubKey; - ssKey >> vchPubKey; + CTxDestination keyID; + if (strType == "keymeta") + { + CPubKey vchPubKey; + ssKey >> vchPubKey; + keyID = vchPubKey.GetID(); + } + else if (strType == "watchmeta") + { + CScript script; + ssKey >> *(CScriptBase*)(&script); + keyID = CScriptID(script); + } + CKeyMetadata keyMeta; ssValue >> keyMeta; wss.nKeyMeta++; - pwallet->LoadKeyMetadata(vchPubKey, keyMeta); - - // find earliest key creation time, as wallet birthday - if (!pwallet->nTimeFirstKey || - (keyMeta.nCreateTime < pwallet->nTimeFirstKey)) - pwallet->nTimeFirstKey = keyMeta.nCreateTime; + pwallet->LoadKeyMetadata(keyID, keyMeta); } else if (strType == "defaultkey") { @@ -658,8 +667,8 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet) wss.nKeys, wss.nCKeys, wss.nKeyMeta, wss.nKeys + wss.nCKeys); // nTimeFirstKey is only reliable if all keys have metadata - if ((wss.nKeys + wss.nCKeys) != wss.nKeyMeta) - pwallet->nTimeFirstKey = 1; // 0 would be considered 'no value' + if ((wss.nKeys + wss.nCKeys + wss.nWatchKeys) != wss.nKeyMeta) + pwallet->UpdateTimeFirstKey(1); BOOST_FOREACH(uint256 hash, wss.vWalletUpgrade) WriteTx(pwallet->mapWallet[hash]); diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 4d5aff29f9..9ea7a088a7 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -96,7 +96,7 @@ public: bool WriteCScript(const uint160& hash, const CScript& redeemScript); - bool WriteWatchOnly(const CScript &script); + bool WriteWatchOnly(const CScript &script, const CKeyMetadata &keymeta); bool EraseWatchOnly(const CScript &script); bool WriteBestBlock(const CBlockLocator& locator); From 75421c37b60e3b575a2b126e46c6152d540ba9d4 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 16 Feb 2017 10:24:21 +0100 Subject: [PATCH 08/12] Merge #9764: wallet: Prevent "overrides a member function but is not marked 'override'" warnings 6c5427d wallet: Prevent "overrides a member function but is not marked 'override'" warnings (Wladimir J. van der Laan) --- src/wallet/wallet.h | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 89bdae7a59..2bec84a155 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -840,17 +840,17 @@ public: */ CPubKey GenerateNewKey(uint32_t nAccountIndex, bool fInternal /*= false*/); //! HaveKey implementation that also checks the mapHdPubKeys - bool HaveKey(const CKeyID &address) const; + bool HaveKey(const CKeyID &address) const override; //! GetPubKey implementation that also checks the mapHdPubKeys - bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const; + bool GetPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const override; //! GetKey implementation that can derive a HD private key on the fly - bool GetKey(const CKeyID &address, CKey& keyOut) const; + bool GetKey(const CKeyID &address, CKey& keyOut) const override; //! Adds a HDPubKey into the wallet(database) bool AddHDPubKey(const CExtPubKey &extPubKey, bool fInternal); //! loads a HDPubKey into the wallets memory bool LoadHDPubKey(const CHDPubKey &hdPubKey); //! Adds a key to the store, and saves it to disk. - bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey); + bool AddKeyPubKey(const CKey& key, const CPubKey &pubkey) override; //! Adds a key to the store, without saving it to disk (used by LoadWallet) bool LoadKey(const CKey& key, const CPubKey &pubkey) { return CCryptoKeyStore::AddKeyPubKey(key, pubkey); } //! Load metadata (used by LoadWallet) @@ -860,10 +860,10 @@ public: void UpdateTimeFirstKey(int64_t nCreateTime); //! Adds an encrypted key to the store, and saves it to disk. - bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret); + bool AddCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret) override; //! Adds an encrypted key to the store, without saving it to disk (used by LoadWallet) bool LoadCryptedKey(const CPubKey &vchPubKey, const std::vector &vchCryptedSecret); - bool AddCScript(const CScript& redeemScript); + bool AddCScript(const CScript& redeemScript) override; bool LoadCScript(const CScript& redeemScript); //! Adds a destination data tuple to the store, and saves it to disk @@ -877,7 +877,7 @@ public: //! Adds a watch-only address to the store, and saves it to disk. bool AddWatchOnly(const CScript& dest, int64_t nCreateTime); - bool RemoveWatchOnly(const CScript &dest); + bool RemoveWatchOnly(const CScript &dest) override; //! Adds a watch-only address to the store, without saving it to disk (used by LoadWallet) bool LoadWatchOnly(const CScript &dest); @@ -899,11 +899,11 @@ public: void MarkDirty(); bool AddToWallet(const CWalletTx& wtxIn, bool fFlushOnClose=true); bool LoadToWallet(const CWalletTx& wtxIn); - void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock); + void SyncTransaction(const CTransaction& tx, const CBlockIndex *pindex, int posInBlock) override; bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex* pIndex, int posInBlock, bool fUpdate); CBlockIndex* ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false); void ReacceptWalletTransactions(); - void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman); + void ResendWalletTransactions(int64_t nBestBlockTime, CConnman* connman) override; std::vector ResendWalletTransactionsBefore(int64_t nTime, CConnman* connman); CAmount GetBalance() const; CAmount GetUnconfirmedBalance() const; @@ -992,7 +992,7 @@ public: CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; CAmount GetChange(const CTransaction& tx) const; - void SetBestChain(const CBlockLocator& loc); + void SetBestChain(const CBlockLocator& loc) override; DBErrors LoadWallet(bool& fFirstRunRet); DBErrors ZapWalletTx(std::vector& vWtx); @@ -1002,9 +1002,9 @@ public: bool DelAddressBook(const CTxDestination& address); - bool UpdatedTransaction(const uint256 &hashTx); + bool UpdatedTransaction(const uint256 &hashTx) override; - void Inventory(const uint256 &hash) + void Inventory(const uint256 &hash) override { { LOCK(cs_wallet); @@ -1014,8 +1014,8 @@ public: } } - void GetScriptForMining(boost::shared_ptr &script); - void ResetRequestCount(const uint256 &hash) + void GetScriptForMining(boost::shared_ptr &script) override; + void ResetRequestCount(const uint256 &hash) override { LOCK(cs_wallet); mapRequestCount[hash] = 0; From 1ba1256217f145c483c379c27560e92a485fa091 Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Fri, 17 Feb 2017 12:53:31 +0100 Subject: [PATCH 09/12] Merge #9761: Use 2 hour grace period for key timestamps in importmulti rescans e662af3 Use 2 hour grace period for key timestamps in importmulti rescans (Russell Yanofsky) 38d3e9e [qa] Extend import-rescan.py to test imports on pruned nodes. (Russell Yanofsky) c28583d [qa] Extend import-rescan.py to test specific key timestamps (Russell Yanofsky) 8be0866 [qa] Simplify import-rescan.py (Russell Yanofsky) --- qa/rpc-tests/import-rescan.py | 239 +++++++++++++++++++--------------- src/wallet/rpcdump.cpp | 5 +- 2 files changed, 137 insertions(+), 107 deletions(-) diff --git a/qa/rpc-tests/import-rescan.py b/qa/rpc-tests/import-rescan.py index d7aa399393..4a86c670b4 100755 --- a/qa/rpc-tests/import-rescan.py +++ b/qa/rpc-tests/import-rescan.py @@ -2,55 +2,105 @@ # Copyright (c) 2014-2016 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 rescan behavior of importaddress, importpubkey, importprivkey, and +importmulti RPCs with different types of keys and rescan options. +In the first part of the test, node 0 creates an address for each type of +import RPC call and sends BTC to it. Then other nodes import the addresses, +and the test makes listtransactions and getbalance calls to confirm that the +importing node either did or did not execute rescans picking up the send +transactions. + +In the second part of the test, node 0 sends more BTC to each address, and the +test makes more listtransactions and getbalance calls to confirm that the +importing nodes pick up the new transactions regardless of whether rescans +happened previously. +""" + +from test_framework.authproxy import JSONRPCException from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import (start_nodes, connect_nodes, sync_blocks, assert_equal) +from test_framework.util import (start_nodes, connect_nodes, sync_blocks, assert_equal, set_node_times) from decimal import Decimal import collections import enum import itertools -import functools Call = enum.Enum("Call", "single multi") Data = enum.Enum("Data", "address pub priv") -ImportNode = collections.namedtuple("ImportNode", "rescan") +Rescan = enum.Enum("Rescan", "no yes late_timestamp") -def call_import_rpc(call, data, address, scriptPubKey, pubkey, key, label, node, rescan): - """Helper that calls a wallet import RPC on a bitcoin node.""" - watchonly = data != Data.priv - if call == Call.single: - if data == Data.address: - response = node.importaddress(address, label, rescan) - elif data == Data.pub: - response = node.importpubkey(pubkey, label, rescan) - elif data == Data.priv: - response = node.importprivkey(key, label, rescan) - assert_equal(response, None) - elif call == Call.multi: - response = node.importmulti([{ - "scriptPubKey": { - "address": address - }, - "timestamp": "now", - "pubkeys": [pubkey] if data == Data.pub else [], - "keys": [key] if data == Data.priv else [], - "label": label, - "watchonly": watchonly - }], {"rescan": rescan}) - assert_equal(response, [{"success": True}]) - return watchonly +class Variant(collections.namedtuple("Variant", "call data rescan prune")): + """Helper for importing one key and verifying scanned transactions.""" + + def do_import(self, timestamp): + """Call one key import RPC.""" + + if self.call == Call.single: + if self.data == Data.address: + response, error = try_rpc(self.node.importaddress, self.address["address"], self.label, + self.rescan == Rescan.yes) + elif self.data == Data.pub: + response, error = try_rpc(self.node.importpubkey, self.address["pubkey"], self.label, + self.rescan == Rescan.yes) + elif self.data == Data.priv: + response, error = try_rpc(self.node.importprivkey, self.key, self.label, self.rescan == Rescan.yes) + assert_equal(response, None) + assert_equal(error, {'message': 'Rescan is disabled in pruned mode', + 'code': -4} if self.expect_disabled else None) + elif self.call == Call.multi: + response = self.node.importmulti([{ + "scriptPubKey": { + "address": self.address["address"] + }, + "timestamp": timestamp + RESCAN_WINDOW + (1 if self.rescan == Rescan.late_timestamp else 0), + "pubkeys": [self.address["pubkey"]] if self.data == Data.pub else [], + "keys": [self.key] if self.data == Data.priv else [], + "label": self.label, + "watchonly": self.data != Data.priv + }], {"rescan": self.rescan in (Rescan.yes, Rescan.late_timestamp)}) + assert_equal(response, [{"success": True}]) + + def check(self, txid=None, amount=None, confirmations=None): + """Verify that getbalance/listtransactions return expected values.""" + + balance = self.node.getbalance(self.label, 0, False, True) + assert_equal(balance, self.expected_balance) + + txs = self.node.listtransactions(self.label, 10000, 0, True) + assert_equal(len(txs), self.expected_txs) + + if txid is not None: + tx, = [tx for tx in txs if tx["txid"] == txid] + assert_equal(tx["account"], self.label) + assert_equal(tx["address"], self.address["address"]) + assert_equal(tx["amount"], amount) + assert_equal(tx["category"], "receive") + assert_equal(tx["label"], self.label) + assert_equal(tx["txid"], txid) + assert_equal(tx["confirmations"], confirmations) + assert_equal("trusted" not in tx, True) + if self.data != Data.priv: + assert_equal(tx["involvesWatchonly"], True) + else: + assert_equal("involvesWatchonly" not in tx, True) -# List of RPCs that import a wallet key or address in various ways. -IMPORT_RPCS = [functools.partial(call_import_rpc, call, data) for call, data in itertools.product(Call, Data)] +# List of Variants for each way a key or address could be imported. +IMPORT_VARIANTS = [Variant(*variants) for variants in itertools.product(Call, Data, Rescan, (False, True))] -# List of bitcoind nodes that will import keys. -IMPORT_NODES = [ - ImportNode(rescan=True), - ImportNode(rescan=False), -] +# List of nodes to import keys to. Half the nodes will have pruning disabled, +# half will have it enabled. Different nodes will be used for imports that are +# expected to cause rescans, and imports that are not expected to cause +# rescans, in order to prevent rescans during later imports picking up +# transactions associated with earlier imports. This makes it easier to keep +# track of expected balances and transactions. +ImportNode = collections.namedtuple("ImportNode", "prune rescan") +IMPORT_NODES = [ImportNode(*fields) for fields in itertools.product((False, True), repeat=2)] + +# Rescans start at the earliest block up to 2 hours before the key timestamp. +RESCAN_WINDOW = 2 * 60 * 60 class ImportRescanTest(BitcoinTestFramework): @@ -60,6 +110,10 @@ class ImportRescanTest(BitcoinTestFramework): def setup_network(self): extra_args = [["-debug=1"] for _ in range(self.num_nodes)] + for i, import_node in enumerate(IMPORT_NODES, 1): + if import_node.prune: + extra_args[i] += ["-prune=1"] + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, extra_args) for i in range(1, self.num_nodes): connect_nodes(self.nodes[i], 0) @@ -67,89 +121,64 @@ class ImportRescanTest(BitcoinTestFramework): def run_test(self): # Create one transaction on node 0 with a unique amount and label for # each possible type of wallet import RPC. - import_rpc_variants = [] - for i, import_rpc in enumerate(IMPORT_RPCS): - label = "label{}".format(i) - addr = self.nodes[0].validateaddress(self.nodes[0].getnewaddress(label)) - key = self.nodes[0].dumpprivkey(addr["address"]) - amount = 24.9375 - i * .0625 - txid = self.nodes[0].sendtoaddress(addr["address"], amount) - import_rpc = functools.partial(import_rpc, addr["address"], addr["scriptPubKey"], addr["pubkey"], key, - label) - import_rpc_variants.append((import_rpc, label, amount, txid, addr)) + for i, variant in enumerate(IMPORT_VARIANTS): + variant.label = "label {} {}".format(i, variant) + variant.address = self.nodes[0].validateaddress(self.nodes[0].getnewaddress(variant.label)) + variant.key = self.nodes[0].dumpprivkey(variant.address["address"]) + variant.initial_amount = 25 - (i + 1) / 4.0 + variant.initial_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.initial_amount) + # Generate a block containing the initial transactions, then another + # block further in the future (past the rescan window). self.nodes[0].generate(1) assert_equal(self.nodes[0].getrawmempool(), []) + timestamp = self.nodes[0].getblockheader(self.nodes[0].getbestblockhash())["time"] + set_node_times(self.nodes, timestamp + RESCAN_WINDOW + 1) + self.nodes[0].generate(1) sync_blocks(self.nodes) - # For each importing node and variation of wallet import RPC, invoke - # the RPC and check the results from getbalance and listtransactions. - for node, import_node in zip(self.nodes[1:], IMPORT_NODES): - for import_rpc, label, amount, txid, addr in import_rpc_variants: - watchonly = import_rpc(node, import_node.rescan) + # For each variation of wallet key import, invoke the import RPC and + # check the results from getbalance and listtransactions. + for variant in IMPORT_VARIANTS: + variant.expect_disabled = variant.rescan == Rescan.yes and variant.prune and variant.call == Call.single + expect_rescan = variant.rescan == Rescan.yes and not variant.expect_disabled + variant.node = self.nodes[1 + IMPORT_NODES.index(ImportNode(variant.prune, expect_rescan))] + variant.do_import(timestamp) + if expect_rescan: + variant.expected_balance = variant.initial_amount + variant.expected_txs = 1 + variant.check(variant.initial_txid, variant.initial_amount, 2) + else: + variant.expected_balance = 0 + variant.expected_txs = 0 + variant.check() - balance = node.getbalance(label, 0, False, True) - if import_node.rescan: - assert_equal(balance, amount) - else: - assert_equal(balance, 0) - - txs = node.listtransactions(label, 10000, 0, True) - if import_node.rescan: - assert_equal(len(txs), 1) - assert_equal(txs[0]["account"], label) - assert_equal(txs[0]["address"], addr["address"]) - assert_equal(txs[0]["amount"], amount) - assert_equal(txs[0]["category"], "receive") - assert_equal(txs[0]["label"], label) - assert_equal(txs[0]["txid"], txid) - assert_equal(txs[0]["confirmations"], 1) - assert_equal("trusted" not in txs[0], True) - if watchonly: - assert_equal(txs[0]["involvesWatchonly"], True) - else: - assert_equal("involvesWatchonly" not in txs[0], True) - else: - assert_equal(len(txs), 0) - - # Create spends for all the imported addresses. - spend_txids = [] + # Create new transactions sending to each address. fee = self.nodes[0].getnetworkinfo()["relayfee"] - for import_rpc, label, amount, txid, addr in import_rpc_variants: - raw_tx = self.nodes[0].getrawtransaction(txid) - decoded_tx = self.nodes[0].decoderawtransaction(raw_tx) - input_vout = next(out["n"] for out in decoded_tx["vout"] - if out["scriptPubKey"]["addresses"] == [addr["address"]]) - inputs = [{"txid": txid, "vout": input_vout}] - outputs = {self.nodes[0].getnewaddress(): Decimal(amount) - fee} - raw_spend_tx = self.nodes[0].createrawtransaction(inputs, outputs) - signed_spend_tx = self.nodes[0].signrawtransaction(raw_spend_tx) - spend_txid = self.nodes[0].sendrawtransaction(signed_spend_tx["hex"]) - spend_txids.append(spend_txid) + for i, variant in enumerate(IMPORT_VARIANTS): + variant.sent_amount = 25 - (2 * i + 1) / 8.0 + variant.sent_txid = self.nodes[0].sendtoaddress(variant.address["address"], variant.sent_amount) + # Generate a block containing the new transactions. self.nodes[0].generate(1) assert_equal(self.nodes[0].getrawmempool(), []) sync_blocks(self.nodes) - # Check the results from getbalance and listtransactions after the spends. - for node, import_node in zip(self.nodes[1:], IMPORT_NODES): - txs = node.listtransactions("*", 10000, 0, True) - for (import_rpc, label, amount, txid, addr), spend_txid in zip(import_rpc_variants, spend_txids): - balance = node.getbalance(label, 0, False, True) - spend_tx = [tx for tx in txs if tx["txid"] == spend_txid] - if import_node.rescan: - assert_equal(balance, amount) - assert_equal(len(spend_tx), 1) - assert_equal(spend_tx[0]["account"], "") - assert_equal(spend_tx[0]["amount"] + spend_tx[0]["fee"], -amount) - assert_equal(spend_tx[0]["category"], "send") - assert_equal("label" not in spend_tx[0], True) - assert_equal(spend_tx[0]["confirmations"], 1) - assert_equal("trusted" not in spend_tx[0], True) - assert_equal("involvesWatchonly" not in txs[0], True) - else: - assert_equal(balance, 0) - assert_equal(spend_tx, []) + # Check the latest results from getbalance and listtransactions. + for variant in IMPORT_VARIANTS: + if not variant.expect_disabled: + variant.expected_balance += variant.sent_amount + variant.expected_txs += 1 + variant.check(variant.sent_txid, variant.sent_amount, 1) + else: + variant.check() + + +def try_rpc(func, *args, **kwargs): + try: + return func(*args, **kwargs), None + except JSONRPCException as e: + return None, e.error if __name__ == "__main__": diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 72d3e12393..18dc5b1a73 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -1191,7 +1191,8 @@ UniValue importmulti(const JSONRPCRequest& mainRequest) " or the string \"now\" to substitute the current synced blockchain time. The timestamp of the oldest\n" " key will determine how far back blockchain rescans need to begin for missing wallet transactions.\n" " \"now\" can be specified to bypass scanning, for keys which are known to never have been used, and\n" - " 0 can be specified to scan the entire blockchain.\n" + " 0 can be specified to scan the entire blockchain. Blocks up to 2 hours before the earliest key\n" + " creation time of all keys being imported by the importmulti call will be scanned.\n" " \"redeemscript\": \"