From b3a9d179f23f654b8fba63924bbca5fd31ad4bb0 Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 25 Jul 2019 10:42:34 -0400 Subject: [wallet] Move CMerkleTx functions into CWalletTx CMerkleTx only exists as a base class for CWalletTx and for wallet file serialization/deserialization. Move CMerkleTx methods into CWalletTx, but leave class hierarchy and serialization logic in place. --- src/wallet/wallet.cpp | 10 ++++----- src/wallet/wallet.h | 62 +++++++++++++++++++++++++++------------------------ 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 638ed569c3..11a2154f53 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -228,7 +228,7 @@ WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000; -const uint256 CMerkleTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); +const uint256 CWalletTx::ABANDON_HASH(uint256S("0000000000000000000000000000000000000000000000000000000000000001")); /** @defgroup mapWallet * @@ -4627,7 +4627,7 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) m_pre_split = false; } -void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) +void CWalletTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) { // Update the tx's hashBlock hashBlock = block_hash; @@ -4636,7 +4636,7 @@ void CMerkleTx::SetMerkleBranch(const uint256& block_hash, int posInBlock) nIndex = posInBlock; } -int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const { if (hashUnset()) return 0; @@ -4644,7 +4644,7 @@ int CMerkleTx::GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const return locked_chain.getBlockDepth(hashBlock) * (nIndex == -1 ? -1 : 1); } -int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const +int CWalletTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const { if (!IsCoinBase()) return 0; @@ -4653,7 +4653,7 @@ int CMerkleTx::GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const return std::max(0, (COINBASE_MATURITY+1) - chain_depth); } -bool CMerkleTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const +bool CWalletTx::IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const { // note GetBlocksToMaturity is 0 for non-coinbase tx return GetBlocksToMaturity(locked_chain) > 0; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 06e22895b5..7fb349a9a9 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -364,13 +364,13 @@ struct COutputEntry int vout; }; -/** A transaction with a merkle branch linking it to the block chain. */ +/** Legacy class used for deserializing vtxPrev for backwards compatibility. + * vtxPrev was removed in commit 93a18a3650292afbb441a47d1fa1b94aeb0164e3, + * but old wallet.dat files may still contain vtxPrev vectors of CMerkleTxs. + * These need to get deserialized for field alignment when deserializing + * a CWalletTx, but the deserialized values are discarded.**/ class CMerkleTx { -private: - /** Constant used in hashBlock to indicate tx has been abandoned */ - static const uint256 ABANDON_HASH; - public: CTransactionRef tx; uint256 hashBlock; @@ -416,30 +416,6 @@ public: READWRITE(nIndex); } - void SetMerkleBranch(const uint256& block_hash, int posInBlock); - - /** - * Return depth of transaction in blockchain: - * <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 - */ - int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; - bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } - - /** - * @return number of blocks to maturity for this transaction: - * 0 : is not a coinbase transaction, or is a mature coinbase transaction - * >0 : is a coinbase transaction which matures in this many blocks - */ - int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; - bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } - bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } - void setAbandoned() { hashBlock = ABANDON_HASH; } - - const uint256& GetHash() const { return tx->GetHash(); } - bool IsCoinBase() const { return tx->IsCoinBase(); } - bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; //Get the marginal bytes of spending the specified output @@ -454,6 +430,9 @@ class CWalletTx : public CMerkleTx private: const CWallet* pwallet; + /** Constant used in hashBlock to indicate tx has been abandoned */ + static const uint256 ABANDON_HASH; + public: /** * Key/value map with information about the transaction. @@ -630,6 +609,31 @@ public: // that we still have the runtime check "AssertLockHeld(pwallet->cs_wallet)" // in place. std::set GetConflicts() const NO_THREAD_SAFETY_ANALYSIS; + + void SetMerkleBranch(const uint256& block_hash, int posInBlock); + + /** + * Return depth of transaction in blockchain: + * <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 + */ + int GetDepthInMainChain(interfaces::Chain::Lock& locked_chain) const; + bool IsInMainChain(interfaces::Chain::Lock& locked_chain) const { return GetDepthInMainChain(locked_chain) > 0; } + + /** + * @return number of blocks to maturity for this transaction: + * 0 : is not a coinbase transaction, or is a mature coinbase transaction + * >0 : is a coinbase transaction which matures in this many blocks + */ + int GetBlocksToMaturity(interfaces::Chain::Lock& locked_chain) const; + bool hashUnset() const { return (hashBlock.IsNull() || hashBlock == ABANDON_HASH); } + bool isAbandoned() const { return (hashBlock == ABANDON_HASH); } + void setAbandoned() { hashBlock = ABANDON_HASH; } + + const uint256& GetHash() const { return tx->GetHash(); } + bool IsCoinBase() const { return tx->IsCoinBase(); } + bool IsImmatureCoinBase(interfaces::Chain::Lock& locked_chain) const; }; class COutput -- cgit v1.2.3 From 783a76f23ba4f33b6e6f609eaf3bf41afd9bcd6f Mon Sep 17 00:00:00 2001 From: John Newbery Date: Thu, 25 Jul 2019 10:43:54 -0400 Subject: [wallet] Flatten CWalletTx class hierarchy Removes CMerkleTx as a base class for CWalletTx. Serialization logic is moved from CMerkleTx to CWalletTx. --- src/wallet/wallet.h | 63 ++++++++++++++++++++++------------------------------- 1 file changed, 26 insertions(+), 37 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 7fb349a9a9..faed0f46a7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -372,43 +372,13 @@ struct COutputEntry class CMerkleTx { public: - CTransactionRef tx; - 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() - { - SetTx(MakeTransactionRef()); - Init(); - } - - explicit CMerkleTx(CTransactionRef arg) - { - SetTx(std::move(arg)); - Init(); - } - - void Init() - { - hashBlock = uint256(); - nIndex = -1; - } - - void SetTx(CTransactionRef arg) - { - tx = std::move(arg); - } - ADD_SERIALIZE_METHODS; template inline void SerializationOp(Stream& s, Operation ser_action) { + CTransactionRef tx; + uint256 hashBlock; + int nIndex; std::vector vMerkleBranch; // For compatibility with older versions. READWRITE(tx); READWRITE(hashBlock); @@ -425,7 +395,7 @@ int CalculateMaximumSignedInputSize(const CTxOut& txout, const CWallet* pwallet, * A transaction with a bunch of additional info that only the owner cares about. * It includes any unrecorded transactions needed to link it back to the block chain. */ -class CWalletTx : public CMerkleTx +class CWalletTx { private: const CWallet* pwallet; @@ -490,7 +460,10 @@ public: mutable bool fInMempool; mutable CAmount nChangeCached; - CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg)) + CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) + : tx(std::move(arg)), + hashBlock(uint256()), + nIndex(-1) { Init(pwalletIn); } @@ -510,6 +483,15 @@ public: nOrderPos = -1; } + CTransactionRef tx; + 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; + template void Serialize(Stream& s) const { @@ -522,7 +504,8 @@ public: mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - s << static_cast(*this); + std::vector dummy_vector; //!< Used to be vMerkleBranch + s << tx << hashBlock << dummy_vector << nIndex; std::vector vUnused; //!< Used to be vtxPrev s << vUnused << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << fSpent; } @@ -533,7 +516,8 @@ public: Init(nullptr); char fSpent; - s >> static_cast(*this); + std::vector dummy_vector; //!< Used to be vMerkleBranch + s >> tx >> hashBlock >> dummy_vector >> nIndex; std::vector vUnused; //!< Used to be vtxPrev s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; @@ -546,6 +530,11 @@ public: mapValue.erase("timesmart"); } + void SetTx(CTransactionRef arg) + { + tx = std::move(arg); + } + //! make sure balances are recalculated void MarkDirty() { -- cgit v1.2.3 From 05b56d1c937b7667ad51400d2f9fb674af72953f Mon Sep 17 00:00:00 2001 From: John Newbery Date: Wed, 24 Jul 2019 14:09:28 -0400 Subject: [wallet] Remove CMerkleTx serialization logic CMerkleTx is only used for deserialization of old wallet files. Remove the serialization logic, and tidy up CWalletTx serialization logic. --- src/wallet/wallet.h | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index faed0f46a7..27f7472806 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -372,20 +372,16 @@ struct COutputEntry class CMerkleTx { public: - ADD_SERIALIZE_METHODS; - - template - inline void SerializationOp(Stream& s, Operation ser_action) { + template + void Unserialize(Stream& s) + { CTransactionRef tx; uint256 hashBlock; + std::vector vMerkleBranch; int nIndex; - std::vector vMerkleBranch; // For compatibility with older versions. - READWRITE(tx); - READWRITE(hashBlock); - READWRITE(vMerkleBranch); - READWRITE(nIndex); - } + s >> tx >> hashBlock >> vMerkleBranch >> nIndex; + } }; //Get the marginal bytes of spending the specified output @@ -495,7 +491,6 @@ public: template void Serialize(Stream& s) const { - char fSpent = false; mapValue_t mapValueCopy = mapValue; mapValueCopy["fromaccount"] = ""; @@ -504,22 +499,21 @@ public: mapValueCopy["timesmart"] = strprintf("%u", nTimeSmart); } - std::vector dummy_vector; //!< Used to be vMerkleBranch - s << tx << hashBlock << dummy_vector << nIndex; - std::vector vUnused; //!< Used to be vtxPrev - s << vUnused << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << fSpent; + std::vector dummy_vector1; //!< Used to be vMerkleBranch + std::vector dummy_vector2; //!< Used to be vtxPrev + char dummy_char = false; //!< Used to be fSpent + s << tx << hashBlock << dummy_vector1 << nIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_char; } template void Unserialize(Stream& s) { Init(nullptr); - char fSpent; - std::vector dummy_vector; //!< Used to be vMerkleBranch - s >> tx >> hashBlock >> dummy_vector >> nIndex; - std::vector vUnused; //!< Used to be vtxPrev - s >> vUnused >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> fSpent; + std::vector dummy_vector1; //!< Used to be vMerkleBranch + std::vector dummy_vector2; //!< Used to be vtxPrev + char dummy_char; //! Used to be fSpent + s >> tx >> hashBlock >> dummy_vector1 >> nIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_char; ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; -- cgit v1.2.3