diff --git a/src/wallet/asyncrpcoperation_mergetoaddress.cpp b/src/wallet/asyncrpcoperation_mergetoaddress.cpp index 515e2b81a..68a554ab4 100644 --- a/src/wallet/asyncrpcoperation_mergetoaddress.cpp +++ b/src/wallet/asyncrpcoperation_mergetoaddress.cpp @@ -327,7 +327,11 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() std::vector> witnesses; { LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->GetSaplingNoteWitnesses(saplingOPs, nAnchorConfirmations, witnesses, anchor); + if (!pwalletMain->GetSaplingNoteWitnesses(saplingOPs, nAnchorConfirmations, witnesses, anchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sapling activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sapling witnesses."); + }; } // Add Sapling spends @@ -447,7 +451,11 @@ bool AsyncRPCOperation_mergetoaddress::main_impl() std::vector vOutPoints = {jso}; uint256 inputAnchor; std::vector> vInputWitnesses; - pwalletMain->GetSproutNoteWitnesses(vOutPoints, nAnchorConfirmations, vInputWitnesses, inputAnchor); + if (!pwalletMain->GetSproutNoteWitnesses(vOutPoints, nAnchorConfirmations, vInputWitnesses, inputAnchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sprout activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sprout witnesses."); + }; jsopWitnessAnchorMap[jso.ToString()] = MergeToAddressWitnessAnchorData{vInputWitnesses[0], inputAnchor}; } } @@ -752,7 +760,11 @@ UniValue AsyncRPCOperation_mergetoaddress::perform_joinsplit(MergeToAddressJSInf uint256 anchor; { LOCK(cs_main); - pwalletMain->GetSproutNoteWitnesses(outPoints, nAnchorConfirmations, witnesses, anchor); + if (!pwalletMain->GetSproutNoteWitnesses(outPoints, nAnchorConfirmations, witnesses, anchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sprout activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sprout witnesses."); + } } return perform_joinsplit(info, witnesses, anchor); } diff --git a/src/wallet/asyncrpcoperation_saplingmigration.cpp b/src/wallet/asyncrpcoperation_saplingmigration.cpp index 6e6dd16a5..b2dadb5da 100644 --- a/src/wallet/asyncrpcoperation_saplingmigration.cpp +++ b/src/wallet/asyncrpcoperation_saplingmigration.cpp @@ -137,12 +137,15 @@ bool AsyncRPCOperation_saplingmigration::main_impl() { libzcash::SproutSpendingKey sproutSk; pwalletMain->GetSproutSpendingKey(sproutEntry.address, sproutSk); std::vector vOutPoints = {sproutEntry.jsop}; - // Each migration transaction SHOULD specify an anchor at height N-10 + // Each migration transaction uses the anchor at height N-nAnchorConfirmations // for each Sprout JoinSplit description - // TODO: the above functionality (in comment) is not implemented in zcashd uint256 inputAnchor; std::vector> vInputWitnesses; - pwalletMain->GetSproutNoteWitnesses(vOutPoints, nAnchorConfirmations, vInputWitnesses, inputAnchor); + if (!pwalletMain->GetSproutNoteWitnesses(vOutPoints, nAnchorConfirmations, vInputWitnesses, inputAnchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sprout activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sprout witnesses."); + } builder.AddSproutInput(sproutSk, sproutEntry.note, vInputWitnesses[0].value()); // Send change to the address of the first input if (!changeAddr.has_value()) { diff --git a/src/wallet/asyncrpcoperation_sendmany.cpp b/src/wallet/asyncrpcoperation_sendmany.cpp index b70c58567..4615ea5a3 100644 --- a/src/wallet/asyncrpcoperation_sendmany.cpp +++ b/src/wallet/asyncrpcoperation_sendmany.cpp @@ -497,7 +497,11 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { std::vector> orchardSpendInfo; { LOCK2(cs_main, pwalletMain->cs_wallet); - pwalletMain->GetSaplingNoteWitnesses(saplingOutPoints, anchordepth_, witnesses, anchor); + if (!pwalletMain->GetSaplingNoteWitnesses(saplingOutPoints, anchordepth_, witnesses, anchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sapling activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sapling witnesses."); + } if (builder_.GetOrchardAnchor().has_value()) { orchardSpendInfo = pwalletMain->GetOrchardSpendInfo(spendable.orchardNoteMetadata, builder_.GetOrchardAnchor().value()); } @@ -585,7 +589,11 @@ uint256 AsyncRPCOperation_sendmany::main_impl() { // inputAnchor is not needed by builder_.AddSproutInput as it is for Sapling. uint256 inputAnchor; - pwalletMain->GetSproutNoteWitnesses(vOutPoints, anchordepth_, vSproutWitnesses, inputAnchor); + if (!pwalletMain->GetSproutNoteWitnesses(vOutPoints, anchordepth_, vSproutWitnesses, inputAnchor)) { + // This error should not appear once we're nAnchorConfirmations blocks past + // Sprout activation. + throw JSONRPCError(RPC_WALLET_ERROR, "Insufficient Sprout witnesses."); + } } // Add Sprout spends diff --git a/src/wallet/gtest/test_wallet.cpp b/src/wallet/gtest/test_wallet.cpp index 5099f9bdc..f37f467c1 100644 --- a/src/wallet/gtest/test_wallet.cpp +++ b/src/wallet/gtest/test_wallet.cpp @@ -126,8 +126,8 @@ std::pair GetWitnessesAndAnchors( saplingWitnesses.clear(); uint256 sproutAnchor; uint256 saplingAnchor; - wallet.GetSproutNoteWitnesses(sproutNotes, anchorDepth, sproutWitnesses, sproutAnchor); - wallet.GetSaplingNoteWitnesses(saplingNotes, anchorDepth, saplingWitnesses, saplingAnchor); + assert(wallet.GetSproutNoteWitnesses(sproutNotes, anchorDepth, sproutWitnesses, sproutAnchor)); + assert(wallet.GetSaplingNoteWitnesses(saplingNotes, anchorDepth, saplingWitnesses, saplingAnchor)); return std::make_pair(sproutAnchor, saplingAnchor); } @@ -1398,7 +1398,7 @@ TEST(WalletTests, CachedWitnessesEmptyChain) { wallet.IncrementNoteWitnesses(params, &index, &block, frontiers, true); EXPECT_DEATH(::GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, nAnchorConfirmations, sproutWitnesses, saplingWitnesses), - ".*it != end.*"); + "GetSproutNoteWitnesses"); for (int i = 1; i <= 8; i++) { CBlock another_block; @@ -1408,7 +1408,7 @@ TEST(WalletTests, CachedWitnessesEmptyChain) { } EXPECT_DEATH(::GetWitnessesAndAnchors(wallet, sproutNotes, saplingNotes, nAnchorConfirmations, sproutWitnesses, saplingWitnesses), - ".*it != end.*"); + "GetSproutNoteWitnesses"); CBlock last_block; CBlockIndex last_index(last_block); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 426ccb74b..6ffd58379 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3529,7 +3529,7 @@ bool CWallet::IsSaplingNullifierFromMe(const uint256& nullifier) const return false; } -void CWallet::GetSproutNoteWitnesses(const std::vector& notes, +bool CWallet::GetSproutNoteWitnesses(const std::vector& notes, unsigned int confirmations, std::vector>& witnesses, uint256 &final_anchor) const @@ -3545,10 +3545,10 @@ void CWallet::GetSproutNoteWitnesses(const std::vector& notes, auto noteWitnesses = mapWallet.at(note.hash).mapSproutNoteData.at(note).witnesses; auto it = noteWitnesses.cbegin(), end = noteWitnesses.cend(); for (int i = 1; i < confirmations; i++) { - assert(it != end); + if (it == end) return false; ++it; } - assert(it != end); + if (it == end) return false; witnesses[i] = *it; if (!rt) { rt = witnesses[i]->root(); @@ -3562,9 +3562,10 @@ void CWallet::GetSproutNoteWitnesses(const std::vector& notes, if (rt) { final_anchor = *rt; } + return true; } -void CWallet::GetSaplingNoteWitnesses(const std::vector& notes, +bool CWallet::GetSaplingNoteWitnesses(const std::vector& notes, unsigned int confirmations, std::vector>& witnesses, uint256 &final_anchor) const @@ -3580,10 +3581,10 @@ void CWallet::GetSaplingNoteWitnesses(const std::vector& notes, auto noteWitnesses = mapWallet.at(note.hash).mapSaplingNoteData.at(note).witnesses; auto it = noteWitnesses.cbegin(), end = noteWitnesses.cend(); for (int i = 1; i < confirmations; i++) { - assert(it != end); + if (it == end) return false; ++it; } - assert(it != end); + if (it == end) return false; witnesses[i] = *it; if (!rt) { rt = witnesses[i]->root(); @@ -3597,6 +3598,7 @@ void CWallet::GetSaplingNoteWitnesses(const std::vector& notes, if (rt) { final_anchor = *rt; } + return true; } std::vector> CWallet::GetOrchardSpendInfo( diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 076e13592..69a097b37 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -1778,12 +1778,12 @@ public: bool IsSproutNullifierFromMe(const uint256& nullifier) const; bool IsSaplingNullifierFromMe(const uint256& nullifier) const; - void GetSproutNoteWitnesses( + bool GetSproutNoteWitnesses( const std::vector& notes, unsigned int confirmations, std::vector>& witnesses, uint256 &final_anchor) const; - void GetSaplingNoteWitnesses( + bool GetSaplingNoteWitnesses( const std::vector& notes, unsigned int confirmations, std::vector>& witnesses,