From 387144f623a50fa03b8c697780579719c62f4ff3 Mon Sep 17 00:00:00 2001 From: Alessandro Rezzi Date: Sat, 22 Apr 2023 23:52:46 +0200 Subject: [PATCH 1/3] Function to rollback, eventually disconnect a block by recreating the witness cache --- src/rpc/blockchain.cpp | 13 ++- src/sapling/saplingscriptpubkeyman.cpp | 146 ++++++++++++++++++++++++- src/sapling/saplingscriptpubkeyman.h | 18 ++- src/wallet/wallet.cpp | 5 +- 4 files changed, 174 insertions(+), 8 deletions(-) diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index c6767903d7d4c..49d4a89c99e59 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -24,6 +24,7 @@ #include "util/system.h" #include "utilmoneystr.h" #include "utilstrencodings.h" +#include "validation.h" #include "validationinterface.h" #include "wallet/wallet.h" #include "warnings.h" @@ -1214,7 +1215,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.\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" "\nArguments:\n" "1. blockhash (string, required) the hash of the block to mark as invalid\n" @@ -1231,6 +1232,16 @@ 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 + 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!"); + } + } + } InvalidateBlock(state, Params(), pblockindex); } diff --git a/src/sapling/saplingscriptpubkeyman.cpp b/src/sapling/saplingscriptpubkeyman.cpp index d027a130e606d..cf3766b9d48f7 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -7,7 +7,14 @@ #include "chain.h" // for CBlockIndex #include "primitives/transaction.h" +#include "primitives/block.h" +#include "sapling/incrementalmerkletree.h" +#include "uint256.h" #include "validation.h" // for ReadBlockFromDisk() +#include "wallet/wallet.h" +#include +#include +#include void SaplingScriptPubKeyMan::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) { @@ -210,6 +217,93 @@ void UpdateWitnessHeights(NoteDataMap& noteDataMap, int indexHeight, int64_t nWi } } +bool SaplingScriptPubKeyMan::BuildWitnessChain(const CBlockIndex* pTargetBlock) +{ + LOCK2(cs_main, wallet->cs_wallet); + // Target is the last block we want to invalidate + rollbackTargetHeight = pTargetBlock->nHeight; + cachedWitnessMap.clear(); + + // Find the oldest sapling note + int minHeight = INT_MAX; + for (auto& it : wallet->mapWallet) { + CWalletTx& wtx = it.second; + if (wtx.mapSaplingNoteData.empty()) continue; + // Skip abandoned and conflicted txs for which the block_height is not defined (more precisely it it set to 0 by default) + if (wtx.m_confirm.status != CWalletTx::CONFIRMED) continue; + 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(); + int currentHeight = GetChainTip()->nHeight; + while (currentHeight >= minHeight) { + CBlock cblock; + ReadBlockFromDisk(cblock, pIndex); + cblocks.insert(cblocks.begin(), cblock); + pIndex = pIndex->pprev; + currentHeight = pIndex->nHeight; + } + + // Load the SaplingMerkleTree for the block before the oldest note + SaplingMerkleTree initialSaplingTree; + if (!pcoinsTip->GetSaplingAnchorAt(pIndex->hashFinalSaplingRoot, initialSaplingTree)) { + return false; + } + // Finally build the witness cache for each sapling note of your wallet + for (CBlock& block : cblocks) { + // Finally build the witness cache for each sapling note + std::vector noteCommitments; + std::vector inBlockArrivingNotes; + for (const auto& tx : block.vtx) { + const auto& hash = tx->GetHash(); + auto it = wallet->mapWallet.find(hash); + bool txIsOurs = it != wallet->mapWallet.end(); + + if (!tx->IsShieldedTx()) continue; + for (uint32_t i = 0; i < tx->sapData->vShieldedOutput.size(); i++) { + const auto& cmu = tx->sapData->vShieldedOutput[i].cmu; + noteCommitments.emplace_back(cmu); + for (auto& item : inBlockArrivingNotes) { + item->witnesses.front().append(cmu); + } + initialSaplingTree.append(cmu); + if (txIsOurs) { + CWalletTx* wtx = &it->second; + auto ndIt = wtx->mapSaplingNoteData.find({hash, i}); + if (ndIt != wtx->mapSaplingNoteData.end()) { + SaplingNoteData* nd = &ndIt->second; + nd->witnesses.push_front(initialSaplingTree.witness()); + inBlockArrivingNotes.emplace_back(nd); + } + } + } + } + for (auto& it2 : cachedWitnessMap) { + it2.second.emplace_front(it2.second.front()); + for (auto& noteComm : noteCommitments) { + it2.second.front().append(noteComm); + } + } + for (auto nd : inBlockArrivingNotes) { + if (nd->nullifier) { + std::list witnesses; + witnesses.push_front(nd->witnesses.front()); + cachedWitnessMap.emplace(*(nd->nullifier), witnesses); + } + } + } + return true; +} + void SaplingScriptPubKeyMan::IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, SaplingMerkleTree& saplingTreeRes) @@ -293,6 +387,34 @@ void SaplingScriptPubKeyMan::IncrementNoteWitnesses(const CBlockIndex* pindex, // CWallet::SetBestChain() (which also ensures that overall consistency // of the wallet.dat is maintained). } +/* + * Clear and eventually reset each witness of noteDataMap with the corresponding front-value of cachedWitnessMap, indexHeight is the blockHeight being invalidated + */ +void ResetNoteWitnesses(std::map& noteDataMap, std::map>& cachedWitnessMap, int indexHeight) +{ + // For each note that you own: + for (auto& item : noteDataMap) { + auto& nd = (item.second); + // skip externally sent notes + if (!nd.IsMyNote()) continue; + // Clear the cache + nd.witnesses.clear(); + // The withnessHeight must be EITHER -1 or equal to the block indexHeight + // The case in which indexHeight > witnessHeight is due to conflicted notes, which are irrelevant + // TODO: Allow invalidating blocks only if there are not conflicted txs? + if (nd.witnessHeight <= indexHeight) { + assert((nd.witnessHeight == -1) || (nd.witnessHeight == indexHeight)); + } + // Decrease the witnessHeight + nd.witnessHeight = indexHeight - 1; + if (nd.nullifier && cachedWitnessMap.at(*nd.nullifier).size() > 0) { + // Update the witness value with the cached one + nd.witnesses.push_front(cachedWitnessMap.at(*nd.nullifier).front()); + cachedWitnessMap.at(*nd.nullifier).pop_front(); + } + } +} + template void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t nWitnessCacheSize) @@ -336,9 +458,30 @@ void DecrementNoteWitnesses(NoteDataMap& noteDataMap, int indexHeight, int64_t n } } -void SaplingScriptPubKeyMan::DecrementNoteWitnesses(int nChainHeight) +void SaplingScriptPubKeyMan::DecrementNoteWitnesses(const CBlockIndex* pindex) { + assert(pindex); LOCK(wallet->cs_wallet); + int nChainHeight = pindex->nHeight; + // if the targetHeight is different from -1 we have a cache to use + if (rollbackTargetHeight != -1) { + for (std::pair& wtxItem : wallet->mapWallet) { + if (!wtxItem.second.mapSaplingNoteData.empty()) { + // For each sapling note that you own reset the current witness with the cached one + ResetNoteWitnesses(wtxItem.second.mapSaplingNoteData, cachedWitnessMap, nChainHeight); + } + } + nWitnessCacheSize = 1; + nWitnessCacheNeedsUpdate = true; + // If we reached the target height empty the cache and reset the target height to -1 + // Remember that the targetHeight is indeed the last block we want to invalidate + if (rollbackTargetHeight == pindex->nHeight) { + cachedWitnessMap.clear(); + rollbackTargetHeight = -1; + } + return; + } + for (std::pair& wtxItem : wallet->mapWallet) { ::DecrementNoteWitnesses(wtxItem.second.mapSaplingNoteData, nChainHeight, nWitnessCacheSize); } @@ -517,7 +660,6 @@ void SaplingScriptPubKeyMan::GetFilteredNotes( for (const auto& it : wtx.mapSaplingNoteData) { const SaplingOutPoint& op = it.first; const SaplingNoteData& nd = it.second; - // skip sent notes if (!nd.IsMyNote()) continue; diff --git a/src/sapling/saplingscriptpubkeyman.h b/src/sapling/saplingscriptpubkeyman.h index 7385b72fbf3d9..0adbc53f81c66 100644 --- a/src/sapling/saplingscriptpubkeyman.h +++ b/src/sapling/saplingscriptpubkeyman.h @@ -6,11 +6,14 @@ #define PIVX_SAPLINGSCRIPTPUBKEYMAN_H #include "consensus/consensus.h" +#include "sapling/incrementalmerkletree.h" #include "sapling/note.h" +#include "uint256.h" #include "wallet/hdchain.h" +#include "wallet/scriptpubkeyman.h" #include "wallet/wallet.h" #include "wallet/walletdb.h" -#include "sapling/incrementalmerkletree.h" +#include //! Size of witness cache // Should be large enough that we can expect not to reorg beyond our cache @@ -47,6 +50,7 @@ class SaplingNoteData /* witnesses/ivk: only for own (received) outputs */ std::list witnesses; + Optional ivk {nullopt}; inline bool IsMyNote() const { return ivk != nullopt; } @@ -154,6 +158,11 @@ class SaplingScriptPubKeyMan { bool IsSaplingSpent(const SaplingOutPoint& op) const; bool IsSaplingSpent(const uint256& nullifier) const; + /** + * Build the old witness chain. + */ + bool BuildWitnessChain(const CBlockIndex* pTargetBlock); + /** * pindex is the new tip being connected. */ @@ -161,9 +170,9 @@ class SaplingScriptPubKeyMan { const CBlock* pblock, SaplingMerkleTree& saplingTree); /** - * nChainHeight is the old tip height being disconnected. + * pindex is the old tip being disconnected. */ - void DecrementNoteWitnesses(int nChainHeight); + void DecrementNoteWitnesses(const CBlockIndex* pindex); /** * Update mapSaplingNullifiersToNotes @@ -407,6 +416,9 @@ class SaplingScriptPubKeyMan { std::map mapSaplingNullifiersToNotes; private: + /* Map hash nullifiers, list Sapling Witness*/ + std::map> cachedWitnessMap; + int rollbackTargetHeight = -1; /* Parent wallet */ CWallet* wallet{nullptr}; /* the HD chain data model (external/internal chain counters) */ diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 5774b168454ff..b51a1f9794942 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -6,6 +6,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "optional.h" +#include "validation.h" #if defined(HAVE_CONFIG_H) #include "config/pivx-config.h" #endif @@ -1369,7 +1370,7 @@ void CWallet::BlockDisconnected(const std::shared_ptr& pblock, con if (Params().GetConsensus().NetworkUpgradeActive(nBlockHeight, Consensus::UPGRADE_V5_0)) { // Update Sapling cached incremental witnesses - m_sspk_man->DecrementNoteWitnesses(nBlockHeight); + m_sspk_man->DecrementNoteWitnesses(mapBlockIndex[blockHash]); m_sspk_man->UpdateSaplingNullifierNoteMapForBlock(pblock.get()); } } @@ -4688,7 +4689,7 @@ void CWallet::IncrementNoteWitnesses(const CBlockIndex* pindex, const CBlock* pblock, SaplingMerkleTree& saplingTree) { m_sspk_man->IncrementNoteWitnesses(pindex, pblock, saplingTree); } -void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { m_sspk_man->DecrementNoteWitnesses(pindex->nHeight); } +void CWallet::DecrementNoteWitnesses(const CBlockIndex* pindex) { m_sspk_man->DecrementNoteWitnesses(pindex); } void CWallet::ClearNoteWitnessCache() { m_sspk_man->ClearNoteWitnessCache(); } From 2379c2daa98d7ab428fe138fc4e25d0965ceaf93 Mon Sep 17 00:00:00 2001 From: Alessandro Rezzi Date: Wed, 26 Apr 2023 21:57:01 +0200 Subject: [PATCH 2/3] 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 49d4a89c99e59..0306cc10326dd 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" @@ -1215,7 +1216,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" @@ -1232,13 +1233,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 cf3766b9d48f7..8ba967c9478fb 100644 --- a/src/sapling/saplingscriptpubkeyman.cpp +++ b/src/sapling/saplingscriptpubkeyman.cpp @@ -7,6 +7,7 @@ #include "chain.h" // for CBlockIndex #include "primitives/transaction.h" +#include "consensus/params.h" #include "primitives/block.h" #include "sapling/incrementalmerkletree.h" #include "uint256.h" @@ -14,6 +15,7 @@ #include "wallet/wallet.h" #include #include +#include #include void SaplingScriptPubKeyMan::AddToSaplingSpends(const uint256& nullifier, const uint256& wtxid) @@ -201,7 +203,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) { @@ -217,8 +219,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; @@ -234,13 +241,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(); @@ -253,12 +253,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; @@ -288,7 +290,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); } @@ -300,6 +305,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 0adbc53f81c66..5f0ba39081efa 100644 --- a/src/sapling/saplingscriptpubkeyman.h +++ b/src/sapling/saplingscriptpubkeyman.h @@ -161,7 +161,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. From be497204a7f32634085be0ccf39436509488f94f Mon Sep 17 00:00:00 2001 From: Alessandro Rezzi Date: Mon, 1 May 2023 13:37:39 +0200 Subject: [PATCH 3/3] Functional test coverage --- test/functional/rpc_invalidateblock.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/functional/rpc_invalidateblock.py b/test/functional/rpc_invalidateblock.py index 19ab691bcdc93..873bf7de9e08f 100755 --- a/test/functional/rpc_invalidateblock.py +++ b/test/functional/rpc_invalidateblock.py @@ -7,10 +7,12 @@ from test_framework.test_framework import PivxTestFramework from test_framework.util import assert_equal, connect_nodes, wait_until + class InvalidateTest(PivxTestFramework): def set_test_params(self): self.setup_clean_chain = True self.num_nodes = 3 + self.extra_args = [["-nuparams=v5_shield:1"]] * self.num_nodes def setup_network(self): self.setup_nodes() @@ -79,6 +81,18 @@ def run_test(self): # Should be back at the tip by now assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + self.log.info("Verify that it works for more than 100 blocks (sapling cache reconstruction)") + blocks = self.nodes[1].generate(200) + assert_equal(self.nodes[0].getblockchaininfo()['upgrades']['v5 shield']['status'], 'active') + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + # Invalidate a block deeper than the maximum cache size (i.e deeper than 100 blocks) + self.nodes[1].invalidateblock(blocks[-140]) + assert_equal(self.nodes[1].getbestblockhash(), blocks[-141]) + # Reconsider only the previous tip + self.nodes[1].reconsiderblock(blocks[-140]) + # Should be back at the tip by now + assert_equal(self.nodes[1].getbestblockhash(), blocks[-1]) + if __name__ == '__main__': InvalidateTest().main()