From b900c4827b775d1d07ecdafca5c4224761cb33eb Mon Sep 17 00:00:00 2001 From: Taylor Hornby Date: Fri, 6 May 2022 11:59:28 -0600 Subject: [PATCH 1/2] Reproduce an assertion failure in the listaddresses RPC --- qa/pull-tester/rpc-tests.py | 1 + qa/rpc-tests/threeofthreerestore.py | 64 +++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100755 qa/rpc-tests/threeofthreerestore.py diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 454cc0670..0e61bee21 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -141,6 +141,7 @@ BASE_SCRIPTS= [ 'wallet_broadcast.py', 'wallet_z_sendmany.py', 'wallet_zero_value.py', + 'threeofthreerestore.py', ] ZMQ_SCRIPTS = [ diff --git a/qa/rpc-tests/threeofthreerestore.py b/qa/rpc-tests/threeofthreerestore.py new file mode 100755 index 000000000..8becfd986 --- /dev/null +++ b/qa/rpc-tests/threeofthreerestore.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 +# Copyright (c) 2014-2016 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://www.opensource.org/licenses/mit-license.php . + +from test_framework.test_framework import BitcoinTestFramework +from test_framework.authproxy import JSONRPCException +from test_framework.util import assert_equal, assert_greater_than, \ + start_nodes, connect_nodes_bi, stop_nodes, wait_and_assert_operationid_status, \ + wait_bitcoinds + +from decimal import Decimal + +class ThreeOfThreeRestoreTest(BitcoinTestFramework): + + def __init__(self): + super().__init__() + self.setup_clean_chain = True + self.num_nodes = 4 + + def setup_network(self, split=False): + self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, + extra_args=[['-experimentalfeatures', '-developerencryptwallet']] * 4) + + 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): + addr1 = self.nodes[2].getnewaddress() + addr2 = self.nodes[2].getnewaddress() + addr3 = self.nodes[2].getnewaddress() + + addr1Obj = self.nodes[2].validateaddress(addr1) + addr2Obj = self.nodes[2].validateaddress(addr2) + addr3Obj = self.nodes[2].validateaddress(addr3) + + key1 = self.nodes[2].dumpprivkey(addr1) + key2 = self.nodes[2].dumpprivkey(addr2) + key3 = self.nodes[2].dumpprivkey(addr3) + + self.nodes[3].importprivkey(key1, "", True) # TODO + self.nodes[3].importprivkey(key2, "", True) + self.nodes[3].importprivkey(key3, "", True) + + + mSigObj = self.nodes[3].addmultisigaddress(3, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']]) + validateObj = self.nodes[3].validateaddress(mSigObj) + + print(validateObj) + + # Causes the node to crash + obj = self.nodes[3].listaddresses() + print(obj) + + assert(validateObj["isvalid"]) + assert(validateObj["ismine"]) + +if __name__ == '__main__': + ThreeOfThreeRestoreTest().main() From 66fad228731f35bc0a96d6fe298aef87d4e50da0 Mon Sep 17 00:00:00 2001 From: Kris Nuttycombe Date: Fri, 6 May 2022 13:24:58 -0600 Subject: [PATCH 2/2] Fix missing handling for imported transparent multisig addresses. --- qa/rpc-tests/threeofthreerestore.py | 27 +++++++++++---------------- src/wallet/rpcwallet.cpp | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 16 deletions(-) diff --git a/qa/rpc-tests/threeofthreerestore.py b/qa/rpc-tests/threeofthreerestore.py index 8becfd986..ce79ae31d 100755 --- a/qa/rpc-tests/threeofthreerestore.py +++ b/qa/rpc-tests/threeofthreerestore.py @@ -1,15 +1,10 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2016 The Bitcoin Core developers +# Copyright (c) 2022 The Zcash developers # Distributed under the MIT software license, see the accompanying # file COPYING or https://www.opensource.org/licenses/mit-license.php . from test_framework.test_framework import BitcoinTestFramework -from test_framework.authproxy import JSONRPCException -from test_framework.util import assert_equal, assert_greater_than, \ - start_nodes, connect_nodes_bi, stop_nodes, wait_and_assert_operationid_status, \ - wait_bitcoinds - -from decimal import Decimal +from test_framework.util import start_nodes, connect_nodes_bi class ThreeOfThreeRestoreTest(BitcoinTestFramework): @@ -43,22 +38,22 @@ class ThreeOfThreeRestoreTest(BitcoinTestFramework): key2 = self.nodes[2].dumpprivkey(addr2) key3 = self.nodes[2].dumpprivkey(addr3) - self.nodes[3].importprivkey(key1, "", True) # TODO + self.nodes[3].importprivkey(key1, "", True) self.nodes[3].importprivkey(key2, "", True) self.nodes[3].importprivkey(key3, "", True) - mSigObj = self.nodes[3].addmultisigaddress(3, [addr1Obj['pubkey'], addr2Obj['pubkey'], addr3Obj['pubkey']]) validateObj = self.nodes[3].validateaddress(mSigObj) - - print(validateObj) - - # Causes the node to crash - obj = self.nodes[3].listaddresses() - print(obj) - assert(validateObj["isvalid"]) assert(validateObj["ismine"]) + # Ensure that the multisig address is returned + list_result = self.nodes[3].listaddresses() + + assert('imported' in obj['source'] for obj in list_result) + for obj in list_result: + if obj['source'] == 'imported': + assert(validateObj['address'] in obj['transparent']['addresses']) + if __name__ == '__main__': ThreeOfThreeRestoreTest().main() diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index faea46efc..9838aa1b3 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -408,12 +408,14 @@ UniValue listaddresses(const UniValue& params, bool fHelp) // Split transparent addresses into several categories: // - Generated randomly. + // - Imported // - Imported watchonly. // - Derived from mnemonic seed. std::set t_generated_dests; std::set t_generated_change_dests; std::set t_mnemonic_dests; std::set t_mnemonic_change_dests; + std::set t_imported_dests; std::set t_watchonly_dests; // Get the CTxDestination values for all the entries in the transparent address book. // This will include any address that has been generated by this wallet. @@ -433,6 +435,9 @@ UniValue listaddresses(const UniValue& params, bool fHelp) case PaymentAddressSource::Random: t_generated_dests.insert(item.first); break; + case PaymentAddressSource::Imported: + t_imported_dests.insert(item.first); + break; case PaymentAddressSource::ImportedWatchOnly: t_watchonly_dests.insert(item.first); break; @@ -453,6 +458,7 @@ UniValue listaddresses(const UniValue& params, bool fHelp) for (const std::pair& item : pwalletMain->GetAddressBalances()) { if (t_generated_dests.count(item.first) == 0 && t_mnemonic_dests.count(item.first) == 0 && + t_imported_dests.count(item.first) == 0 && t_watchonly_dests.count(item.first) == 0) { std::optional source; @@ -607,6 +613,19 @@ UniValue listaddresses(const UniValue& params, bool fHelp) bool hasData = false; + if (!t_imported_dests.empty()) { + UniValue t_imported_addrs(UniValue::VARR); + for (const CTxDestination& dest: t_imported_dests) { + t_imported_addrs.push_back(keyIO.EncodeDestination(dest)); + } + + UniValue imported_t(UniValue::VOBJ); + imported_t.pushKV("addresses", t_imported_addrs); + + entry.pushKV("transparent", imported_t); + hasData = true; + } + { UniValue imported_sprout_addrs(UniValue::VARR); for (const SproutPaymentAddress& addr : sproutAddresses) {