Sapling operation new spend from several different sources feature + send/build decoupling.

src/sapling/address.hpp

@@ -193,6 +193,7 @@ class SaplingExpandedSpendingKey {
     }
 
     SaplingFullViewingKey full_viewing_key() const;
+    bool IsNull() { return ask.IsNull() && nsk.IsNull() && ovk.IsNull(); }
 
     friend inline bool operator==(const SaplingExpandedSpendingKey& a, const SaplingExpandedSpendingKey& b) {
         return a.ask == b.ask && a.nsk == b.nsk && a.ovk == b.ovk;

src/sapling/key_io_sapling.cpp

@@ -146,6 +146,13 @@ namespace KeyIO {
         return libzcash::InvalidEncoding();
     }
 
+    Optional<libzcash::SaplingPaymentAddress> DecodeSaplingPaymentAddress(const std::string& strAddress)
+    {
+        libzcash::PaymentAddress addr = KeyIO::DecodePaymentAddress(strAddress);
+        const auto dest = boost::get<libzcash::SaplingPaymentAddress>(&addr);
+        return (dest) ? Optional<libzcash::SaplingPaymentAddress>(*dest) : nullopt;
+    }
+
     bool IsValidPaymentAddressString(const std::string& str) {
         return IsValidPaymentAddress(DecodePaymentAddress(str));
     }

src/sapling/key_io_sapling.h

@@ -13,6 +13,7 @@ namespace KeyIO {
 
     std::string EncodePaymentAddress(const libzcash::PaymentAddress& zaddr);
     libzcash::PaymentAddress DecodePaymentAddress(const std::string& str);
+    Optional<libzcash::SaplingPaymentAddress> DecodeSaplingPaymentAddress(const std::string& str);
     bool IsValidPaymentAddressString(const std::string& str);
 
     std::string EncodeViewingKey(const libzcash::ViewingKey& vk);

src/sapling/sapling_operation.cpp

@@ -35,15 +35,56 @@ OperationResult SaplingOperation::checkTxValues(TxValues& txValues, bool isFromt
     return OperationResult(true);
 }
 
-OperationResult SaplingOperation::send(std::string& retTxHash)
+OperationResult loadKeysFromShieldedFrom(const libzcash::SaplingPaymentAddress &addr,
+                                         libzcash::SaplingExpandedSpendingKey& expskOut,
+                                         uint256& ovkOut)
+{
+    // Get spending key for address
+    libzcash::SaplingExtendedSpendingKey sk;
+    if (!pwalletMain->GetSaplingExtendedSpendingKey(addr, sk)) {
+        return errorOut("Spending key not in the wallet");
+    }
+    expskOut = sk.expsk;
+    ovkOut = expskOut.full_viewing_key().ovk;
+    return OperationResult(true);
+}
+
+TxValues calculateTarget(std::vector<SendManyRecipient>& taddrRecipients,
+                         std::vector<SendManyRecipient>& shieldedAddrRecipients,
+                         CAmount fee)
+{
+    TxValues txValues;
+    for (SendManyRecipient &t : taddrRecipients) {
+        txValues.transOutTotal += t.amount;
+    }
+
+    // Add shielded outputs
+    for (const SendManyRecipient &t : shieldedAddrRecipients) {
+        txValues.shieldedOutTotal += t.amount;
+    }
+    txValues.target = txValues.shieldedOutTotal + txValues.transOutTotal + fee;
+    return txValues;
+}
+
+OperationResult SaplingOperation::build()
 {
 
     bool isFromtAddress = fromAddress.isFromTAddress();
     bool isFromShielded = fromAddress.isFromSapAddress();
 
-    // It needs to have a from (for now at least)
     if (!isFromtAddress && !isFromShielded) {
-        return errorOut("From address parameter missing");
+        isFromtAddress = selectFromtaddrs;
+        isFromShielded = selectFromShield;
+
+        // It needs to have a from.
+        if (!isFromtAddress && !isFromShielded) {
+            return errorOut("From address parameter missing");
+        }
+
+        // Cannot be from both
+        if (isFromtAddress && isFromShielded) {
+            return errorOut("From address type cannot be shielded and transparent");
+        }
     }
 
     if (taddrRecipients.empty() && shieldedAddrRecipients.empty()) {
@@ -54,17 +95,25 @@ OperationResult SaplingOperation::send(std::string& retTxHash)
         return errorOut("Minconf cannot be zero when sending from shielded address");
     }
 
-    // Get necessary keys
+    // First calculate target values
+    TxValues txValues = calculateTarget(taddrRecipients, shieldedAddrRecipients, fee);
+    OperationResult result(false);
+    // Necessary keys
     libzcash::SaplingExpandedSpendingKey expsk;
     uint256 ovk;
     if (isFromShielded) {
-        // Get spending key for address
-        libzcash::SaplingExtendedSpendingKey sk;
-        if (!pwalletMain->GetSaplingExtendedSpendingKey(fromAddress.fromSapAddr.get(), sk)) {
-            return errorOut("Spending key not in the wallet");
+        // Try to get the sk and ovk if we know the address from, if we don't know it then this will be loaded in loadUnspentNotes
+        // using the sk of the first note input of the transaction.
+        if (fromAddress.isFromSapAddress()) {
+            // Get spending key for address
+            auto loadKeyRes = loadKeysFromShieldedFrom(fromAddress.fromSapAddr.get(), expsk, ovk);
+            if (!loadKeyRes) return loadKeyRes;
+        }
+
+        // Load and select notes to spend
+        if (!(result = loadUnspentNotes(txValues, expsk, ovk))) {
+            return result;
         }
-        expsk = sk.expsk;
-        ovk = expsk.full_viewing_key().ovk;
     } else {
         // Sending from a t-address, which we don't have an ovk for. Instead,
         // generate a common one from the HD seed. This ensures the data is
@@ -73,17 +122,13 @@ OperationResult SaplingOperation::send(std::string& retTxHash)
         ovk = pwalletMain->GetSaplingScriptPubKeyMan()->getCommonOVKFromSeed();
     }
 
-    // Results
-    TxValues txValues;
     // Add transparent outputs
     for (SendManyRecipient &t : taddrRecipients) {
-        txValues.transOutTotal += t.amount;
         txBuilder.AddTransparentOutput(DecodeDestination(t.address), t.amount);
     }
 
     // Add shielded outputs
     for (const SendManyRecipient &t : shieldedAddrRecipients) {
-        txValues.shieldedOutTotal += t.amount;
         auto addr = KeyIO::DecodePaymentAddress(t.address);
         assert(IsValidPaymentAddress(addr));
         auto to = boost::get<libzcash::SaplingPaymentAddress>(addr);
@@ -94,25 +139,13 @@ OperationResult SaplingOperation::send(std::string& retTxHash)
         txBuilder.AddSaplingOutput(ovk, to, t.amount, memo);
     }
 
-    // Load total
-    txValues.target = txValues.shieldedOutTotal + txValues.transOutTotal + fee;
-    OperationResult result(false);
-
     // If from address is a taddr, select UTXOs to spend
     // note: when spending coinbase utxos, you can only specify a single shielded addr as the change must go somewhere
     // and if there are multiple shielded addrs, we don't know where to send it.
     if (isFromtAddress && !(result = loadUtxos(txValues))) {
         return result;
     }
 
-    // If from a shielded addr, select notes to spend
-    if (isFromShielded) {
-        // Load notes
-        if (!(result = loadUnspentNotes(txValues, expsk))) {
-            return result;
-        }
-    }
-
     const auto& retCalc = checkTxValues(txValues, isFromtAddress, isFromShielded);
     if (!retCalc) return retCalc;
 
@@ -124,25 +157,37 @@ OperationResult SaplingOperation::send(std::string& retTxHash)
     LogPrint(BCLog::SAPLING, "%s: fee: %s\n", __func__ , FormatMoney(fee));
 
     // Set change address if we are using transparent funds
-    CReserveKey keyChange(pwalletMain);
     if (isFromtAddress) {
+        if (!tkeyChange) {
+            tkeyChange = new CReserveKey(pwalletMain);
+        }
         CPubKey vchPubKey;
-        if (!keyChange.GetReservedKey(vchPubKey, true)) {
-            // should never fail, as we just unlocked
+        if (!tkeyChange->GetReservedKey(vchPubKey, true)) {
             return errorOut("Could not generate a taddr to use as a change address");
         }
-
         CTxDestination changeAddr = vchPubKey.GetID();
         txBuilder.SendChangeTo(changeAddr);
     }
 
     // Build the transaction
     txBuilder.SetFee(fee);
-    finalTx = txBuilder.Build().GetTxOrThrow();
+    TransactionBuilderResult txResult = txBuilder.Build();
+    auto opTx = txResult.GetTx();
+
+    // Check existent tx
+    if (!opTx) {
+        return errorOut("Failed to build transaction: " + txResult.GetError());
+    }
+
+    finalTx = *opTx;
+    return OperationResult(true);
+}
 
+OperationResult SaplingOperation::send(std::string& retTxHash)
+{
     if (!testMode) {
         CWalletTx wtx(pwalletMain, finalTx);
-        const CWallet::CommitResult& res = pwalletMain->CommitTransaction(wtx, keyChange, g_connman.get());
+        const CWallet::CommitResult& res = pwalletMain->CommitTransaction(wtx, tkeyChange, g_connman.get());
         if (res.status != CWallet::CommitStatus::OK) {
             return errorOut(res.ToString());
         }
@@ -152,6 +197,12 @@ OperationResult SaplingOperation::send(std::string& retTxHash)
     return OperationResult(true);
 }
 
+OperationResult SaplingOperation::buildAndSend(std::string& retTxHash)
+{
+    OperationResult res = build();
+    return (res) ? send(retTxHash) : res;
+}
+
 void SaplingOperation::setFromAddress(const CTxDestination& _dest)
 {
     fromAddress = FromAddress(_dest);
@@ -165,7 +216,7 @@ void SaplingOperation::setFromAddress(const libzcash::SaplingPaymentAddress& _pa
 OperationResult SaplingOperation::loadUtxos(TxValues& txValues)
 {
     std::set<CTxDestination> destinations;
-    destinations.insert(fromAddress.fromTaddr);
+    if (fromAddress.isFromTAddress()) destinations.insert(fromAddress.fromTaddr);
     if (!pwalletMain->AvailableCoins(
             &transInputs,
             nullptr,
@@ -227,11 +278,12 @@ OperationResult SaplingOperation::loadUtxos(TxValues& txValues)
     return OperationResult(true);
 }
 
-OperationResult SaplingOperation::loadUnspentNotes(TxValues& txValues, const libzcash::SaplingExpandedSpendingKey& expsk)
+OperationResult SaplingOperation::loadUnspentNotes(TxValues& txValues,
+                                                   libzcash::SaplingExpandedSpendingKey& expsk,
+                                                   uint256& ovk)
 {
     std::vector<SaplingNoteEntry> saplingEntries;
-    libzcash::PaymentAddress paymentAddress(fromAddress.fromSapAddr.get());
-    pwalletMain->GetSaplingScriptPubKeyMan()->GetFilteredNotes(saplingEntries, paymentAddress, mindepth);
+    pwalletMain->GetSaplingScriptPubKeyMan()->GetFilteredNotes(saplingEntries, fromAddress.fromSapAddr, mindepth);
 
     for (const auto& entry : saplingEntries) {
         shieldedInputs.emplace_back(entry);
@@ -259,6 +311,11 @@ OperationResult SaplingOperation::loadUnspentNotes(TxValues& txValues, const lib
     std::vector<libzcash::SaplingNote> notes;
     CAmount sum = 0;
     for (const auto& t : shieldedInputs) {
+        // if null, load the first input sk
+        if (expsk.IsNull()) {
+            auto resLoadKeys = loadKeysFromShieldedFrom(t.address, expsk, ovk);
+            if (!resLoadKeys) return resLoadKeys;
+        }
         ops.emplace_back(t.op);
         notes.emplace_back(t.note);
         sum += t.note.value();

src/sapling/sapling_operation.h

@@ -41,15 +41,23 @@ class SaplingOperation {
     explicit SaplingOperation(const Consensus::Params& consensusParams, int chainHeight) : txBuilder(consensusParams, chainHeight) {};
     explicit SaplingOperation(TransactionBuilder& _builder) : txBuilder(_builder) {};
 
+    ~SaplingOperation() { delete tkeyChange; }
+
+    OperationResult build();
     OperationResult send(std::string& retTxHash);
+    OperationResult buildAndSend(std::string& retTxHash);
 
     void setFromAddress(const CTxDestination&);
     void setFromAddress(const libzcash::SaplingPaymentAddress&);
+    // In case of no addressFrom filter selected, it will accept any utxo in the wallet as input.
+    SaplingOperation* setSelectTransparentCoins(const bool select) { selectFromtaddrs = select; return this; };
+    SaplingOperation* setSelectShieldedCoins(const bool select) { selectFromShield = select; return this; };
     SaplingOperation* setTransparentRecipients(std::vector<SendManyRecipient>& vec) { taddrRecipients = std::move(vec); return this; };
     SaplingOperation* setShieldedRecipients(std::vector<SendManyRecipient>& vec) { shieldedAddrRecipients = std::move(vec); return this; } ;
     SaplingOperation* setFee(CAmount _fee) { fee = _fee; return this; }
     SaplingOperation* setMinDepth(int _mindepth) { assert(_mindepth >= 0); mindepth = _mindepth; return this; }
     SaplingOperation* setTxBuilder(TransactionBuilder& builder) { txBuilder = builder; return this; }
+    SaplingOperation* setTransparentKeyChange(CReserveKey* reserveKey) { tkeyChange = reserveKey; return this; }
 
     CTransaction getFinalTx() { return finalTx; }
 
@@ -61,19 +69,27 @@ class SaplingOperation {
 
 private:
     FromAddress fromAddress;
+    // In case of no addressFrom filter selected, it will accept any utxo in the wallet as input.
+    bool selectFromtaddrs{false};
+    bool selectFromShield{false};
     std::vector<SendManyRecipient> taddrRecipients;
     std::vector<SendManyRecipient> shieldedAddrRecipients;
     std::vector<COutput> transInputs;
     std::vector<SaplingNoteEntry> shieldedInputs;
     int mindepth{5}; // Min default depth 5.
-    CAmount fee{0};
+    CAmount fee{DEFAULT_SAPLING_FEE}; // Hardcoded fee for now.
+
+    // transparent change
+    CReserveKey* tkeyChange{nullptr};
 
     // Builder
     TransactionBuilder txBuilder;
     CTransaction finalTx;
 
     OperationResult loadUtxos(TxValues& values);
-    OperationResult loadUnspentNotes(TxValues& txValues, const libzcash::SaplingExpandedSpendingKey& expsk);
+    OperationResult loadUnspentNotes(TxValues& txValues,
+                                     libzcash::SaplingExpandedSpendingKey& expsk,
+                                     uint256& ovk);
     OperationResult checkTxValues(TxValues& txValues, bool isFromtAddress, bool isFromShielded);
 };
 

src/sapling/saplingscriptpubkeyman.cpp

@@ -379,15 +379,15 @@ std::vector<libzcash::SaplingPaymentAddress> SaplingScriptPubKeyMan::FindMySapli
  */
 void SaplingScriptPubKeyMan::GetFilteredNotes(
         std::vector<SaplingNoteEntry>& saplingEntries,
-        const libzcash::PaymentAddress& address,
+        Optional<libzcash::SaplingPaymentAddress>& address,
         int minDepth,
         bool ignoreSpent,
         bool requireSpendingKey)
 {
     std::set<libzcash::PaymentAddress> filterAddresses;
 
-    if (IsValidPaymentAddress(address)) {
-        filterAddresses.insert(address);
+    if (address && IsValidPaymentAddress(*address)) {
+        filterAddresses.insert(*address);
     }
 
     GetFilteredNotes(saplingEntries, filterAddresses, minDepth, INT_MAX, ignoreSpent, requireSpendingKey);

src/sapling/saplingscriptpubkeyman.h

@@ -235,7 +235,7 @@ class SaplingScriptPubKeyMan {
 
     /* Find notes filtered by payment address, min depth, ability to spend */
     void GetFilteredNotes(std::vector<SaplingNoteEntry>& saplingEntries,
-                          const libzcash::PaymentAddress& address,
+                          Optional<libzcash::SaplingPaymentAddress>& address,
                           int minDepth=1,
                           bool ignoreSpent=true,
                           bool requireSpendingKey=true);

src/test/librust/sapling_rpc_wallet_tests.cpp

@@ -336,7 +336,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") };
         SaplingOperation operation(consensusParams, 1);
         operation.setFromAddress(DecodeDestination(taddr1));
-        auto res = operation.setShieldedRecipients(recipients)->send(ret);
+        auto res = operation.setShieldedRecipients(recipients)->buildAndSend(ret);
         BOOST_CHECK(!res);
         BOOST_CHECK(res.getError().find("Insufficient funds, no available UTXO to spend") != std::string::npos);
     }
@@ -346,7 +346,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(zaddr1,100.0, "DEADBEEF") };
         SaplingOperation operation(consensusParams, 1);
         operation.setFromAddress(pa);
-        auto res = operation.setShieldedRecipients(recipients)->setMinDepth(0)->send(ret);
+        auto res = operation.setShieldedRecipients(recipients)->setMinDepth(0)->buildAndSend(ret);
         BOOST_CHECK(!res);
         BOOST_CHECK(res.getError().find("Minconf cannot be zero when sending from shielded address") != std::string::npos);
     }
@@ -356,7 +356,7 @@ BOOST_AUTO_TEST_CASE(saplingOperationTests) {
         std::vector<SendManyRecipient> recipients = { SendManyRecipient(taddr1,100.0, "DEADBEEF") };
         SaplingOperation operation(consensusParams, 1);
         operation.setFromAddress(pa);
-        auto res = operation.setTransparentRecipients(recipients)->send(ret);
+        auto res = operation.setTransparentRecipients(recipients)->buildAndSend(ret);
         BOOST_CHECK(!res);
         BOOST_CHECK(res.getError().find("Insufficient funds, no available notes to spend") != std::string::npos);
     }
@@ -464,7 +464,7 @@ BOOST_AUTO_TEST_CASE(rpc_shielded_sendmany_taddr_to_sapling)
     operation.testMode = true; // To not commit the transaction
     BOOST_CHECK(operation.setShieldedRecipients(recipients)
                         ->setMinDepth(0)
-                        ->send(txFinalHash));
+                        ->buildAndSend(txFinalHash));
 
     // Get the transaction
     // Test mode does not send the transaction to the network.