From 792d72e2a17fa34e173a82171bdaee3ba88f3e74 Mon Sep 17 00:00:00 2001 From: Russell Yanofsky Date: Tue, 16 Feb 2021 22:36:26 -0500 Subject: [PATCH] refactor: Make CWalletTx sync state type-safe Current CWalletTx state representation makes it possible to set inconsistent states that won't be handled correctly by wallet sync code or serialized & deserialized back into the same form. For example, it is possible to call setConflicted without setting a conflicting block hash, or setConfirmed with no transaction index. And it's possible update individual m_confirm and fInMempool data fields without setting an overall consistent state that can be serialized and handled correctly. Fix this without changing behavior by using std::variant, instead of an enum and collection of fields, to represent sync state, so state tracking code is safer and more legible. This is a first step to fixing state tracking bugs https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking, by adding an extra margin of safety that can prevent new bugs from being introduced as existing bugs are fixed. --- src/Makefile.am | 1 + src/Makefile.test.include | 1 + src/bench/coin_selection.cpp | 4 +- src/util/overloaded.h | 21 +++ src/wallet/interfaces.cpp | 4 +- src/wallet/rpcdump.cpp | 4 +- src/wallet/rpcwallet.cpp | 10 +- src/wallet/test/coinselector_tests.cpp | 2 +- src/wallet/test/psbt_wallet_tests.cpp | 4 +- src/wallet/test/wallet_tests.cpp | 22 +-- src/wallet/test/wallet_transaction_tests.cpp | 24 +++ src/wallet/transaction.cpp | 2 +- src/wallet/transaction.h | 177 +++++++++++-------- src/wallet/wallet.cpp | 128 +++++++------- src/wallet/wallet.h | 8 +- src/wallet/walletdb.cpp | 2 +- 16 files changed, 245 insertions(+), 169 deletions(-) create mode 100644 src/util/overloaded.h create mode 100644 src/wallet/test/wallet_transaction_tests.cpp diff --git a/src/Makefile.am b/src/Makefile.am index 94bbc6d773f4a..d68ddd9117979 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -240,6 +240,7 @@ BITCOIN_CORE_H = \ util/memory.h \ util/message.h \ util/moneystr.h \ + util/overloaded.h \ util/rbf.h \ util/ref.h \ util/settings.h \ diff --git a/src/Makefile.test.include b/src/Makefile.test.include index e817bb2ee258f..f6bb960944f5f 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -148,6 +148,7 @@ BITCOIN_TESTS =\ if ENABLE_WALLET BITCOIN_TESTS += \ wallet/test/psbt_wallet_tests.cpp \ + wallet/test/wallet_transaction_tests.cpp \ wallet/test/wallet_tests.cpp \ wallet/test/walletdb_tests.cpp \ wallet/test/wallet_crypto_tests.cpp \ diff --git a/src/bench/coin_selection.cpp b/src/bench/coin_selection.cpp index 3e71ff40ba5f1..9a12bcc75479d 100644 --- a/src/bench/coin_selection.cpp +++ b/src/bench/coin_selection.cpp @@ -18,7 +18,7 @@ static void addCoin(const CAmount& nValue, const CWallet& wallet, std::vector(&wallet, MakeTransactionRef(std::move(tx)))); + wtxs.push_back(MakeUnique(MakeTransactionRef(std::move(tx)), TxStateInactive{})); } // Simple benchmark for wallet coin selection. Note that it maybe be necessary @@ -74,7 +74,7 @@ static void add_coin(const CAmount& nValue, int nInput, std::vector CMutableTransaction tx; tx.vout.resize(nInput + 1); tx.vout[nInput].nValue = nValue; - std::unique_ptr wtx = MakeUnique(&testWallet, MakeTransactionRef(std::move(tx))); + std::unique_ptr wtx = MakeUnique(MakeTransactionRef(std::move(tx)), TxStateInactive{}); set.emplace_back(); set.back().Insert(COutput(testWallet, *wtx, nInput, 0, true, true, true).GetInputCoin(), 0, true, 0, 0, false); wtxn.emplace_back(std::move(wtx)); diff --git a/src/util/overloaded.h b/src/util/overloaded.h new file mode 100644 index 0000000000000..9a8f667f5dfd1 --- /dev/null +++ b/src/util/overloaded.h @@ -0,0 +1,21 @@ +// Copyright (c) 2021 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_UTIL_OVERLOADED_H +#define BITCOIN_UTIL_OVERLOADED_H + +#include +#include + +//! Overloaded helper for std::visit, useful to write code that switches on a +//! variant type and triggers a compile error if there are any unhandled cases. +//! +//! Implementation comes from and example usage can be found at +//! https://en.cppreference.com/w/cpp/utility/variant/visit#Example +template struct Overloaded : Ts... { using Ts::operator()...; }; + +//! Explicit deduction guide (not needed as of C++20) +template Overloaded(Ts...) -> Overloaded; + +#endif // BITCOIN_UTIL_OVERLOADED_H diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 8fedf417fb896..f28fa3eebbff5 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -82,7 +82,9 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) WalletTxStatus MakeWalletTxStatus(const CWallet& wallet, const CWalletTx& wtx) { WalletTxStatus result; - result.block_height = wtx.m_confirm.block_height > 0 ? wtx.m_confirm.block_height : std::numeric_limits::max(); + int height = wtx.state() ? wtx.state()->confirmed_block_height : + wtx.state() ? wtx.state()->conflicting_block_height : 0; + result.block_height = height > 0 ? height : std::numeric_limits::max(); result.blocks_to_maturity = wallet.GetTxBlocksToMaturity(wtx); result.depth_in_main_chain = wallet.GetTxDepthInMainChain(wtx); result.time_received = wtx.nTimeReceived; diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 99803a91d2724..92c4191a7a868 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -369,11 +369,9 @@ RPCHelpMan importprunedfunds() unsigned int txnIndex = vIndex[it - vMatch.begin()]; - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, height, merkleBlock.header.GetHash(), txnIndex); - CTransactionRef tx_ref = MakeTransactionRef(tx); if (pwallet->IsMine(*tx_ref)) { - pwallet->AddToWallet(std::move(tx_ref), confirm); + pwallet->AddToWallet(std::move(tx_ref), TxStateConfirmed{merkleBlock.header.GetHash(), height, (int)txnIndex}); return NullUniValue; } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 4f2875047e07f..7bec6b6571c19 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -154,13 +154,13 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue entry.pushKV("confirmations", confirms); if (wtx.IsCoinBase()) entry.pushKV("generated", true); - if (confirms > 0) + if (auto* conf = wtx.state()) { - entry.pushKV("blockhash", wtx.m_confirm.hashBlock.GetHex()); - entry.pushKV("blockheight", wtx.m_confirm.block_height); - entry.pushKV("blockindex", wtx.m_confirm.nIndex); + entry.pushKV("blockhash", conf->confirmed_block_hash.GetHex()); + entry.pushKV("blockheight", conf->confirmed_block_height); + entry.pushKV("blockindex", conf->position_in_block); int64_t block_time; - CHECK_NONFATAL(chain.findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(block_time))); + CHECK_NONFATAL(chain.findBlock(conf->confirmed_block_hash, FoundBlock().time(block_time))); entry.pushKV("blocktime", block_time); } else { entry.pushKV("trusted", CachedTxIsTrusted(wallet, wtx)); diff --git a/src/wallet/test/coinselector_tests.cpp b/src/wallet/test/coinselector_tests.cpp index 608a9eb9593c7..1c017870327d6 100644 --- a/src/wallet/test/coinselector_tests.cpp +++ b/src/wallet/test/coinselector_tests.cpp @@ -74,7 +74,7 @@ static void add_coin(CWallet& wallet, const CAmount& nValue, int nAge = 6*24, bo // so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe() tx.vin.resize(1); } - CWalletTx* wtx = wallet.AddToWallet(MakeTransactionRef(std::move(tx)), /* confirm= */ {}); + CWalletTx* wtx = wallet.AddToWallet(MakeTransactionRef(std::move(tx)), TxStateInactive{}); if (fIsFromMe) { wtx->m_amounts[CWalletTx::DEBIT].Set(ISMINE_SPENDABLE, 1); diff --git a/src/wallet/test/psbt_wallet_tests.cpp b/src/wallet/test/psbt_wallet_tests.cpp index ce7e661b677f1..df948442d67cc 100644 --- a/src/wallet/test/psbt_wallet_tests.cpp +++ b/src/wallet/test/psbt_wallet_tests.cpp @@ -22,12 +22,12 @@ BOOST_AUTO_TEST_CASE(psbt_updater_test) CDataStream s_prev_tx1(ParseHex("0200000000010158e87a21b56daf0c23be8e7070456c336f7cbaa5c8757924f545887bb2abdd7501000000171600145f275f436b09a8cc9a2eb2a2f528485c68a56323feffffff02d8231f1b0100000017a914aed962d6654f9a2b36608eb9d64d2b260db4f1118700c2eb0b0000000017a914b7f5faf40e3d40a5a459b1db3535f2b72fa921e88702483045022100a22edcc6e5bc511af4cc4ae0de0fcd75c7e04d8c1c3a8aa9d820ed4b967384ec02200642963597b9b1bc22c75e9f3e117284a962188bf5e8a74c895089046a20ad770121035509a48eb623e10aace8bfd0212fdb8a8e5af3c94b0b133b95e114cab89e4f7965000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx1; s_prev_tx1 >> prev_tx1; - m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx1->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx1)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx1->GetHash()), std::forward_as_tuple(prev_tx1, TxStateInactive{})); CDataStream s_prev_tx2(ParseHex("0200000001aad73931018bd25f84ae400b68848be09db706eac2ac18298babee71ab656f8b0000000048473044022058f6fc7c6a33e1b31548d481c826c015bd30135aad42cd67790dab66d2ad243b02204a1ced2604c6735b6393e5b41691dd78b00f0c5942fb9f751856faa938157dba01feffffff0280f0fa020000000017a9140fb9463421696b82c833af241c78c17ddbde493487d0f20a270100000017a91429ca74f8a08f81999428185c97b5d852e4063f618765000000"), SER_NETWORK, PROTOCOL_VERSION); CTransactionRef prev_tx2; s_prev_tx2 >> prev_tx2; - m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(&m_wallet, prev_tx2)); + m_wallet.mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(prev_tx2->GetHash()), std::forward_as_tuple(prev_tx2, TxStateInactive{})); // Add scripts CScript rs1; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 1f6354793c0ee..742ec65c20b0d 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -314,14 +314,11 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) { CWallet wallet(m_node.chain.get(), "", CreateDummyWalletDatabase()); auto spk_man = wallet.GetOrCreateLegacyScriptPubKeyMan(); - CWalletTx wtx(&wallet, m_coinbase_txns.back()); + CWalletTx wtx{m_coinbase_txns.back(), TxStateConfirmed{::ChainActive().Tip()->GetBlockHash(), ::ChainActive().Height(), /* position_in_block= */ 0}}; LOCK2(wallet.cs_wallet, spk_man->cs_KeyStore); wallet.SetLastBlockProcessed(::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 0); - wtx.m_confirm = confirm; - // Call GetImmatureCredit() once before adding the key to the wallet to // cache the current immature credit amount, which is 0. BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 0); @@ -336,7 +333,7 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup) static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime) { CMutableTransaction tx; - CWalletTx::Confirmation confirm; + TxState state = TxStateInactive{}; tx.nLockTime = lockTime; SetMockTime(mockTime); CBlockIndex* block = nullptr; @@ -348,13 +345,13 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock block = inserted.first->second; block->nTime = blockTime; block->phashBlock = &hash; - confirm = {CWalletTx::Status::CONFIRMED, block->nHeight, hash, 0}; + state = TxStateConfirmed{hash, block->nHeight, /* position_in_block= */ 0}; } - - // If transaction is already in map, to avoid inconsistencies, unconfirmation - // is needed before confirm again with different block. - return wallet.AddToWallet(MakeTransactionRef(tx), confirm, [&](CWalletTx& wtx, bool /* new_tx */) { - wtx.setUnconfirmed(); + return wallet.AddToWallet(MakeTransactionRef(tx), state, [&](CWalletTx& wtx, bool /* new_tx */) { + // Assign wtx.m_state to simplify test and avoid the need to simulate + // reorg events. Without this, AddToWallet asserts false when the same + // transaction is confirmed in different blocks. + wtx.m_state = state; return true; })->nTimeSmart; } @@ -532,8 +529,7 @@ class ListCoinsTestingSetup : public TestChain100Setup wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, ::ChainActive().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - CWalletTx::Confirmation confirm(CWalletTx::Status::CONFIRMED, ::ChainActive().Height(), ::ChainActive().Tip()->GetBlockHash(), 1); - it->second.m_confirm = confirm; + it->second.m_state = TxStateConfirmed{::ChainActive().Tip()->GetBlockHash(), ::ChainActive().Height(), /* position_in_block= */ 1}; return it->second; } diff --git a/src/wallet/test/wallet_transaction_tests.cpp b/src/wallet/test/wallet_transaction_tests.cpp new file mode 100644 index 0000000000000..5ef2904f669b5 --- /dev/null +++ b/src/wallet/test/wallet_transaction_tests.cpp @@ -0,0 +1,24 @@ +// Copyright (c) 2021 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 + +#include + +#include + +BOOST_FIXTURE_TEST_SUITE(wallet_transaction_tests, WalletTestingSetup) + +BOOST_AUTO_TEST_CASE(roundtrip) +{ + for (uint8_t hash = 0; hash < 5; ++hash) { + for (int index = -2; index < 3; ++index) { + TxState state = TxStateInterpretSerialized(TxStateUnrecognized{uint256{hash}, index}); + BOOST_CHECK_EQUAL(TxStateSerializedBlockHash(state), uint256{hash}); + BOOST_CHECK_EQUAL(TxStateSerializedIndex(state), index); + } + } +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp index cf98b516f1773..a926c0ecc1572 100644 --- a/src/wallet/transaction.cpp +++ b/src/wallet/transaction.cpp @@ -15,7 +15,7 @@ bool CWalletTx::IsEquivalentTo(const CWalletTx& _tx) const bool CWalletTx::InMempool() const { - return fInMempool; + return state(); } int64_t CWalletTx::GetTxTime() const diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index f6c524c566da5..8b4dc9db3b871 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -11,12 +11,99 @@ #include #include #include +#include #include #include #include +#include #include +//! State of transaction confirmed in a block. +struct TxStateConfirmed { + uint256 confirmed_block_hash; + int confirmed_block_height; + int position_in_block; + + explicit TxStateConfirmed(const uint256& block_hash, int height, int index) : confirmed_block_hash(block_hash), confirmed_block_height(height), position_in_block(index) {} +}; + +//! State of transaction added to mempool. +struct TxStateInMempool { +}; + +//! State of rejected transaction that conflicts with a confirmed block. +struct TxStateConflicted { + uint256 conflicting_block_hash; + int conflicting_block_height; + + explicit TxStateConflicted(const uint256& block_hash, int height) : conflicting_block_hash(block_hash), conflicting_block_height(height) {} +}; + +//! State of tranaction not in confirmed or conflicting with a known block and +//! not in the mempool. May conflict with the mempool, or with an unknown block, +//! or be abandoned, never broadcast, or rejected from the mempool for another +//! reason. +struct TxStateInactive { + bool abandoned; + + explicit TxStateInactive(bool abandoned=false) : abandoned(abandoned) {} +}; + +//! State of transaction loaded in an unrecognized state with unexpected hash or +//! index values. Treated as inactive (with serialized hash and index values +//! preserved) by default, but may enter another state if transaction is added +//! to the mempool, or confirmed, or abandoned, or found conflicting. +struct TxStateUnrecognized { + uint256 block_hash; + int index; + + TxStateUnrecognized(const uint256& block_hash, int index) : block_hash(block_hash), index(index) {} +}; + +//! All possible CWalletTx states +using TxState = std::variant; + +//! Subset of states transaction sync logic is implemented to handle. +using SyncTxState = std::variant; + +//! Interpret TxState serialized fields as a recognized state. +static TxState TxStateInterpretSerialized(TxStateUnrecognized data) { + if (data.block_hash == uint256::ZERO) { + if (data.index == 0) return TxStateInactive{}; + } else if (data.block_hash == uint256::ONE) { + if (data.index == -1) return TxStateInactive{/* abandoned= */ true}; + } else if (data.index >= 0) { + return TxStateConfirmed{data.block_hash, /* height= */ -1, data.index}; + } else if (data.index == -1) { + return TxStateConflicted{data.block_hash, /* height= */ -1}; + } + return data; +} + +//! Get TxState serialized block hash. Inverse of TxStateInterpretSerialized. +static uint256 TxStateSerializedBlockHash(const TxState& state) { + return std::visit(Overloaded { + [](const TxStateConfirmed& confirmed) { return confirmed.confirmed_block_hash; }, + [](const TxStateConflicted& conflicted) { return conflicted.conflicting_block_hash; }, + [](const TxStateInMempool& in_mempool) { return uint256::ZERO; }, + [](const TxStateInactive& inactive) { return inactive.abandoned ? uint256::ONE : uint256::ZERO; }, + [](const TxStateUnrecognized& unrecognized) { return unrecognized.block_hash; } + }, state); +} + +//! Get TxState serialized block hash. Inverse of TxStateInterpretSerialized. +static int TxStateSerializedIndex(const TxState& state) { + return std::visit(Overloaded { + [](const TxStateConfirmed& confirmed) { return confirmed.position_in_block; }, + [](const TxStateConflicted& conflicted) { return -1; }, + [](const TxStateInMempool& in_mempool) { return 0; }, + [](const TxStateInactive& inactive) { return inactive.abandoned ? -1 : 0; }, + [](const TxStateUnrecognized& unrecognized) { return unrecognized.index; } + }, state); +} + + typedef std::map mapValue_t; @@ -64,12 +151,6 @@ class CMerkleTx */ class CWalletTx { -private: - /** Constant used in hashBlock to indicate tx has been abandoned, only used at - * serialization/deserialization to avoid ambiguity with conflicted. - */ - static constexpr const uint256& ABANDON_HASH = uint256::ONE; - public: /** * Key/value map with information about the transaction. @@ -130,11 +211,11 @@ class CWalletTx */ mutable bool m_is_cache_empty{true}; mutable bool fChangeCached; - mutable bool fInMempool; mutable CAmount nChangeCached; - CWalletTx(const CWallet* wallet, CTransactionRef arg) - : tx(std::move(arg)) + TxState m_state; + + CWalletTx(CTransactionRef tx, const TxState& state) : tx(std::move(tx)), m_state(state) { Init(); } @@ -148,44 +229,12 @@ class CWalletTx nTimeSmart = 0; fFromMe = false; fChangeCached = false; - fInMempool = false; nChangeCached = 0; nOrderPos = -1; - m_confirm = Confirmation{}; } CTransactionRef tx; - /* New transactions start as UNCONFIRMED. At BlockConnected, - * they will transition to CONFIRMED. In case of reorg, at BlockDisconnected, - * they roll back to UNCONFIRMED. If we detect a conflicting transaction at - * block connection, we update conflicted tx and its dependencies as CONFLICTED. - * If tx isn't confirmed and outside of mempool, the user may switch it to ABANDONED - * by using the abandontransaction call. This last status may be override by a CONFLICTED - * or CONFIRMED transition. - */ - enum Status { - UNCONFIRMED, - CONFIRMED, - CONFLICTED, - ABANDONED - }; - - /* Confirmation includes tx status and a triplet of {block height/block hash/tx index in block} - * at which tx has been confirmed. All three are set to 0 if tx is unconfirmed or abandoned. - * Meaning of these fields changes with CONFLICTED state where they instead point to block hash - * and block height of the deepest conflicting tx. - */ - struct Confirmation { - Status status; - int block_height; - uint256 hashBlock; - int nIndex; - Confirmation(Status s = UNCONFIRMED, int b = 0, uint256 h = uint256(), int i = 0) : status(s), block_height(b), hashBlock(h), nIndex(i) {} - }; - - Confirmation m_confirm; - template void Serialize(Stream& s) const { @@ -200,8 +249,8 @@ class CWalletTx std::vector dummy_vector1; //!< Used to be vMerkleBranch std::vector dummy_vector2; //!< Used to be vtxPrev bool dummy_bool = false; //!< Used to be fSpent - uint256 serializedHash = isAbandoned() ? ABANDON_HASH : m_confirm.hashBlock; - int serializedIndex = isAbandoned() || isConflicted() ? -1 : m_confirm.nIndex; + uint256 serializedHash = TxStateSerializedBlockHash(m_state); + int serializedIndex = TxStateSerializedIndex(m_state); s << tx << serializedHash << dummy_vector1 << serializedIndex << dummy_vector2 << mapValueCopy << vOrderForm << fTimeReceivedIsTxTime << nTimeReceived << fFromMe << dummy_bool; } @@ -213,24 +262,11 @@ class CWalletTx std::vector dummy_vector1; //!< Used to be vMerkleBranch std::vector dummy_vector2; //!< Used to be vtxPrev bool dummy_bool; //! Used to be fSpent + uint256 serialized_block_hash; int serializedIndex; - s >> tx >> m_confirm.hashBlock >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_bool; - - /* At serialization/deserialization, an nIndex == -1 means that hashBlock refers to - * the earliest block in the chain we know this or any in-wallet ancestor conflicts - * with. If nIndex == -1 and hashBlock is ABANDON_HASH, it means transaction is abandoned. - * In same context, an nIndex >= 0 refers to a confirmed transaction (if hashBlock set) or - * unconfirmed one. Older clients interpret nIndex == -1 as unconfirmed for backward - * compatibility (pre-commit 9ac63d6). - */ - if (serializedIndex == -1 && m_confirm.hashBlock == ABANDON_HASH) { - setAbandoned(); - } else if (serializedIndex == -1) { - setConflicted(); - } else if (!m_confirm.hashBlock.IsNull()) { - m_confirm.nIndex = serializedIndex; - setConfirmed(); - } + s >> tx >> serialized_block_hash >> dummy_vector1 >> serializedIndex >> dummy_vector2 >> mapValue >> vOrderForm >> fTimeReceivedIsTxTime >> nTimeReceived >> fFromMe >> dummy_bool; + + m_state = TxStateInterpretSerialized({serialized_block_hash, serializedIndex}); ReadOrderPos(nOrderPos, mapValue); nTimeSmart = mapValue.count("timesmart") ? (unsigned int)atoi64(mapValue["timesmart"]) : 0; @@ -264,20 +300,13 @@ class CWalletTx int64_t GetTxTime() const; - bool isAbandoned() const { return m_confirm.status == CWalletTx::ABANDONED; } - void setAbandoned() - { - m_confirm.status = CWalletTx::ABANDONED; - m_confirm.hashBlock = uint256(); - m_confirm.block_height = 0; - m_confirm.nIndex = 0; - } - bool isConflicted() const { return m_confirm.status == CWalletTx::CONFLICTED; } - void setConflicted() { m_confirm.status = CWalletTx::CONFLICTED; } - bool isUnconfirmed() const { return m_confirm.status == CWalletTx::UNCONFIRMED; } - void setUnconfirmed() { m_confirm.status = CWalletTx::UNCONFIRMED; } - bool isConfirmed() const { return m_confirm.status == CWalletTx::CONFIRMED; } - void setConfirmed() { m_confirm.status = CWalletTx::CONFIRMED; } + template const T* state() const { return std::get_if(&m_state); } + template T* state() { return std::get_if(&m_state); } + + bool isAbandoned() const { return state() && state()->abandoned; } + bool isConflicted() const { return state(); } + bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } + bool isConfirmed() const { return state(); } const uint256& GetHash() const { return tx->GetHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 313f3adb70800..706b67db59631 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -835,7 +835,7 @@ bool CWallet::IsSpentKey(const uint256& hash, unsigned int n) const return false; } -CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose) +CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx, bool fFlushOnClose) { LOCK(cs_wallet); @@ -856,12 +856,11 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio } // Inserts only if not already there, returns tx inserted or tx found - auto ret = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, tx)); + auto ret = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(tx, state)); CWalletTx& wtx = (*ret.first).second; bool fInsertedNew = ret.second; bool fUpdated = update_wtx && update_wtx(wtx, fInsertedNew); if (fInsertedNew) { - wtx.m_confirm = confirm; wtx.nTimeReceived = chain().getAdjustedTime(); wtx.nOrderPos = IncOrderPosNext(&batch); wtx.m_it_wtxOrdered = wtxOrdered.insert(std::make_pair(wtx.nOrderPos, &wtx)); @@ -871,16 +870,12 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio if (!fInsertedNew) { - if (confirm.status != wtx.m_confirm.status) { - wtx.m_confirm.status = confirm.status; - wtx.m_confirm.nIndex = confirm.nIndex; - wtx.m_confirm.hashBlock = confirm.hashBlock; - wtx.m_confirm.block_height = confirm.block_height; + if (state.index() != wtx.m_state.index()) { + wtx.m_state = state; fUpdated = true; } else { - assert(wtx.m_confirm.nIndex == confirm.nIndex); - assert(wtx.m_confirm.hashBlock == confirm.hashBlock); - assert(wtx.m_confirm.block_height == confirm.block_height); + assert(TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state)); + assert(TxStateSerializedBlockHash(wtx.m_state) == TxStateSerializedBlockHash(state)); } // If we have a witness-stripped version of this transaction, and we // see a new version with a witness, then we must be upgrading a pre-segwit @@ -932,7 +927,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const CWalletTx::Confirmatio bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) { - const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(this, nullptr)); + const auto& ins = mapWallet.emplace(std::piecewise_construct, std::forward_as_tuple(hash), std::forward_as_tuple(nullptr, TxStateInactive{})); CWalletTx& wtx = ins.first->second; if (!fill_wtx(wtx, ins.second)) { return false; @@ -940,22 +935,21 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx // If wallet doesn't have a chain (e.g wallet-tool), don't bother to update txn. if (HaveChain()) { bool active; - int height; - if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().inActiveChain(active).height(height)) && active) { - // Update cached block height variable since it not stored in the - // serialized transaction. - wtx.m_confirm.block_height = height; - } else if (wtx.isConflicted() || wtx.isConfirmed()) { + auto lookup_block = [&](const uint256& hash, int& height, TxState& state) { // If tx block (or conflicting block) was reorged out of chain // while the wallet was shutdown, change tx status to UNCONFIRMED // and reset block height, hash, and index. ABANDONED tx don't have // associated blocks and don't need to be updated. The case where a // transaction was reorged out while online and then reconfirmed // while offline is covered by the rescan logic. - wtx.setUnconfirmed(); - wtx.m_confirm.hashBlock = uint256(); - wtx.m_confirm.block_height = 0; - wtx.m_confirm.nIndex = 0; + if (!chain().findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) { + state = TxStateInactive{}; + } + }; + if (auto* conf = wtx.state()) { + lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, wtx.m_state); + } else if (auto* conf = wtx.state()) { + lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, wtx.m_state); } } if (/* insertion took place */ ins.second) { @@ -966,27 +960,27 @@ bool CWallet::LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx auto it = mapWallet.find(txin.prevout.hash); if (it != mapWallet.end()) { CWalletTx& prevtx = it->second; - if (prevtx.isConflicted()) { - MarkConflicted(prevtx.m_confirm.hashBlock, prevtx.m_confirm.block_height, wtx.GetHash()); + if (auto* prev = prevtx.state()) { + MarkConflicted(prev->conflicting_block_hash, prev->conflicting_block_height, wtx.GetHash()); } } } return true; } -bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool fUpdate) +bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, const SyncTxState& state, bool fUpdate) { const CTransaction& tx = *ptx; { AssertLockHeld(cs_wallet); - if (!confirm.hashBlock.IsNull()) { + if (auto* conf = std::get_if(&state)) { 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()) { - WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), confirm.hashBlock.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); - MarkConflicted(confirm.hashBlock, confirm.block_height, range.first->second); + WalletLogPrintf("Transaction %s (in block %s) conflicts with wallet transaction %s (both spend %s:%i)\n", tx.GetHash().ToString(), conf->confirmed_block_hash.ToString(), range.first->second.ToString(), range.first->first.hash.ToString(), range.first->first.n); + MarkConflicted(conf->confirmed_block_hash, conf->confirmed_block_height, range.first->second); } range.first++; } @@ -1012,7 +1006,8 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransactionRef& ptx, CWalletTx::Co // Block disconnection override an abandoned tx as unconfirmed // which means user may have to call abandontransaction again - return AddToWallet(MakeTransactionRef(tx), confirm, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false); + TxState tx_state = std::visit([](auto&& s) -> TxState { return s; }, state); + return AddToWallet(MakeTransactionRef(tx), tx_state, /* update_wtx= */ nullptr, /* fFlushOnClose= */ false); } } return false; @@ -1068,7 +1063,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) 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.setAbandoned(); + wtx.m_state = TxStateInactive{/* abandoned= */ true}; wtx.MarkDirty(); batch.WriteTx(wtx); NotifyTransactionChanged(this, wtx.GetHash(), CT_UPDATED); @@ -1120,10 +1115,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c if (conflictconfirms < currentconfirm) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. - wtx.m_confirm.nIndex = 0; - wtx.m_confirm.hashBlock = hashBlock; - wtx.m_confirm.block_height = conflicting_height; - wtx.setConflicted(); + wtx.m_state = TxStateConflicted{hashBlock, conflicting_height}; wtx.MarkDirty(); batch.WriteTx(wtx); // Iterate over all its outputs, and mark transactions in the wallet that spend them conflicted too @@ -1141,9 +1133,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c } } -void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmation confirm, bool update_tx) +void CWallet::SyncTransaction(const CTransactionRef& ptx, const SyncTxState& state, bool update_tx) { - if (!AddToWalletIfInvolvingMe(ptx, confirm, update_tx)) + if (!AddToWalletIfInvolvingMe(ptx, state, update_tx)) return; // Not one of ours // If a transaction changes 'conflicted' state, that changes the balance @@ -1154,19 +1146,19 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, CWalletTx::Confirmatio void CWallet::transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) { LOCK(cs_wallet); - SyncTransaction(tx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}); + SyncTransaction(tx, TxStateInMempool{}); auto it = mapWallet.find(tx->GetHash()); if (it != mapWallet.end()) { - it->second.fInMempool = true; + CHECK_NONFATAL(it->second.InMempool()); } } void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRemovalReason reason, uint64_t mempool_sequence) { LOCK(cs_wallet); auto it = mapWallet.find(tx->GetHash()); - if (it != mapWallet.end()) { - it->second.fInMempool = false; + if (it != mapWallet.end() && it->second.InMempool()) { + it->second.m_state = TxStateInactive{}; } // Handle transactions that were removed from the mempool because they // conflict with transactions in a newly connected block. @@ -1195,7 +1187,7 @@ void CWallet::transactionRemovedFromMempool(const CTransactionRef& tx, MemPoolRe // distinguishing between conflicted and unconfirmed transactions are // imperfect, and could be improved in general, see // https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking - SyncTransaction(tx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}); + SyncTransaction(tx, TxStateInactive{}); } } @@ -1207,7 +1199,7 @@ void CWallet::blockConnected(const CBlock& block, int height) m_last_block_processed_height = height; m_last_block_processed = block_hash; for (size_t index = 0; index < block.vtx.size(); index++) { - SyncTransaction(block.vtx[index], {CWalletTx::Status::CONFIRMED, height, block_hash, (int)index}); + SyncTransaction(block.vtx[index], TxStateConfirmed{block_hash, height, (int)index}); transactionRemovedFromMempool(block.vtx[index], MemPoolRemovalReason::BLOCK, 0 /* mempool_sequence */); } } @@ -1223,7 +1215,7 @@ void CWallet::blockDisconnected(const CBlock& block, int height) m_last_block_processed_height = height - 1; m_last_block_processed = block.hashPrevBlock; for (const CTransactionRef& ptx : block.vtx) { - SyncTransaction(ptx, {CWalletTx::Status::UNCONFIRMED, /* block height */ 0, /* block hash */ {}, /* index */ 0}); + SyncTransaction(ptx, TxStateInactive{}); } } @@ -1585,7 +1577,7 @@ CWallet::ScanResult CWallet::ScanForWalletTransactions(const uint256& start_bloc break; } for (size_t posInBlock = 0; posInBlock < block.vtx.size(); ++posInBlock) { - SyncTransaction(block.vtx[posInBlock], {CWalletTx::Status::CONFIRMED, block_height, block_hash, (int)posInBlock}, fUpdate); + SyncTransaction(block.vtx[posInBlock], TxStateConfirmed{block_hash, block_height, (int)posInBlock}, fUpdate); } // scan succeeded, record block as most recent successfully scanned result.last_scanned_block = block_hash; @@ -1660,7 +1652,7 @@ void CWallet::ReacceptWalletTransactions() } } -bool CWallet::SubmitTxMemoryPoolAndRelay(const CWalletTx& wtx, std::string& err_string, bool relay) const +bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const { // Can't relay if wallet is not broadcasting if (!GetBroadcastTransactions()) return false; @@ -1684,7 +1676,7 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(const CWalletTx& wtx, std::string& err_ // out-of-order is incorrect - it should be unmarked when // TransactionRemovedFromMempool fires. bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, relay, err_string); - wtx.fInMempool |= ret; + if (ret) wtx.m_state = TxStateInMempool{}; return ret; } @@ -1771,7 +1763,8 @@ bool CWallet::SignTransaction(CMutableTransaction& tx) const return false; } const CWalletTx& wtx = mi->second; - coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], wtx.m_confirm.block_height, wtx.IsCoinBase()); + int prev_height = wtx.state() ? wtx.state()->confirmed_block_height : 0; + coins[input.prevout] = Coin(wtx.tx->vout[input.prevout.n], prev_height, wtx.IsCoinBase()); } std::map input_errors; return SignTransaction(tx, coins, SIGHASH_ALL, input_errors); @@ -1889,7 +1882,7 @@ void CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve // Add tx to wallet, because if it has change it's also ours, // otherwise just for transaction history. - AddToWallet(tx, {}, [&](CWalletTx& wtx, bool new_tx) { + AddToWallet(tx, TxStateInactive{}, [&](CWalletTx& wtx, bool new_tx) { CHECK_NONFATAL(wtx.mapValue.empty()); CHECK_NONFATAL(wtx.vOrderForm.empty()); wtx.mapValue = std::move(mapValue); @@ -2237,10 +2230,10 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { } // map in which we'll infer heights of other keys - std::map mapKeyFirstBlock; - CWalletTx::Confirmation max_confirm; - max_confirm.block_height = GetLastBlockHeight() > 144 ? GetLastBlockHeight() - 144 : 0; // the tip can be reorganized; use a 144-block safety margin - CHECK_NONFATAL(chain().findAncestorByHeight(GetLastBlockHash(), max_confirm.block_height, FoundBlock().hash(max_confirm.hashBlock))); + std::map mapKeyFirstBlock; + TxStateConfirmed max_confirm{uint256{}, /* height */ -1, /* index */ -1}; + max_confirm.confirmed_block_height = GetLastBlockHeight() > 144 ? GetLastBlockHeight() - 144 : 0; // the tip can be reorganized; use a 144-block safety margin + CHECK_NONFATAL(chain().findAncestorByHeight(GetLastBlockHash(), max_confirm.confirmed_block_height, FoundBlock().hash(max_confirm.confirmed_block_hash))); for (const CKeyID &keyid : spk_man->GetKeys()) { if (mapKeyBirth.count(keyid) == 0) mapKeyFirstBlock[keyid] = &max_confirm; @@ -2254,15 +2247,15 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { for (const auto& entry : mapWallet) { // iterate over all wallet transactions... const CWalletTx &wtx = entry.second; - if (wtx.m_confirm.status == CWalletTx::CONFIRMED) { + if (auto* conf = wtx.state()) { // ... which are already in a block for (const CTxOut &txout : wtx.tx->vout) { // iterate over all their outputs for (const auto &keyid : GetAffectedKeys(txout.scriptPubKey, *spk_man)) { // ... and all their affected keys auto rit = mapKeyFirstBlock.find(keyid); - if (rit != mapKeyFirstBlock.end() && wtx.m_confirm.block_height < rit->second->block_height) { - rit->second = &wtx.m_confirm; + if (rit != mapKeyFirstBlock.end() && conf->confirmed_block_height < rit->second->confirmed_block_height) { + rit->second = conf; } } } @@ -2272,7 +2265,7 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { // Extract block timestamps for those keys for (const auto& entry : mapKeyFirstBlock) { int64_t block_time; - CHECK_NONFATAL(chain().findBlock(entry.second->hashBlock, FoundBlock().time(block_time))); + CHECK_NONFATAL(chain().findBlock(entry.second->confirmed_block_hash, FoundBlock().time(block_time))); mapKeyBirth[entry.first] = block_time - TIMESTAMP_WINDOW; // block times can be 2h off } } @@ -2300,10 +2293,17 @@ void CWallet::GetKeyBirthTimes(std::map& mapKeyBirth) const { */ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const { + std::optional block_hash; + if (auto* conf = wtx.state()) { + block_hash = conf->confirmed_block_hash; + } else if (auto* conf = wtx.state()) { + block_hash = conf->conflicting_block_hash; + } + unsigned int nTimeSmart = wtx.nTimeReceived; - if (!wtx.isUnconfirmed() && !wtx.isAbandoned()) { + if (block_hash) { int64_t blocktime; - if (chain().findBlock(wtx.m_confirm.hashBlock, FoundBlock().time(blocktime))) { + if (chain().findBlock(*block_hash, FoundBlock().time(blocktime))) { int64_t latestNow = wtx.nTimeReceived; int64_t latestEntry = 0; @@ -2331,7 +2331,7 @@ unsigned int CWallet::ComputeTimeSmart(const CWalletTx& wtx) const nTimeSmart = std::max(latestEntry, std::min(blocktime, latestNow)); } else { - WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), wtx.m_confirm.hashBlock.ToString()); + WalletLogPrintf("%s: found %s in block %s not in index\n", __func__, wtx.GetHash().ToString(), block_hash->ToString()); } } return nTimeSmart; @@ -2786,9 +2786,13 @@ CKeyPool::CKeyPool(const CPubKey& vchPubKeyIn, bool internalIn) int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const { AssertLockHeld(cs_wallet); - if (wtx.isUnconfirmed() || wtx.isAbandoned()) return 0; - - return (GetLastBlockHeight() - wtx.m_confirm.block_height + 1) * (wtx.isConflicted() ? -1 : 1); + if (auto* conf = wtx.state()) { + return GetLastBlockHeight() - conf->confirmed_block_height + 1; + } else if (auto* conf = wtx.state()) { + return -1 * (GetLastBlockHeight() - conf->conflicting_block_height + 1); + } else { + return 0; + } } int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index c19aeee082c17..81087a764a235 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -284,7 +284,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati * Abandoned state should probably be more carefully tracked via different * posInBlock signals or by checking mempool presence when necessary. */ - bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + bool AddToWalletIfInvolvingMe(const CTransactionRef& tx, const SyncTxState& state, bool fUpdate) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); /* Mark a transaction (and its in-wallet descendants) as conflicting with a particular block. */ void MarkConflicted(const uint256& hashBlock, int conflicting_height, const uint256& hashTx); @@ -296,7 +296,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati /* Used by TransactionAddedToMemorypool/BlockConnected/Disconnected/ScanForWalletTransactions. * Should be called with non-zero block_hash and posInBlock if this is for a transaction that is included in a block. */ - void SyncTransaction(const CTransactionRef& tx, CWalletTx::Confirmation confirm, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); + void SyncTransaction(const CTransactionRef& tx, const SyncTxState& state, bool update_tx = true) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); std::atomic m_wallet_flags{0}; @@ -506,7 +506,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati //! @return true if wtx is changed and needs to be saved to disk, otherwise false using UpdateWalletTxFn = std::function; - CWalletTx* AddToWallet(CTransactionRef tx, const CWalletTx::Confirmation& confirm, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true); + CWalletTx* AddToWallet(CTransactionRef tx, const TxState& state, const UpdateWalletTxFn& update_wtx=nullptr, bool fFlushOnClose=true); bool LoadToWallet(const uint256& hash, const UpdateWalletTxFn& fill_wtx) EXCLUSIVE_LOCKS_REQUIRED(cs_wallet); void transactionAddedToMempool(const CTransactionRef& tx, uint64_t mempool_sequence) override; void blockConnected(const CBlock& block, int height) override; @@ -574,7 +574,7 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati void CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::vector> orderForm); // Pass this transaction to node for mempool insertion and relay to peers if flag set to true - bool SubmitTxMemoryPoolAndRelay(const CWalletTx& wtx, std::string& err_string, bool relay) const; + bool SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string, bool relay) const; bool DummySignTx(CMutableTransaction &txNew, const std::set &txouts, bool use_max_sig = false) const { diff --git a/src/wallet/walletdb.cpp b/src/wallet/walletdb.cpp index 69854cae054ed..62a996827129c 100644 --- a/src/wallet/walletdb.cpp +++ b/src/wallet/walletdb.cpp @@ -891,7 +891,7 @@ DBErrors WalletBatch::FindWalletTx(std::vector& vTxHash, std::list> hash; vTxHash.push_back(hash); - vWtx.emplace_back(nullptr /* wallet */, nullptr /* tx */); + vWtx.emplace_back(nullptr /* tx */, TxStateInactive{}); ssValue >> vWtx.back(); } }