From 20266ac91112cf753a6c74b8b13d2d2c49540937 Mon Sep 17 00:00:00 2001
From: Kris Nuttycombe <kris@nutty.land>
Date: Wed, 29 Dec 2021 13:44:55 -0700
Subject: [PATCH] Remove uses of KeyIO::DecodeDestination

---
 qa/rpc-tests/wallet_shieldingcoinbase.py      |   8 +-
 src/chainparams.cpp                           |   8 +-
 src/consensus/params.cpp                      |  29 ++-
 src/init.cpp                                  |  15 +-
 .../asyncrpcoperation_mergetoaddress.cpp      |  39 +++-
 src/wallet/asyncrpcoperation_sendmany.cpp     |  38 +++-
 src/wallet/asyncrpcoperation_sendmany.h       |  10 +-
 src/wallet/rpcwallet.cpp                      | 190 ++++++++++--------
 src/wallet/test/rpc_wallet_tests.cpp          |  17 +-
 9 files changed, 216 insertions(+), 138 deletions(-)

diff --git a/qa/rpc-tests/wallet_shieldingcoinbase.py b/qa/rpc-tests/wallet_shieldingcoinbase.py
index 01f7ee8e6..129af7011 100755
--- a/qa/rpc-tests/wallet_shieldingcoinbase.py
+++ b/qa/rpc-tests/wallet_shieldingcoinbase.py
@@ -82,9 +82,11 @@ class WalletShieldingCoinbaseTest (BitcoinTestFramework):
         # Node 3 will test that watch only address utxos are not selected
         self.nodes[3].importaddress(mytaddr)
         recipients= [{"address":myzaddr, "amount": Decimal('1')}]
-        myopid = self.nodes[3].z_sendmany(mytaddr, recipients)
-
-        wait_and_assert_operationid_status(self.nodes[3], myopid, "failed", "Insufficient transparent funds, no UTXOs found for taddr from address.", 10)
+        try:
+            myopid = self.nodes[3].z_sendmany(mytaddr, recipients)
+        except JSONRPCException as e:
+            errorString = e.error['message']
+        assert_equal("Invalid from address: does not belong to this node, spending key not found.", errorString);
 
         # This send will fail because our wallet does not allow any change when shielding a coinbase utxo,
         # as it's currently not possible to specify a change address in z_sendmany.
diff --git a/src/chainparams.cpp b/src/chainparams.cpp
index d9a7cb182..d117db8ee 100644
--- a/src/chainparams.cpp
+++ b/src/chainparams.cpp
@@ -822,10 +822,10 @@ CScript CChainParams::GetFoundersRewardScriptAtHeight(int nHeight) const {
     assert(nHeight > 0 && nHeight <= consensus.GetLastFoundersRewardBlockHeight(nHeight));
 
     KeyIO keyIO(*this);
-    CTxDestination address = keyIO.DecodeDestination(GetFoundersRewardAddressAtHeight(nHeight).c_str());
-    assert(IsValidDestination(address));
-    assert(IsScriptDestination(address));
-    CScriptID scriptID = std::get<CScriptID>(address); // address is a variant
+    auto address = keyIO.DecodePaymentAddress(GetFoundersRewardAddressAtHeight(nHeight).c_str());
+    assert(address.has_value());
+    assert(std::holds_alternative<CScriptID>(address.value()));
+    CScriptID scriptID = std::get<CScriptID>(address.value());
     CScript script = CScript() << OP_HASH160 << ToByteVector(scriptID) << OP_EQUAL;
     return script;
 }
diff --git a/src/consensus/params.cpp b/src/consensus/params.cpp
index 58528b7b4..f1641e7aa 100644
--- a/src/consensus/params.cpp
+++ b/src/consensus/params.cpp
@@ -9,6 +9,7 @@
 #include <script/standard.h>
 #include "upgrades.h"
 #include "util.h"
