From d29b6b042f42045f259c9613c4a3b34abd7bbe41 Mon Sep 17 00:00:00 2001 From: Nathan Wilcox Date: Mon, 18 Sep 2017 14:56:37 +0900 Subject: [PATCH 1/7] key_import_export rpc-test: verify that UTXO view co-evolves for nodes sharing a key. --- qa/pull-tester/rpc-tests.sh | 1 + qa/rpc-tests/key_import_export.py | 111 ++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+) create mode 100755 qa/rpc-tests/key_import_export.py diff --git a/qa/pull-tester/rpc-tests.sh b/qa/pull-tester/rpc-tests.sh index bd3357642..1483ead83 100755 --- a/qa/pull-tester/rpc-tests.sh +++ b/qa/pull-tester/rpc-tests.sh @@ -34,6 +34,7 @@ testScripts=( 'fundrawtransaction.py' 'signrawtransactions.py' 'walletbackup.py' + 'key_import_export.py' 'nodehandling.py' 'reindex.py' 'decodescript.py' diff --git a/qa/rpc-tests/key_import_export.py b/qa/rpc-tests/key_import_export.py new file mode 100755 index 000000000..a80babd5f --- /dev/null +++ b/qa/rpc-tests/key_import_export.py @@ -0,0 +1,111 @@ +#!/usr/bin/env python2 +# Copyright (c) 2017 The Zcash developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or http://www.opensource.org/licenses/mit-license.php. + +from time import sleep +from decimal import Decimal +from test_framework.test_framework import BitcoinTestFramework +from test_framework.util import assert_equal, assert_greater_than, sync_blocks + +import logging + +logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.INFO) + + +class KeyImportExportTest (BitcoinTestFramework): + + def run_test(self): + [alice, bob, charlie, miner] = self.nodes + + def wait_until_miner_sees(txid): + attempts = 123 + delay = 0.5 + + for _ in range(attempts): + try: + miner.getrawtransaction(txid) + except Exception: + logging.debug( + 'txid %r not yet seen by miner; sleep %r', + txid, + delay, + ) + sleep(delay) + else: + return + + raise Exception( + 'miner failed to see txid {!r} after {!r} attempts...'.format( + txid, + attempts, + ), + ) + + def alice_to_bob(amount): + txid = alice.sendtoaddress(addr, Decimal(amount)) + wait_until_miner_sees(txid) + miner.generate(1) + sync_blocks(self.nodes) + + def verify_utxos(node, amounts): + utxos = node.listunspent(1, 10**9, [addr]) + + def cmp_confirmations_high_to_low(a, b): + return cmp(b["confirmations"], a["confirmations"]) + + utxos.sort(cmp_confirmations_high_to_low) + + try: + assert_equal(amounts, [utxo["amount"] for utxo in utxos]) + except AssertionError: + logging.error( + 'Expected amounts: %r; utxos: %r', + amounts, utxos) + raise + + # The first address already mutated by the startup process, ignore it: + # BUG: Why is this necessary? + bob.getnewaddress() + + # Now get a pristine address for receiving transfers: + addr = bob.getnewaddress() + verify_utxos(bob, []) + + # the amounts of each txn embodied which generates a single UTXO: + amounts = map(Decimal, ['2.3', '3.7', '0.1', '0.5', '1.0', '0.19']) + + # Internal test consistency assertion: + assert_greater_than( + alice.getbalance(), + reduce(Decimal.__add__, amounts)) + + logging.info("Sending pre-export txns...") + for amount in amounts[0:2]: + alice_to_bob(amount) + + logging.info("Exporting privkey from bob...") + privkey = bob.dumpprivkey(addr) + + logging.info("Sending post-export txns...") + for amount in amounts[2:4]: + alice_to_bob(amount) + + verify_utxos(bob, amounts[:4]) + + logging.info("Importing privkey into charlie...") + charlie.importprivkey(privkey, '', True) + + # importprivkey should have rescanned, so this should pass: + verify_utxos(charlie, amounts[:4]) + + logging.info("Sending post-import txns...") + for amount in amounts[4:]: + alice_to_bob(amount) + + verify_utxos(bob, amounts) + verify_utxos(charlie, amounts) + + +if __name__ == '__main__': + KeyImportExportTest().main() From 63cdea423354c9bf769a5eb51c5c3b9f5905e8a4 Mon Sep 17 00:00:00 2001 From: Nathan Wilcox Date: Mon, 18 Sep 2017 15:19:24 +0900 Subject: [PATCH 2/7] Add a new rpc-test-specified requirement: `importprivkey` outputs the associated address. (Test fails.) --- qa/rpc-tests/key_import_export.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/key_import_export.py b/qa/rpc-tests/key_import_export.py index a80babd5f..00129914a 100755 --- a/qa/rpc-tests/key_import_export.py +++ b/qa/rpc-tests/key_import_export.py @@ -94,7 +94,8 @@ class KeyImportExportTest (BitcoinTestFramework): verify_utxos(bob, amounts[:4]) logging.info("Importing privkey into charlie...") - charlie.importprivkey(privkey, '', True) + ipkaddr = charlie.importprivkey(privkey, '', True) + assert_equal(addr, ipkaddr) # importprivkey should have rescanned, so this should pass: verify_utxos(charlie, amounts[:4]) From d187317083fedbac06c12decfa6a99655c200f86 Mon Sep 17 00:00:00 2001 From: Nathan Wilcox Date: Mon, 18 Sep 2017 15:24:30 +0900 Subject: [PATCH 3/7] [tests pass] Output address on new key import. --- src/wallet/rpcdump.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 936532837..578c0e1fe 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -144,7 +144,7 @@ UniValue importprivkey(const UniValue& params, bool fHelp) } } - return NullUniValue; + return CBitcoinAddress(vchAddress).ToString(); } UniValue importaddress(const UniValue& params, bool fHelp) From 3e92c028ce5cfe5cfbdb9ac067e2055020aeebca Mon Sep 17 00:00:00 2001 From: Nathan Wilcox Date: Mon, 18 Sep 2017 15:27:22 +0900 Subject: [PATCH 4/7] Add a new requirement that `importprivkey` API is idempotent. --- qa/rpc-tests/key_import_export.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/qa/rpc-tests/key_import_export.py b/qa/rpc-tests/key_import_export.py index 00129914a..debade1a3 100755 --- a/qa/rpc-tests/key_import_export.py +++ b/qa/rpc-tests/key_import_export.py @@ -97,6 +97,10 @@ class KeyImportExportTest (BitcoinTestFramework): ipkaddr = charlie.importprivkey(privkey, '', True) assert_equal(addr, ipkaddr) + # Verify idempotent behavior: + ipkaddr2 = charlie.importprivkey(privkey, '', True) + assert_equal(addr, ipkaddr2) + # importprivkey should have rescanned, so this should pass: verify_utxos(charlie, amounts[:4]) From de422c066b70e6f54994c123b56d1bb3bdcc2733 Mon Sep 17 00:00:00 2001 From: Nathan Wilcox Date: Mon, 18 Sep 2017 15:29:10 +0900 Subject: [PATCH 5/7] [tests pass] Ensure `importprivkey` outputs the address in case key is already imported. --- src/wallet/rpcdump.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 578c0e1fe..fe5b83e8d 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -128,8 +128,9 @@ UniValue importprivkey(const UniValue& params, bool fHelp) pwalletMain->SetAddressBook(vchAddress, strLabel, "receive"); // Don't throw error in case a key is already there - if (pwalletMain->HaveKey(vchAddress)) - return NullUniValue; + if (pwalletMain->HaveKey(vchAddress)) { + return CBitcoinAddress(vchAddress).ToString(); + } pwalletMain->mapKeyMetadata[vchAddress].nCreateTime = 1; From 5a0721f0fe5f236cdc716f5b05e0dd88a1a233a6 Mon Sep 17 00:00:00 2001 From: Simon Date: Tue, 19 Sep 2017 20:37:02 -0700 Subject: [PATCH 6/7] Set up a clean chain. Delete redundant method wait_until_miner_sees() via use of sync_all(). --- qa/rpc-tests/key_import_export.py | 51 +++++++++++++------------------ 1 file changed, 21 insertions(+), 30 deletions(-) diff --git a/qa/rpc-tests/key_import_export.py b/qa/rpc-tests/key_import_export.py index debade1a3..6b46c189e 100755 --- a/qa/rpc-tests/key_import_export.py +++ b/qa/rpc-tests/key_import_export.py @@ -6,7 +6,7 @@ from time import sleep from decimal import Decimal from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import assert_equal, assert_greater_than, sync_blocks +from test_framework.util import assert_equal, assert_greater_than, sync_blocks, start_nodes, initialize_chain_clean, connect_nodes_bi import logging @@ -15,38 +15,27 @@ logging.basicConfig(format='%(levelname)s:%(message)s', level=logging.INFO) class KeyImportExportTest (BitcoinTestFramework): + def setup_chain(self): + print("Initializing test directory "+self.options.tmpdir) + initialize_chain_clean(self.options.tmpdir, 4) + + def setup_network(self, split=False): + self.nodes = start_nodes(4, self.options.tmpdir ) + connect_nodes_bi(self.nodes,0,1) + connect_nodes_bi(self.nodes,1,2) + connect_nodes_bi(self.nodes,0,2) + connect_nodes_bi(self.nodes,0,3) + self.is_network_split=False + self.sync_all() + def run_test(self): [alice, bob, charlie, miner] = self.nodes - def wait_until_miner_sees(txid): - attempts = 123 - delay = 0.5 - - for _ in range(attempts): - try: - miner.getrawtransaction(txid) - except Exception: - logging.debug( - 'txid %r not yet seen by miner; sleep %r', - txid, - delay, - ) - sleep(delay) - else: - return - - raise Exception( - 'miner failed to see txid {!r} after {!r} attempts...'.format( - txid, - attempts, - ), - ) - def alice_to_bob(amount): txid = alice.sendtoaddress(addr, Decimal(amount)) - wait_until_miner_sees(txid) + self.sync_all() miner.generate(1) - sync_blocks(self.nodes) + self.sync_all() def verify_utxos(node, amounts): utxos = node.listunspent(1, 10**9, [addr]) @@ -64,9 +53,11 @@ class KeyImportExportTest (BitcoinTestFramework): amounts, utxos) raise - # The first address already mutated by the startup process, ignore it: - # BUG: Why is this necessary? - bob.getnewaddress() + # Seed Alice with some funds + alice.generate(10) + self.sync_all() + miner.generate(100) + self.sync_all() # Now get a pristine address for receiving transfers: addr = bob.getnewaddress() From 109fed51ff496a3aeaead6b869793e85d7373483 Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Thu, 21 Sep 2017 22:21:44 +0100 Subject: [PATCH 7/7] Additional test cases for importprivkey RPC test --- qa/rpc-tests/key_import_export.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/qa/rpc-tests/key_import_export.py b/qa/rpc-tests/key_import_export.py index 6b46c189e..3f075bc76 100755 --- a/qa/rpc-tests/key_import_export.py +++ b/qa/rpc-tests/key_import_export.py @@ -62,6 +62,7 @@ class KeyImportExportTest (BitcoinTestFramework): # Now get a pristine address for receiving transfers: addr = bob.getnewaddress() verify_utxos(bob, []) + verify_utxos(charlie, []) # the amounts of each txn embodied which generates a single UTXO: amounts = map(Decimal, ['2.3', '3.7', '0.1', '0.5', '1.0', '0.19']) @@ -83,16 +84,20 @@ class KeyImportExportTest (BitcoinTestFramework): alice_to_bob(amount) verify_utxos(bob, amounts[:4]) + verify_utxos(charlie, []) logging.info("Importing privkey into charlie...") ipkaddr = charlie.importprivkey(privkey, '', True) assert_equal(addr, ipkaddr) + # importprivkey should have rescanned, so this should pass: + verify_utxos(charlie, amounts[:4]) + # Verify idempotent behavior: ipkaddr2 = charlie.importprivkey(privkey, '', True) assert_equal(addr, ipkaddr2) - # importprivkey should have rescanned, so this should pass: + # amounts should be unchanged verify_utxos(charlie, amounts[:4]) logging.info("Sending post-import txns...")