Merge pull request 'Lock sapling input notes (zins) in z_sendmany' (#465) from lockzins into dev

Reviewed-on: https://git.hush.is/hush/hush3/pulls/465

qa/rpc-tests/lockzins.py

@@ -46,8 +46,9 @@ class LockZinsTest (BitcoinTestFramework):
                     #'-debug',
                     '-regtest',
                     '--daemon',
-                    #'-rpcuser=hush',
+                    '-zrpc',
-                    #'-rpcpassword=puppy'
+                    '-zdebug',
+                    '-zrpcunsafe'
                     ]]
         )
         self.is_network_split = False
@@ -75,7 +76,7 @@ class LockZinsTest (BitcoinTestFramework):
 
         rpc.generate(11)
         self.sync_all()
-        rpc.z_listunspent()
+        # rpc.z_listunspent()
         rpc.z_getbalances()
 
         recipients = []
@@ -84,20 +85,24 @@ class LockZinsTest (BitcoinTestFramework):
         # queue 4 ztxs, which will try to spend the same funds multiple times
         # without correct locking of zins
         opid1 = rpc.z_sendmany(zaddr1,recipients,1,0)
+        rpc.z_listlockunspent()
         opid2 = rpc.z_sendmany(zaddr1,recipients,1,0)
+        rpc.z_listlockunspent()
         opid3 = rpc.z_sendmany(zaddr1,recipients,1,0)
+        rpc.z_listlockunspent()
         opid4 = rpc.z_sendmany(zaddr1,recipients,1,0)
 
         rpc.generate(1)
         self.sync_all()
+
+        # now that we have fullyNotified, this may not be needed?
+        #time.sleep(3)
+
         wait_and_assert_operationid_status(self.nodes[0], opid1)
         wait_and_assert_operationid_status(self.nodes[0], opid2)
         wait_and_assert_operationid_status(self.nodes[0], opid3)
         wait_and_assert_operationid_status(self.nodes[0], opid4)
 
-        # give time for all z_sendmany's to run
-        #time.sleep(10)
-
         rpc.z_getoperationstatus()
 
 if __name__ == '__main__':

qa/rpc-tests/test_framework/authproxy.py

