Remove spurious variant from asyncrpcoperation_sendmany

The `spendingkey_` field previously used by asyncrpcoperation_sendmany
caused a situation where call sites would assume the type of the
spending key based upon boolean flags that were derived elsewhere.
In attempting to support unified addresses, it is desirable to remove
these boolean deductions and instead work primarily based on the
direct evidence of the data at hand. This commit therefore removes
the `spendingkey_` field that could potentially produce results
that would disagree with these boolean values, in favor of such
direct evidence, making these call sites a bit less error prone.

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -106,7 +106,6 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
 
             isfromzaddr_ = true;
             frompaymentaddress_ = address.value();
-            spendingkey_ = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value()).value();
         } else {
             throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid from address");
         }
@@ -306,9 +305,9 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         // Get various necessary keys
         SaplingExpandedSpendingKey expsk;
         uint256 ovk;
-        if (isfromzaddr_) {
-            auto sk = std::get<libzcash::SaplingExtendedSpendingKey>(spendingkey_);
-            expsk = sk.expsk;
+        auto saplingKey = std::visit(GetSaplingKeyForPaymentAddress(pwalletMain), frompaymentaddress_);
+        if (saplingKey.has_value()) {
+            expsk = saplingKey.value().expsk;
             ovk = expsk.full_viewing_key().ovk;
         } else {
             // Sending from a t-address, which we don't have an ovk for. Instead,
@@ -523,6 +522,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         UniValue obj(UniValue::VOBJ);
         while (zOutputsDeque.size() > 0) {
             AsyncJoinSplitInfo info;
+            // FIXME: make sure this .value() call is safe
             info.vpub_old = 0;
             info.vpub_new = 0;
             int n = 0;
@@ -641,7 +641,9 @@ bool AsyncRPCOperation_sendmany::main_impl() {
             intermediates.insert(std::make_pair(tree.root(), tree));    // chained js are interstitial (found in between block boundaries)
 
             // Decrypt the change note's ciphertext to retrieve some data we need
-            ZCNoteDecryption decryptor(std::get<libzcash::SproutSpendingKey>(spendingkey_).receiving_key());
+            // FIXME: make sure this .value() call is safe
+            auto sk = std::visit(GetSproutKeyForPaymentAddress(pwalletMain), frompaymentaddress_).value();
+            ZCNoteDecryption decryptor(sk.receiving_key());
             auto hSig = ZCJoinSplit::h_sig(
                 prevJoinSplit.randomSeed,
                 prevJoinSplit.nullifiers,
@@ -1023,7 +1025,8 @@ UniValue AsyncRPCOperation_sendmany::perform_joinsplit(
         if (!witnesses[i]) {
             throw runtime_error("joinsplit input could not be found in tree");
         }
-        info.vjsin.push_back(JSInput(*witnesses[i], info.notes[i], std::get<libzcash::SproutSpendingKey>(spendingkey_)));
+        auto sk = std::visit(GetSproutKeyForPaymentAddress(pwalletMain), frompaymentaddress_).value();
+        info.vjsin.push_back(JSInput(*witnesses[i], info.notes[i], sk));
     }
 
     // Make sure there are two inputs and two outputs

src/wallet/asyncrpcoperation_sendmany.h

@@ -104,7 +104,6 @@ private:
     bool isfromzaddr_;
     CTxDestination fromtaddr_;
     PaymentAddress frompaymentaddress_;
-    SpendingKey spendingkey_;
 
     Ed25519VerificationKey joinSplitPubKey_;
     Ed25519SigningKey joinSplitPrivKey_;

src/wallet/rpcdump.cpp

@@ -3,6 +3,7 @@
 // file COPYING or https://www.opensource.org/licenses/mit-license.php .
 
 #include "chain.h"
+#include "core_io.h"
 #include "key_io.h"
 #include "rpc/server.h"
 #include "init.h"
@@ -11,6 +12,7 @@
 #include "script/standard.h"
 #include "sync.h"
 #include "util.h"
+#include "util/match.h"
 #include "utiltime.h"
 #include "wallet.h"
 
@@ -909,12 +911,48 @@ UniValue z_exportkey(const UniValue& params, bool fHelp)
         throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid zaddr");
     }
 
-    // Sapling support
-    auto sk = std::visit(GetSpendingKeyForPaymentAddress(pwalletMain), address.value());
-    if (!sk) {
-        throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold private zkey for this zaddr");
+    std::string result = std::visit(match {
+        [&](const CKeyID& addr) {
+            CKey key;
+            if (pwalletMain->GetKey(addr, key)) {
+                return keyIO.EncodeSecret(key);
+            } else {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private key for this address.");
             }
-    return keyIO.EncodeSpendingKey(sk.value());
+        },
+        [&](const CScriptID& addr) {
+            CScript redeemScript;
+            if (pwalletMain->GetCScript(addr, redeemScript)) {
+                return FormatScript(redeemScript);
+            } else {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private key for this address.");
+            }
+        },
+        [&](const libzcash::SproutPaymentAddress& addr) {
+            libzcash::SproutSpendingKey key;
+            if (pwalletMain->GetSproutSpendingKey(addr, key)) {
+                return keyIO.EncodeSpendingKey(key);
+            } else {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private zkey for this zaddr");
+            }
+        },
+        [&](const libzcash::SaplingPaymentAddress& addr) {
+            libzcash::SaplingExtendedSpendingKey extsk;
+            if (pwalletMain->GetSaplingExtendedSpendingKey(addr, extsk)) {
+                return keyIO.EncodeSpendingKey(extsk);
+            } else {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Wallet does not hold the private zkey for this zaddr");
+            }
+        },
+        [&](const libzcash::UnifiedAddress& ua) {
+            throw JSONRPCError(
+                    RPC_WALLET_ERROR,
+                    "No serialized form is defined for unified spending keys. "
+                    "Use the emergency recovery phrase for this wallet for backup purposes instead.");
+            return std::string(); //unreachable, here to make the compiler happy
+        }
+    }, address.value());
+    return result;
 }
 
 UniValue z_exportviewingkey(const UniValue& params, bool fHelp)

src/wallet/wallet.cpp

@@ -5341,43 +5341,79 @@ bool HaveSpendingKeyForPaymentAddress::operator()(const libzcash::UnifiedAddress
     return false;
 }
 
-// GetSpendingKeyForPaymentAddress
+// GetSproutKeyForPaymentAddress
 
-std::optional<libzcash::SpendingKey> GetSpendingKeyForPaymentAddress::operator()(
+std::optional<libzcash::SproutSpendingKey> GetSproutKeyForPaymentAddress::operator()(
     const CKeyID &zaddr) const
 {
     return std::nullopt;
 }
-std::optional<libzcash::SpendingKey> GetSpendingKeyForPaymentAddress::operator()(
+std::optional<libzcash::SproutSpendingKey> GetSproutKeyForPaymentAddress::operator()(
     const CScriptID &zaddr) const
 {
     return std::nullopt;
 }
-std::optional<libzcash::SpendingKey> GetSpendingKeyForPaymentAddress::operator()(
+std::optional<libzcash::SproutSpendingKey> GetSproutKeyForPaymentAddress::operator()(
     const libzcash::SproutPaymentAddress &zaddr) const
 {
     libzcash::SproutSpendingKey k;
     if (m_wallet->GetSproutSpendingKey(zaddr, k)) {
-        return libzcash::SpendingKey(k);
+        return k;
     } else {
         return std::nullopt;
     }
 }
-std::optional<libzcash::SpendingKey> GetSpendingKeyForPaymentAddress::operator()(
+std::optional<libzcash::SproutSpendingKey> GetSproutKeyForPaymentAddress::operator()(
+    const libzcash::SaplingPaymentAddress &zaddr) const
+{
+    return std::nullopt;
+}
+std::optional<libzcash::SproutSpendingKey> GetSproutKeyForPaymentAddress::operator()(
+    const libzcash::UnifiedAddress &uaddr) const
+{
+    return std::nullopt;
+}
+
+// GetSaplingKeyForPaymentAddress
+
+std::optional<libzcash::SaplingExtendedSpendingKey> GetSaplingKeyForPaymentAddress::operator()(
+    const CKeyID &zaddr) const
+{
+    return std::nullopt;
+}
+std::optional<libzcash::SaplingExtendedSpendingKey> GetSaplingKeyForPaymentAddress::operator()(
+    const CScriptID &zaddr) const
+{
+    return std::nullopt;
+}
+std::optional<libzcash::SaplingExtendedSpendingKey> GetSaplingKeyForPaymentAddress::operator()(
+    const libzcash::SproutPaymentAddress &zaddr) const
+{
+    return std::nullopt;
+}
+std::optional<libzcash::SaplingExtendedSpendingKey> GetSaplingKeyForPaymentAddress::operator()(
     const libzcash::SaplingPaymentAddress &zaddr) const
 {
     libzcash::SaplingExtendedSpendingKey extsk;
     if (m_wallet->GetSaplingExtendedSpendingKey(zaddr, extsk)) {
-        return libzcash::SpendingKey(extsk);
+        return extsk;
     } else {
         return std::nullopt;
     }
 }
-std::optional<libzcash::SpendingKey> GetSpendingKeyForPaymentAddress::operator()(
+std::optional<libzcash::SaplingExtendedSpendingKey> GetSaplingKeyForPaymentAddress::operator()(
     const libzcash::UnifiedAddress &uaddr) const
 {
-    // TODO
-    return libzcash::SpendingKey();
+    for (const libzcash::Receiver& receiver: uaddr) {
+        auto saplingAddr = std::get_if<SaplingPaymentAddress>(&receiver);
+        if (saplingAddr != nullptr) {
+            libzcash::SaplingExtendedSpendingKey extsk;
+            if (m_wallet->GetSaplingExtendedSpendingKey(*saplingAddr, extsk)) {
+                return extsk;
+            }
+        }
+    }
+    return std::nullopt;
 }
 
 // AddViewingKeyToWallet

src/wallet/wallet.h

@@ -1404,19 +1404,32 @@ public:
     bool operator()(const libzcash::UnifiedAddress &uaddr) const;
 };
 
-class GetSpendingKeyForPaymentAddress
+class GetSproutKeyForPaymentAddress
 {
 private:
     CWallet *m_wallet;
 public:
-    GetSpendingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {}
+    GetSproutKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {}
 
-    std::optional<libzcash::SpendingKey> operator()(const CKeyID &zaddr) const;
-    std::optional<libzcash::SpendingKey> operator()(const CScriptID &zaddr) const;
-    std::optional<libzcash::SpendingKey> operator()(const libzcash::SproutPaymentAddress &zaddr) const;
-    std::optional<libzcash::SpendingKey> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
-    // FIXME: this doesn't make sense
-    std::optional<libzcash::SpendingKey> operator()(const libzcash::UnifiedAddress &uaddr) const;
+    std::optional<libzcash::SproutSpendingKey> operator()(const CKeyID &zaddr) const;
+    std::optional<libzcash::SproutSpendingKey> operator()(const CScriptID &zaddr) const;
+    std::optional<libzcash::SproutSpendingKey> operator()(const libzcash::SproutPaymentAddress &zaddr) const;
+    std::optional<libzcash::SproutSpendingKey> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
+    std::optional<libzcash::SproutSpendingKey> operator()(const libzcash::UnifiedAddress &uaddr) const;
+};
+
+class GetSaplingKeyForPaymentAddress
+{
+private:
+    CWallet *m_wallet;
+public:
+    GetSaplingKeyForPaymentAddress(CWallet *wallet) : m_wallet(wallet) {}
+
+    std::optional<libzcash::SaplingExtendedSpendingKey> operator()(const CKeyID &zaddr) const;
+    std::optional<libzcash::SaplingExtendedSpendingKey> operator()(const CScriptID &zaddr) const;
+    std::optional<libzcash::SaplingExtendedSpendingKey> operator()(const libzcash::SproutPaymentAddress &zaddr) const;
+    std::optional<libzcash::SaplingExtendedSpendingKey> operator()(const libzcash::SaplingPaymentAddress &zaddr) const;
+    std::optional<libzcash::SaplingExtendedSpendingKey> operator()(const libzcash::UnifiedAddress &uaddr) const;
 };
 
 enum PaymentAddressSource {