+#include "util/match.h"
 
 namespace Consensus {
     bool Params::NetworkUpgradeActive(int nHeight, Consensus::UpgradeIndex idx) const {
@@ -163,18 +164,26 @@ namespace Consensus {
 
         // Parse the address strings into concrete types.
         std::vector<FundingStreamAddress> addresses;
-        for (auto addr : strAddresses) {
-            auto taddr = keyIO.DecodeDestination(addr);
-            if (IsValidDestination(taddr)) {
-                addresses.push_back(GetScriptForDestination(taddr));
-            } else {
-                auto zaddr = keyIO.DecodePaymentAddress(addr);
-                if (!(zaddr.has_value() && std::holds_alternative<libzcash::SaplingPaymentAddress>(zaddr.value()))) {
+        for (const auto& strAddr : strAddresses) {
+            auto addr = keyIO.DecodePaymentAddress(strAddr);
+            if (!addr.has_value()) {
+                throw std::runtime_error("Funding stream address was not valid Zcash address.");
+            }
+
+            std::visit(match {
+                [&](const CKeyID& keyId) {
+                    addresses.push_back(GetScriptForDestination(keyId));
+                },
+                [&](const CScriptID& scriptId) {
+                    addresses.push_back(GetScriptForDestination(scriptId));
+                },
+                [&](const libzcash::SaplingPaymentAddress& zaddr) {
+                    addresses.push_back(zaddr);
+                },
+                [&](const auto& zaddr) {
                     throw std::runtime_error("Funding stream address was not a valid transparent or Sapling address.");
                 }
-
-                addresses.push_back(std::get<libzcash::SaplingPaymentAddress>(zaddr.value()));
-            }
+            }, addr.value());
         }
 
         auto validationResult = FundingStream::ValidateFundingStream(params, startHeight, endHeight, addresses);
diff --git a/src/init.cpp b/src/init.cpp
index 851c1f9bb..300c45816 100644
--- a/src/init.cpp
+++ b/src/init.cpp
@@ -1098,16 +1098,11 @@ bool AppInit2(boost::thread_group& threadGroup, CScheduler& scheduler)
     KeyIO keyIO(chainparams);
 #ifdef ENABLE_MINING
     if (mapArgs.count("-mineraddress")) {
-        CTxDestination addr = keyIO.DecodeDestination(mapArgs["-mineraddress"]);
-        if (!IsValidDestination(addr)) {
-            // Try a payment address
-            auto zaddr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]);
-            if (!zaddr.has_value() || std::visit(ExtractMinerAddress(), zaddr.value()).has_value())
-            {
-                return InitError(strprintf(
-                    _("Invalid address for -mineraddress=<addr>: '%s' (must be a Sapling or transparent address)"),
-                    mapArgs["-mineraddress"]));
-            }
+        auto addr = keyIO.DecodePaymentAddress(mapArgs["-mineraddress"]);
+        if (!(addr.has_value() && std::visit(ExtractMinerAddress(), addr.value()).has_value())) {
+            return InitError(strprintf(
+                _("Invalid address for -mineraddress=<addr>: '%s' (must be a Sapling or transparent address)"),
+                mapArgs["-mineraddress"]));
         }
     }
 #endif
diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.cpp b/src/wallet/asyncrpcoperation_mergetoaddress.cpp
index b3ea22bbe..459e52402 100644
--- a/src/wallet/asyncrpcoperation_mergetoaddress.cpp
+++ b/src/wallet/asyncrpcoperation_mergetoaddress.cpp
@@ -22,6 +22,7 @@
 #include "timedata.h"
 #include "transaction_builder.h"
 #include "util.h"
+#include "util/match.h"
 #include "utilmoneystr.h"
 #include "utiltime.h"
 #include "wallet.h"
@@ -96,18 +97,36 @@ AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress(
     }
 
     KeyIO keyIO(Params());
-    toTaddr_ = keyIO.DecodeDestination(std::get<0>(recipient));
-    isToTaddr_ = IsValidDestination(toTaddr_);
+    isToTaddr_ = false;
     isToZaddr_ = false;
 
-    if (!isToTaddr_) {
-        auto address = keyIO.DecodePaymentAddress(std::get<0>(recipient));
-        if (address.has_value()) {
-            isToZaddr_ = true;
-            toPaymentAddress_ = address.value();
-        } else {
-            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address");
-        }
+    auto address = keyIO.DecodePaymentAddress(std::get<0>(recipient));
+    if (address.has_value()) {
+        std::visit(match {
+            [&](const CKeyID& keyId) {
+                toTaddr_ = keyId;
+                isToTaddr_ = true;
+            },
+            [&](const CScriptID& scriptId) {
+                toTaddr_ = scriptId;
+                isToTaddr_ = true;
+            },
+            [&](const libzcash::SproutPaymentAddress& addr) {
+                toPaymentAddress_ = addr;
+                isToZaddr_ = true;
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) {
+                toPaymentAddress_ = addr;
+                isToZaddr_ = true;
+            },
+            [&](const libzcash::UnifiedAddress& addr) {
+                throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "z_mergetoaddress does not yet support sending to unified addresses");
+            },
+        }, address.value());
+    } else {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid recipient address");
     }
 
     // Log the context info i.e. the call parameters to z_mergetoaddress
diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp
index ac2880e86..91766d5a5 100644
--- a/src/wallet/asyncrpcoperation_sendmany.cpp
+++ b/src/wallet/asyncrpcoperation_sendmany.cpp
@@ -21,6 +21,7 @@
 #include "transaction_builder.h"
 #include "timedata.h"
 #include "util.h"
+#include "util/match.h"
 #include "utilmoneystr.h"
 #include "wallet.h"
 #include "walletdb.h"
@@ -92,20 +93,41 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
     KeyIO keyIO(Params());
 
     useanyutxo_ = fromAddress == "ANY_TADDR";
-    fromtaddr_ = keyIO.DecodeDestination(fromAddress);
-    isfromtaddr_ = useanyutxo_ || IsValidDestination(fromtaddr_);
-    isfromzaddr_ = false;
-
-    if (!isfromtaddr_) {
+    if (useanyutxo_) {
+        isfromtaddr_ = true;
+    } else {
         auto address = keyIO.DecodePaymentAddress(fromAddress);
         if (address.has_value()) {
             // We don't need to lock on the wallet as spending key related methods are thread-safe
             if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), address.value())) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, no spending key found for zaddr");
+                throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "Invalid from address, no spending key found for address");
             }
 
-            isfromzaddr_ = true;
-            frompaymentaddress_ = address.value();
+            std::visit(match {
+                [&](const CKeyID& keyId) {
+                    fromtaddr_ = keyId;
+                    isfromtaddr_ = true;
+                },
+                [&](const CScriptID& scriptId) {
+                    fromtaddr_ = scriptId;
+                    isfromtaddr_ = true;
+                },
+                [&](const libzcash::SproutPaymentAddress& addr) {
+                    frompaymentaddress_ = addr;
+                    isfromzaddr_ = true;
+                },
+                [&](const libzcash::SaplingPaymentAddress& addr) {
+                    frompaymentaddress_ = addr;
+                    isfromzaddr_ = true;
+                },
+                [&](const libzcash::UnifiedAddress& addr) {
+                    throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "Unified addresses are not yet supported by z_sendmany");
+                }
+            }, address.value());
         } else {
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address");
         }
