txgraph: drop move assignment operator
What the developer wrote
Authored by Anthony Towns
txgraph: drop move assignment operator
What changed, and why it matters
This commit removes the ability to reassign a TxGraph::Ref after it is created (move assignment), while keeping the ability to construct a new Ref by moving an existing one (move construction). The change is framed as a cleanup to prevent misuse of a reference-counting-like handle in Bitcoin Core's transaction graph code. There is no direct evidence in the commit that this fixes an active security bug, but removing a complex operator can reduce the risk of lifetime or double-management bugs.
Treat as a defensive hardening change. Reviewers should verify that no remaining code paths attempt move assignment of TxGraph::Ref and that the move constructor correctly maintains graph invariants. No urgent action is required unless additional context shows this was a fix for a reachable bug.
Security signals we found
Removal of move assignment for a resource-managing handle
Simplification of object lifetime and graph reference tracking
Fuzz target updated to avoid the deleted operator
No CVE, advisory, or security disclosure referenced in commit
Evidence from the diff
The patch deletes TxGraph::Ref::operator=(Ref&&) in src/txgraph.cpp and marks it = delete in src/txgraph.h. Move construction (Ref(Ref&&)) remains. The fuzz test is updated so that a Ref is passed into the simulation by move construction rather than by default construction followed by move assignment. The deleted operator previously unlinked the current Ref from its graph, updated the other graph’s pointer to the new object address, and nullified the source. Removing it eliminates a code path that could be invoked incorrectly and simplifies the invariant that a Ref is either default-constructed or move-constructed exactly once.
Changed components
src/txgraph.hsrc/txgraph.cppsrc/test/fuzz/txgraph.cppInspect captured patch +5 / −22
diff --git a/src/test/fuzz/txgraph.cpp b/src/test/fuzz/txgraph.cpp
index fbf22bfc..79ee0a16 100644
--- a/src/test/fuzz/txgraph.cpp
+++ b/src/test/fuzz/txgraph.cpp
@@ -138,19 +138,18 @@ struct SimTxGraph
}
/** Add a new transaction to the simulation. */
- TxGraph::Ref* AddTransaction(const FeePerWeight& feerate)
+ void AddTransaction(TxGraph::Ref&& ref, const FeePerWeight& feerate)
{
assert(graph.TxCount() < MAX_TRANSACTIONS);
auto simpos = graph.AddTransaction(feerate);
real_is_optimal = false;
MakeModified(simpos);
assert(graph.Positions()[simpos]);
- simmap[simpos] = std::make_shared<TxGraph::Ref>();
+ simmap[simpos] = std::make_shared<TxGraph::Ref>(std::move(ref));
auto ptr = simmap[simpos].get();
simrevmap[ptr] = simpos;
// This may invalidate our cached oversized value.
if (oversized.has_value() && !*oversized) oversized = std::nullopt;
- return ptr;
}
/** Add a dependency between two positions in this graph. */
@@ -459,9 +458,7 @@ FUZZ_TARGET(txgraph)
// Create a real TxGraph::Ref.
auto ref = real->AddTransaction(feerate);
// Create a shared_ptr place in the simulation to put the Ref in.
- auto ref_loc = top_sim.AddTransaction(feerate);
- // Move it in place.
- *ref_loc = std::move(ref);
+ top_sim.AddTransaction(std::move(ref), feerate);
break;
} else if ((block_builders.empty() || sims.size() > 1) && top_sim.GetTransactionCount() + top_sim.removed.size() > 1 && command-- == 0) {
// AddDependency.
diff --git a/src/txgraph.cpp b/src/txgraph.cpp
index ae85c546..23094bd8 100644
--- a/src/txgraph.cpp
+++ b/src/txgraph.cpp
@@ -3451,20 +3451,6 @@ TxGraph::Ref::~Ref()
}
}
-TxGraph::Ref& TxGraph::Ref::operator=(Ref&& other) noexcept
-{
- // Unlink the current graph, if any.
- if (m_graph) m_graph->UnlinkRef(m_index);
- // Inform the other's graph about the move, if any.
- if (other.m_graph) other.m_graph->UpdateRef(other.m_index, *this);
- // Actually update the contents.
- m_graph = other.m_graph;
- m_index = other.m_index;
- other.m_graph = nullptr;
- other.m_index = GraphIndex(-1);
- return *this;
-}
-
TxGraph::Ref::Ref(Ref&& other) noexcept
{
// Inform the TxGraph of other that its Ref is being moved.
diff --git a/src/txgraph.h b/src/txgraph.h
index 1d8d31a5..0e1f2c79 100644
--- a/src/txgraph.h
+++ b/src/txgraph.h
@@ -241,8 +241,8 @@ public:
/** Destroy this Ref. If it is not empty, the corresponding transaction is removed (in both
* main and staging, if it exists). */
virtual ~Ref();
- // Support moving a Ref.
- Ref& operator=(Ref&& other) noexcept;
+ // Support move-constructing a Ref.
+ Ref& operator=(Ref&& other) noexcept = delete;
Ref(Ref&& other) noexcept;
// Do not permit copy constructing or copy assignment. A TxGraph entry can have at most one
// Ref pointing to it.
Why this scored 24/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.