From 1a1a5ffc8f553bc90e83c4aa15d4c59f643ddfed Mon Sep 17 00:00:00 2001 From: Alessandro Rezzi Date: Wed, 26 Apr 2023 21:57:01 +0200 Subject: [PATCH] Improved error coverage and added default behaviour for null merkle tree --- src/rpc/blockchain.cpp | 9 ++++---- src/sapling/saplingscriptpubkeyman.cpp | 32 +++++++++++++++----------- src/sapling/saplingscriptpubkeyman.h | 2 +- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 0ead810d419daf..175e1861cbbda7 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -6,6 +6,7 @@ // file COPYING or https://www.opensource.org/licenses/mit-license.php. #include "budget/budgetmanager.h" +#include "chainparams.h" #include "checkpoints.h" #include "clientversion.h" #include "consensus/upgrades.h" @@ -1212,7 +1213,7 @@ UniValue invalidateblock(const JSONRPCRequest& request) if (request.fHelp || request.params.size() != 1) throw std::runtime_error( "invalidateblock \"blockhash\"\n" - "\nPermanently marks a block as invalid, as if it violated a consensus rule. Note: Your oldest sapling note must not be more than 43200 blocks behind the chain tip\n" + "\nPermanently marks a block as invalid, as if it violated a consensus rule. Note: it might take up to some minutes and after calling it's reccomended to run recover transactions. \n" "\nArguments:\n" "1. blockhash (string, required) the hash of the block to mark as invalid\n" @@ -1229,13 +1230,13 @@ UniValue invalidateblock(const JSONRPCRequest& request) throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Block not found"); CBlockIndex* pblockindex = mapBlockIndex[hash]; - //For each walllet in your wallet list + // For each wallet in your wallet list std::string errString = ""; for (auto* pwallet : vpwallets) { //Do we need to recreate the witnesscache or is the current one enough? if (pwallet->GetSaplingScriptPubKeyMan()->nWitnessCacheSize <= (chainActive.Height() - pblockindex->nHeight + 1)) { - if (!pwallet->GetSaplingScriptPubKeyMan()->BuildWitnessChain(pblockindex)) { - throw JSONRPCError(RPC_DATABASE_ERROR, "Sapling notes are too old!"); + if (!pwallet->GetSaplingScriptPubKeyMan()->BuildWitnessChain(pblockindex, Params().GetConsensus(), errString)) { + throw JSONRPCError(RPC_DATABASE_ERROR, errString); } } } diff --git a/src/sapling/saplingscriptpubkeyman.cpp b/src/sapling/saplingscriptpubkeyman.cpp index e361d33908aff7..43cf1fdd1a9a0a 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -6,6 +6,7 @@ #include "sapling/saplingscriptpubkeyman.h" #include "chain.h" // for CBlockIndex +#include "consensus/params.h" #include "primitives/block.h" #include "sapling/incrementalmerkletree.h" #include "uint256.h" @@ -13,6 +14,7 @@ #include "wallet/wallet.h" #include #include +#include #include void SaplingScriptPubKeyMan::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) @@ -189,7 +191,7 @@ void WitnessNoteIfMine(SaplingNoteData* nd, assert(nWitnessCacheSize >= (int64_t) nd->witnesses.size()); } -template +template void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize) { for (auto& item : noteDataMap) { @@ -205,8 +207,13 @@ void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWi } } -bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) +bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock, const Consensus::Params& params, std::string& errorStr) { + // If V5 is not enforced building the witness cache is useless + if (!params.NetworkUpgradeActive(chainActive.Height(), Consensus::UPGRADE_V5_0)) { + return true; + } + LOCK2(cs_main, wallet->cs_wallet); // Target is the last block we want to invalidate rollbackTargetHeight = pTargetBlock->nHeight; @@ -222,13 +229,6 @@ bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) minHeight = std::min(wtx.m_confirm.block_height, minHeight); } - // For the moment as a maximum rollback span we use 1 month or 43200 blocks - if ((chainActive.Height() - minHeight) > 43200) { - cachedWitnessMap.clear(); - rollbackTargetHeight = -1; - return false; - } - // Read blocks from the disk from chaintip to the minimum found height std::vector cblocks; const CBlockIndex* pIndex = GetChainTip(); @@ -241,12 +241,14 @@ bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) currentHeight = pIndex->nHeight; } - // Load the SaplingMerkleTree for the block before the oldest note - SaplingMerkleTree initialSaplingTree; - if (!pcoinsTip->GetSaplingAnchorAt(pIndex->hashFinalSaplingRoot, initialSaplingTree)) { + SaplingMerkleTree initialSaplingTree = SaplingMerkleTree(); + // Load the SaplingMerkleTree for the block before the oldest note (if the hash is zero then continue with an empty merkle tree) + if (!(pIndex->hashFinalSaplingRoot == UINT256_ZERO) && !pcoinsTip->GetSaplingAnchorAt(pIndex->hashFinalSaplingRoot, initialSaplingTree)) { + errorStr = "Cannot fetch the sapling anchor!"; return false; } // Finally build the witness cache for each sapling note of your wallet + int height = minHeight; for (CBlock& block : cblocks) { // Finally build the witness cache for each sapling note std::vector noteCommitments; @@ -276,7 +278,10 @@ bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) } } for (auto& it2 : cachedWitnessMap) { - it2.second.emplace_front(it2.second.front()); + // Don't duplicate if the block is too old + if (height >= rollbackTargetHeight) { + it2.second.emplace_front(it2.second.front()); + } for (auto& noteComm : noteCommitments) { it2.second.front().append(noteComm); } @@ -288,6 +293,7 @@ bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) cachedWitnessMap.emplace(*(nd->nullifier), witnesses); } } + height++; } return true; } diff --git a/src/sapling/saplingscriptpubkeyman.h b/src/sapling/saplingscriptpubkeyman.h index 23bbabd369ff1b..09a75917e0d225 100644 --- a/src/sapling/saplingscriptpubkeyman.h +++ b/src/sapling/saplingscriptpubkeyman.h @@ -160,7 +160,7 @@ class SaplingScriptPubKeyMan { /** * Build the old witness chain. */ - bool BuildWitnessChain(const CBlockIndex* pTargetBlock); + bool BuildWitnessChain(const CBlockIndex* pTargetBlock, const Consensus::Params& params, std::string& errorStr); /** * pindex is the new tip being connected.