lockzins test finally passes because z_sendmany correctly locks notes now

src/transaction_builder.cpp

@@ -150,7 +150,7 @@ boost::optional<CTransaction> TransactionBuilder::Build()
         change -= tOut.nValue;
     }
     if (change < 0) {
-        LogPrintf("%s: negative change=%lu!\n", __func__, change);
+        LogPrintf("%s: negative change=%lu mtx.valueBalance=%lu fee=%lu!\n", __func__, change, mtx.valueBalance, fee);
         return boost::none;
     }
 

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -63,11 +63,12 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
         std::string fromAddress,
         std::vector<SendManyRecipient> tOutputs,
         std::vector<SendManyRecipient> zOutputs,
+        std::vector<SendManyInputSaplingNote> saplingNoteInputs,
         int minDepth,
         CAmount fee,
         UniValue contextInfo,
         CScript opret) :
-        tx_(contextualTx), fromaddress_(fromAddress), t_outputs_(tOutputs), z_outputs_(zOutputs), mindepth_(minDepth), fee_(fee), contextinfo_(contextInfo), opret_(opret)
+        tx_(contextualTx), fromaddress_(fromAddress), t_outputs_(tOutputs), z_outputs_(zOutputs), saplingNoteInputs_(saplingNoteInputs), mindepth_(minDepth), fee_(fee), contextinfo_(contextInfo), opret_(opret)
 {
     assert(fee_ >= 0);
 
@@ -120,6 +121,10 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
         LogPrint("zrpc", "%s: z_sendmany initialized\n", getId());
     }
 