@@ -1,3 +1,4 @@
+# Copyright (c) 2016-2025 The Hush developers
 """
   Copyright 2011 Jeff Garzik
 
@@ -40,11 +41,11 @@ import logging
 from http.client import HTTPConnection, HTTPSConnection, BadStatusLine
 from urllib.parse import urlparse
 
-USER_AGENT = "AuthServiceProxy/0.1"
+USER_AGENT = "HushAuthServiceProxy/0.1"
 
 HTTP_TIMEOUT = 600
 
-log = logging.getLogger("BitcoinRPC")
+log = logging.getLogger("RPC")
 
 class JSONRPCException(Exception):
     def __init__(self, rpc_error):

qa/rpc-tests/test_framework/test_framework.py

@@ -123,11 +123,16 @@ class BitcoinTestFramework(object):
             print("JSONRPC error: "+e.error['message'])
             traceback.print_tb(sys.exc_info()[2])
         except AssertionError as e:
-            print("Assertion failed: "+e.message)
+            print("Assertion failed: " + str(e))
+            traceback.print_tb(sys.exc_info()[2])
+        except KeyError as e:
+            print("key not found: "+ str(e))
             traceback.print_tb(sys.exc_info()[2])
         except Exception as e:
             print("Unexpected exception caught during testing: "+str(e))
             traceback.print_tb(sys.exc_info()[2])
+        except KeyboardInterrupt as e:
+            print("Exiting after " + repr(e))
 
         if not self.options.noshutdown:
             print("Stopping nodes")

qa/rpc-tests/test_framework/util.py

@@ -57,8 +57,6 @@ def sync_blocks(rpc_connections, wait=1):
             break
         time.sleep(wait)
 
-    return
-
     # Now that the block counts are in sync, wait for the internal
     # notifications to finish
     while True:
@@ -276,7 +274,7 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
     devnull.close()
     port     = extra_args[3]
     #port     = rpc_port(i)
-    print("port=%s" % port)
+    #print("port=%s" % port)
     username = rpc_username()
     password = rpc_password()
     url      = "http://%s:%s@%s:%s" % (username, password, rpchost or '127.0.0.1', port[9:])
@@ -285,7 +283,7 @@ def start_node(i, dirname, extra_args=None, rpchost=None, timewait=None, binary=
         proxy = AuthServiceProxy(url, timeout=timewait)
     else:
         proxy = AuthServiceProxy(url)
-    print("created proxy")
+    #print("created proxy")
     proxy.url = url # store URL on proxy for info
     return proxy
 

src/hush_utils.h

@@ -1339,7 +1339,7 @@ void hush_statefname(char *fname,char *symbol,char *str)
             fname[len - n] = 0;
         else
         {
-            if ( strcmp(symbol,"REGTEST") != 0 )
+            if ( strcmp(symbol,"ZZZ") != 0 )
                 printf("unexpected fname.(%s) vs %s [%s] n.%d len.%d (%s)\n",fname,symbol,SMART_CHAIN_SYMBOL,n,len,&fname[len - n]);
             return;
         }

src/init.cpp

@@ -146,7 +146,9 @@ std::atomic<bool> fRequestShutdown(false);
 
 void StartShutdown()
 {
-	fprintf(stderr,"%s: fRequestShudown=true\n", __FUNCTION__);
+    if(fDebug) {
+	    fprintf(stderr,"%s: fRequestShudown=true\n", __FUNCTION__);
+    }
     fRequestShutdown = true;
 }
 bool ShutdownRequested()
@@ -208,7 +210,9 @@ void Shutdown()
     RenameThread(shutoffstr);
     mempool.AddTransactionsUpdated(1);
 
-	fprintf(stderr,"%s: stopping HUSH HTTP/REST/RPC\n", __FUNCTION__);
+    if(fDebug) {
+	    fprintf(stderr,"%s: stopping HUSH HTTP/REST/RPC\n", __FUNCTION__);
+    }
     StopHTTPRPC();
     StopREST();
     StopRPC();
@@ -225,7 +229,9 @@ void Shutdown()
     GenerateBitcoins(false, 0);
  #endif
 #endif
-	fprintf(stderr,"%s: stopping node\n", __FUNCTION__);
+    if(fDebug) {
+	    fprintf(stderr,"%s: stopping node\n", __FUNCTION__);
+    }
     StopNode();
     StopTorControl();
     UnregisterNodeSignals(GetNodeSignals());

src/rpc/blockchain.cpp

@@ -1335,6 +1335,12 @@ UniValue getblockchaininfo(const UniValue& params, bool fHelp, const CPubKey& my
 
         obj.push_back(Pair("pruneheight",        block->GetHeight()));
     }
+
+    // this helps our tests work correctly
+    if (Params().NetworkIDString() == "regtest") {
+        obj.pushKV("fullyNotified", ChainIsFullyNotified());
+    }
+
     return obj;
 }
 

src/rpc/server.cpp

@@ -460,6 +460,7 @@ static const CRPCCommand vRPCCommands[] =
     { "wallet",             "walletlock",             &walletlock,             true  },
     { "wallet",             "walletpassphrasechange", &walletpassphrasechange, true  },
     { "wallet",             "walletpassphrase",       &walletpassphrase,       true  },
+    { "wallet",             "z_listlockunspent",      &z_listlockunspent,      false },
     { "wallet",             "z_listreceivedbyaddress",&z_listreceivedbyaddress,false },
     { "wallet",             "z_listreceivedaddress",  &z_listreceivedaddress,  false },
     { "wallet",             "z_getbalance",           &z_getbalance,           false },

src/rpc/server.h

@@ -295,6 +295,7 @@ extern UniValue getrawtransaction(const UniValue& params, bool fHelp, const CPub
 extern UniValue listunspent(const UniValue& params, bool fHelp, const CPubKey& mypk);
 extern UniValue lockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk);
 extern UniValue listlockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk);
+extern UniValue z_listlockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk);
 extern UniValue createrawtransaction(const UniValue& params, bool fHelp, const CPubKey& mypk);
 extern UniValue decoderawtransaction(const UniValue& params, bool fHelp, const CPubKey& mypk);
 extern UniValue decodescript(const UniValue& params, bool fHelp, const CPubKey& mypk);

src/transaction_builder.cpp

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

src/txdb.cpp

@@ -271,13 +271,13 @@ bool CCoinsViewDB::GetStats(CCoinsStats &stats) const {
 
 bool CBlockTreeDB::WriteBatchSync(const std::vector<std::pair<int, const CBlockFileInfo*> >& fileInfo, int nLastFile, const std::vector<CBlockIndex*>& blockinfo) {
     CDBBatch batch(*this);
-    if (fZdebug)
+    if (fDebug)
 	    fprintf(stderr, "%s: Writing block files\n", __FUNCTION__);
     for (const auto& it : fileInfo) {
         batch.Write(make_pair(DB_BLOCK_FILES, it.first), *it.second);
     }
     batch.Write(DB_LAST_BLOCK, nLastFile);
-    if (fZdebug)
+    if (fDebug)
 	    fprintf(stderr, "%s: Writing block index\n", __FUNCTION__);
     for (const auto& it : blockinfo) {
         std::pair<char, uint256> key = make_pair(DB_BLOCK_INDEX, it->GetBlockHash());

src/wallet/asyncrpcoperation_mergetoaddress.cpp

@@ -53,13 +53,13 @@ using namespace libzcash;
 extern UniValue sendrawtransaction(const UniValue& params, bool fHelp, const CPubKey& mypk);
 
 AsyncRPCOperation_mergetoaddress::AsyncRPCOperation_mergetoaddress(
-                                                                   boost::optional<TransactionBuilder> builder,
+   boost::optional<TransactionBuilder> builder,
-                                                                   CMutableTransaction contextualTx,
+   CMutableTransaction contextualTx,
-                                                                   std::vector<MergeToAddressInputUTXO> utxoInputs,
+   std::vector<MergeToAddressInputUTXO> utxoInputs,
-                                                                   std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs,
+   std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs,
-                                                                   MergeToAddressRecipient recipient,
+   MergeToAddressRecipient recipient,
-                                                                   CAmount fee,
+   CAmount fee,
-                                                                   UniValue contextInfo) :
+   UniValue contextInfo) :
 tx_(contextualTx), utxoInputs_(utxoInputs), saplingNoteInputs_(saplingNoteInputs), recipient_(recipient), fee_(fee), contextinfo_(contextInfo)
 {
     if (fee < 0 || fee > MAX_MONEY) {
@@ -182,7 +182,6 @@ void AsyncRPCOperation_mergetoaddress::main()
 
 // Notes:
 // 1. #1359 Currently there is no limit set on the number of inputs+outputs, 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_mergetoaddress::main_impl()
 {
     assert(isToTaddr_ != isToZaddr_);
@@ -192,22 +191,6 @@ bool AsyncRPCOperation_mergetoaddress::main_impl()
 
     size_t numInputs = utxoInputs_.size();
 
-    /*
-    // Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
-    size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
-    {
-        LOCK(cs_main);
-        if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-            limit = 0;
-        }
-    }
-    if (limit > 0 && numInputs > limit) {
-        throw JSONRPCError(RPC_WALLET_ERROR,
-                           strprintf("Number of transparent inputs %d is greater than mempooltxinputlimit of %d",
-                                     numInputs, limit));
-    }
-    */
-
     CAmount t_inputs_total = 0;
     for (MergeToAddressInputUTXO& t : utxoInputs_) {
         t_inputs_total += std::get<1>(t);

src/wallet/asyncrpcoperation_mergetoaddress.h

@@ -1,6 +1,5 @@
 // Copyright (c) 2017 The Zcash developers
 // Copyright (c) 2016-2024 The Hush developers
-
 // Distributed under the GPLv3 software license, see the accompanying
 // file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html
 
@@ -28,12 +27,9 @@
 #include "transaction_builder.h"
 #include "wallet.h"
 #include "zcash/Address.hpp"
-#include "zcash/JoinSplit.hpp"
-
 #include <array>
 #include <tuple>
 #include <unordered_map>
-
 #include <univalue.h>
 
 // Default transaction fee if caller does not specify one.
@@ -90,9 +86,6 @@ private:
     CTxDestination toTaddr_;
     PaymentAddress toPaymentAddress_;
     
-    uint256 joinSplitPubKey_;
-    unsigned char joinSplitPrivKey_[crypto_sign_SECRETKEYBYTES];
-    
     std::vector<MergeToAddressInputUTXO> utxoInputs_;
     std::vector<MergeToAddressInputSaplingNote> saplingNoteInputs_;
     

src/wallet/asyncrpcoperation_saplingconsolidation.cpp

@@ -74,6 +74,8 @@ void AsyncRPCOperation_saplingconsolidation::main() {
     }
 
     LogPrintf("%s", s);
+    unlock_notes(); // clean up
+    LogPrint("zrpc", "%s: consolidation input notes unlocked\n", getId());
 }
 
 bool AsyncRPCOperation_saplingconsolidation::main_impl() {
@@ -102,6 +104,11 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() {
             return true;
         }
 
+        // store sapling inputs so we can correctly lock and unlock them
+        for (auto entry : saplingEntries) {
+            z_sapling_inputs_.push_back(entry);
+        }
+
         if (fConsolidationMapUsed) {
             const vector<string>& v = mapMultiArgs["-consolidatesaplingaddress"];
             for(int i = 0; i < v.size(); i++) {
@@ -233,6 +240,10 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() {
                 break;
             }
 
+            // Lock shielded input notes
+            lock_notes();
+            LogPrint("zrpc", "%s: consolidation input notes locked\n", getId());
+
             if(pwalletMain->CommitAutomatedTx(tx)) {
                 LogPrintf("%s: Committed consolidation transaction with txid=%s\n",opid, tx.GetHash().ToString());
                 amountConsolidated += actualAmountToSend;
@@ -240,10 +251,15 @@ bool AsyncRPCOperation_saplingconsolidation::main_impl() {
                 numTxCreated++;
             } else {
                 LogPrintf("%s: Consolidation transaction FAILED in CommitTransaction, txid=%s\n",opid , tx.GetHash().ToString());
+
+                unlock_notes();
+                LogPrint("zrpc", "%s: consolidatoin input notes unlocked\n", getId());
+
                 setConsolidationResult(numTxCreated, amountConsolidated, consolidationTxIds);
                 status = false;
                 break;
             }
+
         }
     }
 
@@ -275,3 +291,19 @@ UniValue AsyncRPCOperation_saplingconsolidation::getStatus() const {
     obj.push_back(Pair("target_height", targetHeight_));
     return obj;
 }
+
+// Lock input notes
+void AsyncRPCOperation_saplingconsolidation::lock_notes() {
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    for (auto note : z_sapling_inputs_) {
+        pwalletMain->LockNote(note.op);
+    }
+}
+
+// Unlock input notes
+void AsyncRPCOperation_saplingconsolidation::unlock_notes() {
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    for (auto note : z_sapling_inputs_) {
+        pwalletMain->UnlockNote(note.op);
+    }
+}

src/wallet/asyncrpcoperation_saplingconsolidation.h

@@ -8,6 +8,7 @@
 #include "univalue.h"
 #include "zcash/Address.hpp"
 #include "zcash/zip32.h"
+#include "wallet.h" // for SaplingNoteEntry
 
 //Default fee used for consolidation transactions, in puposhis
 static const CAmount DEFAULT_CONSOLIDATION_FEE = 10000;
@@ -32,10 +33,14 @@ public:
 
     virtual UniValue getStatus() const;
 
+    void lock_notes();
+    void unlock_notes();
+
 private:
     int targetHeight_;
 
     bool main_impl();
+    std::vector<SaplingNoteEntry> z_sapling_inputs_;
 
     void setConsolidationResult(int numTxCreated, const CAmount& amountConsolidated, const std::vector<std::string>& consolidationTxIds);
 

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);
 
@@ -119,14 +120,25 @@ AsyncRPCOperation_sendmany::AsyncRPCOperation_sendmany(
     } else {
         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() {
 }
 
 void AsyncRPCOperation_sendmany::main() {
-    if (isCancelled())
+
+    // clean up locks if we are cancelled
+    if (isCancelled()) {
+        // We are more likely to be spending notes, so unlock them first
+        unlock_notes();
+        unlock_utxos();
         return;
+    }
 
     set_state(OperationStatus::EXECUTING);
     start_execution_clock();
@@ -162,6 +174,10 @@ void AsyncRPCOperation_sendmany::main() {
         set_error_message("unknown error");
     }
 
+    unlock_notes();
+    unlock_utxos();
+    LogPrintf("%s: %s z_sendmany input notes+utxos unlocked\n", __func__, getId());
+
 #ifdef ENABLE_MINING
   #ifdef ENABLE_WALLET
     GenerateBitcoins(GetBoolArg("-gen",false), pwalletMain, GetArg("-genproclimit", 1));
@@ -172,6 +188,7 @@ void AsyncRPCOperation_sendmany::main() {
 
     stop_execution_clock();
 
+
     if (success) {
         set_state(OperationStatus::SUCCESS);
     } else {
@@ -185,22 +202,14 @@ void AsyncRPCOperation_sendmany::main() {
         s += strprintf(", error=%s)\n", getErrorMessage());
     }
     LogPrintf("%s",s);
+
 }
 
 // Notes:
-// 1. #1159 Currently there is no limit set on the number of shielded spends, so size of tx could be invalid.
+// 1. Currently there is no limit set on the number of shielded spends, so size of tx could be invalid.
-// 2. #1360 Note selection is not optimal
-// 3. #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_);
 
-    /* TODO: this needs to allow DPoW addresses. Consensus-time checks do it correctly.
-    if(t_outputs_.size() > 0) {
-        throw JSONRPCError(RPC_WALLET_ERROR, "Extreme Privacy! You cannot send to a transparent address.");
-    }
-    */
-
     bool isSingleZaddrOutput   = (t_outputs_.size()==0 && z_outputs_.size()==1);
     bool isMultipleZaddrOutput = (t_outputs_.size()==0 && z_outputs_.size()>=1);
     bool isPureTaddrOnlyTx     = (isfromtaddr_ && z_outputs_.size() == 0);
@@ -227,9 +236,8 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         }
     }
 
-    if (isfromzaddr_ && !find_unspent_notes()) {
+    // Lock UTXOs
-        throw JSONRPCError(RPC_WALLET_INSUFFICIENT_FUNDS, "Insufficient funds, no unspent notes found for zaddr from address.");
+    lock_utxos();
-    }
 
     CAmount t_inputs_total = 0;
     for (SendManyInputUTXO & t : t_inputs_) {
@@ -237,8 +245,8 @@ 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 += std::get<1>(t).value();
     }
 
     CAmount t_outputs_total = 0;
@@ -252,8 +260,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);
@@ -309,23 +320,6 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         t_inputs_      = selectedTInputs;
         t_inputs_total = selectedUTXOAmount;
 
-        /*
-        // Check mempooltxinputlimit to avoid creating a transaction which the local mempool rejects
-        const size_t limit = (size_t)GetArg("-mempooltxinputlimit", 0);
-        {
-            LOCK(cs_main);
-            if (NetworkUpgradeActive(chainActive.Height() + 1, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-                limit = 0;
-            }
-        }
-        if (limit > 0) {
-            size_t n = t_inputs_.size();
-            if (n > limit) {
-                throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Too many transparent inputs %zu > limit %zu", n, limit));
-            }
-        }
-        */
-
         // update the transaction with these inputs
         if (isUsingBuilder_) {
             CScript scriptPubKey;
@@ -415,15 +409,10 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         if(fZdebug)
             LogPrintf("%s: Selecting Sapling notes\n", __FUNCTION__);
         std::vector<SaplingOutPoint> ops;
-        std::vector<SaplingNote> notes;
         CAmount sum = 0;
-        for (auto t : z_sapling_inputs_) {
+
-            ops.push_back(t.op);
+        for(const auto t : saplingNoteInputs_) {
-            notes.push_back(t.note);
+            ops.push_back(std::get<0>(t));
-            sum += t.note.value();
-            if (sum >= targetAmount) {
-                break;
-            }
         }
 
         // Fetch Sapling anchor and witnesses
@@ -435,14 +424,19 @@ 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++) {
+        for (size_t i = 0; i < saplingNoteInputs_.size(); i++) {
-            if (!witnesses[i]) {
+            if (!witnesses.at(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", 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()));
         }
 
         // Add Sapling outputs
@@ -454,7 +448,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);
 
@@ -488,6 +482,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);
@@ -659,37 +654,6 @@ bool AsyncRPCOperation_sendmany::find_utxos(bool fAcceptCoinbase=false) {
     return t_inputs_.size() > 0;
 }
 
-
-bool AsyncRPCOperation_sendmany::find_unspent_notes() {
-    if(fZdebug)
-        LogPrintf("%s: For address %s depth=%d\n", __FUNCTION__, fromaddress_.c_str(), mindepth_);
-
-    std::vector<SaplingNoteEntry> saplingEntries;
-    {
-        LOCK2(cs_main, pwalletMain->cs_wallet);
-        pwalletMain->GetFilteredNotes(saplingEntries, fromaddress_, mindepth_);
-    }
-
-    for (auto entry : saplingEntries) {
-        z_sapling_inputs_.push_back(entry);
-        std::string data(entry.memo.begin(), entry.memo.end());
-        LogPrint("zrpcunsafe", "%s: found unspent Sapling note (txid=%s, vShieldedSpend=%d, amount=%s, memo=%s)\n",
-            getId(),
-            entry.op.hash.ToString().substr(0, 10),
-            entry.op.n,
-            FormatMoney(entry.note.value()),
-            HexStr(data).substr(0, 10));
-    }
-
-    // sort in descending order, so big notes appear first
-    std::sort(z_sapling_inputs_.begin(), z_sapling_inputs_.end(),
-        [](SaplingNoteEntry i, SaplingNoteEntry j) -> bool {
-            return i.note.value() > j.note.value();
-        });
-
-    return true;
-}
-
 void AsyncRPCOperation_sendmany::add_taddr_outputs_to_tx() {
 
     CMutableTransaction rawTx(tx_);
@@ -781,3 +745,53 @@ UniValue AsyncRPCOperation_sendmany::getStatus() const {
     obj.push_back(Pair("params", contextinfo_ ));
     return obj;
 }
+
+// Lock input utxos
+ void AsyncRPCOperation_sendmany::lock_utxos() {
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    for (auto utxo : t_inputs_) {
+        uint256 txid = std::get<0>(utxo);
+        int vout = std::get<1>(utxo);
+
+        COutPoint outpt(txid, vout);
+        pwalletMain->LockCoin(outpt);
+    }
+}
+
+// Unlock input utxos
+void AsyncRPCOperation_sendmany::unlock_utxos() {
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    for (auto utxo : t_inputs_) {
+        uint256 txid = std::get<0>(utxo);
+        int vout = std::get<1>(utxo);
+
+        COutPoint outpt(txid, vout);
+        pwalletMain->UnlockCoin(outpt);
+    }
+}
+
+// Lock input notes
+void AsyncRPCOperation_sendmany::lock_notes() {
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    LogPrintf("%s: found %lu notes to lock\n", __func__, saplingNoteInputs_.size() );
+    for (auto note : saplingNoteInputs_) {
+        if(pwalletMain->IsLockedNote(std::get<0>(note))) {
+            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);
+    LogPrintf("%s: found %lu notes to unlock\n", __func__, saplingNoteInputs_.size() );
+    for (auto note : saplingNoteInputs_) {
+        if(pwalletMain->IsLockedNote(std::get<0>(note))) {
+            pwalletMain->UnlockNote(std::get<0>(note));
+        } else {
+            LogPrintf("%s: note already unlocked!\n", __func__);
+        }
+    }
+}

src/wallet/asyncrpcoperation_sendmany.h

@@ -25,14 +25,11 @@
 #include "amount.h"
 #include "primitives/transaction.h"
 #include "transaction_builder.h"
-#include "zcash/JoinSplit.hpp"
 #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.
@@ -46,11 +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;
 
-// A struct to help us track the witness and anchor for a given JSOutPoint
+// Input note is a tuple of output, note, amount, spending key
-struct WitnessAnchorData {
+typedef std::tuple<SaplingOutPoint, SaplingNote, CAmount, SaplingExpandedSpendingKey> SendManyInputSaplingNote;
-	boost::optional<SproutWitness> witness;
-	uint256 anchor;
-};
 
 class AsyncRPCOperation_sendmany : public AsyncRPCOperation {
 public:
@@ -60,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,
@@ -96,17 +91,11 @@ private:
     SpendingKey spendingkey_;
     CScript opret_ = CScript();
     
-    uint256 joinSplitPubKey_;
-    unsigned char joinSplitPrivKey_[crypto_sign_SECRETKEYBYTES];
-
-    // The key is the result string from calling JSOutPoint::ToString()
-    std::unordered_map<std::string, WitnessAnchorData> jsopWitnessAnchorMap;
-
     std::vector<SendManyRecipient> t_outputs_;
     std::vector<SendManyRecipient> z_outputs_;
     std::vector<SendManyInputUTXO> t_inputs_;
-    //std::vector<SendManyInputJSOP> z_sprout_inputs_;
+    //std::vector<SaplingNoteEntry> z_sapling_inputs_;
-    std::vector<SaplingNoteEntry> z_sapling_inputs_;
+    std::vector<SendManyInputSaplingNote> saplingNoteInputs_;
 
     TransactionBuilder builder_;
     CTransaction tx_;
@@ -119,6 +108,11 @@ private:
     bool main_impl();
 
     void sign_send_raw_transaction(UniValue obj);     // throws exception if there was an error
+
+    void lock_utxos();
+    void unlock_utxos();
+    void lock_notes();
+    void unlock_notes();
 };
 
 
@@ -172,6 +166,4 @@ public:
     }
 };
 
-
 #endif /* ASYNCRPCOPERATION_SENDMANY_H */
-

src/wallet/asyncrpcoperation_shieldcoinbase.h

@@ -74,9 +74,6 @@ private:
     CAmount fee_;
     PaymentAddress tozaddr_;
 
-    uint256 joinSplitPubKey_;
-    unsigned char joinSplitPrivKey_[crypto_sign_SECRETKEYBYTES];
-
     std::vector<ShieldCoinbaseUTXO> inputs_;
 
     TransactionBuilder builder_;
@@ -87,7 +84,6 @@ private:
     void sign_send_raw_transaction(UniValue obj);     // throws exception if there was an error
 
     void lock_utxos();
-
     void unlock_utxos();
 };
 

src/wallet/rpcwallet.cpp

@@ -2645,6 +2645,49 @@ UniValue lockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
     return true;
 }
 
+UniValue z_listlockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
+{
+    if (!EnsureWalletIsAvailable(fHelp))
+        return NullUniValue;
+
+    if (fHelp || params.size() > 0)
+        throw runtime_error(
+            "z_listlockunspent\n"
+            "\nReturns list of temporarily locked shielded outputs which are currently unspendable. They are locked\n"
+            "\nbecause they are currently in the process of being spent by an operation such as z_sendmany/z_mergetoaddress/etc.\n"
+            "\nIf that operation succeeds, they will become spent. If it fails they will be unlocked and become\n"
+            "\nspendable again.\n"
+            "\nResult:\n"
+            "[\n"
+            "  {\n"
+            "    \"txid\"     : \"transactionid\",     (string) The transaction id locked\n"
+            "    \"outindex\" : n                      (integer) The shielded output index\n"
+            "  }\n"
+            "  ,...\n"
+            "]\n"
+            "\nExamples:\n"
+            "\nList the locked Sapling notes\n"
+            + HelpExampleCli("z_listlockunspent", "") +
+            "\nAs a json rpc call\n"
+            + HelpExampleRpc("z_listlockunspent", "")
+        );
+
+    LOCK2(cs_main, pwalletMain->cs_wallet);
+    vector<SaplingOutPoint> ops = pwalletMain->ListLockedSaplingNotes();
+
+    UniValue ret(UniValue::VARR);
+
+    BOOST_FOREACH(SaplingOutPoint &op, ops) {
+        UniValue o(UniValue::VOBJ);
+
+        o.push_back(Pair("txid", op.hash.GetHex()));
+        o.push_back(Pair("outindex", (int) op.n));
+        ret.push_back(o);
+    }
+
+    return ret;
+}
+
 UniValue listlockunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
 {
     if (!EnsureWalletIsAvailable(fHelp))
@@ -4081,7 +4124,6 @@ UniValue z_getbalances(const UniValue& params, bool fHelp, const CPubKey& mypk)
     return results;
 }
 
-
 UniValue z_listunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
 {
     if (!EnsureWalletIsAvailable(fHelp))
@@ -4111,6 +4153,7 @@ UniValue z_listunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
             "    \"outindex\" (sapling) : n          (numeric) the output index\n"
             "    \"confirmations\" : n       (numeric) the number of confirmations\n"
             "    \"spendable\" : true|false  (boolean) true if note can be spent by wallet, false if note has zero confirmations, false if address is watchonly\n"
+            "    \"locked\" : true|false  (boolean) true if note is part of an in-process async operation to spend it, such as z_mergetoaddress/z_sendmany/etc\n"
             "    \"address\" : \"address\",    (string) the shielded address\n"
             "    \"amount\": xxxxx,          (numeric) the amount of value in the note\n"
             "    \"memo\": xxxxx,            (string) hexademical string representation of memo field\n"
@@ -4216,8 +4259,12 @@ UniValue z_listunspent(const UniValue& params, bool fHelp, const CPubKey& mypk)
             libzcash::SaplingFullViewingKey fvk;
             pwalletMain->GetSaplingIncomingViewingKey(boost::get<libzcash::SaplingPaymentAddress>(entry.address), ivk);
             pwalletMain->GetSaplingFullViewingKey(ivk, fvk);
-            bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk);
+            const bool hasSaplingSpendingKey = pwalletMain->HaveSaplingSpendingKey(fvk);
             obj.push_back(Pair("spendable", hasSaplingSpendingKey));
+
+            const bool isLocked = pwalletMain->IsLockedNote(entry.op);
+            obj.push_back(Pair("locked", isLocked));
+
             obj.push_back(Pair("address", EncodePaymentAddress(entry.address)));
             obj.push_back(Pair("amount", ValueFromAmount(CAmount(entry.note.value())))); // note.value() is equivalent to plaintext.value()
             obj.push_back(Pair("memo", HexStr(entry.memo)));
@@ -5124,7 +5171,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
             fprintf(stderr,"%s: Selecting one of %lu potential source zaddrs\n", __func__, nPotentials);
             fromaddress = vPotentialAddresses[ GetRandInt(nPotentials) ];
         } else {
-            // Automagic zaddr source election failed, exit honorably
+            // Automagic zaddr source selection failed, exit honorably
             throw JSONRPCError(RPC_INVALID_PARAMETER, "No single zaddr currently has enough funds to make that transaction, you may need to wait for confirmations.");
         }
     } else {
@@ -5147,6 +5194,7 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
         }
     }
 
+
     // Recipients
     std::vector<SendManyRecipient> taddrRecipients;
     std::vector<SendManyRecipient> zaddrRecipients;
@@ -5236,6 +5284,38 @@ 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);
+
+    // sort notes from largest to smallest, which means
+    // we will spend the largest first
+    std::sort(saplingEntries.begin(), saplingEntries.end(),
+        [](SaplingNoteEntry i, SaplingNoteEntry j) -> bool {
+            return i.note.value() > j.note.value();
+        });
+
+    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
@@ -5288,18 +5368,6 @@ UniValue z_sendmany(const UniValue& params, bool fHelp, const CPubKey& mypk)
     mtx.nVersionGroupId = SAPLING_VERSION_GROUP_ID;
     mtx.nVersion = SAPLING_TX_VERSION;
     unsigned int max_tx_size = MAX_TX_SIZE_AFTER_SAPLING;
-    /*
-    const bool sapling = true; //NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_SAPLING)) {
-    if (!sapling) {
-        if (NetworkUpgradeActive(nextBlockHeight, Params().GetConsensus(), Consensus::UPGRADE_OVERWINTER)) {
-            mtx.nVersionGroupId = OVERWINTER_VERSION_GROUP_ID;
-            mtx.nVersion        = OVERWINTER_TX_VERSION;
-        } else {
-            mtx.fOverwintered = false;
-            mtx.nVersion      = 2;
-        }
-    }
-    */
 
     // As a sanity check, estimate and verify that the size of the transaction will be valid.
     // Depending on the input notes, the actual tx size may turn out to be larger and perhaps invalid.
@@ -5368,7 +5436,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.
@@ -5391,8 +5459,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();
@@ -5951,7 +6020,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp, const CPubKey& myp
     // Contextual transaction we will build on
     CMutableTransaction contextualTx; //= CreateNewContextualCMutableTransaction( Params().GetConsensus(), nextBlockHeight);
 
-    // Builder (used if Sapling addresses are involved)
+    // Builder (used if zaddrs are involved)
     boost::optional<TransactionBuilder> builder;
     if (isToSaplingZaddr || saplingNoteInputs.size() > 0) {
         builder = TransactionBuilder(Params().GetConsensus(), nextBlockHeight, pwalletMain);
@@ -5962,7 +6031,7 @@ UniValue z_mergetoaddress(const UniValue& params, bool fHelp, const CPubKey& myp
     // Create operation and add to global queue
     std::shared_ptr<AsyncRPCQueue> q = getAsyncRPCQueue();
     std::shared_ptr<AsyncRPCOperation> operation(
-                                                 new AsyncRPCOperation_mergetoaddress(builder, contextualTx, utxoInputs, saplingNoteInputs, recipient, nFee, contextInfo) );
+         new AsyncRPCOperation_mergetoaddress(builder, contextualTx, utxoInputs, saplingNoteInputs, recipient, nFee, contextInfo) );
     q->addOperation(operation);
     AsyncRPCOperationId operationId = operation->getId();
 

src/wallet/wallet.cpp

@@ -1839,19 +1839,17 @@ void CWallet::GetSaplingNoteWitnesses(std::vector<SaplingOutPoint> notes,
         auto noteData   = mapWallet[note.hash].mapSaplingNoteData;
         auto nWitnesses = noteData[note].witnesses.size();
         if (mapWallet.count(note.hash) && noteData.count(note) && nWitnesses > 0) {
-            fprintf(stderr,"%s: Found %lu witnesses for note %s\n", __func__, nWitnesses, note.hash.ToString().c_str() );
+            fprintf(stderr,"%s: Found %lu witnesses for note %s...\n", __func__, nWitnesses, note.hash.ToString().substr(0,8).c_str() );
             witnesses[i] = noteData[note].witnesses.front();
             if (!rt) {
                 //fprintf(stderr,"%s: Setting witness root\n",__func__);
                 rt = witnesses[i]->root();
             } else {
                 if(*rt == witnesses[i]->root()) {
-                    //fprintf(stderr,"%s: rt=%s\n",__func__,rt.GetHash().ToString().c_str());
+                } else {
-                    //fprintf(stderr,"%s: witnesses[%d]->root()=%s\n",__func__,i,witnesses[i]->root().GetHash().ToString().c_str());
                     // Something is fucky
                     std::string err = string("CWallet::GetSaplingNoteWitnesses: Invalid witness root! rt=") + rt.get().ToString();
                     err            += string("\n!= witness[i]->root()=") + witnesses[i]->root().ToString();
-                    //throw std::logic_error(err);
                     fprintf(stderr,"%s: IGNORING %s\n", __func__,err.c_str());
                 }
 
@@ -4586,12 +4584,14 @@ void CWallet::LockNote(const SaplingOutPoint& output)
 {
     AssertLockHeld(cs_wallet);
     setLockedSaplingNotes.insert(output);
+    fprintf(stderr,"%s: locking note %s...\n", __func__, output.hash.ToString().substr(0,8).c_str() );
 }
 
 void CWallet::UnlockNote(const SaplingOutPoint& output)
 {
     AssertLockHeld(cs_wallet);
     setLockedSaplingNotes.erase(output);
+    fprintf(stderr,"%s: unlocking note %s...\n", __func__, output.hash.ToString().substr(0,8).c_str() );
 }
 
 void CWallet::UnlockAllSaplingNotes()
@@ -4878,6 +4878,8 @@ void CWallet::GetFilteredNotes(
 {
     LOCK2(cs_main, cs_wallet);
 
+    LogPrintf("%s ignoreLocked=%d\n", __func__, ignoreLocked);
+
     for (auto & p : mapWallet) {
         CWalletTx wtx = p.second;
 
@@ -4939,8 +4941,8 @@ void CWallet::GetFilteredNotes(
             }
 
             // skip locked notes
-            // TODO: Add locking for Sapling notes -> done
              if (ignoreLocked && IsLockedNote(op)) {
+                 LogPrintf("%s: skipping locked note %s\n", __func__, op.hash.ToString().substr(0,10).c_str());
                  continue;
              }
 

test.sh

@@ -1,4 +1,7 @@
 #!/usr/bin/env bash
+# Copyright 2016-2025 The Hush developers
+# Distributed under the GPLv3 software license, see the accompanying
+# file COPYING or https://www.gnu.org/licenses/gpl-3.0.en.html
 
 #export PYTHON_DEBUG=1
 export PYTHONPATH=./qa/rpc-tests/test_framework/