wallet: Remove unused CWalletTx CopyFrom and copy constructor
What the developer wrote
Authored by Ava Chow
wallet: Remove unused CWalletTx CopyFrom and copy constructor
What changed, and why it matters
This commit removes an unused explicit copy helper and strengthens a safety guard in Bitcoin Core's wallet code. Previously, copying a wallet transaction object was allowed through a private default copy constructor and an explicit CopyFrom() method. The change deletes both, so the object can no longer be accidentally duplicated. The commit message and code comment explain this is meant to prevent bugs where updates happen on the wrong copy of a transaction, not to fix an active security vulnerability.
No urgent action required. Treat as routine defensive cleanup. Reviewers may verify that no remaining code path relies on CWalletTx copy semantics, and that move semantics cover legitimate use cases.
Security signals we found
Defensive hardening: explicitly deleting copy operations for a mutable wallet object
Code comment explicitly frames change as bug-prevention, not security fix
No functional bug or exploit path is described in commit or diff
Evidence from the diff
The patch deletes CWalletTx::CopyFrom() and changes the copy constructor and copy-assignment operator from private-default to deleted. The accompanying comment already stated the goal: avoid bugs caused by CWalletTx instances being copied into/out of mapWallet and then updated on the wrong copy. The move constructor remains default. No callers of CopyFrom existed, so this is a hardening/cleanup change rather than a fix for an exploitable flaw.
Changed components
src/wallet/transaction.hsrc/wallet/transaction.cppCWalletTx classInspect captured patch +2 / −11
diff --git a/src/wallet/transaction.cpp b/src/wallet/transaction.cpp
index 3d82d55f..f1bf62aa 100644
--- a/src/wallet/transaction.cpp
+++ b/src/wallet/transaction.cpp
@@ -55,9 +55,4 @@ void CWalletTx::updateState(interfaces::Chain& chain)
lookup_block(conf->conflicting_block_hash, conf->conflicting_block_height, m_state);
}
}
-
-void CWalletTx::CopyFrom(const CWalletTx& _tx)
-{
- *this = _tx;
-}
} // namespace wallet
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index a715aa81..c65d2c69 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -382,15 +382,11 @@ public:
const Wtxid& GetWitnessHash() const LIFETIMEBOUND { return tx->GetWitnessHash(); }
bool IsCoinBase() const { return tx->IsCoinBase(); }
-private:
// Disable copying of CWalletTx objects to prevent bugs where instances get
// copied in and out of the mapWallet map, and fields are updated in the
// wrong copy.
- CWalletTx(const CWalletTx&) = default;
- CWalletTx& operator=(const CWalletTx&) = default;
-public:
- // Instead have an explicit copy function
- void CopyFrom(const CWalletTx&);
+ CWalletTx(const CWalletTx&) = delete;
+ CWalletTx& operator=(const CWalletTx&) = delete;
// Enable the default move constructor
CWalletTx(CWalletTx&&) = default;
Why this scored 22/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.