diff --git a/src/wallet/asyncrpcoperation_sendmany.h b/src/wallet/asyncrpcoperation_sendmany.h
index cb3c4b9cb..dfe991811 100644
--- a/src/wallet/asyncrpcoperation_sendmany.h
+++ b/src/wallet/asyncrpcoperation_sendmany.h
@@ -94,14 +94,14 @@ private:
 
     UniValue contextinfo_;     // optional data to include in return value from getStatus()
 
-    bool isUsingBuilder_; // Indicates that no Sprout addresses are involved
+    bool isUsingBuilder_{false}; // Indicates that no Sprout addresses are involved
     uint32_t consensusBranchId_;
     CAmount fee_;
-    int mindepth_;
+    int mindepth_{0};
     std::string fromaddress_;
-    bool useanyutxo_;
-    bool isfromtaddr_;
-    bool isfromzaddr_;
+    bool useanyutxo_{false};
+    bool isfromtaddr_{false};
+    bool isfromzaddr_{false};
     CTxDestination fromtaddr_;
     PaymentAddress frompaymentaddress_;
 
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index 445175d73..847ce4fe8 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2991,24 +2991,13 @@ UniValue z_listaddresses(const UniValue& params, bool fHelp)
     return ret;
 }
 
-CAmount getBalanceTaddr(std::string transparentAddress, int minDepth=1, bool ignoreUnspendable=true) {
-    std::set<CTxDestination> destinations;
+CAmount getBalanceTaddr(const std::optional<CTxDestination>& taddr, int minDepth=1, bool ignoreUnspendable=true) {
     vector<COutput> vecOutputs;
     CAmount balance = 0;
 
-    KeyIO keyIO(Params());
-    if (transparentAddress.length() > 0) {
-        CTxDestination taddr = keyIO.DecodeDestination(transparentAddress);
-        if (!IsValidDestination(taddr)) {
-            throw std::runtime_error("invalid transparent address");
-        }
-        destinations.insert(taddr);
-    }
-
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
     pwalletMain->AvailableCoins(vecOutputs, false, NULL, true);
-
     for (const COutput& out : vecOutputs) {
         if (out.nDepth < minDepth) {
             continue;
@@ -3018,13 +3007,13 @@ CAmount getBalanceTaddr(std::string transparentAddress, int minDepth=1, bool ign
             continue;
         }
 
-        if (destinations.size()) {
+        if (taddr.has_value()) {
             CTxDestination address;
             if (!ExtractDestination(out.tx->vout[out.i].scriptPubKey, address)) {
                 continue;
             }
 
-            if (!destinations.count(address)) {
+            if (address != taddr.value()) {
                 continue;
             }
         }
@@ -3244,26 +3233,35 @@ UniValue z_getbalance(const UniValue& params, bool fHelp)
     KeyIO keyIO(Params());
     // Check that the from address is valid.
     auto fromaddress = params[0].get_str();
-    bool fromTaddr = false;
-    CTxDestination taddr = keyIO.DecodeDestination(fromaddress);
     auto pa = keyIO.DecodePaymentAddress(fromaddress);
-    fromTaddr = IsValidDestination(taddr);
-    if (!fromTaddr) {
-        if (!pa.has_value()) {
-            throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr.");
-        }
-        if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), pa.value())) {
-             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, spending key or viewing key not found.");
-        }
+
+    if (!pa.has_value()) {
+        throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr.");
+    }
+    if (!std::visit(PaymentAddressBelongsToWallet(pwalletMain), pa.value())) {
+         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node.");
     }
 
     CAmount nBalance = 0;
-    if (fromTaddr) {
-        nBalance = getBalanceTaddr(fromaddress, nMinDepth, false);
-    } else {
-        // TODO: Return an error if a UA is provided (once we support UAs).
-        nBalance = getBalanceZaddr(pa, nMinDepth, INT_MAX, false);
-    }
+    std::visit(match {
+        [&](const CKeyID& addr) {
+            nBalance = getBalanceTaddr(addr, nMinDepth, false);
+        },
+        [&](const CScriptID& addr) {
+            nBalance = getBalanceTaddr(addr, nMinDepth, false);
+        },
+        [&](const libzcash::SproutPaymentAddress& addr) {
+            nBalance = getBalanceZaddr(addr, nMinDepth, INT_MAX, false);
+        },
+        [&](const libzcash::SaplingPaymentAddress& addr) {
+            nBalance = getBalanceZaddr(addr, nMinDepth, INT_MAX, false);
+        },
+        [&](const libzcash::UnifiedAddress& addr) {
+             throw JSONRPCError(
+                     RPC_INVALID_ADDRESS_OR_KEY,
+                     "Unified addresses are not yet supported for z_getbalance.");
+        },
+    }, pa.value());
 
     // inZat
     if (params.size() > 2 && params[2].get_bool()) {
@@ -3323,7 +3321,7 @@ UniValue z_gettotalbalance(const UniValue& params, bool fHelp)
     // but they don't because wtx.GetAmounts() does not handle tx where there are no outputs
     // pwalletMain->GetBalance() does not accept min depth parameter
     // so we use our own method to get balance of utxos.
-    CAmount nBalance = getBalanceTaddr("", nMinDepth, !fIncludeWatchonly);
+    CAmount nBalance = getBalanceTaddr(std::nullopt, nMinDepth, !fIncludeWatchonly);
     CAmount nPrivateBalance = getBalanceZaddr(std::nullopt, nMinDepth, INT_MAX, !fIncludeWatchonly);
     CAmount nTotalBalance = nBalance + nPrivateBalance;
     UniValue result(UniValue::VOBJ);
@@ -3730,24 +3728,40 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
     if (fromaddress == "ANY_TADDR") {
         fromTaddr = true;
     } else {
-        CTxDestination taddr = keyIO.DecodeDestination(fromaddress);
-        fromTaddr = IsValidDestination(taddr);
-        if (!fromTaddr) {
-            auto addr = keyIO.DecodePaymentAddress(fromaddress);
-            if (!addr.has_value()) {
-                // invalid
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address, should be a taddr or zaddr.");
-            }
-
-            // This is a sanity check; the actual checks will come later when the spend is attempted.
-            if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), addr.value())) {
-                throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "From address does not belong to this node, zaddr spending key not found.");
-            }
-
-            // Remember whether this is a Sprout or Sapling address
-            fromSapling = std::holds_alternative<libzcash::SaplingPaymentAddress>(addr.value());
-            fromSprout = std::holds_alternative<libzcash::SproutPaymentAddress>(addr.value());
+        auto addr = keyIO.DecodePaymentAddress(fromaddress);
+        if (!addr.has_value()) {
+            throw JSONRPCError(
+                    RPC_INVALID_ADDRESS_OR_KEY,
+                    "Invalid from address: should be a taddr, a zaddr, or the string 'ANY_TADDR'.");
         }
+
+        // This is a sanity check; the actual checks will come later when the spend is attempted.
+        if (!std::visit(HaveSpendingKeyForPaymentAddress(pwalletMain), addr.value())) {
+            throw JSONRPCError(
+                    RPC_INVALID_ADDRESS_OR_KEY,
+                    "Invalid from address: does not belong to this node, spending key not found.");
+        }
+
+        // Remember what sort of address this is
+        std::visit(match {
+            [&](const CKeyID&) {
+                fromTaddr = true;
+            },
+            [&](const CScriptID&) {
+                fromTaddr = true;
+            },
+            [&](const libzcash::SaplingPaymentAddress& addr) {
+                fromSapling = true;
+            },
+            [&](const libzcash::SproutPaymentAddress& addr) {
+                fromSprout = true;
+            },
+            [&](const libzcash::UnifiedAddress& ua) {
+                throw JSONRPCError(
+                        RPC_INVALID_ADDRESS_OR_KEY,
+                        "Invalid from address: unified addresses are not yet supported.");
+            }
+        }, addr.value());
     }
 
     UniValue outputs = params[1].get_array();
@@ -3780,45 +3794,57 @@ UniValue z_sendmany(const UniValue& params, bool fHelp)
         }
 
         string address = find_value(o, "address").get_str();
+
         bool isZaddr = false;
-        CTxDestination taddr = keyIO.DecodeDestination(address);
-        if (!IsValidDestination(taddr)) {
-            auto addr = keyIO.DecodePaymentAddress(address);
-            if (addr.has_value()) {
-                isZaddr = true;
-
-                bool toSapling = std::holds_alternative<libzcash::SaplingPaymentAddress>(addr.value());
-                bool toSprout = std::holds_alternative<libzcash::SproutPaymentAddress>(addr.value());
-                noSproutAddrs = !toSprout && noSproutAddrs && toSapling;
-
-                containsSproutOutput |= toSprout;
-                containsSaplingOutput |= toSapling;
-
-                // Sending to both Sprout and Sapling is currently unsupported using z_sendmany
-                if (containsSproutOutput && containsSaplingOutput) {
+        auto toAddr = keyIO.DecodePaymentAddress(address);
+        if (toAddr.has_value()) {
+            bool toSprout = false;
+            bool toSapling = false;
+            std::visit(match {
+                [&](const CKeyID&) { },
+                [&](const CScriptID&) { },
+                [&](const libzcash::SaplingPaymentAddress& addr) {
+                    isZaddr = true;
+                    toSapling = true;
+                    containsSaplingOutput = true;
+                },
+                [&](const libzcash::SproutPaymentAddress& addr) {
+                    isZaddr = true;
+                    toSprout = true;
+                    containsSproutOutput = true;
+                    noSproutAddrs = false;
+                },
+                [&](const libzcash::UnifiedAddress& ua) {
                     throw JSONRPCError(
-                        RPC_INVALID_PARAMETER,
-                        "Cannot send to both Sprout and Sapling addresses using z_sendmany");
+                            RPC_INVALID_ADDRESS_OR_KEY,
+                            "Invalid recipient address: unified addresses are not yet supported.");
                 }
+            }, toAddr.value());
 
-                // If sending between shielded addresses, they must be the same type
-                if ((fromSprout && toSapling) || (fromSapling && toSprout)) {
-                    throw JSONRPCError(
-                        RPC_INVALID_PARAMETER,
-                        "Cannot send between Sprout and Sapling addresses using z_sendmany");
-                }
-
-                int nextBlockHeight = chainActive.Height() + 1;
-
-                if (fromTaddr && toSprout) {
-                    const bool canopyActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_CANOPY);
-                    if (canopyActive) {
-                        throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy");
-                    }
-                }
-            } else {
-                throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ")+address );
+            // Sending to both Sprout and Sapling is currently unsupported using z_sendmany
+            if (containsSproutOutput && containsSaplingOutput) {
+                throw JSONRPCError(
+                    RPC_INVALID_PARAMETER,
+                    "Cannot send to both Sprout and Sapling addresses using z_sendmany");
             }
+
+            // If sending between shielded addresses, they must be the same type
+            if ((fromSprout && toSapling) || (fromSapling && toSprout)) {
+                throw JSONRPCError(
+                    RPC_INVALID_PARAMETER,
+                    "Cannot send between Sprout and Sapling addresses using z_sendmany");
+            }
+
+            int nextBlockHeight = chainActive.Height() + 1;
+
+            if (fromTaddr && toSprout) {
+                const bool canopyActive = Params().GetConsensus().NetworkUpgradeActive(nextBlockHeight, Consensus::UPGRADE_CANOPY);
+                if (canopyActive) {
+                    throw JSONRPCError(RPC_VERIFY_REJECTED, "Sprout shielding is not supported after Canopy");
+                }
+            }
+        } else {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, string("Invalid parameter, unknown address format: ")+address );
         }
 
         if (setAddress.count(address))
diff --git a/src/wallet/test/rpc_wallet_tests.cpp b/src/wallet/test/rpc_wallet_tests.cpp
index 5947c444a..ee5db391e 100644
--- a/src/wallet/test/rpc_wallet_tests.cpp
+++ b/src/wallet/test/rpc_wallet_tests.cpp
@@ -362,20 +362,25 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_getbalance)
 
     LOCK2(cs_main, pwalletMain->cs_wallet);
 
