Merge pull request #5930 from defuse/2022-05-rpc-assertion-failure

Test an assertion failure in the listaddresses RPC

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 = [

qa/rpc-tests/threeofthreerestore.py

@@ -0,0 +1,59 @@
+#!/usr/bin/env python3
+# 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.util import start_nodes, connect_nodes_bi 
+
+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)
+        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)
+        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()

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<CTxDestination> t_generated_dests;
     std::set<CTxDestination> t_generated_change_dests;
     std::set<CTxDestination> t_mnemonic_dests;
     std::set<CTxDestination> t_mnemonic_change_dests;
+    std::set<CTxDestination> t_imported_dests;
     std::set<CTxDestination> 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<CTxDestination, CAmount>& 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<PaymentAddressSource> 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) {