From fa51521e953ac41ad7d4005bd526c351ccd110ba Mon Sep 17 00:00:00 2001 From: Ava Chow Date: Tue, 20 Feb 2024 11:54:42 -0500 Subject: [PATCH] wallet: Make CWalletTx::m_state private with {get,set}ters --- src/wallet/receive.cpp | 2 +- src/wallet/test/wallet_tests.cpp | 4 ++-- src/wallet/transaction.cpp | 8 ++++---- src/wallet/transaction.h | 5 +++++ src/wallet/wallet.cpp | 30 +++++++++++++++--------------- 5 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp index d8aa2c1718b..035f59dd157 100644 --- a/src/wallet/receive.cpp +++ b/src/wallet/receive.cpp @@ -259,7 +259,7 @@ bool CachedTxIsTrusted(const CWallet& wallet, const TxState& state, const uint25 bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx, std::set& trusted_parents) { - return CachedTxIsTrusted(wallet, wtx.m_state, wtx.GetHash(), trusted_parents); + return CachedTxIsTrusted(wallet, wtx.GetState(), wtx.GetHash(), trusted_parents); } bool CachedTxIsTrusted(const CWallet& wallet, const CWalletTx& wtx) diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 165fc2c03e7..19f2cd273c6 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -384,7 +384,7 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock // 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; + wtx.SetState(state); return true; })->nTimeSmart; } @@ -583,7 +583,7 @@ public: wallet->SetLastBlockProcessed(wallet->GetLastBlockHeight() + 1, m_node.chainman->ActiveChain().Tip()->GetBlockHash()); auto it = wallet->mapWallet.find(tx->GetHash()); BOOST_CHECK(it != wallet->mapWallet.end()); - it->second.m_state = TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}; + it->second.SetState(TxStateConfirmed{m_node.chainman->ActiveChain().Tip()->GetBlockHash(), m_node.chainman->ActiveChain().Height(), /*index=*/1}); return it->second; } diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp index 561880482f8..6dfaabd0921 100644 --- a/src/wallet/transaction.cpp +++ b/src/wallet/transaction.cpp @@ -32,7 +32,7 @@ int64_t CWalletTx::GetTxTime() const void CWalletTx::updateState(interfaces::Chain& chain) { bool active; - auto lookup_block = [&](const uint256& hash, int& height, TxState& state) { + auto lookup_block = [&](const uint256& hash, int& height) { // 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 @@ -40,13 +40,13 @@ void CWalletTx::updateState(interfaces::Chain& chain) // transaction was reorged out while online and then reconfirmed // while offline is covered by the rescan logic. if (!chain.findBlock(hash, FoundBlock().inActiveChain(active).height(height)) || !active) { - state = TxStateInactive{}; + SetState(TxStateInactive{}); } }; if (auto* conf = state()) { - lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height, m_state); + lookup_block(conf->confirmed_block_hash, conf->confirmed_block_height); } else if (auto* conf = state()) { - lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, m_state); + lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height); } } diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index b5c9450bb97..fa0286eac99 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -256,8 +256,11 @@ public: } CTransactionRef tx; + +private: TxState m_state; +public: // Set of mempool transactions that conflict // directly with the transaction, or that conflict // with an ancestor transaction. This set will be @@ -339,6 +342,8 @@ public: template const T* state() const { return std::get_if(&m_state); } template T* state() { return std::get_if(&m_state); } + void SetState(const TxState& state) { m_state = state; } + const TxState& GetState() const { return m_state; } //! Update transaction state when attaching to a chain, filling in heights //! of conflicted and confirmed blocks diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 0156dae0094..c370c48af9b 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -147,7 +147,7 @@ static void RefreshMempoolStatus(CWallet& wallet, CWalletTx& tx, interfaces::Cha state = TxStateInactive(); } if (state) { - tx.m_state = *state; + tx.SetState(*state); wallet.RefreshSingleTxTXOs(tx); } } @@ -1110,8 +1110,8 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const if (!fInsertedNew) { - if (state.index() != wtx.m_state.index()) { - wtx.m_state = state; + if (state.index() != wtx.GetState().index()) { + wtx.SetState(state); for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) { COutPoint outpoint(wtx.GetHash(), i); auto it = m_txos.find(outpoint); @@ -1121,8 +1121,8 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const } fUpdated = true; } else { - assert(TxStateSerializedIndex(wtx.m_state) == TxStateSerializedIndex(state)); - assert(TxStateSerializedBlockHash(wtx.m_state) == TxStateSerializedBlockHash(state)); + assert(TxStateSerializedIndex(wtx.GetState()) == TxStateSerializedIndex(state)); + assert(TxStateSerializedBlockHash(wtx.GetState()) == 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 @@ -1144,7 +1144,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const while (!txs.empty()) { CWalletTx* desc_tx = txs.back(); txs.pop_back(); - desc_tx->m_state = inactive_state; + desc_tx->SetState(inactive_state); // Break caches since we have changed the state desc_tx->MarkDirty(); batch.WriteTx(*desc_tx); @@ -1349,7 +1349,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx) assert(!wtx.InMempool()); // If already conflicted or abandoned, no need to set abandoned if (!wtx.isBlockConflicted() && !wtx.isAbandoned()) { - wtx.m_state = TxStateInactive{/*abandoned=*/true}; + wtx.SetState(TxStateInactive{/*abandoned=*/true}); return TxUpdate::NOTIFY_CHANGED; } return TxUpdate::UNCHANGED; @@ -1385,7 +1385,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, int conflicting_height, c if (conflictconfirms < GetTxDepthInMainChain(wtx)) { // Block is 'more conflicted' than current confirm; update. // Mark transaction as conflicted with this block. - wtx.m_state = TxStateBlockConflicted{hashBlock, conflicting_height}; + wtx.SetState(TxStateBlockConflicted{hashBlock, conflicting_height}); return TxUpdate::CHANGED; } return TxUpdate::UNCHANGED; @@ -1425,7 +1425,7 @@ void CWallet::RecursiveUpdateTxState(WalletBatch* batch, const uint256& tx_hash, COutPoint outpoint(Txid::FromUint256(wtx.GetHash()), i); auto it = m_txos.find(outpoint); if (it != m_txos.end()) { - it->second.SetState(wtx.m_state); + it->second.SetState(wtx.GetState()); } std::pair range = mapTxSpends.equal_range(COutPoint(Txid::FromUint256(now), i)); for (TxSpends::const_iterator iter = range.first; iter != range.second; ++iter) { @@ -1587,7 +1587,7 @@ void CWallet::blockDisconnected(const interfaces::BlockInfo& block) auto try_updating_state = [&](CWalletTx& tx) { if (!tx.isBlockConflicted()) return TxUpdate::UNCHANGED; if (tx.state()->conflicting_block_height >= disconnect_height) { - tx.m_state = TxStateInactive{}; + tx.SetState(TxStateInactive{}); return TxUpdate::CHANGED; } return TxUpdate::UNCHANGED; @@ -2073,7 +2073,7 @@ bool CWallet::SubmitTxMemoryPoolAndRelay(CWalletTx& wtx, std::string& err_string // TransactionRemovedFromMempool fires. bool ret = chain().broadcastTransaction(wtx.tx, m_default_max_tx_fee, relay, err_string); if (ret) { - wtx.m_state = TxStateInMempool{}; + wtx.SetState(TxStateInMempool{}); for (unsigned int i = 0; i < wtx.tx->vout.size(); ++i) { COutPoint outpoint(wtx.GetHash(), i); auto it = m_txos.find(outpoint); @@ -3499,7 +3499,7 @@ int CWallet::GetTxStateDepthInMainChain(const TxState& state) const int CWallet::GetTxDepthInMainChain(const CWalletTx& wtx) const { AssertLockHeld(cs_wallet); - return GetTxStateDepthInMainChain(wtx.m_state); + return GetTxStateDepthInMainChain(wtx.GetState()); } int CWallet::GetTxStateBlocksToMaturity(const TxState& state) const @@ -3517,7 +3517,7 @@ int CWallet::GetTxBlocksToMaturity(const CWalletTx& wtx) const if (!wtx.IsCoinBase()) { return 0; } - return GetTxStateBlocksToMaturity(wtx.m_state); + return GetTxStateBlocksToMaturity(wtx.GetState()); } bool CWallet::IsTxImmatureCoinBase(const CWalletTx& wtx) const @@ -4707,9 +4707,9 @@ void CWallet::RefreshSingleTxTXOs(const CWalletTx& wtx) if (it != m_txos.end()) { it->second.SetIsMine(ismine); - it->second.SetState(wtx.m_state); + it->second.SetState(wtx.GetState()); } else { - m_txos.emplace(outpoint, WalletTXO{wtx, txout, ismine, wtx.m_state, wtx.IsCoinBase(), wtx.m_from_me, wtx.GetTxTime()}); + m_txos.emplace(outpoint, WalletTXO{wtx, txout, ismine, wtx.GetState(), wtx.IsCoinBase(), wtx.m_from_me, wtx.GetTxTime()}); } } }