Try harder to avoid selecting locked notes to spend

src/wallet/asyncrpcoperation_sendmany.cpp

@@ -129,8 +129,9 @@ void AsyncRPCOperation_sendmany::main() {
 
     // clean up locks if we are cancelled
     if (isCancelled()) {
-        unlock_utxos();
+        // We are more likely to be spending notes, so unlock them first
         unlock_notes();
+        unlock_utxos();
         return;
     }
 
@@ -192,9 +193,9 @@ void AsyncRPCOperation_sendmany::main() {
     }
     LogPrintf("%s",s);
 
-    unlock_utxos(); // clean up
     unlock_notes(); // clean up
-    LogPrint("zrpc", "%s: z_sendmany input notes unlocked\n", getId());
+    unlock_utxos(); // clean up
+    LogPrint("zrpc", "%s: z_sendmany input notes+utxos unlocked\n", getId());
 }
 
 // Notes:
@@ -411,7 +412,14 @@ bool AsyncRPCOperation_sendmany::main_impl() {
         //      saplingNoteInputs_ is a list of notes we will actually spend
         //      and need to lock. It is a subset of z_sapling_inputs_
         for (const auto t : z_sapling_inputs_) {
-            // keep track of notes to lock later on in lock_notes()
+            // locked status of these inputs may have changed, check again
+            const bool isLocked = pwalletMain->IsLockedNote(t.op);
+            if (isLocked) {
+                LogPrintf("%s: skipping locked note %s\n", __func__, t.op.hash.ToString().substr(0,10).c_str());
+                continue;
+            }
+
+            // keep track of currently unlocked notes to lock later on in lock_notes()
             saplingNoteInputs_.emplace_back(t.op, t.note, t.note.value() );
 
             ops.push_back(t.op);
@@ -658,7 +666,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)
         LogPrintf("%s: For address %s depth=%d\n", __FUNCTION__, fromaddress_.c_str(), mindepth_);
@@ -666,11 +674,19 @@ bool AsyncRPCOperation_sendmany::find_unspent_notes() {
     std::vector<SaplingNoteEntry> saplingEntries;
     {
         LOCK2(cs_main, pwalletMain->cs_wallet);
+        // GetFilteredNotes ignores locked notes by default
         pwalletMain->GetFilteredNotes(saplingEntries, fromaddress_, mindepth_);
     }
 
-
     for (auto entry : saplingEntries) {
+        // locked status of note may have changed since GetFilteredNotes()
+        // returned data, so we check again
+        const bool isLocked = pwalletMain->IsLockedNote(entry.op);
+        if (isLocked) {
+            LogPrintf("%s: skipping locked note %s:%d\n", __func__, entry.op.hash.ToString().substr(0,10).c_str(), entry.op.n);
+            continue;
+        }
+
         z_sapling_inputs_.push_back(entry);
 
         std::string data(entry.memo.begin(), entry.memo.end());

src/wallet/wallet.cpp

@@ -4942,8 +4942,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;
              }