+    //TODO: lock_utxos() ?
+    // Lock shielded input notes (zins) stored in saplingNoteInputs
+    lock_notes();
+    LogPrintf("%s: %s z_sendmany input notes locked\n", __func__, getId());
 }
 
 AsyncRPCOperation_sendmany::~AsyncRPCOperation_sendmany() {
@@ -171,7 +176,7 @@ void AsyncRPCOperation_sendmany::main() {
 
     unlock_notes();
     unlock_utxos();
-    LogPrintf("%s: z_sendmany input notes+utxos unlocked\n", __func__, getId());
+    LogPrintf("%s: %s z_sendmany input notes+utxos unlocked\n", __func__, getId());
 
 #ifdef ENABLE_MINING
   #ifdef ENABLE_WALLET
@@ -204,7 +209,6 @@ void AsyncRPCOperation_sendmany::main() {
 // 1. #1159 Currently there is no limit set on the number of shielded spends, so size of tx could be invalid.
 // 2. #1277 Spendable notes are not locked, so an operation running in parallel could also try to use them
 bool AsyncRPCOperation_sendmany::main_impl() {
-
     assert(isfromtaddr_ != isfromzaddr_);
 
     bool isSingleZaddrOutput   = (t_outputs_.size()==0 && z_outputs_.size()==1);
@@ -233,9 +237,11 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         }
     }
 
+    /*
     if (isfromzaddr_ && !find_unspent_notes()) {
         throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds, no unspent notes found for zaddr from address.");
     }
+    */
 
     // Lock UTXOs
     lock_utxos();
@@ -246,8 +252,9 @@ bool AsyncRPCOperation_sendmany::main_impl() {
     }
 
     CAmount z_inputs_total = 0;
-    for (auto t : z_sapling_inputs_) {
+    for (auto t : saplingNoteInputs_) {
-        z_inputs_total += t.note.value();
+        //z_inputs_total += t.note.value();
+        z_inputs_total += std::get<1>(t).value();
     }
 
     CAmount t_outputs_total = 0;
@@ -261,8 +268,11 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         z_outputs_total += std::get<1>(t);
     }
 
+    LogPrintf("%s: z_inputs_total=%s z_outputs_total=%s\n", __func__, FormatMoney(z_inputs_total), FormatMoney(z_outputs_total) );
+
     CAmount sendAmount   = z_outputs_total + t_outputs_total;
     CAmount targetAmount = sendAmount + minersFee;
+    LogPrintf("%s: targetAmount=%s sendAmount=%s minersFee=%s\n", __func__, FormatMoney(targetAmount), FormatMoney(sendAmount), FormatMoney(minersFee) );
 
     assert(!isfromtaddr_ || z_inputs_total == 0);
     assert(!isfromzaddr_ || t_inputs_total == 0);
@@ -410,6 +420,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         std::vector<SaplingNote> notes;
         CAmount sum = 0;
 
+        /*
         //NOTE: z_sapling_inputs_ is a list of all potential notes to spend
         //      saplingNoteInputs_ is a list of notes we will actually spend
         //      and need to lock. It is a subset of z_sapling_inputs_
@@ -431,9 +442,11 @@ bool AsyncRPCOperation_sendmany::main_impl() {
                 break;
             }
         }
+        */
 
-        // Lock shielded input notes (zins) stored in saplingNoteInputs
+        for(const auto t : saplingNoteInputs_) {
-        lock_notes();
+            ops.push_back(std::get<0>(t));
+        }
 
         // Fetch Sapling anchor and witnesses
         //LogPrintf("%s: Gathering anchors and witnesses\n", __FUNCTION__);
@@ -444,14 +457,30 @@ bool AsyncRPCOperation_sendmany::main_impl() {
             pwalletMain->GetSaplingNoteWitnesses(ops, witnesses, anchor);
         }
 
+        LogPrintf("%s: ops.size=%d witnesses.size=%d\n", __func__, ops.size(), witnesses.size() );
+
         // Add Sapling spends
-        for (size_t i = 0; i < notes.size(); i++) {
+        //TODO: should be using saplingNoteInputs_
+        for (size_t i = 0; i < saplingNoteInputs_.size(); i++) {
+            //LOCK2(cs_main, pwalletMain->cs_wallet);
+
             if (!witnesses[i]) {
                 throw JSONRPCError(RPC_WALLET_ERROR,
-                    strprintf( "Missing witness for Sapling note at outpoint %s", z_sapling_inputs_[i].op.ToString())
+                    //strprintf( "Missing witness for Sapling note at outpoint %s", saplingNoteInputs_[i].op.ToString())
+                    strprintf( "Missing witness for Sapling note at outpoint %s", std::get<0>(saplingNoteInputs_[i]).ToString())
                 );
             }
-            assert(builder_.AddSaplingSpend(expsk, notes[i], anchor, witnesses[i].get()));
+            if(fZdebug)
+                LogPrintf("%s: Adding Sapling spend\n", __func__);
+            assert(builder_.AddSaplingSpend(expsk, std::get<1>(saplingNoteInputs_[i]), anchor, witnesses[i].get()));
+
+            /*
+            // notes we are currently spending should be locked
+            if(pwalletMain->IsLockedNote(ops[i])) {
+            } else {
+                throw JSONRPCError(RPC_WALLET_ERROR, "Note we are spending is not locked!" );
+            }
+            */
         }
 
         // Add Sapling outputs
@@ -463,7 +492,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
             assert(boost::get<libzcash::SaplingPaymentAddress>(&addr) != nullptr);
             auto to = boost::get<libzcash::SaplingPaymentAddress>(addr);
             if(fZdebug)
-                LogPrintf("%s: Adding Sapling output to address %s\n", __FUNCTION__, address.c_str());
+                LogPrintf("%s: Adding Sapling output with value=%s to address %s\n", __func__, FormatMoney(value), address.c_str());
 
             auto memo = get_memo_from_hex_string(hexMemo);
 
@@ -497,6 +526,7 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         // Send the transaction
         // TODO: Use CWallet::CommitTransaction instead of sendrawtransaction
         auto signedtxn = EncodeHexTx(tx_);
+
         if (!testmode) {
             UniValue params = UniValue(UniValue::VARR);
             params.push_back(signedtxn);
@@ -668,6 +698,7 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
     return t_inputs_.size() > 0;
 }
 
+/*
 // find unspent notes which are also unlocked
 bool AsyncRPCOperation_sendmany::find_unspent_notes() {
     if(fZdebug)
@@ -708,6 +739,7 @@ bool AsyncRPCOperation_sendmany::find_unspent_notes() {
 
     return true;
 }
+*/
 
 void AsyncRPCOperation_sendmany::add_taddr_outputs_to_tx() {
 
@@ -828,17 +860,27 @@ void AsyncRPCOperation_sendmany::unlock_utxos() {
 // Lock input notes
 void AsyncRPCOperation_sendmany::lock_notes() {
     LOCK2(cs_main, pwalletMain->cs_wallet);
-    fprintf(stderr,"%s: found %lu notes to lock\n", __func__, saplingNoteInputs_.size() );
+    LogPrintf("%s: found %lu notes to lock\n", __func__, saplingNoteInputs_.size() );
     for (auto note : saplingNoteInputs_) {
-        pwalletMain->LockNote(std::get<0>(note));
+        if(pwalletMain->IsLockedNote(std::get<0>(note))) {
+            //TODO: deal with this
+            LogPrintf("%s: note already locked!\n", __func__);
+        } else {
+            pwalletMain->LockNote(std::get<0>(note));
+        }
     }
 }
 
 // Unlock input notes
 void AsyncRPCOperation_sendmany::unlock_notes() {
     LOCK2(cs_main, pwalletMain->cs_wallet);
-    fprintf(stderr,"%s: found %lu notes to unlock\n", __func__, saplingNoteInputs_.size() );
+    LogPrintf("%s: found %lu notes to unlock\n", __func__, saplingNoteInputs_.size() );
     for (auto note : saplingNoteInputs_) {
-        pwalletMain->UnlockNote(std::get<0>(note));
+        if(pwalletMain->IsLockedNote(std::get<0>(note))) {
+            //TODO: deal with this
+            pwalletMain->UnlockNote(std::get<0>(note));
+        } else {
+            LogPrintf("%s: note already unlocked!\n", __func__);
+        }
     }
 }

src/wallet/asyncrpcoperation_sendmany.h

@@ -27,11 +27,9 @@
 #include "transaction_builder.h"
 #include "zcash/Address.hpp"
 #include "wallet.h"
-
 #include <array>
 #include <unordered_map>
 #include <tuple>
-
 #include <univalue.h>
 
 // Default transaction fee if caller does not specify one.
@@ -45,8 +43,8 @@ typedef std::tuple<std::string, CAmount, std::string> SendManyRecipient;
 // Input UTXO is a tuple (quadruple) of txid, vout, amount, coinbase)
 typedef std::tuple<uint256, int, CAmount, bool, CTxDestination> SendManyInputUTXO;
 
-// Input note is a tuple of output, note, amount
+// Input note is a tuple of output, note, amount, spending key
-typedef std::tuple<SaplingOutPoint, SaplingNote, CAmount> SendManyInputSaplingNote;
+typedef std::tuple<SaplingOutPoint, SaplingNote, CAmount, SaplingExpandedSpendingKey> SendManyInputSaplingNote;
 
 class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
 public:
@@ -56,6 +54,7 @@ public:
         std::string fromAddress,
         std::vector<SendManyRecipient> tOutputs,
         std::vector<SendManyRecipient> zOutputs,
+        std::vector<SendManyInputSaplingNote> saplingNoteInputs,
         int minDepth,
         CAmount fee = ASYNC_RPC_OPERATION_DEFAULT_MINERS_FEE,
         UniValue contextInfo = NullUniValue,
@@ -95,7 +94,7 @@ private:
     std::vector<SendManyRecipient> t_outputs_;
     std::vector<SendManyRecipient> z_outputs_;
     std::vector<SendManyInputUTXO> t_inputs_;
-    std::vector<SaplingNoteEntry> z_sapling_inputs_;
+    //std::vector<SaplingNoteEntry> z_sapling_inputs_;
     std::vector<SendManyInputSaplingNote> saplingNoteInputs_;
 
     TransactionBuilder builder_;

src/wallet/rpcwallet.cpp

@@ -5191,6 +5191,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
         }
     }
 
+
     // Recipients
     std::vector<SendManyRecipient> taddrRecipients;
     std::vector<SendManyRecipient> zaddrRecipients;
@@ -5280,6 +5281,31 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
 
         nTotalOut += nAmount;
     }
+
+    std::vector<SaplingNoteEntry> saplingEntries;
+    // find all unspent and unlocked notes in this zaddr
+    pwalletMain->GetFilteredNotes(saplingEntries, fromaddress);
+
+    CAmount total_value = 0;
+
+    std::vector<SendManyInputSaplingNote> saplingNoteInputs;
+    // Decide which sapling notes will be spent
+    for (const SaplingNoteEntry& entry : saplingEntries) {
+        CAmount nValue = entry.note.value();
+        libzcash::SaplingExtendedSpendingKey extsk;
+        if (!pwalletMain->GetSaplingExtendedSpendingKey(entry.address, extsk)) {
+            throw JSONRPCError(RPC_INVALID_PARAMETER, "Could not find spending key for payment address.");
+        }
+        saplingNoteInputs.emplace_back(entry.op, entry.note, nValue, extsk.expsk);
+        total_value += nValue;
+        LogPrintf("%s: adding note to spend with value=%s, total_value=%s\n", __func__, FormatMoney(nValue), FormatMoney(total_value) );
+        if (total_value >= nTotalOut) {
+            // we have enough note value to make the tx
+            LogPrintf("%s: found enough notes, nTotalOut=%s total_value=%s\n", __func__, FormatMoney(nTotalOut), FormatMoney(total_value) );
+            break;
+        }
+    }
+
    // SIETCH: Sprinkle our cave with some magic privacy zdust
    // End goal is to have this be as large as possible without slowing xtns down too much
    // A value of 7 will provide much stronger linkability privacy versus pre-Sietch operations
@@ -5400,7 +5426,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
                         throw JSONRPCError(RPC_INVALID_PARAMETER, strprintf("Fee %s is greater than the sum of outputs %s and also greater than the default fee", FormatMoney(nFee), FormatMoney(nTotalOut)));
                     }
             }
-	}
+	    }
     }
 
     // Use input parameters as the optional context info to be returned by z_getoperationstatus and z_getoperationresult.
@@ -5423,8 +5449,9 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
 
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
-    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(builder, contextualTx, fromaddress, taddrRecipients, zaddrRecipients, nMinDepth, nFee, contextInfo, opret) );
+    std::shared_ptr<AsyncRPCOperation> operation( new AsyncRPCOperation_sendmany(builder, contextualTx, fromaddress, taddrRecipients, zaddrRecipients, saplingNoteInputs, nMinDepth, nFee, contextInfo, opret) );
     q->addOperation(operation);
+
     if(fZdebug)
         LogPrintf("%s: Submitted to async queue\n", __FUNCTION__);
     AsyncRPCOperationId operationId = operation->getId();