From 391dff16fe9ace90fc0f3308a5c63c453370e713 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Tue, 11 Aug 2015 21:03:31 +0200 Subject: [PATCH 1/2] Do not store Merkle branches in the wallet. Assume that when a wallet transaction has a valid block hash and transaction position in it, the transaction is actually there. We're already trusting wallet data in a much more fundamental way anyway. To prevent backward compatibility issues, a new record is used for storing the block locator in the wallet. Old wallets will see a wallet file synchronized up to the genesis block, and rescan automatically. --- doc/release-notes.md | 9 +++++++++ src/chainparams.cpp | 2 +- src/core_memusage.h | 2 +- src/main.cpp | 2 +- src/miner.cpp | 2 +- src/primitives/block.cpp | 39 ++------------------------------------- src/primitives/block.h | 10 ++-------- src/test/miner_tests.cpp | 2 +- src/test/pmt_tests.cpp | 2 +- src/wallet/wallet.cpp | 15 +-------------- src/wallet/wallet.h | 7 +------ src/wallet/walletdb.cpp | 6 ++++-- 12 files changed, 25 insertions(+), 73 deletions(-) diff --git a/doc/release-notes.md b/doc/release-notes.md index a06075334..bef9af60e 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -105,6 +105,15 @@ In this version, it is only enforced for peers that send protocol versions removed. It is recommended to update SPV clients to check for the `NODE_BLOOM` service bit for nodes that report versions newer than 70011. +Merkle branches removed from wallet +----------------------------------- + +Previously, every wallet transaction stored a Merkle branch to prove its +presence in blocks. This wasn't being used for more than an expensive +sanity check. Since 0.12, these are no longer stored. When loading a +0.12 wallet into an older version, it will automatically rescan to avoid +failed checks. + 0.12.0 Change log ================= diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 9c843f6e7..15b86cdda 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -31,7 +31,7 @@ static CBlock CreateGenesisBlock(const char* pszTimestamp, const CScript& genesi genesis.nVersion = nVersion; genesis.vtx.push_back(txNew); genesis.hashPrevBlock.SetNull(); - genesis.hashMerkleRoot = genesis.BuildMerkleTree(); + genesis.hashMerkleRoot = genesis.ComputeMerkleRoot(); return genesis; } diff --git a/src/core_memusage.h b/src/core_memusage.h index 711135bb4..a05f59ee0 100644 --- a/src/core_memusage.h +++ b/src/core_memusage.h @@ -48,7 +48,7 @@ static inline size_t RecursiveDynamicUsage(const CMutableTransaction& tx) { } static inline size_t RecursiveDynamicUsage(const CBlock& block) { - size_t mem = memusage::DynamicUsage(block.vtx) + memusage::DynamicUsage(block.vMerkleTree); + size_t mem = memusage::DynamicUsage(block.vtx); for (std::vector::const_iterator it = block.vtx.begin(); it != block.vtx.end(); it++) { mem += RecursiveDynamicUsage(*it); } diff --git a/src/main.cpp b/src/main.cpp index 2a24d38e5..1e0194e2c 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2595,7 +2595,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo // Check the merkle root. if (fCheckMerkleRoot) { bool mutated; - uint256 hashMerkleRoot2 = block.BuildMerkleTree(&mutated); + uint256 hashMerkleRoot2 = block.ComputeMerkleRoot(&mutated); if (block.hashMerkleRoot != hashMerkleRoot2) return state.DoS(100, error("CheckBlock(): hashMerkleRoot mismatch"), REJECT_INVALID, "bad-txnmrklroot", true); diff --git a/src/miner.cpp b/src/miner.cpp index b2a356e52..42c8bb970 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -368,7 +368,7 @@ void IncrementExtraNonce(CBlock* pblock, const CBlockIndex* pindexPrev, unsigned assert(txCoinbase.vin[0].scriptSig.size() <= 100); pblock->vtx[0] = txCoinbase; - pblock->hashMerkleRoot = pblock->BuildMerkleTree(); + pblock->hashMerkleRoot = pblock->ComputeMerkleRoot(); } ////////////////////////////////////////////////////////////////////////////// diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index 5b9c13d87..7a58074d2 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -15,7 +15,7 @@ uint256 CBlockHeader::GetHash() const return SerializeHash(*this); } -uint256 CBlock::BuildMerkleTree(bool* fMutated) const +uint256 CBlock::ComputeMerkleRoot(bool* fMutated) const { /* WARNING! If you're reading this because you're learning about crypto and/or designing a new system that will use merkle trees, keep in mind @@ -52,7 +52,7 @@ uint256 CBlock::BuildMerkleTree(bool* fMutated) const known ways of changing the transactions without affecting the merkle root. */ - vMerkleTree.clear(); + std::vector vMerkleTree; vMerkleTree.reserve(vtx.size() * 2 + 16); // Safe upper bound for the number of total nodes. for (std::vector::const_iterator it(vtx.begin()); it != vtx.end(); ++it) vMerkleTree.push_back(it->GetHash()); @@ -78,37 +78,6 @@ uint256 CBlock::BuildMerkleTree(bool* fMutated) const return (vMerkleTree.empty() ? uint256() : vMerkleTree.back()); } -std::vector CBlock::GetMerkleBranch(int nIndex) const -{ - if (vMerkleTree.empty()) - BuildMerkleTree(); - std::vector vMerkleBranch; - int j = 0; - for (int nSize = vtx.size(); nSize > 1; nSize = (nSize + 1) / 2) - { - int i = std::min(nIndex^1, nSize-1); - vMerkleBranch.push_back(vMerkleTree[j+i]); - nIndex >>= 1; - j += nSize; - } - return vMerkleBranch; -} - -uint256 CBlock::CheckMerkleBranch(uint256 hash, const std::vector& vMerkleBranch, int nIndex) -{ - if (nIndex == -1) - return uint256(); - for (std::vector::const_iterator it(vMerkleBranch.begin()); it != vMerkleBranch.end(); ++it) - { - if (nIndex & 1) - hash = Hash(BEGIN(*it), END(*it), BEGIN(hash), END(hash)); - else - hash = Hash(BEGIN(hash), END(hash), BEGIN(*it), END(*it)); - nIndex >>= 1; - } - return hash; -} - std::string CBlock::ToString() const { std::stringstream s; @@ -123,9 +92,5 @@ std::string CBlock::ToString() const { s << " " << vtx[i].ToString() << "\n"; } - s << " vMerkleTree: "; - for (unsigned int i = 0; i < vMerkleTree.size(); i++) - s << " " << vMerkleTree[i].ToString(); - s << "\n"; return s.str(); } diff --git a/src/primitives/block.h b/src/primitives/block.h index 59f46deb1..7fe8c84cb 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -77,9 +77,6 @@ public: // network and disk std::vector vtx; - // memory only - mutable std::vector vMerkleTree; - CBlock() { SetNull(); @@ -103,7 +100,6 @@ public: { CBlockHeader::SetNull(); vtx.clear(); - vMerkleTree.clear(); } CBlockHeader GetBlockHeader() const @@ -118,14 +114,12 @@ public: return block; } - // Build the in-memory merkle tree for this block and return the merkle root. + // Build the merkle tree for this block and return the merkle root. // If non-NULL, *mutated is set to whether mutation was detected in the merkle // tree (a duplication of transactions in the block leading to an identical // merkle root). - uint256 BuildMerkleTree(bool* mutated = NULL) const; + uint256 ComputeMerkleRoot(bool* mutated = NULL) const; - std::vector GetMerkleBranch(int nIndex) const; - static uint256 CheckMerkleBranch(uint256 hash, const std::vector& vMerkleBranch, int nIndex); std::string ToString() const; }; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index ad79a558c..91a3a5738 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -87,7 +87,7 @@ BOOST_AUTO_TEST_CASE(CreateNewBlock_validity) pblock->vtx[0] = CTransaction(txCoinbase); if (txFirst.size() < 2) txFirst.push_back(new CTransaction(pblock->vtx[0])); - pblock->hashMerkleRoot = pblock->BuildMerkleTree(); + pblock->hashMerkleRoot = pblock->ComputeMerkleRoot(); pblock->nNonce = blockinfo[i].nonce; CValidationState state; BOOST_CHECK(ProcessNewBlock(state, NULL, pblock, true, NULL)); diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp index f6d06d680..d9f3c3e46 100644 --- a/src/test/pmt_tests.cpp +++ b/src/test/pmt_tests.cpp @@ -48,7 +48,7 @@ BOOST_AUTO_TEST_CASE(pmt_test1) } // calculate actual merkle root and height - uint256 merkleRoot1 = block.BuildMerkleTree(); + uint256 merkleRoot1 = block.ComputeMerkleRoot(); std::vector vTxid(nTx, uint256()); for (unsigned int j=0; jhashMerkleRoot) - return 0; - fMerkleVerified = true; - } - pindexRet = pindex; return chainActive.Height() - pindex->nHeight + 1; } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index bd30b67b0..34e98cfb8 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -151,13 +151,8 @@ private: public: uint256 hashBlock; - std::vector vMerkleBranch; int nIndex; - // memory only - mutable bool fMerkleVerified; - - CMerkleTx() { Init(); @@ -172,13 +167,13 @@ public: { hashBlock = uint256(); nIndex = -1; - fMerkleVerified = false; } ADD_SERIALIZE_METHODS; template inline void SerializationOp(Stream& s, Operation ser_action, int nType, int nVersion) { + std::vector vMerkleBranch; // For compatibility with older versions. READWRITE(*(CTransaction*)this); nVersion = this->nVersion; READWRITE(hashBlock); diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index c1eb18458..0624e442d 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -131,12 +131,14 @@ bool CWalletDB::EraseWatchOnly(const CScript &dest) bool CWalletDB::WriteBestBlock(const CBlockLocator& locator) { nWalletDBUpdated++; - return Write(std::string("bestblock"), locator); + Write(std::string("bestblock"), CBlockLocator()); // Write empty block locator so versions that require a merkle branch automatically rescan + return Write(std::string("bestblock_nomerkle"), locator); } bool CWalletDB::ReadBestBlock(CBlockLocator& locator) { - return Read(std::string("bestblock"), locator); + if (Read(std::string("bestblock"), locator) && !locator.vHave.empty()) return true; + return Read(std::string("bestblock_nomerkle"), locator); } bool CWalletDB::WriteOrderPosNext(int64_t nOrderPosNext) From 3b33ec85ed00ba7e7525858e3701f9f55071c58b Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 15 Aug 2015 23:32:38 +0200 Subject: [PATCH 2/2] Avoid duplicate CheckBlock checks --- src/main.cpp | 6 ++++++ src/primitives/block.h | 4 ++++ 2 files changed, 10 insertions(+) diff --git a/src/main.cpp b/src/main.cpp index 1e0194e2c..e8bc68fe6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -2587,6 +2587,9 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo { // These are checks that are independent of context. + if (block.fChecked) + return true; + // Check that the header is valid (particularly PoW). This is mostly // redundant with the call in AcceptBlockHeader. if (!CheckBlockHeader(block, state, fCheckPOW)) @@ -2642,6 +2645,9 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo return state.DoS(100, error("CheckBlock(): out-of-bounds SigOpCount"), REJECT_INVALID, "bad-blk-sigops", true); + if (fCheckPOW && fCheckMerkleRoot) + block.fChecked = true; + return true; } diff --git a/src/primitives/block.h b/src/primitives/block.h index 7fe8c84cb..86106098f 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -77,6 +77,9 @@ public: // network and disk std::vector vtx; + // memory only + mutable bool fChecked; + CBlock() { SetNull(); @@ -100,6 +103,7 @@ public: { CBlockHeader::SetNull(); vtx.clear(); + fChecked = false; } CBlockHeader GetBlockHeader() const