+    UniValue retValue;
+    BOOST_CHECK_NO_THROW(retValue = CallRPC("getnewaddress"));
+    std::string taddr1 = retValue.get_str();
 
     BOOST_CHECK_THROW(CallRPC("z_getbalance too many args"), runtime_error);
     BOOST_CHECK_THROW(CallRPC("z_getbalance invalidaddress"), runtime_error);
-    BOOST_CHECK_NO_THROW(CallRPC("z_getbalance tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab"));
-    BOOST_CHECK_THROW(CallRPC("z_getbalance tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab -1"), runtime_error);
-    BOOST_CHECK_NO_THROW(CallRPC("z_getbalance tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab 0"));
+    // address does not belong to wallet
+    BOOST_CHECK_THROW(CallRPC("z_getbalance tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab"), runtime_error);
+    BOOST_CHECK_NO_THROW(CallRPC(std::string("z_getbalance ") + taddr1));
+    // negative minconf not allowed
+    BOOST_CHECK_THROW(CallRPC(std::string("z_getbalance ") + taddr1 + " -1"), runtime_error);
+    BOOST_CHECK_NO_THROW(CallRPC(std::string("z_getbalance ") + taddr1 + std::string(" 0")));
+    // don't have the spending key
     BOOST_CHECK_THROW(CallRPC("z_getbalance tnRZ8bPq2pff3xBWhTJhNkVUkm2uhzksDeW5PvEa7aFKGT9Qi3YgTALZfjaY4jU3HLVKBtHdSXxoPoLA3naMPcHBcY88FcF 1"), runtime_error);
 
