← Watch feed
clusterlin: avoid depgraph argument in SanityCheck (cleanup)
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Pieter Wuille
clusterlin: avoid depgraph argument in SanityCheck (cleanup)
Since the deterministic ordering change, SpanningForestState holds a
reference to the DepGraph it is linearizing. So this means we do not
need to pass it to SanityCheck() as an argument anymore.
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a small internal cleanup in Bitcoin Core's transaction-cluster linearization code. It removes an unnecessary argument from a debugging/self-check function called SanityCheck(), because the object already stores a reference to the same data. There is no user-facing change, no network behavior change, and no security fix.
Recommended action
No security action needed. Treat as normal code-cleanup/refactor.
Security signals we found
No strong security signals were identified.
Evidence from the diff
SpanningForestState already holds m_depgraph, so passing depgraph to SanityCheck() was redundant. The patch changes SanityCheck() to use the member m_depgraph instead of a parameter, and updates the fuzz test call site. All assertions remain identical. This is a pure refactor with no functional or security change.
Changed components
src/cluster_linearize.hsrc/test/fuzz/cluster_linearize.cppInspect captured patch +10 / −10
diff --git a/src/cluster_linearize.h b/src/cluster_linearize.h
index 9a2166bc..d2041b9b 100644
--- a/src/cluster_linearize.h
+++ b/src/cluster_linearize.h
@@ -1440,7 +1440,7 @@ public:
uint64_t GetCost() const noexcept { return m_cost; }
/** Verify internal consistency of the data structure. */
- void SanityCheck(const DepGraph<SetType>& depgraph) const
+ void SanityCheck() const
{
//
// Verify dependency parent/child information, and build list of (active) dependencies.
@@ -1448,8 +1448,8 @@ public:
std::vector<std::pair<TxIdx, TxIdx>> expected_dependencies;
std::vector<std::tuple<TxIdx, TxIdx, DepIdx>> all_dependencies;
std::vector<std::tuple<TxIdx, TxIdx, DepIdx>> active_dependencies;
- for (auto parent_idx : depgraph.Positions()) {
- for (auto child_idx : depgraph.GetReducedChildren(parent_idx)) {
+ for (auto parent_idx : m_depgraph.Positions()) {
+ for (auto child_idx : m_depgraph.GetReducedChildren(parent_idx)) {
expected_dependencies.emplace_back(parent_idx, child_idx);
}
}
@@ -1473,7 +1473,7 @@ public:
//
// Verify the chunks against the list of active dependencies
//
- for (auto tx_idx: depgraph.Positions()) {
+ for (auto tx_idx: m_depgraph.Positions()) {
// Only process chunks for now.
if (m_tx_data[tx_idx].chunk_rep == tx_idx) {
const auto& chunk_data = m_tx_data[tx_idx];
@@ -1505,14 +1505,14 @@ public:
assert(chunk_data.chunk_setinfo.transactions == expected_chunk);
// Verify the chunk's feerate.
assert(chunk_data.chunk_setinfo.feerate ==
- depgraph.FeeRate(chunk_data.chunk_setinfo.transactions));
+ m_depgraph.FeeRate(chunk_data.chunk_setinfo.transactions));
}
}
//
// Verify other transaction data.
//
- assert(m_transaction_idxs == depgraph.Positions());
+ assert(m_transaction_idxs == m_depgraph.Positions());
for (auto tx_idx : m_transaction_idxs) {
const auto& tx_data = m_tx_data[tx_idx];
// Verify it has a valid chunk representative, and that chunk includes this
@@ -1520,8 +1520,8 @@ public:
assert(m_tx_data[tx_data.chunk_rep].chunk_rep == tx_data.chunk_rep);
assert(m_tx_data[tx_data.chunk_rep].chunk_setinfo.transactions[tx_idx]);
// Verify parents/children.
- assert(tx_data.parents == depgraph.GetReducedParents(tx_idx));
- assert(tx_data.children == depgraph.GetReducedChildren(tx_idx));
+ assert(tx_data.parents == m_depgraph.GetReducedParents(tx_idx));
+ assert(tx_data.children == m_depgraph.GetReducedChildren(tx_idx));
// Verify list of child dependencies.
std::vector<DepIdx> expected_child_deps;
for (const auto& [par_idx, chl_idx, dep_idx] : all_dependencies) {
@@ -1559,7 +1559,7 @@ public:
assert(dep_data.top_setinfo.transactions == expected_top);
// Verify the top_info's feerate.
assert(dep_data.top_setinfo.feerate ==
- depgraph.FeeRate(dep_data.top_setinfo.transactions));
+ m_depgraph.FeeRate(dep_data.top_setinfo.transactions));
}
//
diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp
index b4df22a5..5dc7eb21 100644
--- a/src/test/fuzz/cluster_linearize.cpp
+++ b/src/test/fuzz/cluster_linearize.cpp
@@ -919,7 +919,7 @@ FUZZ_TARGET(clusterlin_sfl)
if (rng.randbits(4) == 0) {
// Perform sanity checks from time to time (too computationally expensive to do after
// every step).
- sfl.SanityCheck(depgraph);
+ sfl.SanityCheck();
}
auto diagram = sfl.GetDiagram();
if (rng.randbits(4) == 0) {
Risk score
Our methodology →Why this scored 15/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.