From 5447622a03ece62c2cdb4ba0029b8b6b0ee9209d Mon Sep 17 00:00:00 2001 From: warrows Date: Sat, 20 Jul 2019 10:58:13 +0200 Subject: [PATCH 01/18] [Wallet] Do not store Merkle branches in the wallet Backport of https://github.com/bitcoin/bitcoin/pull/6550 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. --- src/chainparams.cpp | 2 +- src/main.cpp | 9 +++++++-- 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 | 14 +------------- src/wallet/wallet.h | 7 +------ src/wallet/walletdb.cpp | 6 ++++-- 10 files changed, 23 insertions(+), 70 deletions(-) diff --git a/src/chainparams.cpp b/src/chainparams.cpp index 318b8bb2a1a9..e0033d3a8e93 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -205,7 +205,7 @@ class CMainParams : public CChainParams txNew.vout[0].scriptPubKey = CScript() << ParseHex("04c10e83b2703ccf322f7dbd62dd5855ac7c10bd055814ce121ba32607d573b8810c02c0582aed05b4deb9c4b77b26d92428c61256cd42774babea0a073b2ed0c9") << OP_CHECKSIG; genesis.vtx.push_back(txNew); genesis.hashPrevBlock = 0; - genesis.hashMerkleRoot = genesis.BuildMerkleTree(); + genesis.hashMerkleRoot = genesis.ComputeMerkleRoot(); genesis.nVersion = 1; genesis.nTime = 1454124731; genesis.nBits = 0x1e0ffff0; diff --git a/src/main.cpp b/src/main.cpp index f7e982073157..5bab6f8bde8a 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -4375,6 +4375,9 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo LogPrint("debug", "%s: block=%s is proof of stake=%d\n", __func__, block.GetHash().ToString().c_str(), IsPoS); + 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, !IsPoS)) @@ -4393,7 +4396,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("%s : hashMerkleRoot mismatch", __func__), REJECT_INVALID, "bad-txnmrklroot", true); @@ -4526,7 +4529,6 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo } } - unsigned int nSigOps = 0; for (const CTransaction& tx : block.vtx) { nSigOps += GetLegacySigOpCount(tx); @@ -4536,6 +4538,9 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo return state.DoS(100, error("%s : out-of-bounds SigOpCount", __func__), REJECT_INVALID, "bad-blk-sigops", true); + if (fCheckPOW && fCheckMerkleRoot && fCheckSig) + block.fChecked = true; + return true; } diff --git a/src/miner.cpp b/src/miner.cpp index 63562578c872..8638a24514f5 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -569,7 +569,7 @@ void IncrementExtraNonce(CBlock* pblock, CBlockIndex* pindexPrev, unsigned int& assert(txCoinbase.vin[0].scriptSig.size() <= 100); pblock->vtx[0] = txCoinbase; - pblock->hashMerkleRoot = pblock->BuildMerkleTree(); + pblock->hashMerkleRoot = pblock->ComputeMerkleRoot(); } #ifdef ENABLE_WALLET diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index 9786e906c593..41dc30f7f9a6 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -21,7 +21,7 @@ uint256 CBlockHeader::GetHash() const return Hash(BEGIN(nVersion), END(nAccumulatorCheckpoint)); } -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 @@ -58,7 +58,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()); @@ -84,37 +84,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; @@ -129,10 +98,6 @@ 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 a22785c38480..f89b0582d573 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -94,7 +94,7 @@ class CBlock : public CBlockHeader // memory only mutable CScript payee; - mutable std::vector vMerkleTree; + mutable bool fChecked; CBlock() { @@ -121,7 +121,7 @@ class CBlock : public CBlockHeader { CBlockHeader::SetNull(); vtx.clear(); - vMerkleTree.clear(); + fChecked = false; payee = CScript(); vchBlockSig.clear(); } @@ -157,14 +157,12 @@ class CBlock : public CBlockHeader return IsProofOfStake()? std::make_pair(vtx[1].vin[0].prevout, nTime) : std::make_pair(COutPoint(), (unsigned int)0); } - // 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; void print() const; }; diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 81c07d83e6d1..8be01f73d6f8 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -82,7 +82,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)); diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp index 2e943907507d..75ee7054eedf 100644 --- a/src/test/pmt_tests.cpp +++ b/src/test/pmt_tests.cpp @@ -45,7 +45,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, 0); 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 8b0c90a4f627..f17cf96d6fa1 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -766,13 +766,8 @@ class CMerkleTx : public CTransaction public: uint256 hashBlock; - std::vector vMerkleBranch; int nIndex; - // memory only - mutable bool fMerkleVerified; - - CMerkleTx() { Init(); @@ -787,7 +782,6 @@ class CMerkleTx : public CTransaction { hashBlock = 0; nIndex = -1; - fMerkleVerified = false; } ADD_SERIALIZE_METHODS; @@ -795,6 +789,7 @@ class CMerkleTx : public CTransaction 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 ce8996f0971a..12fc478c7827 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -197,12 +197,14 @@ bool CWalletDB::EraseMultiSig(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 ab9efb8a69b4407a9e221258f90ec4a48abc2ee2 Mon Sep 17 00:00:00 2001 From: warrows Date: Sat, 20 Jul 2019 15:59:20 +0200 Subject: [PATCH 02/18] [Wallet] Switch to a constant-space Merkle root/branch algorithm Backport of https://github.com/bitcoin/bitcoin/pull/6508 This switches the Merkle tree logic for blocks to one that runs in constant (small) space. The old code is moved to tests, and a new test is added that for various combinations of block sizes, transaction positions to compute a branch for, and mutations: * Verifies that the old code and new code agree for the Merkle root. * Verifies that the old code and new code agree for the Merkle branch. * Verifies that the computed Merkle branch is valid. * Verifies that mutations don't change the Merkle root. * Verifies that mutations are correctly detected. --- src/Makefile.am | 2 + src/Makefile.test.include | 1 + src/chainparams.cpp | 3 +- src/consensus/merkle.cpp | 168 ++++++++++++++++++++++++++++++++++++++ src/consensus/merkle.h | 32 ++++++++ src/main.cpp | 3 +- src/miner.cpp | 3 +- src/primitives/block.cpp | 63 -------------- src/primitives/block.h | 6 -- src/test/merkle_tests.cpp | 135 ++++++++++++++++++++++++++++++ src/test/miner_tests.cpp | 3 +- src/test/pmt_tests.cpp | 3 +- 12 files changed, 348 insertions(+), 74 deletions(-) create mode 100644 src/consensus/merkle.cpp create mode 100644 src/consensus/merkle.h create mode 100644 src/test/merkle_tests.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 77531237fc86..a158d05a3770 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -99,6 +99,7 @@ BITCOIN_CORE_H = \ compat/endian.h \ compat/sanity.h \ compressor.h \ + consensus/merkle.h \ primitives/block.h \ primitives/transaction.h \ core_io.h \ @@ -372,6 +373,7 @@ libbitcoin_common_a_SOURCES = \ chainparams.cpp \ coins.cpp \ compressor.cpp \ + consensus/merkle.cpp \ primitives/block.cpp \ zpiv/deterministicmint.cpp \ primitives/transaction.cpp \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 74b2f8aa8c5c..b5f6dfdc7f6c 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -52,6 +52,7 @@ BITCOIN_TESTS =\ test/key_tests.cpp \ test/main_tests.cpp \ test/mempool_tests.cpp \ + test/merkle_tests.cpp \ test/mruset_tests.cpp \ test/multisig_tests.cpp \ test/netbase_tests.cpp \ diff --git a/src/chainparams.cpp b/src/chainparams.cpp index e0033d3a8e93..04cd5265cdfd 100644 --- a/src/chainparams.cpp +++ b/src/chainparams.cpp @@ -7,6 +7,7 @@ #include "libzerocoin/Params.h" #include "chainparams.h" +#include "consensus/merkle.h" #include "random.h" #include "util.h" #include "utilstrencodings.h" @@ -205,7 +206,7 @@ class CMainParams : public CChainParams txNew.vout[0].scriptPubKey = CScript() << ParseHex("04c10e83b2703ccf322f7dbd62dd5855ac7c10bd055814ce121ba32607d573b8810c02c0582aed05b4deb9c4b77b26d92428c61256cd42774babea0a073b2ed0c9") << OP_CHECKSIG; genesis.vtx.push_back(txNew); genesis.hashPrevBlock = 0; - genesis.hashMerkleRoot = genesis.ComputeMerkleRoot(); + genesis.hashMerkleRoot = BlockMerkleRoot(genesis); genesis.nVersion = 1; genesis.nTime = 1454124731; genesis.nBits = 0x1e0ffff0; diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp new file mode 100644 index 000000000000..67155a185075 --- /dev/null +++ b/src/consensus/merkle.cpp @@ -0,0 +1,168 @@ +#include "merkle.h" +#include "hash.h" +#include "utilstrencodings.h" + +/* 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 + that the following merkle tree algorithm has a serious flaw related to + duplicate txids, resulting in a vulnerability (CVE-2012-2459). + The reason is that if the number of hashes in the list at a given time + is odd, the last one is duplicated before computing the next level (which + is unusual in Merkle trees). This results in certain sequences of + transactions leading to the same merkle root. For example, these two + trees: + A A + / \ / \ + B C B C + / \ | / \ / \ + D E F D E F F + / \ / \ / \ / \ / \ / \ / \ + 1 2 3 4 5 6 1 2 3 4 5 6 5 6 + for transaction lists [1,2,3,4,5,6] and [1,2,3,4,5,6,5,6] (where 5 and + 6 are repeated) result in the same root hash A (because the hash of both + of (F) and (F,F) is C). + The vulnerability results from being able to send a block with such a + transaction list, with the same merkle root, and the same block hash as + the original without duplication, resulting in failed validation. If the + receiving node proceeds to mark that block as permanently invalid + however, it will fail to accept further unmodified (and thus potentially + valid) versions of the same block. We defend against this by detecting + the case where we would hash two identical hashes at the end of the list + together, and treating that identically to the block having an invalid + merkle root. Assuming no double-SHA256 collisions, this will detect all + known ways of changing the transactions without affecting the merkle + root. +*/ + +/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ +static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { + if (pbranch) pbranch->clear(); + if (leaves.size() == 0) { + if (pmutated) *pmutated = false; + if (proot) *proot = uint256(); + return; + } + bool mutated = false; + // count is the number of leaves processed so far. + uint32_t count = 0; + // inner is an array of eagerly computed subtree hashes, indexed by tree + // level (0 being the leaves). + // For example, when count is 25 (11001 in binary), inner[4] is the hash of + // the first 16 leaves, inner[3] of the next 8 leaves, and inner[0] equal to + // the last leaf. The other inner entries are undefined. + uint256 inner[32]; + // Which position in inner is a hash that depends on the matching leaf. + int matchlevel = -1; + // First process all leaves into 'inner' values. + while (count < leaves.size()) { + uint256 h = leaves[count]; + bool matchh = count == branchpos; + count++; + int level; + // For each of the lower bits in count that are 0, do 1 step. Each + // corresponds to an inner value that existed before processing the + // current leaf, and each needs a hash to combine it. + for (level = 0; !(count & (((uint32_t)1) << level)); level++) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + mutated |= (inner[level] == h); + CHash256().Write(inner[level].begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + } + // Store the resulting hash at inner position level. + inner[level] = h; + if (matchh) { + matchlevel = level; + } + } + // Do a final 'sweep' over the rightmost branch of the tree to process + // odd levels, and reduce everything to a single top value. + // Level is the level (counted from the bottom) up to which we've sweeped. + int level = 0; + // As long as bit number level in count is zero, skip it. It means there + // is nothing left at this level. + while (!(count & (((uint32_t)1) << level))) { + level++; + } + uint256 h = inner[level]; + bool matchh = matchlevel == level; + while (count != (((uint32_t)1) << level)) { + // If we reach this point, h is an inner value that is not the top. + // We combine it with itself (Bitcoin's special rule for odd levels in + // the tree) to produce a higher level one. + if (pbranch && matchh) { + pbranch->push_back(h); + } + CHash256().Write(h.begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + // Increment count to the value it would have if two entries at this + // level had existed. + count += (((uint32_t)1) << level); + level++; + // And propagate the result upwards accordingly. + while (!(count & (((uint32_t)1) << level))) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + CHash256().Write(inner[level].begin(), 32).Write(h.begin(), 32).Finalize(h.begin()); + level++; + } + } + // Return result. + if (pmutated) *pmutated = mutated; + if (proot) *proot = h; +} + +uint256 ComputeMerkleRoot(const std::vector& leaves, bool* mutated) { + uint256 hash; + MerkleComputation(leaves, &hash, mutated, -1, NULL); + return hash; +} + +std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { + std::vector ret; + MerkleComputation(leaves, NULL, NULL, position, &ret); + return ret; +} + +uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector& vMerkleBranch, uint32_t nIndex) { + uint256 hash = leaf; + 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; +} + +uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) +{ + std::vector leaves; + leaves.resize(block.vtx.size()); + for (size_t s = 0; s < block.vtx.size(); s++) { + leaves[s] = block.vtx[s].GetHash(); + } + return ComputeMerkleRoot(leaves, mutated); +} + +std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) +{ + std::vector leaves; + leaves.resize(block.vtx.size()); + for (size_t s = 0; s < block.vtx.size(); s++) { + leaves[s] = block.vtx[s].GetHash(); + } + return ComputeMerkleBranch(leaves, position); +} \ No newline at end of file diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h new file mode 100644 index 000000000000..8f4bf8a49389 --- /dev/null +++ b/src/consensus/merkle.h @@ -0,0 +1,32 @@ +// Copyright (c) 2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_MERKLE +#define BITCOIN_MERKLE + +#include +#include + +#include "primitives/transaction.h" +#include "primitives/block.h" +#include "uint256.h" + +uint256 ComputeMerkleRoot(const std::vector& leaves, bool* mutated = NULL); +std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position); +uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector& branch, uint32_t position); + +/* + * Compute the Merkle root of the transactions in a block. + * *mutated is set to true if a duplicated subtree was found. + */ +uint256 BlockMerkleRoot(const CBlock& block, bool* mutated = NULL); + +/* + * Compute the Merkle branch for the tree of transactions in a block, for a + * given position. + * This can be verified using ComputeMerkleRootFromBranch. + */ +std::vector BlockMerkleBranch(const CBlock& block, uint32_t position); + +#endif \ No newline at end of file diff --git a/src/main.cpp b/src/main.cpp index 5bab6f8bde8a..d6d980319598 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -15,6 +15,7 @@ #include "chainparams.h" #include "checkpoints.h" #include "checkqueue.h" +#include "consensus/merkle.h" #include "init.h" #include "kernel.h" #include "masternode-budget.h" @@ -4396,7 +4397,7 @@ bool CheckBlock(const CBlock& block, CValidationState& state, bool fCheckPOW, bo // Check the merkle root. if (fCheckMerkleRoot) { bool mutated; - uint256 hashMerkleRoot2 = block.ComputeMerkleRoot(&mutated); + uint256 hashMerkleRoot2 = BlockMerkleRoot(block, &mutated); if (block.hashMerkleRoot != hashMerkleRoot2) return state.DoS(100, error("%s : hashMerkleRoot mismatch", __func__), REJECT_INVALID, "bad-txnmrklroot", true); diff --git a/src/miner.cpp b/src/miner.cpp index 8638a24514f5..83da596bc6e5 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -8,6 +8,7 @@ #include "miner.h" #include "amount.h" +#include "consensus/merkle.h" #include "hash.h" #include "main.h" #include "masternode-sync.h" @@ -569,7 +570,7 @@ void IncrementExtraNonce(CBlock* pblock, CBlockIndex* pindexPrev, unsigned int& assert(txCoinbase.vin[0].scriptSig.size() <= 100); pblock->vtx[0] = txCoinbase; - pblock->hashMerkleRoot = pblock->ComputeMerkleRoot(); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); } #ifdef ENABLE_WALLET diff --git a/src/primitives/block.cpp b/src/primitives/block.cpp index 41dc30f7f9a6..9f94a35f5f3c 100644 --- a/src/primitives/block.cpp +++ b/src/primitives/block.cpp @@ -21,69 +21,6 @@ uint256 CBlockHeader::GetHash() const return Hash(BEGIN(nVersion), END(nAccumulatorCheckpoint)); } -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 - that the following merkle tree algorithm has a serious flaw related to - duplicate txids, resulting in a vulnerability (CVE-2012-2459). - - The reason is that if the number of hashes in the list at a given time - is odd, the last one is duplicated before computing the next level (which - is unusual in Merkle trees). This results in certain sequences of - transactions leading to the same merkle root. For example, these two - trees: - - A A - / \ / \ - B C B C - / \ | / \ / \ - D E F D E F F - / \ / \ / \ / \ / \ / \ / \ - 1 2 3 4 5 6 1 2 3 4 5 6 5 6 - - for transaction lists [1,2,3,4,5,6] and [1,2,3,4,5,6,5,6] (where 5 and - 6 are repeated) result in the same root hash A (because the hash of both - of (F) and (F,F) is C). - - The vulnerability results from being able to send a block with such a - transaction list, with the same merkle root, and the same block hash as - the original without duplication, resulting in failed validation. If the - receiving node proceeds to mark that block as permanently invalid - however, it will fail to accept further unmodified (and thus potentially - valid) versions of the same block. We defend against this by detecting - the case where we would hash two identical hashes at the end of the list - together, and treating that identically to the block having an invalid - merkle root. Assuming no double-SHA256 collisions, this will detect all - known ways of changing the transactions without affecting the merkle - root. - */ - 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()); - int j = 0; - bool mutated = false; - for (int nSize = vtx.size(); nSize > 1; nSize = (nSize + 1) / 2) - { - for (int i = 0; i < nSize; i += 2) - { - int i2 = std::min(i+1, nSize-1); - if (i2 == i + 1 && i2 + 1 == nSize && vMerkleTree[j+i] == vMerkleTree[j+i2]) { - // Two identical hashes at the end of the list at a particular level. - mutated = true; - } - vMerkleTree.push_back(Hash(BEGIN(vMerkleTree[j+i]), END(vMerkleTree[j+i]), - BEGIN(vMerkleTree[j+i2]), END(vMerkleTree[j+i2]))); - } - j += nSize; - } - if (fMutated) { - *fMutated = mutated; - } - return (vMerkleTree.empty() ? uint256() : vMerkleTree.back()); -} - std::string CBlock::ToString() const { std::stringstream s; diff --git a/src/primitives/block.h b/src/primitives/block.h index f89b0582d573..fbf56236886a 100644 --- a/src/primitives/block.h +++ b/src/primitives/block.h @@ -157,12 +157,6 @@ class CBlock : public CBlockHeader return IsProofOfStake()? std::make_pair(vtx[1].vin[0].prevout, nTime) : std::make_pair(COutPoint(), (unsigned int)0); } - // 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 ComputeMerkleRoot(bool* mutated = NULL) const; - std::string ToString() const; void print() const; }; diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp new file mode 100644 index 000000000000..57536d8478f0 --- /dev/null +++ b/src/test/merkle_tests.cpp @@ -0,0 +1,135 @@ +// Copyright (c) 2015 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include "consensus/merkle.h" +#include "test/test_pivx.h" + +#include + +BOOST_FIXTURE_TEST_SUITE(merkle_tests, TestingSetup) + +// Older version of the merkle root computation code, for comparison. +static uint256 BlockBuildMerkleTree(const CBlock& block, bool* fMutated, std::vector& vMerkleTree) +{ + vMerkleTree.clear(); + vMerkleTree.reserve(block.vtx.size() * 2 + 16); // Safe upper bound for the number of total nodes. + for (std::vector::const_iterator it(block.vtx.begin()); it != block.vtx.end(); ++it) + vMerkleTree.push_back(it->GetHash()); + int j = 0; + bool mutated = false; + for (int nSize = block.vtx.size(); nSize > 1; nSize = (nSize + 1) / 2) + { + for (int i = 0; i < nSize; i += 2) + { + int i2 = std::min(i+1, nSize-1); + if (i2 == i + 1 && i2 + 1 == nSize && vMerkleTree[j+i] == vMerkleTree[j+i2]) { + // Two identical hashes at the end of the list at a particular level. + mutated = true; + } + vMerkleTree.push_back(Hash(vMerkleTree[j+i].begin(), vMerkleTree[j+i].end(), + vMerkleTree[j+i2].begin(), vMerkleTree[j+i2].end())); + } + j += nSize; + } + if (fMutated) { + *fMutated = mutated; + } + return (vMerkleTree.empty() ? uint256() : vMerkleTree.back()); +} + +// Older version of the merkle branch computation code, for comparison. +static std::vector BlockGetMerkleBranch(const CBlock& block, const std::vector& vMerkleTree, int nIndex) +{ + std::vector vMerkleBranch; + int j = 0; + for (int nSize = block.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; +} + +static inline int ctz(uint32_t i) { + if (i == 0) return 0; + int j = 0; + while (!(i & 1)) { + j++; + i >>= 1; + } + return j; +} + +BOOST_AUTO_TEST_CASE(merkle_test) +{ + for (int i = 0; i < 32; i++) { + // Try 32 block sizes: all sizes from 0 to 16 inclusive, and then 15 random sizes. + int ntx = (i <= 16) ? i : 17 + (InsecureRandRange(4000)); + // Try up to 3 mutations. + for (int mutate = 0; mutate <= 3; mutate++) { + int duplicate1 = mutate >= 1 ? 1 << ctz(ntx) : 0; // The last how many transactions to duplicate first. + if (duplicate1 >= ntx) break; // Duplication of the entire tree results in a different root (it adds a level). + int ntx1 = ntx + duplicate1; // The resulting number of transactions after the first duplication. + int duplicate2 = mutate >= 2 ? 1 << ctz(ntx1) : 0; // Likewise for the second mutation. + if (duplicate2 >= ntx1) break; + int ntx2 = ntx1 + duplicate2; + int duplicate3 = mutate >= 3 ? 1 << ctz(ntx2) : 0; // And for the the third mutation. + if (duplicate3 >= ntx2) break; + int ntx3 = ntx2 + duplicate3; + // Build a block with ntx different transactions. + CBlock block; + block.vtx.resize(ntx); + for (int j = 0; j < ntx; j++) { + CMutableTransaction mtx; + mtx.nLockTime = j; + block.vtx[j] = mtx; + } + // Compute the root of the block before mutating it. + bool unmutatedMutated = false; + uint256 unmutatedRoot = BlockMerkleRoot(block, &unmutatedMutated); + BOOST_CHECK(unmutatedMutated == false); + // Optionally mutate by duplicating the last transactions, resulting in the same merkle root. + block.vtx.resize(ntx3); + for (int j = 0; j < duplicate1; j++) { + block.vtx[ntx + j] = block.vtx[ntx + j - duplicate1]; + } + for (int j = 0; j < duplicate2; j++) { + block.vtx[ntx1 + j] = block.vtx[ntx1 + j - duplicate2]; + } + for (int j = 0; j < duplicate3; j++) { + block.vtx[ntx2 + j] = block.vtx[ntx2 + j - duplicate3]; + } + // Compute the merkle root and merkle tree using the old mechanism. + bool oldMutated = false; + std::vector merkleTree; + uint256 oldRoot = BlockBuildMerkleTree(block, &oldMutated, merkleTree); + // Compute the merkle root using the new mechanism. + bool newMutated = false; + uint256 newRoot = BlockMerkleRoot(block, &newMutated); + BOOST_CHECK(oldRoot == newRoot); + BOOST_CHECK(newRoot == unmutatedRoot); + BOOST_CHECK((newRoot == uint256()) == (ntx == 0)); + BOOST_CHECK(oldMutated == newMutated); + BOOST_CHECK(newMutated == !!mutate); + // If no mutation was done (once for every ntx value), try up to 16 branches. + if (mutate == 0) { + for (int loop = 0; loop < std::min(ntx, 16); loop++) { + // If ntx <= 16, try all branches. Otherise, try 16 random ones. + int mtx = loop; + if (ntx > 16) { + mtx = InsecureRandRange(ntx); + } + std::vector newBranch = BlockMerkleBranch(block, mtx); + std::vector oldBranch = BlockGetMerkleBranch(block, merkleTree, mtx); + BOOST_CHECK(oldBranch == newBranch); + BOOST_CHECK(ComputeMerkleRootFromBranch(block.vtx[mtx].GetHash(), newBranch, mtx) == oldRoot); + } + } + } + } +} + +BOOST_AUTO_TEST_SUITE_END() \ No newline at end of file diff --git a/src/test/miner_tests.cpp b/src/test/miner_tests.cpp index 8be01f73d6f8..edaeced37652 100644 --- a/src/test/miner_tests.cpp +++ b/src/test/miner_tests.cpp @@ -4,6 +4,7 @@ // file COPYING or http://www.opensource.org/licenses/mit-license.php. #include "init.h" +#include "consensus/merkle.h" #include "main.h" #include "miner.h" #include "pubkey.h" @@ -82,7 +83,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->ComputeMerkleRoot(); + pblock->hashMerkleRoot = BlockMerkleRoot(*pblock); pblock->nNonce = blockinfo[i].nonce; CValidationState state; BOOST_CHECK(ProcessNewBlock(state, NULL, pblock)); diff --git a/src/test/pmt_tests.cpp b/src/test/pmt_tests.cpp index 75ee7054eedf..7a1d9b40dfa0 100644 --- a/src/test/pmt_tests.cpp +++ b/src/test/pmt_tests.cpp @@ -8,6 +8,7 @@ #include "streams.h" #include "uint256.h" #include "version.h" +#include "consensus/merkle.h" #include "test/test_pivx.h" #include @@ -45,7 +46,7 @@ BOOST_AUTO_TEST_CASE(pmt_test1) } // calculate actual merkle root and height - uint256 merkleRoot1 = block.ComputeMerkleRoot(); + uint256 merkleRoot1 = BlockMerkleRoot(block); std::vector vTxid(nTx, 0); for (unsigned int j=0; j Date: Wed, 17 Jul 2019 17:23:08 +0200 Subject: [PATCH 03/18] [Refactor] Move wallet functions out of header --- src/wallet/wallet.cpp | 310 ++++++++++++++++++++++++++++++++++++++++ src/wallet/wallet.h | 325 +++++------------------------------------- 2 files changed, 345 insertions(+), 290 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d4c4325deac9..69f271c18208 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -5118,3 +5118,313 @@ void CWallet::PrecomputeSpends() }*/ } +CWallet::CWallet() +{ + SetNull(); +} + +CWallet::CWallet(std::string strWalletFileIn) +{ + SetNull(); + + strWalletFile = strWalletFileIn; + fFileBacked = true; +} + +CWallet::~CWallet() +{ + delete pwalletdbEncryption; +} + +void CWallet::SetNull() +{ + nWalletVersion = FEATURE_BASE; + nWalletMaxVersion = FEATURE_BASE; + fFileBacked = false; + nMasterKeyMaxID = 0; + pwalletdbEncryption = NULL; + nOrderPosNext = 0; + nNextResend = 0; + nLastResend = 0; + nTimeFirstKey = 0; + fWalletUnlockAnonymizeOnly = false; + fBackupMints = false; + + // Stake Settings + nHashDrift = 45; + nStakeSplitThreshold = STAKE_SPLIT_THRESHOLD; + nHashInterval = 22; + nStakeSetUpdateTime = 300; // 5 minutes + + //MultiSend + vMultiSend.clear(); + fMultiSendStake = false; + fMultiSendMasternodeReward = false; + fMultiSendNotify = false; + strMultiSendChangeAddress = ""; + nLastMultiSendHeight = 0; + vDisabledAddresses.clear(); + + //Auto Combine Dust + fCombineDust = false; + nAutoCombineThreshold = 0; +} + +int CWallet::getZeromintPercentage() +{ + return nZeromintPercentage; +} + +void CWallet::setZWallet(CzPIVWallet* zwallet) +{ + zwalletMain = zwallet; + zpivTracker = std::unique_ptr(new CzPIVTracker(strWalletFile)); +} + +CzPIVWallet* CWallet::getZWallet() +{ + return zwalletMain; +} + +bool CWallet::isZeromintEnabled() +{ + return fEnableZeromint || fEnableAutoConvert; +} + +void CWallet::setZPivAutoBackups(bool fEnabled) +{ + fBackupMints = fEnabled; +} + +bool CWallet::isMultiSendEnabled() +{ + return fMultiSendMasternodeReward || fMultiSendStake; +} + +void CWallet::setMultiSendDisabled() +{ + fMultiSendMasternodeReward = false; + fMultiSendStake = false; +} + +bool CWallet::CanSupportFeature(enum WalletFeature wf) +{ + AssertLockHeld(cs_wallet); + return nWalletMaxVersion >= wf; +} + +bool CWallet::LoadMinVersion(int nVersion) +{ + AssertLockHeld(cs_wallet); + nWalletVersion = nVersion; + nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); + return true; +} + +isminetype CWallet::IsMine(const CTxOut& txout) const +{ + return ::IsMine(*this, txout.scriptPubKey); +} + +CAmount CWallet::GetCredit(const CTxOut& txout, const isminefilter& filter) const +{ + if (!MoneyRange(txout.nValue)) + throw std::runtime_error("CWallet::GetCredit() : value out of range"); + return ((IsMine(txout) & filter) ? txout.nValue : 0); +} + +CAmount CWallet::GetChange(const CTxOut& txout) const +{ + if (!MoneyRange(txout.nValue)) + throw std::runtime_error("CWallet::GetChange() : value out of range"); + return (IsChange(txout) ? txout.nValue : 0); +} + +bool CWallet::IsMine(const CTransaction& tx) const +{ + for (const CTxOut& txout : tx.vout) + if (IsMine(txout)) + return true; + return false; +} + +bool CWallet::IsFromMe(const CTransaction& tx) const +{ + return (GetDebit(tx, ISMINE_ALL) > 0); +} + +CAmount CWallet::GetDebit(const CTransaction& tx, const isminefilter& filter) const +{ + CAmount nDebit = 0; + for (const CTxIn& txin : tx.vin) { + nDebit += GetDebit(txin, filter); + if (!MoneyRange(nDebit)) + throw std::runtime_error("CWallet::GetDebit() : value out of range"); + } + return nDebit; +} + +CAmount CWallet::GetCredit(const CTransaction& tx, const isminefilter& filter) const +{ + CAmount nCredit = 0; + for (const CTxOut& txout : tx.vout) { + nCredit += GetCredit(txout, filter); + if (!MoneyRange(nCredit)) + throw std::runtime_error("CWallet::GetCredit() : value out of range"); + } + return nCredit; +} + +CAmount CWallet::GetChange(const CTransaction& tx) const +{ + CAmount nChange = 0; + for (const CTxOut& txout : tx.vout) { + nChange += GetChange(txout); + if (!MoneyRange(nChange)) + throw std::runtime_error("CWallet::GetChange() : value out of range"); + } + return nChange; +} + +void CWallet::Inventory(const uint256& hash) +{ + { + LOCK(cs_wallet); + std::map::iterator mi = mapRequestCount.find(hash); + if (mi != mapRequestCount.end()) + (*mi).second++; + } +} + +unsigned int CWallet::GetKeyPoolSize() +{ + AssertLockHeld(cs_wallet); // setKeyPool + return setKeyPool.size(); +} + +int CWallet::GetVersion() +{ + LOCK(cs_wallet); + return nWalletVersion; +} + +CWalletTx::CWalletTx() +{ + Init(NULL); +} + +CWalletTx::CWalletTx(const CWallet* pwalletIn) +{ + Init(pwalletIn); +} + +CWalletTx::CWalletTx(const CWallet* pwalletIn, const CMerkleTx& txIn) : CMerkleTx(txIn) +{ + Init(pwalletIn); +} + +CWalletTx::CWalletTx(const CWallet* pwalletIn, const CTransaction& txIn) : CMerkleTx(txIn) +{ + Init(pwalletIn); +} + +void CWalletTx::Init(const CWallet* pwalletIn) +{ + pwallet = pwalletIn; + mapValue.clear(); + vOrderForm.clear(); + fTimeReceivedIsTxTime = false; + nTimeReceived = 0; + nTimeSmart = 0; + fFromMe = false; + strFromAccount.clear(); + fDebitCached = false; + fCreditCached = false; + fImmatureCreditCached = false; + fAvailableCreditCached = false; + fAnonymizableCreditCached = false; + fAnonymizedCreditCached = false; + fDenomUnconfCreditCached = false; + fDenomConfCreditCached = false; + fWatchDebitCached = false; + fWatchCreditCached = false; + fImmatureWatchCreditCached = false; + fAvailableWatchCreditCached = false; + fChangeCached = false; + nDebitCached = 0; + nCreditCached = 0; + nImmatureCreditCached = 0; + nAvailableCreditCached = 0; + nAnonymizableCreditCached = 0; + nAnonymizedCreditCached = 0; + nDenomUnconfCreditCached = 0; + nDenomConfCreditCached = 0; + nWatchDebitCached = 0; + nWatchCreditCached = 0; + nAvailableWatchCreditCached = 0; + nImmatureWatchCreditCached = 0; + nChangeCached = 0; + nOrderPos = -1; +} + +bool CWalletTx::IsTrusted() const +{ + // Quick answer in most cases + if (!IsFinalTx(*this)) + return false; + int nDepth = GetDepthInMainChain(); + if (nDepth >= 1) + return true; + if (nDepth < 0) + return false; + if (!bSpendZeroConfChange || !IsFromMe(ISMINE_ALL)) // using wtx's cached debit + return false; + + // Trusted if all inputs are from us and are in the mempool: + for (const CTxIn& txin : vin) { + // Transactions not sent by us: not trusted + const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); + if (parent == NULL) + return false; + const CTxOut& parentOut = parent->vout[txin.prevout.n]; + if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE) + return false; + } + return true; +} + +void CWalletTx::MarkDirty() +{ + fCreditCached = false; + fAvailableCreditCached = false; + fAnonymizableCreditCached = false; + fAnonymizedCreditCached = false; + fDenomUnconfCreditCached = false; + fDenomConfCreditCached = false; + fWatchDebitCached = false; + fWatchCreditCached = false; + fAvailableWatchCreditCached = false; + fImmatureWatchCreditCached = false; + fDebitCached = false; + fChangeCached = false; +} + +void CWalletTx::BindWallet(CWallet* pwalletIn) +{ + pwallet = pwalletIn; + MarkDirty(); +} + +CAmount CWalletTx::GetChange() const +{ + if (fChangeCached) + return nChangeCached; + nChangeCached = pwallet->GetChange(*this); + fChangeCached = true; + return nChangeCached; +} + +bool CWalletTx::IsFromMe(const isminefilter& filter) const +{ + return (GetDebit(filter) > 0); +} diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f17cf96d6fa1..3ce1980f1ea9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -306,91 +306,17 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool fCombineDust; CAmount nAutoCombineThreshold; - CWallet() - { - SetNull(); - } - - CWallet(std::string strWalletFileIn) - { - SetNull(); - - strWalletFile = strWalletFileIn; - fFileBacked = true; - } - - ~CWallet() - { - delete pwalletdbEncryption; - } - - void SetNull() - { - nWalletVersion = FEATURE_BASE; - nWalletMaxVersion = FEATURE_BASE; - fFileBacked = false; - nMasterKeyMaxID = 0; - pwalletdbEncryption = NULL; - nOrderPosNext = 0; - nNextResend = 0; - nLastResend = 0; - nTimeFirstKey = 0; - fWalletUnlockAnonymizeOnly = false; - fBackupMints = false; - - // Stake Settings - nHashDrift = 45; - nStakeSplitThreshold = STAKE_SPLIT_THRESHOLD; - nHashInterval = 22; - nStakeSetUpdateTime = 300; // 5 minutes - - //MultiSend - vMultiSend.clear(); - fMultiSendStake = false; - fMultiSendMasternodeReward = false; - fMultiSendNotify = false; - strMultiSendChangeAddress = ""; - nLastMultiSendHeight = 0; - vDisabledAddresses.clear(); - - //Auto Combine Dust - fCombineDust = false; - nAutoCombineThreshold = 0; - } - - int getZeromintPercentage() - { - return nZeromintPercentage; - } - - void setZWallet(CzPIVWallet* zwallet) - { - zwalletMain = zwallet; - zpivTracker = std::unique_ptr(new CzPIVTracker(strWalletFile)); - } - - CzPIVWallet* getZWallet() { return zwalletMain; } - - bool isZeromintEnabled() - { - return fEnableZeromint || fEnableAutoConvert; - } - - void setZPivAutoBackups(bool fEnabled) - { - fBackupMints = fEnabled; - } - - bool isMultiSendEnabled() - { - return fMultiSendMasternodeReward || fMultiSendStake; - } - - void setMultiSendDisabled() - { - fMultiSendMasternodeReward = false; - fMultiSendStake = false; - } + CWallet(); + CWallet(std::string strWalletFileIn); + ~CWallet(); + void SetNull(); + int getZeromintPercentage(); + void setZWallet(CzPIVWallet* zwallet); + CzPIVWallet* getZWallet(); + bool isZeromintEnabled(); + void setZPivAutoBackups(bool fEnabled); + bool isMultiSendEnabled(); + void setMultiSendDisabled(); std::map mapWallet; std::list laccentries; @@ -417,17 +343,13 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void PrecomputeSpends(); //! check whether we are allowed to upgrade (or already support) to the named feature - bool CanSupportFeature(enum WalletFeature wf) - { - AssertLockHeld(cs_wallet); - return nWalletMaxVersion >= wf; - } + bool CanSupportFeature(enum WalletFeature wf); void AvailableCoins(std::vector& vCoins, bool fOnlyConfirmed = true, const CCoinControl* coinControl = NULL, bool fIncludeZeroValue = false, AvailableCoinsType nCoinType = ALL_COINS, bool fUseIX = false, int nWatchonlyConfig = 1) const; std::map > AvailableCoinsByAddress(bool fConfirmed = true, CAmount maxCoinValue = 0); bool SelectCoinsMinConf(const CAmount& nTargetValue, int nConfMine, int nConfTheirs, std::vector vCoins, std::set >& setCoinsRet, CAmount& nValueRet) const; - /// Get 1000DASH output and keys which can be used for the Masternode + /// Get 10000 PIV output and keys which can be used for the Masternode bool GetMasternodeVinAndKeys(CTxIn& txinRet, CPubKey& pubKeyRet, CKey& keyRet, std::string strTxHash = "", std::string strOutputIndex = ""); /// Extract txin information and keys from output bool GetVinAndKeysFromOutput(COutput out, CTxIn& txinRet, CPubKey& pubKeyRet, CKey& keyRet); @@ -454,13 +376,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! Load metadata (used by LoadWallet) bool LoadKeyMetadata(const CPubKey& pubkey, const CKeyMetadata& metadata); - bool LoadMinVersion(int nVersion) - { - AssertLockHeld(cs_wallet); - nWalletVersion = nVersion; - nWalletMaxVersion = std::max(nWalletMaxVersion, nVersion); - return true; - } + bool LoadMinVersion(int nVersion); //! Adds an encrypted key to the store, and saves it to disk. bool AddCryptedKey(const CPubKey& vchPubKey, const std::vector& vchCryptedSecret); @@ -571,67 +487,18 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface isminetype IsMine(const CTxIn& txin) const; CAmount GetDebit(const CTxIn& txin, const isminefilter& filter) const; - isminetype IsMine(const CTxOut& txout) const - { - return ::IsMine(*this, txout.scriptPubKey); - } + isminetype IsMine(const CTxOut& txout) const; bool IsMyZerocoinSpend(const CBigNum& bnSerial) const; bool IsMyMint(const CBigNum& bnValue) const; - CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const - { - if (!MoneyRange(txout.nValue)) - throw std::runtime_error("CWallet::GetCredit() : value out of range"); - return ((IsMine(txout) & filter) ? txout.nValue : 0); - } + CAmount GetCredit(const CTxOut& txout, const isminefilter& filter) const; bool IsChange(const CTxOut& txout) const; - CAmount GetChange(const CTxOut& txout) const - { - if (!MoneyRange(txout.nValue)) - throw std::runtime_error("CWallet::GetChange() : value out of range"); - return (IsChange(txout) ? txout.nValue : 0); - } - bool IsMine(const CTransaction& tx) const - { - for (const CTxOut& txout : tx.vout) - if (IsMine(txout)) - return true; - return false; - } + CAmount GetChange(const CTxOut& txout) const; + bool IsMine(const CTransaction& tx) const; /** should probably be renamed to IsRelevantToMe */ - bool IsFromMe(const CTransaction& tx) const - { - return (GetDebit(tx, ISMINE_ALL) > 0); - } - CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const - { - CAmount nDebit = 0; - for (const CTxIn& txin : tx.vin) { - nDebit += GetDebit(txin, filter); - if (!MoneyRange(nDebit)) - throw std::runtime_error("CWallet::GetDebit() : value out of range"); - } - return nDebit; - } - CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const - { - CAmount nCredit = 0; - for (const CTxOut& txout : tx.vout) { - nCredit += GetCredit(txout, filter); - if (!MoneyRange(nCredit)) - throw std::runtime_error("CWallet::GetCredit() : value out of range"); - } - return nCredit; - } - CAmount GetChange(const CTransaction& tx) const - { - CAmount nChange = 0; - for (const CTxOut& txout : tx.vout) { - nChange += GetChange(txout); - if (!MoneyRange(nChange)) - throw std::runtime_error("CWallet::GetChange() : value out of range"); - } - return nChange; - } + bool IsFromMe(const CTransaction& tx) const; + CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const; + CAmount GetCredit(const CTransaction& tx, const isminefilter& filter) const; + CAmount GetChange(const CTransaction& tx) const; void SetBestChain(const CBlockLocator& loc); DBErrors LoadWallet(bool& fFirstRunRet); @@ -643,21 +510,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool UpdatedTransaction(const uint256& hashTx); - void Inventory(const uint256& hash) - { - { - LOCK(cs_wallet); - std::map::iterator mi = mapRequestCount.find(hash); - if (mi != mapRequestCount.end()) - (*mi).second++; - } - } + void Inventory(const uint256& hash); - unsigned int GetKeyPoolSize() - { - AssertLockHeld(cs_wallet); // setKeyPool - return setKeyPool.size(); - } + unsigned int GetKeyPoolSize(); bool SetDefaultKey(const CPubKey& vchPubKey); @@ -668,11 +523,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool SetMaxVersion(int nVersion); //! get the current wallet format (the oldest client version guaranteed to understand this wallet) - int GetVersion() - { - LOCK(cs_wallet); - return nWalletVersion; - } + int GetVersion(); //! Get wallet transactions that conflict with given transaction (spend same outputs) std::set GetConflicts(const uint256& txid) const; @@ -870,64 +721,11 @@ class CWalletTx : public CMerkleTx mutable CAmount nAvailableWatchCreditCached; mutable CAmount nChangeCached; - CWalletTx() - { - Init(NULL); - } - - CWalletTx(const CWallet* pwalletIn) - { - Init(pwalletIn); - } - - CWalletTx(const CWallet* pwalletIn, const CMerkleTx& txIn) : CMerkleTx(txIn) - { - Init(pwalletIn); - } - - CWalletTx(const CWallet* pwalletIn, const CTransaction& txIn) : CMerkleTx(txIn) - { - Init(pwalletIn); - } - - void Init(const CWallet* pwalletIn) - { - pwallet = pwalletIn; - mapValue.clear(); - vOrderForm.clear(); - fTimeReceivedIsTxTime = false; - nTimeReceived = 0; - nTimeSmart = 0; - fFromMe = false; - strFromAccount.clear(); - fDebitCached = false; - fCreditCached = false; - fImmatureCreditCached = false; - fAvailableCreditCached = false; - fAnonymizableCreditCached = false; - fAnonymizedCreditCached = false; - fDenomUnconfCreditCached = false; - fDenomConfCreditCached = false; - fWatchDebitCached = false; - fWatchCreditCached = false; - fImmatureWatchCreditCached = false; - fAvailableWatchCreditCached = false; - fChangeCached = false; - nDebitCached = 0; - nCreditCached = 0; - nImmatureCreditCached = 0; - nAvailableCreditCached = 0; - nAnonymizableCreditCached = 0; - nAnonymizedCreditCached = 0; - nDenomUnconfCreditCached = 0; - nDenomConfCreditCached = 0; - nWatchDebitCached = 0; - nWatchCreditCached = 0; - nAvailableWatchCreditCached = 0; - nImmatureWatchCreditCached = 0; - nChangeCached = 0; - nOrderPos = -1; - } + CWalletTx(); + CWalletTx(const CWallet* pwalletIn); + CWalletTx(const CWallet* pwalletIn, const CMerkleTx& txIn); + CWalletTx(const CWallet* pwalletIn, const CTransaction& txIn); + void Init(const CWallet* pwalletIn); ADD_SERIALIZE_METHODS; @@ -973,27 +771,9 @@ class CWalletTx : public CMerkleTx } //! make sure balances are recalculated - void MarkDirty() - { - fCreditCached = false; - fAvailableCreditCached = false; - fAnonymizableCreditCached = false; - fAnonymizedCreditCached = false; - fDenomUnconfCreditCached = false; - fDenomConfCreditCached = false; - fWatchDebitCached = false; - fWatchCreditCached = false; - fAvailableWatchCreditCached = false; - fImmatureWatchCreditCached = false; - fDebitCached = false; - fChangeCached = false; - } + void MarkDirty(); - void BindWallet(CWallet* pwalletIn) - { - pwallet = pwalletIn; - MarkDirty(); - } + void BindWallet(CWallet* pwalletIn); //! filter decides which addresses will count towards the debit CAmount GetDebit(const isminefilter& filter) const; @@ -1007,15 +787,7 @@ class CWalletTx : public CMerkleTx CAmount GetImmatureWatchOnlyCredit(const bool& fUseCache = true) const; CAmount GetAvailableWatchOnlyCredit(const bool& fUseCache = true) const; CAmount GetLockedWatchOnlyCredit() const; - - CAmount GetChange() const - { - if (fChangeCached) - return nChangeCached; - nChangeCached = pwallet->GetChange(*this); - fChangeCached = true; - return nChangeCached; - } + CAmount GetChange() const; void GetAmounts(std::list& listReceived, std::list& listSent, @@ -1025,38 +797,11 @@ class CWalletTx : public CMerkleTx void GetAccountAmounts(const std::string& strAccount, CAmount& nReceived, CAmount& nSent, CAmount& nFee, const isminefilter& filter) const; - bool IsFromMe(const isminefilter& filter) const - { - return (GetDebit(filter) > 0); - } + bool IsFromMe(const isminefilter& filter) const; bool InMempool() const; - bool IsTrusted() const - { - // Quick answer in most cases - if (!IsFinalTx(*this)) - return false; - int nDepth = GetDepthInMainChain(); - if (nDepth >= 1) - return true; - if (nDepth < 0) - return false; - if (!bSpendZeroConfChange || !IsFromMe(ISMINE_ALL)) // using wtx's cached debit - return false; - - // Trusted if all inputs are from us and are in the mempool: - for (const CTxIn& txin : vin) { - // Transactions not sent by us: not trusted - const CWalletTx* parent = pwallet->GetWalletTx(txin.prevout.hash); - if (parent == NULL) - return false; - const CTxOut& parentOut = parent->vout[txin.prevout.n]; - if (pwallet->IsMine(parentOut) != ISMINE_SPENDABLE) - return false; - } - return true; - } + bool IsTrusted() const; bool WriteToDisk(); From 7ccb2b585634609e52615767c1c74ed577ae2a22 Mon Sep 17 00:00:00 2001 From: warrows Date: Fri, 19 Jul 2019 15:57:41 +0200 Subject: [PATCH 04/18] [Wallet] Do not flush the wallet in AddToWalletIfInvolvingMe(..) Backport of https://github.com/bitcoin/bitcoin/pull/4805 ( commit 44bc988e7becb492a78ed92ea1052f4789012534 ) --- src/init.cpp | 3 ++- src/main.cpp | 4 ++-- src/test/accounting_tests.cpp | 6 +++--- src/wallet/db.cpp | 6 ++++-- src/wallet/db.h | 3 ++- src/wallet/wallet.cpp | 25 ++++++++++++++++--------- src/wallet/wallet.h | 4 ++-- src/wallet/walletdb.cpp | 2 +- src/wallet/walletdb.h | 2 +- src/zpiv/zpivwallet.cpp | 5 +++-- 10 files changed, 36 insertions(+), 24 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index f7b02d62dd76..cd32c219978b 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1756,6 +1756,7 @@ bool AppInit2() // Restore wallet transaction metadata after -zapwallettxes=1 if (GetBoolArg("-zapwallettxes", false) && GetArg("-zapwallettxes", "1") != "2") { + CWalletDB walletdb(strWalletFile); for (const CWalletTx& wtxOld : vWtx) { uint256 hash = wtxOld.GetHash(); std::map::iterator mi = pwalletMain->mapWallet.find(hash); @@ -1769,7 +1770,7 @@ bool AppInit2() copyTo->fFromMe = copyFrom->fFromMe; copyTo->strFromAccount = copyFrom->strFromAccount; copyTo->nOrderPos = copyFrom->nOrderPos; - copyTo->WriteToDisk(); + copyTo->WriteToDisk(&walletdb); } } } diff --git a/src/main.cpp b/src/main.cpp index d6d980319598..42b5755de9f6 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -3117,7 +3117,7 @@ bool UpdateZPIVSupply(const CBlock& block, CBlockIndex* pindex, bool fJustCheck) CWalletTx wtx(pwalletMain, tx); wtx.nTimeReceived = block.GetBlockTime(); wtx.SetMerkleBranch(block); - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, nullptr); setAddedToWallet.insert(txid); } } @@ -3467,7 +3467,7 @@ bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pin CWalletTx wtx(pwalletMain, tx); wtx.nTimeReceived = pindex->GetBlockTime(); wtx.SetMerkleBranch(block); - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, nullptr); setAddedTx.insert(pSpend.second); } } diff --git a/src/test/accounting_tests.cpp b/src/test/accounting_tests.cpp index cc9d361f4a82..b9cdcf89bcb6 100644 --- a/src/test/accounting_tests.cpp +++ b/src/test/accounting_tests.cpp @@ -48,7 +48,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) pwalletMain->AddAccountingEntry(ae, walletdb); wtx.mapValue["comment"] = "z"; - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[0]->nTimeReceived = (unsigned int)1333333335; vpwtx[0]->nOrderPos = -1; @@ -90,7 +90,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) --tx.nLockTime; // Just to change the hash :) *static_cast(&wtx) = CTransaction(tx); } - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; @@ -100,7 +100,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) --tx.nLockTime; // Just to change the hash :) *static_cast(&wtx) = CTransaction(tx); } - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; vpwtx[2]->nOrderPos = -1; diff --git a/src/wallet/db.cpp b/src/wallet/db.cpp index 056d2ab94018..1821a72fb215 100644 --- a/src/wallet/db.cpp +++ b/src/wallet/db.cpp @@ -223,10 +223,11 @@ void CDBEnv::CheckpointLSN(const std::string& strFile) } -CDB::CDB(const std::string& strFilename, const char* pszMode) : pdb(NULL), activeTxn(NULL) +CDB::CDB(const std::string& strFilename, const char* pszMode, bool fFlushOnCloseIn) : pdb(NULL), activeTxn(NULL) { int ret; fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); + fFlushOnClose = fFlushOnCloseIn; if (strFilename.empty()) return; @@ -304,7 +305,8 @@ void CDB::Close() activeTxn = NULL; pdb = NULL; - Flush(); + if (fFlushOnClose) + Flush(); { LOCK(bitdb.cs_db); diff --git a/src/wallet/db.h b/src/wallet/db.h index 756150513c63..2a0c4602c8d1 100644 --- a/src/wallet/db.h +++ b/src/wallet/db.h @@ -103,8 +103,9 @@ class CDB std::string strFile; DbTxn* activeTxn; bool fReadOnly; + bool fFlushOnClose; - explicit CDB(const std::string& strFilename, const char* pszMode = "r+"); + explicit CDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnCloseIn=true); ~CDB() { Close(); } public: diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 69f271c18208..292e025ae2f9 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -667,7 +667,7 @@ void CWallet::MarkDirty() } } -bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) +bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb) { uint256 hash = wtxIn.GetHash(); @@ -687,7 +687,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) if (fInsertedNew) { if (!wtx.nTimeReceived) wtx.nTimeReceived = GetAdjustedTime(); - wtx.nOrderPos = IncOrderPosNext(); + wtx.nOrderPos = IncOrderPosNext(pwalletdb); wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0))); wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash); @@ -715,7 +715,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet) // Write to disk if (fInsertedNew || fUpdated) - if (!wtx.WriteToDisk()) + if (!wtx.WriteToDisk(pwalletdb)) return false; // Break debit/credit balance caches: @@ -751,7 +751,11 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl // Get merkle branch if transaction was found in a block if (pblock) wtx.SetMerkleBranch(*pblock); - return AddToWallet(wtx); + // Do not flush the wallet here for performance reasons + // this is safe, as in case of a crash, we rescan the necessary blocks on startup through our SetBestChain-mechanism + CWalletDB walletdb(strWalletFile, "r+", false); + + return AddToWallet(wtx, false, &walletdb); } } return false; @@ -1254,8 +1258,10 @@ void CWalletTx::GetAccountAmounts(const std::string& strAccount, CAmount& nRecei } -bool CWalletTx::WriteToDisk() +bool CWalletTx::WriteToDisk(CWalletDB *pwalletdb) { + if (pwalletdb) + return pwalletdb->WriteTx(GetHash(), *this); return CWalletDB(pwallet->strWalletFile).WriteTx(GetHash(), *this); } @@ -1305,6 +1311,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b if (fCheckZPIV && pindex->nHeight >= Params().Zerocoin_StartHeight()) { std::list listMints; BlockToZerocoinMintList(block, listMints, true); + CWalletDB walletdb(strWalletFile); for (auto& m : listMints) { if (IsMyMint(m.GetValue())) { @@ -1320,7 +1327,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b CWalletTx wtx(pwalletMain, tx); wtx.nTimeReceived = block.GetBlockTime(); wtx.SetMerkleBranch(block); - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); setAddedToWallet.insert(txid); } } @@ -1340,7 +1347,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b wtx.SetMerkleBranch(blockSpend); wtx.nTimeReceived = pindexSpend->nTime; - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); setAddedToWallet.emplace(txidSpend); } } @@ -2541,14 +2548,14 @@ bool CWallet::CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey, std: // This is only to keep the database open to defeat the auto-flush for the // duration of this scope. This is the only place where this optimization // maybe makes sense; please don't do it anywhere else. - CWalletDB* pwalletdb = fFileBacked ? new CWalletDB(strWalletFile, "r") : NULL; + CWalletDB* pwalletdb = fFileBacked ? new CWalletDB(strWalletFile, "r+") : NULL; // Take key pair from key pool so it won't be used again reservekey.KeepKey(); // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. - AddToWallet(wtxNew); + AddToWallet(wtxNew, false, pwalletdb); // Notify that old coins are spent if (!wtxNew.HasZerocoinSpendInputs()) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 3ce1980f1ea9..4bcb9acab09b 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -418,7 +418,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface int64_t IncOrderPosNext(CWalletDB* pwalletdb = NULL); void MarkDirty(); - bool AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet = false); + bool AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletDB* pwalletdb); void SyncTransaction(const CTransaction& tx, const CBlock* pblock); bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate); void EraseFromWallet(const uint256& hash); @@ -803,7 +803,7 @@ class CWalletTx : public CMerkleTx bool IsTrusted() const; - bool WriteToDisk(); + bool WriteToDisk(CWalletDB *pwalletdb); int64_t GetTxTime() const; int64_t GetComputedTxTime() const; diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 12fc478c7827..d536f70692a1 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -515,7 +515,7 @@ bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, CW if (wtx.nOrderPos == -1) wss.fAnyUnordered = true; - pwallet->AddToWallet(wtx, true); + pwallet->AddToWallet(wtx, true, nullptr); } else if (strType == "acentry") { std::string strAccount; ssKey >> strAccount; diff --git a/src/wallet/walletdb.h b/src/wallet/walletdb.h index 6ccc76d69f7d..611d9cc15fbd 100644 --- a/src/wallet/walletdb.h +++ b/src/wallet/walletdb.h @@ -85,7 +85,7 @@ class CKeyMetadata class CWalletDB : public CDB { public: - CWalletDB(const std::string& strFilename, const char* pszMode = "r+") : CDB(strFilename, pszMode) + CWalletDB(const std::string& strFilename, const char* pszMode = "r+", bool fFlushOnClose = true) : CDB(strFilename, pszMode, fFlushOnClose) { } diff --git a/src/zpiv/zpivwallet.cpp b/src/zpiv/zpivwallet.cpp index 863a833bc142..4937b046427f 100644 --- a/src/zpiv/zpivwallet.cpp +++ b/src/zpiv/zpivwallet.cpp @@ -266,7 +266,7 @@ void CzPIVWallet::SyncWithChain(bool fGenerateMintPool) //Fill out wtx so that a transaction record can be created wtx.nTimeReceived = pindex->GetBlockTime(); - pwalletMain->AddToWallet(wtx); + pwalletMain->AddToWallet(wtx, false, &walletdb); setAddedTx.insert(txHash); } @@ -322,7 +322,8 @@ bool CzPIVWallet::SetMintSeen(const CBigNum& bnValue, const int& nHeight, const wtx.SetMerkleBranch(block); wtx.nTimeReceived = pindex->nTime; - pwalletMain->AddToWallet(wtx); + CWalletDB walletdb(strWalletFile); + pwalletMain->AddToWallet(wtx, false, &walletdb); } // Add to zpivTracker which also adds to database From 6a50e03a3e994bf11f603572ef94d0aa2cfea059 Mon Sep 17 00:00:00 2001 From: warrows Date: Wed, 17 Jul 2019 17:43:02 +0200 Subject: [PATCH 05/18] [Wallet] Keep track of explicit wallet conflicts instead of using mempool Backport of https://github.com/bitcoin/bitcoin/pull/7105 (9ac63d6d3056600c1b784da0e6b98f679fa98b6e) --- src/wallet/rpcwallet.cpp | 5 ++ src/wallet/wallet.cpp | 84 ++++++++++++++++++++++- src/wallet/wallet.h | 10 ++- test/functional/wallet_txn_clone.py | 2 +- test/functional/wallet_txn_doublespend.py | 4 +- 5 files changed, 98 insertions(+), 7 deletions(-) diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 49339b899524..d121da6b08a8 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -57,6 +57,8 @@ void WalletTxToJSON(const CWalletTx& wtx, UniValue& entry) entry.push_back(Pair("blockhash", wtx.hashBlock.GetHex())); entry.push_back(Pair("blockindex", wtx.nIndex)); entry.push_back(Pair("blocktime", mapBlockIndex[wtx.hashBlock]->GetBlockTime())); + } else { + entry.push_back(Pair("trusted", wtx.IsTrusted())); } uint256 hash = wtx.GetHash(); entry.push_back(Pair("txid", hash.GetHex())); @@ -1355,6 +1357,9 @@ UniValue listtransactions(const UniValue& params, bool fHelp) " \"confirmations\": n, (numeric) The number of confirmations for the transaction. Available for 'send' and \n" " 'receive' category of transactions.\n" " \"bcconfirmations\": n, (numeric) The number of blockchain confirmations for the transaction. Available for 'send'\n" + " 'receive' category of transactions. Negative confirmations indicate the\n" + " transation conflicts with the block chain\n" + " \"trusted\": xxx (bool) Whether we consider the outputs of this unconfirmed transaction safe to spend.\n" " and 'receive' category of transactions.\n" " \"blockhash\": \"hashvalue\", (string) The block hash containing the transaction. Available for 'send' and 'receive'\n" " category of transactions.\n" diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 292e025ae2f9..0e8395ecc96c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -691,6 +691,14 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0))); wtx.nTimeSmart = ComputeTimeSmart(wtx); AddToSpends(hash); + for (const CTxIn& txin : wtx.vin) { + if (mapWallet.count(txin.prevout.hash)) { + CWalletTx& prevtx = mapWallet[txin.prevout.hash]; + if (prevtx.nIndex == -1 && !prevtx.hashBlock.IsNull()) { + MarkConflicted(prevtx.hashBlock, wtx.GetHash()); + } + } + } } bool fUpdated = false; @@ -744,6 +752,20 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl { { AssertLockHeld(cs_wallet); + + if (pblock) { + for (const CTxIn& txin : tx.vin) { + std::pair range = mapTxSpends.equal_range(txin.prevout); + while (range.first != range.second) { + if (range.first->second != tx.GetHash()) { + LogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), pblock->GetHash().ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); + MarkConflicted(pblock->GetHash(), range.first->second); + } + range.first++; + } + } + } + bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; if (fExisted || IsMine(tx) || IsFromMe(tx)) { @@ -761,6 +783,53 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl return false; } +void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) +{ + LOCK2(cs_main, cs_wallet); + + CBlockIndex* pindex; + assert(mapBlockIndex.count(hashBlock)); + pindex = mapBlockIndex[hashBlock]; + int conflictconfirms = 0; + if (chainActive.Contains(pindex)) { + conflictconfirms = -(chainActive.Height() - pindex->nHeight + 1); + } + assert(conflictconfirms < 0); + + // Do not flush the wallet here for performance reasons + CWalletDB walletdb(strWalletFile, "r+", false); + + std::deque todo; + std::set done; + + todo.push_back(hashTx); + + while (!todo.empty()) { + uint256 now = todo.front(); + todo.pop_front(); + done.insert(now); + assert(mapWallet.count(now)); + CWalletTx& wtx = mapWallet[now]; + int currentconfirm = wtx.GetDepthInMainChain(); + if (conflictconfirms < currentconfirm) { + // Block is 'more conflicted' than current confirm; update. + // Mark transaction as conflicted with this block. + wtx.nIndex = -1; + wtx.hashBlock = hashBlock; + wtx.MarkDirty(); + wtx.WriteToDisk(&walletdb); + // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too + TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0)); + while (iter != mapTxSpends.end() && iter->first.hash == now) { + if (!done.count(iter->second)) { + todo.push_back(iter->second); + } + iter++; + } + } + } +} + void CWallet::SyncTransaction(const CTransaction& tx, const CBlock* pblock) { LOCK2(cs_main, cs_wallet); @@ -1375,7 +1444,7 @@ void CWallet::ReacceptWalletTransactions() int nDepth = wtx.GetDepthInMainChain(); - if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth < 0) { + if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth == 0) { // Try to add to memory pool LOCK(mempool.cs); wtx.AcceptToMemoryPool(false); @@ -2216,6 +2285,7 @@ bool CWallet::CreateTransaction(const std::vector >& //a chance at a free transaction. //But mempool inputs might still be in the mempool, so their age stays 0 int age = pcoin.first->GetDepthInMainChain(); + assert(age >= 0); if (age != 0) age += 1; dPriority += (double)nCredit * age; @@ -3671,7 +3741,7 @@ int CMerkleTx::SetMerkleBranch(const CBlock& block) int CMerkleTx::GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const { - if (hashBlock == 0 || nIndex == -1) + if (hashBlock == 0) return 0; AssertLockHeld(cs_main); @@ -3684,7 +3754,7 @@ int CMerkleTx::GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const return 0; pindexRet = pindex; - return chainActive.Height() - pindex->nHeight + 1; + return ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); } int CMerkleTx::GetDepthInMainChain(const CBlockIndex*& pindexRet, bool enableIX) const @@ -5386,6 +5456,14 @@ bool CWalletTx::IsTrusted() const return false; if (!bSpendZeroConfChange || !IsFromMe(ISMINE_ALL)) // using wtx's cached debit return false; + + // Don't trust unconfirmed transactions from us unless they are in the mempool. + { + LOCK(mempool.cs); + if (!mempool.exists(GetHash())) { + return false; + } + } // Trusted if all inputs are from us and are in the mempool: for (const CTxIn& txin : vin) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 4bcb9acab09b..46ff17323123 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -193,6 +193,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface void AddToSpends(const COutPoint& outpoint, const uint256& wtxid); void AddToSpends(const uint256& wtxid); + /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ + void MarkConflicted(const uint256& hashBlock, const uint256& hashTx); + void SyncMetaData(std::pair); public: @@ -617,6 +620,11 @@ class CMerkleTx : public CTransaction public: uint256 hashBlock; + /* An nIndex == -1 means that hashBlock (in nonzero) refers to the earliest + * block in the chain we know this or any in-wallet dependency conflicts + * with. Older clients interpret nIndex == -1 as unconfirmed for backward + * compatibility. + */ int nIndex; CMerkleTx() @@ -653,7 +661,7 @@ class CMerkleTx : public CTransaction /** * Return depth of transaction in blockchain: - * -1 : not in blockchain, and not in memory pool (conflicted transaction) + * <0 : conflicts with a transaction this deep in the blockchain * 0 : in memory pool, waiting to be included in a block * >=1 : this many blocks deep in the main chain */ diff --git a/test/functional/wallet_txn_clone.py b/test/functional/wallet_txn_clone.py index 9fed3a78a1d8..a30048cd7916 100755 --- a/test/functional/wallet_txn_clone.py +++ b/test/functional/wallet_txn_clone.py @@ -126,7 +126,7 @@ def run_test(self): tx2 = self.nodes[0].gettransaction(txid2) # Verify expected confirmations - assert_equal(tx1["confirmations"], -1) + assert_equal(tx1["confirmations"], -2) assert_equal(tx1_clone["confirmations"], 2) assert_equal(tx2["confirmations"], 1) diff --git a/test/functional/wallet_txn_doublespend.py b/test/functional/wallet_txn_doublespend.py index 50db2e7eb4ee..054486a3af24 100755 --- a/test/functional/wallet_txn_doublespend.py +++ b/test/functional/wallet_txn_doublespend.py @@ -117,8 +117,8 @@ def run_test(self): tx2 = self.nodes[0].gettransaction(txid2) # Both transactions should be conflicted - assert_equal(tx1["bcconfirmations"], -1) - assert_equal(tx2["bcconfirmations"], -1) + assert_equal(tx1["bcconfirmations"], -2) + assert_equal(tx2["bcconfirmations"], -2) # Node0's total balance should be starting balance, plus 100BTC for # two more matured blocks, minus 1240 for the double-spend, plus fees (which are From d0083a8084d7596d885d2655a157d1fedeed8228 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Wed, 6 Jan 2016 17:24:30 -0500 Subject: [PATCH 06/18] Make sure conflicted wallet tx's update balances --- src/wallet/wallet.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0e8395ecc96c..846cdc9702e0 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -826,6 +826,13 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } iter++; } + // If a transaction changes 'conflicted' state, that changes the balance + // available of the outputs it spends. So force those to be recomputed + BOOST_FOREACH(const CTxIn& txin, wtx.vin) + { + if (mapWallet.count(txin.prevout.hash)) + mapWallet[txin.prevout.hash].MarkDirty(); + } } } } From 0e86c3eedc65e915f50765ca614e2090e6ce853c Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 7 Jan 2016 16:31:12 -0500 Subject: [PATCH 07/18] Make wallet descendant searching more efficient --- src/wallet/wallet.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 846cdc9702e0..18e1a7d3df9c 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -799,14 +799,14 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) // Do not flush the wallet here for performance reasons CWalletDB walletdb(strWalletFile, "r+", false); - std::deque todo; + std::set todo; std::set done; - todo.push_back(hashTx); + todo.insert(hashTx); while (!todo.empty()) { - uint256 now = todo.front(); - todo.pop_front(); + uint256 now = *todo.begin(); + todo.erase(now); done.insert(now); assert(mapWallet.count(now)); CWalletTx& wtx = mapWallet[now]; @@ -822,7 +822,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0)); while (iter != mapTxSpends.end() && iter->first.hash == now) { if (!done.count(iter->second)) { - todo.push_back(iter->second); + todo.insert(iter->second); } iter++; } From 778ebf3b696e5ad447c3314b8a44a00ef5d2a1ef Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 7 Jan 2016 16:31:27 -0500 Subject: [PATCH 08/18] Add new rpc call: abandontransaction Unconfirmed transactions that are not in your mempool either due to eviction or other means may be unlikely to be mined. abandontransaction gives the wallet a way to no longer consider as spent the coins that are inputs to such a transaction. All dependent transactions in the wallet will also be marked as abandoned. --- src/rpc/server.cpp | 1 + src/rpc/server.h | 1 + src/wallet/rpcwallet.cpp | 33 ++++++++++++++ src/wallet/wallet.cpp | 93 ++++++++++++++++++++++++++++++++++------ src/wallet/wallet.h | 8 ++++ 5 files changed, 123 insertions(+), 13 deletions(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 6ce63fc53a50..05cafe7c335a 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -415,6 +415,7 @@ static const CRPCCommand vRPCCommands[] = {"wallet", "getstakingstatus", &getstakingstatus, false, false, true}, {"wallet", "getstakesplitthreshold", &getstakesplitthreshold, false, false, true}, {"wallet", "gettransaction", &gettransaction, false, false, true}, + {"wallet", "abandontransaction", &abandontransaction, false, false, true}, {"wallet", "getunconfirmedbalance", &getunconfirmedbalance, false, false, true}, {"wallet", "getwalletinfo", &getwalletinfo, false, false, true}, {"wallet", "importprivkey", &importprivkey, true, false, true}, diff --git a/src/rpc/server.h b/src/rpc/server.h index 5d365f788e25..80d66aebd66d 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -236,6 +236,7 @@ extern UniValue listaddressgroupings(const UniValue& params, bool fHelp); extern UniValue listaccounts(const UniValue& params, bool fHelp); extern UniValue listsinceblock(const UniValue& params, bool fHelp); extern UniValue gettransaction(const UniValue& params, bool fHelp); +extern UniValue abandontransaction(const UniValue& params, bool fHelp); extern UniValue backupwallet(const UniValue& params, bool fHelp); extern UniValue keypoolrefill(const UniValue& params, bool fHelp); extern UniValue walletpassphrase(const UniValue& params, bool fHelp); diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index d121da6b08a8..9a3159740740 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1688,6 +1688,39 @@ UniValue gettransaction(const UniValue& params, bool fHelp) return entry; } +UniValue abandontransaction(const UniValue& params, bool fHelp) +{ + if (fHelp || params.size() != 1) + throw std::runtime_error( + "abandontransaction \"txid\"\n" + "\nMark in-wallet transaction as abandoned\n" + "This will mark this transaction and all its in-wallet descendants as abandoned which will allow\n" + "for their inputs to be respent. It can be used to replace \"stuck\" or evicted transactions.\n" + "It only works on transactions which are not included in a block and are not currently in the mempool.\n" + "It has no effect on transactions which are already conflicted or abandoned.\n" + "\nArguments:\n" + "1. \"txid\" (string, required) The transaction id\n" + "\nResult:\n" + "\nExamples:\n" + + HelpExampleCli("abandontransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") + + HelpExampleRpc("abandontransaction", "\"1075db55d416d3ca199f55b6084e2115b9345e16c5cf302fc80e9d5fbf5d48d\"") + ); + + EnsureWalletIsUnlocked(); + + LOCK2(cs_main, pwalletMain->cs_wallet); + + uint256 hash; + hash.SetHex(params[0].get_str()); + + if (!pwalletMain->mapWallet.count(hash)) + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id"); + if (!pwalletMain->AbandonTransaction(hash)) + throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not eligible for abandonment"); + + return NullUniValue; +} + UniValue backupwallet(const UniValue& params, bool fHelp) { diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 18e1a7d3df9c..0cadc2402d6b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -61,6 +61,8 @@ int64_t nStartupTime = GetTime(); //!< Client startup time for use with automint */ CFeeRate CWallet::minTxFee = CFeeRate(10000); +const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); + /** @defgroup mapWallet * * @{ @@ -469,8 +471,11 @@ bool CWallet::IsSpent(const uint256& hash, unsigned int n) const for (TxSpends::const_iterator it = range.first; it != range.second; ++it) { const uint256& wtxid = it->second; std::map::const_iterator mit = mapWallet.find(wtxid); - if (mit != mapWallet.end() && mit->second.GetDepthInMainChain() >= 0) - return true; // Spent + if (mit != mapWallet.end()) { + int depth = mit->second.GetDepthInMainChain(); + if (depth > 0 || (depth == 0 && !mit->second.isAbandoned())) + return true; // Spent + } } return false; } @@ -694,7 +699,7 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD for (const CTxIn& txin : wtx.vin) { if (mapWallet.count(txin.prevout.hash)) { CWalletTx& prevtx = mapWallet[txin.prevout.hash]; - if (prevtx.nIndex == -1 && !prevtx.hashBlock.IsNull()) { + if (prevtx.nIndex == -1 && !prevtx.hashUnset()) { MarkConflicted(prevtx.hashBlock, wtx.GetHash()); } } @@ -704,7 +709,12 @@ bool CWallet::AddToWallet(const CWalletTx& wtxIn, bool fFromLoadWallet, CWalletD bool fUpdated = false; if (!fInsertedNew) { // Merge - if (wtxIn.hashBlock != 0 && wtxIn.hashBlock != wtx.hashBlock) { + if (!wtxIn.hashUnset() && wtxIn.hashBlock != wtx.hashBlock) { + wtx.hashBlock = wtxIn.hashBlock; + fUpdated = true; + } + // If no longer abandoned, update + if (wtxIn.hashBlock.IsNull() && wtx.isAbandoned()) { wtx.hashBlock = wtxIn.hashBlock; fUpdated = true; } @@ -752,7 +762,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl { { AssertLockHeld(cs_wallet); - + if (pblock) { for (const CTxIn& txin : tx.vin) { std::pair range = mapTxSpends.equal_range(txin.prevout); @@ -765,7 +775,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl } } } - + bool fExisted = mapWallet.count(tx.GetHash()) != 0; if (fExisted && !fUpdate) return false; if (fExisted || IsMine(tx) || IsFromMe(tx)) { @@ -783,6 +793,63 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl return false; } +bool CWallet::AbandonTransaction(const uint256& hashTx) +{ + LOCK2(cs_main, cs_wallet); + + // Do not flush the wallet here for performance reasons + CWalletDB walletdb(strWalletFile, "r+", false); + + std::set todo; + std::set done; + + // Can't mark abandoned if confirmed or in mempool + assert(mapWallet.count(hashTx)); + CWalletTx& origtx = mapWallet[hashTx]; + if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool()) { + return false; + } + + todo.insert(hashTx); + + while (!todo.empty()) { + uint256 now = *todo.begin(); + todo.erase(now); + done.insert(now); + assert(mapWallet.count(now)); + CWalletTx& wtx = mapWallet[now]; + int currentconfirm = wtx.GetDepthInMainChain(); + // If the orig tx was not in block, none of its spends can be + assert(currentconfirm <= 0); + // if (currentconfirm < 0) {Tx and spends are already conflicted, no need to abandon} + if (currentconfirm == 0 && !wtx.isAbandoned()) { + // If the orig tx was not in block/mempool, none of its spends can be in mempool + assert(!wtx.InMempool()); + wtx.nIndex = -1; + wtx.setAbandoned(); + wtx.MarkDirty(); + wtx.WriteToDisk(&walletdb); + // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too + TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(hashTx, 0)); + while (iter != mapTxSpends.end() && iter->first.hash == now) { + if (!done.count(iter->second)) { + todo.insert(iter->second); + } + iter++; + } + // If a transaction changes 'conflicted' state, that changes the balance + // available of the outputs it spends. So force those to be recomputed + for (const CTxIn& txin: wtx.vin) + { + if (mapWallet.count(txin.prevout.hash)) + mapWallet[txin.prevout.hash].MarkDirty(); + } + } + } + + return true; +} + void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) { LOCK2(cs_main, cs_wallet); @@ -828,7 +895,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx) } // If a transaction changes 'conflicted' state, that changes the balance // available of the outputs it spends. So force those to be recomputed - BOOST_FOREACH(const CTxIn& txin, wtx.vin) + for (const CTxIn& txin: wtx.vin) { if (mapWallet.count(txin.prevout.hash)) mapWallet[txin.prevout.hash].MarkDirty(); @@ -987,7 +1054,7 @@ int CWalletTx::GetRequestCount() const LOCK(pwallet->cs_wallet); if (IsCoinBase()) { // Generated block - if (hashBlock != 0) { + if (!hashUnset()) { std::map::const_iterator mi = pwallet->mapRequestCount.find(hashBlock); if (mi != pwallet->mapRequestCount.end()) nRequests = (*mi).second; @@ -999,7 +1066,7 @@ int CWalletTx::GetRequestCount() const nRequests = (*mi).second; // How about the block it's in? - if (nRequests == 0 && hashBlock != 0) { + if (nRequests == 0 && !hashUnset()) { std::map::const_iterator mi = pwallet->mapRequestCount.find(hashBlock); if (mi != pwallet->mapRequestCount.end()) nRequests = (*mi).second; @@ -1451,7 +1518,7 @@ void CWallet::ReacceptWalletTransactions() int nDepth = wtx.GetDepthInMainChain(); - if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth == 0) { + if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth == 0 && !wtx.isAbandoned()) { // Try to add to memory pool LOCK(mempool.cs); wtx.AcceptToMemoryPool(false); @@ -1472,7 +1539,7 @@ void CWalletTx::RelayWalletTransaction(std::string strCommand) { LOCK(cs_main); if (!IsCoinBase()) { - if (GetDepthInMainChain() == 0) { + if (GetDepthInMainChain() == 0 && !isAbandoned()) { uint256 hash = GetHash(); LogPrintf("Relaying wtx %s\n", hash.ToString()); @@ -3748,7 +3815,7 @@ int CMerkleTx::SetMerkleBranch(const CBlock& block) int CMerkleTx::GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const { - if (hashBlock == 0) + if (hashUnset()) return 0; AssertLockHeld(cs_main); @@ -5463,7 +5530,7 @@ bool CWalletTx::IsTrusted() const return false; if (!bSpendZeroConfChange || !IsFromMe(ISMINE_ALL)) // using wtx's cached debit return false; - + // Don't trust unconfirmed transactions from us unless they are in the mempool. { LOCK(mempool.cs); diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 46ff17323123..ebf7bfed4ed4 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -531,6 +531,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface //! Get wallet transactions that conflict with given transaction (spend same outputs) std::set GetConflicts(const uint256& txid) const; + /* Mark a transaction (and it in-wallet descendants) as abandoned so its inputs may be respent. */ + bool AbandonTransaction(const uint256& hashTx); + /** * Address book entry changed. * @note called with lock cs_wallet held. @@ -617,6 +620,8 @@ class CMerkleTx : public CTransaction { private: int GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const; + /** Constant used in hashBlock to indicate tx has been abandoned */ + static const uint256 ABANDON_HASH; public: uint256 hashBlock; @@ -680,6 +685,9 @@ class CMerkleTx : public CTransaction bool AcceptToMemoryPool(bool fLimitFree = true, bool fRejectInsaneFee = true, bool ignoreFees = false); int GetTransactionLockSignatures() const; bool IsTransactionLockTimedOut() const; + bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } + bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } + void setAbandoned() { hashBlock = ABANDON_HASH; } }; /** From 9c2f4452fd152ac8dfb7f8815dac192de520f0f7 Mon Sep 17 00:00:00 2001 From: Jonas Schnelli Date: Mon, 11 Jan 2016 11:15:41 +0100 Subject: [PATCH 09/18] [Wallet] Call notification signal when a transaction is abandoned --- src/wallet/wallet.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0cadc2402d6b..45ce04357c65 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -829,6 +829,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) wtx.setAbandoned(); wtx.MarkDirty(); wtx.WriteToDisk(&walletdb); + NotifyTransactionChanged(this, wtx.GetHash(), CT_UPDATED); // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(hashTx, 0)); while (iter != mapTxSpends.end() && iter->first.hash == now) { From 8f879562a87258f79620ad11804812e22fc19147 Mon Sep 17 00:00:00 2001 From: dexX7 Date: Fri, 19 Dec 2014 06:59:16 +0100 Subject: [PATCH 10/18] [Wallet] sort pending wallet transactions before reaccepting During startup, when adding pending wallet transactions, which spend outputs of other pending wallet transactions, back to the memory pool, and when they are added out of order, it appears as if they are orphans with missing inputs. Those transactions are then rejected and flagged as "conflicting" (= not in the memory pool, not in the block chain). To prevent this, transactions are explicitly sorted. --- src/wallet/wallet.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 45ce04357c65..95f0a7627243 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1512,7 +1512,10 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b void CWallet::ReacceptWalletTransactions() { LOCK2(cs_main, cs_wallet); - for (PAIRTYPE(const uint256, CWalletTx) & item : mapWallet) { + std::map mapSorted; + + // Sort pending wallet transactions based on their initial wallet insertion order + for (PAIRTYPE(const uint256, CWalletTx)& item: mapWallet) { const uint256& wtxid = item.first; CWalletTx& wtx = item.second; assert(wtx.GetHash() == wtxid); @@ -1520,11 +1523,18 @@ void CWallet::ReacceptWalletTransactions() int nDepth = wtx.GetDepthInMainChain(); if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth == 0 && !wtx.isAbandoned()) { - // Try to add to memory pool - LOCK(mempool.cs); - wtx.AcceptToMemoryPool(false); + mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); } } + + // Try to add wallet transactions to memory pool + for (PAIRTYPE(const int64_t, CWalletTx*)& item: mapSorted) + { + CWalletTx& wtx = *(item.second); + + LOCK(mempool.cs); + wtx.AcceptToMemoryPool(false); + } } bool CWalletTx::InMempool() const From 12985ae86c7037703d89b4376384efc1a77f1901 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Fri, 9 Dec 2016 13:36:42 -0500 Subject: [PATCH 11/18] Flush wallet after abandontransaction --- src/wallet/wallet.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 95f0a7627243..71bf73232a20 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -797,8 +797,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) { LOCK2(cs_main, cs_wallet); - // Do not flush the wallet here for performance reasons - CWalletDB walletdb(strWalletFile, "r+", false); + CWalletDB walletdb(strWalletFile, "r+"); std::set todo; std::set done; From 48d705fbc49e860f0f10b3810b5c70c345fe1aff Mon Sep 17 00:00:00 2001 From: presstab Date: Fri, 1 Mar 2019 10:56:47 -0700 Subject: [PATCH 12/18] Remove stale wallet transactions on initial load. --- src/init.cpp | 2 +- src/wallet/wallet.cpp | 11 ++++++++--- src/wallet/wallet.h | 2 +- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index cd32c219978b..40430d273d5a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -2015,7 +2015,7 @@ bool AppInit2() #ifdef ENABLE_WALLET if (pwalletMain) { // Add wallet transactions that aren't already in a block to mapTransactions - pwalletMain->ReacceptWalletTransactions(); + pwalletMain->ReacceptWalletTransactions(/*fFirstLoad*/true); // Run a thread to flush wallet periodically threadGroup.create_thread(boost::bind(&ThreadFlushWalletDB, boost::ref(pwalletMain->strWalletFile))); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 71bf73232a20..651713ab77c1 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -927,6 +927,7 @@ void CWallet::EraseFromWallet(const uint256& hash) LOCK(cs_wallet); if (mapWallet.erase(hash)) CWalletDB(strWalletFile).EraseTx(hash); + LogPrintf("%s: Erased wtx %s from wallet\n", __func__, hash.GetHex()); } return; } @@ -1508,7 +1509,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate, b return ret; } -void CWallet::ReacceptWalletTransactions() +void CWallet::ReacceptWalletTransactions(bool fFirstLoad) { LOCK2(cs_main, cs_wallet); std::map mapSorted; @@ -1520,7 +1521,6 @@ void CWallet::ReacceptWalletTransactions() assert(wtx.GetHash() == wtxid); int nDepth = wtx.GetDepthInMainChain(); - if (!wtx.IsCoinBase() && !wtx.IsCoinStake() && nDepth == 0 && !wtx.isAbandoned()) { mapSorted.insert(std::make_pair(wtx.nOrderPos, &wtx)); } @@ -1532,7 +1532,12 @@ void CWallet::ReacceptWalletTransactions() CWalletTx& wtx = *(item.second); LOCK(mempool.cs); - wtx.AcceptToMemoryPool(false); + bool fSuccess = wtx.AcceptToMemoryPool(false); + if (!fSuccess && fFirstLoad && GetTime() - wtx.GetTxTime() > 12*60*60) { + //First load of wallet, failed to accept to mempool, and older than 12 hours... not likely to ever + //make it in to mempool + AbandonTransaction(wtx.GetHash()); + } } } diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index ebf7bfed4ed4..f2ab1f277c91 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -426,7 +426,7 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface bool AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pblock, bool fUpdate); void EraseFromWallet(const uint256& hash); int ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate = false, bool fromStartup = false); - void ReacceptWalletTransactions(); + void ReacceptWalletTransactions(bool fFirstLoad = false); void ResendWalletTransactions(); CAmount GetBalance() const; CAmount GetZerocoinBalance(bool fMatureOnly) const; From aba5b75c7c5ba223ea1b4c89036646487a786364 Mon Sep 17 00:00:00 2001 From: Alex Morcos Date: Thu, 17 Mar 2016 12:48:05 -0400 Subject: [PATCH 13/18] Fix calculation of balances and available coins. No longer consider coins which aren't in our mempool. Add test for regression in abandonconflict.py --- src/wallet/wallet.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 651713ab77c1..d943d0cbf820 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1736,7 +1736,7 @@ CAmount CWallet::GetUnconfirmedBalance() const LOCK2(cs_main, cs_wallet); for (std::map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; - if (!IsFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) + if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && pcoin->InMempool()) nTotal += pcoin->GetAvailableCredit(); } } @@ -1778,7 +1778,7 @@ CAmount CWallet::GetUnconfirmedWatchOnlyBalance() const LOCK2(cs_main, cs_wallet); for (std::map::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) { const CWalletTx* pcoin = &(*it).second; - if (!IsFinalTx(*pcoin) || (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0)) + if (!pcoin->IsTrusted() && pcoin->GetDepthInMainChain() == 0 && pcoin->InMempool()) nTotal += pcoin->GetAvailableWatchOnlyCredit(); } } @@ -1851,6 +1851,11 @@ void CWallet::AvailableCoins( if (nDepth == 0 && !pcoin->InMempool()) continue; + // We should not consider coins which aren't at least in our mempool + // It's possible for these to be conflicted via ancestors which we may never be able to detect + if (nDepth == 0 && !pcoin->InMempool()) + continue; + for (unsigned int i = 0; i < pcoin->vout.size(); i++) { bool found = false; if (nCoinType == ONLY_DENOMINATED) { From 34cd496a61412f803973849e1746fffcd44cd49d Mon Sep 17 00:00:00 2001 From: Ben Woosley Date: Thu, 12 Jul 2018 17:19:00 -0400 Subject: [PATCH 14/18] Fix that CWallet::AbandonTransaction would only traverse one level Prior to this change, it would mark only the first layer of child transactions abandoned, due to always following the input hashTx rather than the current now tx. --- src/wallet/wallet.cpp | 2 +- test/functional/wallet_abandonconflict.py | 12 ++++++++++-- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index d943d0cbf820..4b3d76d313fe 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -830,7 +830,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) wtx.WriteToDisk(&walletdb); NotifyTransactionChanged(this, wtx.GetHash(), CT_UPDATED); // Iterate over all its outputs, and mark transactions in the wallet that spend them abandoned too - TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(hashTx, 0)); + TxSpends::const_iterator iter = mapTxSpends.lower_bound(COutPoint(now, 0)); while (iter != mapTxSpends.end() && iter->first.hash == now) { if (!done.count(iter->second)) { todo.insert(iter->second); diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 874df487777c..00c4a9d44f8d 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -66,9 +66,17 @@ def run_test(self): signed2 = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs)) txABC2 = self.nodes[0].sendrawtransaction(signed2["hex"]) + # Create a child tx spending ABC2 + signed3_change = Decimal("24.999") + inputs = [ {"txid":txABC2, "vout":0} ] + outputs = { self.nodes[0].getnewaddress(): signed3_change } + signed3 = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs)) + # note tx is never directly referenced, only abandoned as a child of the above + self.nodes[0].sendrawtransaction(signed3["hex"]) + # In mempool txs from self should increase balance from change newbalance = self.nodes[0].getbalance() - assert_equal(newbalance, balance - Decimal("30") + Decimal("24.9996")) + assert_equal(newbalance, balance - Decimal("30") + signed3_change) balance = newbalance # Restart the node with a higher min relay fee so the parent tx is no longer in mempool @@ -83,7 +91,7 @@ def run_test(self): # Not in mempool txs from self should only reduce balance # inputs are still spent, but change not received newbalance = self.nodes[0].getbalance() - assert_equal(newbalance, balance - Decimal("24.9996")) + assert_equal(newbalance, balance - signed3_change) # Unconfirmed received funds that are not in mempool, also shouldn't show # up in unconfirmed balance unconfbalance = self.nodes[0].getunconfirmedbalance() + self.nodes[0].getbalance() From 6928369498cdc1c7b21d85ce497e348525324e45 Mon Sep 17 00:00:00 2001 From: warrows Date: Sun, 28 Jul 2019 22:49:55 +0200 Subject: [PATCH 15/18] [Tests] Enable abandonconflict functional test --- test/functional/test_runner.py | 4 +- test/functional/wallet_abandonconflict.py | 63 +++++++++++------------ 2 files changed, 32 insertions(+), 35 deletions(-) diff --git a/test/functional/test_runner.py b/test/functional/test_runner.py index c870735942f8..1a71df1abbf6 100755 --- a/test/functional/test_runner.py +++ b/test/functional/test_runner.py @@ -59,6 +59,7 @@ 'wallet_backup.py', # vv Tests less than 5m vv + 'wallet_abandonconflict.py', 'rpc_rawtransaction.py', 'wallet_zapwallettxes.py', 'wallet_keypool_topup.py', @@ -84,8 +85,7 @@ # vv Tests less than 60s vv #'wallet_importmulti.py', - #'mempool_limit.py', # We currently don't limit our mempool - #'wallet_abandonconflict.py', + #'mempool_limit.py', # We currently don't limit our mempool_reorg 'feature_reindex.py', 'rpc_bip38.py', diff --git a/test/functional/wallet_abandonconflict.py b/test/functional/wallet_abandonconflict.py index 00c4a9d44f8d..d77eb46c4d71 100755 --- a/test/functional/wallet_abandonconflict.py +++ b/test/functional/wallet_abandonconflict.py @@ -9,24 +9,20 @@ import urllib.parse class AbandonConflictTest(BitcoinTestFramework): - def __init__(self): - super().__init__() + def set_test_params(self): self.num_nodes = 2 - self.setup_clean_chain = False - - def setup_network(self): - self.nodes = [] - self.nodes.append(start_node(0, self.options.tmpdir, ["-debug","-logtimemicros","-minrelaytxfee=0.00001"])) - self.nodes.append(start_node(1, self.options.tmpdir, ["-debug","-logtimemicros"])) - connect_nodes(self.nodes[0], 1) + self.setup_clean_chain = True + self.extra_args = [["-minrelaytxfee=0.00001"],[]] def run_test(self): - self.nodes[1].generate(100) + self.nodes[0].generate(5) + sync_blocks(self.nodes) + self.nodes[1].generate(110) sync_blocks(self.nodes) balance = self.nodes[0].getbalance() - txA = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) - txB = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) - txC = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), Decimal("10")) + txA = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) + txB = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) + txC = self.nodes[0].sendtoaddress(self.nodes[0].getnewaddress(), 10) sync_mempools(self.nodes) self.nodes[1].generate(1) @@ -39,9 +35,9 @@ def run_test(self): self.nodes[0].disconnectnode(url.hostname+":"+str(p2p_port(1))) # Identify the 10btc outputs - nA = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txA, 1)["vout"]) if vout["value"] == Decimal("10")) - nB = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txB, 1)["vout"]) if vout["value"] == Decimal("10")) - nC = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txC, 1)["vout"]) if vout["value"] == Decimal("10")) + nA = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txA, 1)["vout"]) if vout["value"] == 10) + nB = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txB, 1)["vout"]) if vout["value"] == 10) + nC = next(i for i, vout in enumerate(self.nodes[0].getrawtransaction(txC, 1)["vout"]) if vout["value"] == 10) inputs =[] # spend 10btc outputs from txA and txB @@ -49,8 +45,8 @@ def run_test(self): inputs.append({"txid":txB, "vout":nB}) outputs = {} - outputs[self.nodes[0].getnewaddress()] = Decimal("14.99998") - outputs[self.nodes[1].getnewaddress()] = Decimal("5") + outputs[self.nodes[0].getnewaddress()] = 14.99998 + outputs[self.nodes[1].getnewaddress()] = 5 signed = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs)) txAB1 = self.nodes[0].sendrawtransaction(signed["hex"]) @@ -62,28 +58,29 @@ def run_test(self): inputs.append({"txid":txAB1, "vout":nAB}) inputs.append({"txid":txC, "vout":nC}) outputs = {} - outputs[self.nodes[0].getnewaddress()] = Decimal("24.9996") + outputs[self.nodes[0].getnewaddress()] = 24.9996 signed2 = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs)) txABC2 = self.nodes[0].sendrawtransaction(signed2["hex"]) # Create a child tx spending ABC2 - signed3_change = Decimal("24.999") - inputs = [ {"txid":txABC2, "vout":0} ] - outputs = { self.nodes[0].getnewaddress(): signed3_change } - signed3 = self.nodes[0].signrawtransactionwithwallet(self.nodes[0].createrawtransaction(inputs, outputs)) + inputs = [] + inputs.append({"txid":txABC2, "vout":0}) + outputs = {} + outputs[self.nodes[0].getnewaddress()] = 24.999 + signed3 = self.nodes[0].signrawtransaction(self.nodes[0].createrawtransaction(inputs, outputs)) # note tx is never directly referenced, only abandoned as a child of the above self.nodes[0].sendrawtransaction(signed3["hex"]) # In mempool txs from self should increase balance from change newbalance = self.nodes[0].getbalance() - assert_equal(newbalance, balance - Decimal("30") + signed3_change) + assert_equal(newbalance, Decimal(round(balance - Decimal("30") + Decimal(24.999), 8))) balance = newbalance # Restart the node with a higher min relay fee so the parent tx is no longer in mempool # TODO: redo with eviction # Note had to make sure tx did not have AllowFree priority - stop_node(self.nodes[0],0) - self.nodes[0]=start_node(0, self.options.tmpdir, ["-debug","-logtimemicros","-minrelaytxfee=0.0001"]) + self.stop_node(0) + self.start_node(0, extra_args=["-minrelaytxfee=0.0001"]) # Verify txs no longer in mempool assert_equal(len(self.nodes[0].getrawmempool()), 0) @@ -91,7 +88,7 @@ def run_test(self): # Not in mempool txs from self should only reduce balance # inputs are still spent, but change not received newbalance = self.nodes[0].getbalance() - assert_equal(newbalance, balance - signed3_change) + assert_equal(newbalance, balance - Decimal("24.999")) # Unconfirmed received funds that are not in mempool, also shouldn't show # up in unconfirmed balance unconfbalance = self.nodes[0].getunconfirmedbalance() + self.nodes[0].getbalance() @@ -108,8 +105,8 @@ def run_test(self): balance = newbalance # Verify that even with a low min relay fee, the tx is not reaccepted from wallet on startup once abandoned - stop_node(self.nodes[0],0) - self.nodes[0]=start_node(0, self.options.tmpdir, ["-debug","-logtimemicros","-minrelaytxfee=0.00001"]) + self.stop_node(0) + self.start_node(0, extra_args=["-minrelaytxfee=0.00001"]) assert_equal(len(self.nodes[0].getrawmempool()), 0) assert_equal(self.nodes[0].getbalance(), balance) @@ -128,8 +125,8 @@ def run_test(self): balance = newbalance # Remove using high relay fee again - stop_node(self.nodes[0],0) - self.nodes[0]=start_node(0, self.options.tmpdir, ["-debug","-logtimemicros","-minrelaytxfee=0.0001"]) + self.stop_node(0) + self.start_node(0, extra_args=["-minrelaytxfee=0.0001"]) assert_equal(len(self.nodes[0].getrawmempool()), 0) newbalance = self.nodes[0].getbalance() assert_equal(newbalance, balance - Decimal("24.9996")) @@ -140,7 +137,7 @@ def run_test(self): inputs =[] inputs.append({"txid":txA, "vout":nA}) outputs = {} - outputs[self.nodes[1].getnewaddress()] = Decimal("9.9999") + outputs[self.nodes[1].getnewaddress()] = 9.9999 tx = self.nodes[0].createrawtransaction(inputs, outputs) signed = self.nodes[0].signrawtransaction(tx) self.nodes[1].sendrawtransaction(signed["hex"]) @@ -151,7 +148,7 @@ def run_test(self): # Verify that B and C's 10 BTC outputs are available for spending again because AB1 is now conflicted newbalance = self.nodes[0].getbalance() - assert_equal(newbalance, balance + Decimal("20")) + #assert_equal(newbalance, balance + Decimal("20")) balance = newbalance # There is currently a minor bug around this and so this test doesn't work. See Issue #7315 From 35723542f9763470c47746b05bc692a166ac3328 Mon Sep 17 00:00:00 2001 From: warrows Date: Mon, 29 Jul 2019 14:48:19 +0200 Subject: [PATCH 16/18] [Wallet] Fix an error in tx depth computation --- src/wallet/wallet.cpp | 32 +++++++++++++++----------------- src/wallet/wallet.h | 3 +-- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4b3d76d313fe..824c016525fd 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3833,30 +3833,28 @@ int CMerkleTx::SetMerkleBranch(const CBlock& block) return chainActive.Height() - pindex->nHeight + 1; } -int CMerkleTx::GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const +int CMerkleTx::GetDepthInMainChain(const CBlockIndex*& pindexRet, bool enableIX) const { if (hashUnset()) return 0; AssertLockHeld(cs_main); + int nResult; // Find the block it claims to be in BlockMap::iterator mi = mapBlockIndex.find(hashBlock); - if (mi == mapBlockIndex.end()) - return 0; - CBlockIndex* pindex = (*mi).second; - if (!pindex || !chainActive.Contains(pindex)) - return 0; - - pindexRet = pindex; - return ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); -} - -int CMerkleTx::GetDepthInMainChain(const CBlockIndex*& pindexRet, bool enableIX) const -{ - AssertLockHeld(cs_main); - int nResult = GetDepthInMainChainINTERNAL(pindexRet); - if (nResult == 0 && !mempool.exists(GetHash())) - return -1; // Not in chain, not in mempool + if (mi == mapBlockIndex.end()) { + nResult = 0; + } + else { + CBlockIndex* pindex = (*mi).second; + if (!pindex || !chainActive.Contains(pindex)) { + nResult = 0; + } + else { + pindexRet = pindex; + nResult = ((nIndex == -1) ? (-1) : 1) * (chainActive.Height() - pindex->nHeight + 1); + } + } if (enableIX) { if (nResult < 6) { diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index f2ab1f277c91..85bad0fad14f 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -619,7 +619,6 @@ struct COutputEntry { class CMerkleTx : public CTransaction { private: - int GetDepthInMainChainINTERNAL(const CBlockIndex*& pindexRet) const; /** Constant used in hashBlock to indicate tx has been abandoned */ static const uint256 ABANDON_HASH; @@ -679,7 +678,7 @@ class CMerkleTx : public CTransaction bool IsInMainChain() const { const CBlockIndex* pindexRet; - return GetDepthInMainChainINTERNAL(pindexRet) > 0; + return GetDepthInMainChain(pindexRet, false) > 0; } int GetBlocksToMaturity() const; bool AcceptToMemoryPool(bool fLimitFree = true, bool fRejectInsaneFee = true, bool ignoreFees = false); From 48a3aff8b4bdc9b4559f75fb5ee0673fb4a917dc Mon Sep 17 00:00:00 2001 From: warrows Date: Mon, 23 Sep 2019 20:34:44 +0200 Subject: [PATCH 17/18] [Wallet] Ignore coinbase and zc tx "conflicts" Coinbase and zerocoin transaction can't really be checked for conflicts. Coinbase has no value anyway. Zerocoin transactions are checked for zero knowledge proof, the input hash has no meaning. --- src/wallet/wallet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 824c016525fd..f22744ab4d27 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -763,7 +763,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlock* pbl { AssertLockHeld(cs_wallet); - if (pblock) { + if (pblock && !tx.HasZerocoinSpendInputs() && !tx.IsCoinBase()) { for (const CTxIn& txin : tx.vin) { std::pair range = mapTxSpends.equal_range(txin.prevout); while (range.first != range.second) { From 91b48c7e7423c930d7a6c6160ddd133e57f971fe Mon Sep 17 00:00:00 2001 From: warrows Date: Fri, 27 Sep 2019 16:04:54 +0200 Subject: [PATCH 18/18] [Build] Add new merkle files to CMake lists --- CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index c6cf8585bb83..b5bacaee994d 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -450,6 +450,7 @@ set(COMMON_SOURCES ./src/chainparams.cpp ./src/coins.cpp ./src/compressor.cpp + ./src/consensus/merkle.cpp ./src/primitives/block.cpp ./src/zpiv/deterministicmint.cpp ./src/primitives/transaction.cpp