-
     BOOST_CHECK_THROW(CallRPC("z_gettotalbalance too manyargs"), runtime_error);
     BOOST_CHECK_THROW(CallRPC("z_gettotalbalance -1"), runtime_error);
     BOOST_CHECK_NO_THROW(CallRPC("z_gettotalbalance 0"));
 
-
     BOOST_CHECK_THROW(CallRPC("z_listreceivedbyaddress too many args"), runtime_error);
     // negative minconf not allowed
     BOOST_CHECK_THROW(CallRPC("z_listreceivedbyaddress tmC6YZnCUhm19dEXxh3Jb7srdBJxDawaCab -1"), runtime_error);
@@ -1217,7 +1222,7 @@ BOOST_AUTO_TEST_CASE(rpc_z_sendmany_parameters)
         std::vector<SendManyRecipient> recipients = { SendManyRecipient("dummy", 1*COIN, "") };
         std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(std::nullopt, mtx, "ztjiDe569DPNbyTE6TSdJTaSDhoXEHLGvYoUnBU1wfVNU52TEyT6berYtySkd21njAeEoh8fFJUT42kua9r8EnhBaEKqCpP", recipients, {}, 1) );
     } catch (const UniValue& objError) {
-        BOOST_CHECK( find_error(objError, "no spending key found for zaddr"));
+        BOOST_CHECK( find_error(objError, "no spending key found for address"));
     }
 }