What the developer wrote
Authored by SChernykh
Refactored tx verification NIC logic
Encapsulated `nic_verified_hf_version` and `txs_by_txid` to make it impossible for callers to skip updating it.
What changed, and why it matters
This commit is a defensive code cleanup in Monero's transaction verification system. It restructures how a temporary list of extra block transactions (the 'pool supplement') is handled so that callers cannot accidentally forget to mark the list as unverified when adding new transactions. Before, outside code directly inserted transactions and separately reset a verification flag, which created a risk that someone could add a transaction without resetting the flag and potentially use a stale 'already verified' status. Now, adding or clearing transactions automatically resets the flag, and only the dedicated verification function can set it to a valid version. There is no direct evidence in the commit of an actual exploit or bug being fixed, but the change removes a fragile pattern that could lead to consensus or security issues.
Treat this as a hardening improvement rather than an urgent security patch. Reviewers should verify that all call sites now use `add_tx()` and `clear()` and that no direct access to `txs_by_txid` or `nic_verified_hf_version` remains elsewhere in the codebase. Consider whether any existing tests cover pool supplement invalidation behavior, and add regression tests if absent. No immediate deployment urgency is indicated by the supplied materials.
Security signals we found
Encapsulation of mutable verification state to prevent caller-forgotten invalidation
Automatic reset of `nic_verified_hf_version` on any transaction addition
Restriction of `nic_verified_hf_version` writes to a single friend verification function
Refactoring of transaction verification 'non-input consensus' (NIC) bookkeeping
No explicit bug, CVE, or exploit described in commit or references
Evidence from the diff
The patch refactors pool_supplement in src/cryptonote_core/tx_verification_utils.h to encapsulate its internal txs_by_txid map and nic_verified_hf_version field. It adds accessor methods (begin, end, find, erase, size, clear) and an add_tx() method that automatically invalidates nic_verified_hf_version. The verification flag is now private and mutable only by the friend ver_non_input_consensus(pool_supplement&, ...) function. Call sites in blockchain.cpp, cryptonote_core.cpp, and cryptonote_protocol_handler.inl are updated to use the new interface. The protocol handler no longer manually resets the flag before populating the supplement because add_tx() does so. This is a hardening change that prevents callers from skipping the invalidation step, but the commit message and diff do not describe a specific vulnerability or incident.
Changed components
src/cryptonote_core/tx_verification_utils.hsrc/cryptonote_core/tx_verification_utils.cppsrc/cryptonote_core/blockchain.cppsrc/cryptonote_core/cryptonote_core.cppsrc/cryptonote_protocol/cryptonote_protocol_handler.inlInspect captured patch +46 / −15
diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index 09e6a3f..a7a0e01 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -2057,7 +2057,7 @@ bool Blockchain::handle_alternative_block(const block& b, const crypto::hash& id
// Add pool supplement txs to the main mempool with relay_method::block
CRITICAL_REGION_LOCAL(m_tx_pool);
- for (auto& extra_block_tx : extra_block_txs.txs_by_txid)
+ for (auto& extra_block_tx : extra_block_txs)
{
const crypto::hash& txid = extra_block_tx.first;
transaction& tx = extra_block_tx.second.first;
@@ -2084,8 +2084,7 @@ bool Blockchain::handle_alternative_block(const block& b, const crypto::hash& id
.weight = get_transaction_weight(tx),
.res = true}});
}
- extra_block_txs.txs_by_txid.clear();
- extra_block_txs.nic_verified_hf_version = 0;
+ extra_block_txs.clear();
bei.block_cumulative_weight = cryptonote::get_transaction_weight(b.miner_tx);
for (const crypto::hash &txid: b.tx_hashes)
@@ -4176,15 +4175,15 @@ leave:
bool find_tx_failure{!found_tx_in_pool};
if (!found_tx_in_pool) // if not in mempool:
{
- const auto extra_txs_it{extra_block_txs.txs_by_txid.find(tx_id)};
- if (extra_txs_it != extra_block_txs.txs_by_txid.end()) // if in block supplement:
+ const auto extra_txs_it{extra_block_txs.find(tx_id)};
+ if (extra_txs_it != extra_block_txs.end()) // if in block supplement:
{
tx = std::move(extra_txs_it->second.first);
txblob = std::move(extra_txs_it->second.second);
tx_weight = tx.pruned ? get_pruned_transaction_weight(tx) : get_transaction_weight(tx, txblob.size());
fee = get_tx_fee(tx);
pruned = tx.pruned;
- extra_block_txs.txs_by_txid.erase(extra_txs_it);
+ extra_block_txs.erase(extra_txs_it);
txpool_events.emplace_back(txpool_event{tx, tx_id, txblob.size(), tx_weight, true});
find_tx_failure = false;
}
@@ -4197,7 +4196,7 @@ leave:
// txs twice.
if (find_tx_failure) // did not find txid in mempool or provided extra block txs
{
- const bool fully_supplemented_block = extra_block_txs.txs_by_txid.size() >= bl.tx_hashes.size();
+ const bool fully_supplemented_block = extra_block_txs.size() >= bl.tx_hashes.size();
if (fully_supplemented_block)
MERROR_VER("Block with id: " << id << " has at least one unknown transaction with id: " << tx_id);
else
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index 4ab8b1d..dcbb2d2 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -1460,7 +1460,7 @@ namespace cryptonote
// saving each block, then it doesn't matter either way: cleanup_handle_incoming_blocks()
// always triggers a sync.
size_t block_total_bytes = block_blob.size();
- for (const auto &t : extra_block_txs.txs_by_txid)
+ for (const auto &t : extra_block_txs)
block_total_bytes += t.second.second.size();
CRITICAL_REGION_LOCAL(m_incoming_tx_lock);
diff --git a/src/cryptonote_core/tx_verification_utils.cpp b/src/cryptonote_core/tx_verification_utils.cpp
index 809faf7..aee5aa3 100644
--- a/src/cryptonote_core/tx_verification_utils.cpp
+++ b/src/cryptonote_core/tx_verification_utils.cpp
@@ -469,7 +469,7 @@ bool ver_non_input_consensus(const transaction& tx, tx_verification_context& tvc
return ver_non_input_consensus_templated(&tx, &tx + 1, tvc, hf_version);
}
-bool ver_non_input_consensus(const pool_supplement& ps, tx_verification_context& tvc,
+bool ver_non_input_consensus(pool_supplement& ps, tx_verification_context& tvc,
const std::uint8_t hf_version)
{
// We already verified the pool supplement for this hard fork version! Yippee!
diff --git a/src/cryptonote_core/tx_verification_utils.h b/src/cryptonote_core/tx_verification_utils.h
index e27b21b..9af705a 100644
--- a/src/cryptonote_core/tx_verification_utils.h
+++ b/src/cryptonote_core/tx_verification_utils.h
@@ -89,12 +89,46 @@ bool ver_mixed_rct_semantics(std::vector<const rct::rctSig*> rvv);
*/
struct pool_supplement
{
+public:
+ using txs_by_txid_t = std::unordered_map<crypto::hash, std::pair<transaction, blobdata>>;
+
+ bool add_tx(const crypto::hash& id, transaction&& tx, const blobdata& blob)
+ {
+ // Any new tx invalidates previous verification
+ nic_verified_hf_version = 0;
+
+ return txs_by_txid.emplace(id, std::make_pair(std::move(tx), blob)).second;
+ }
+
+ void clear() {
+ txs_by_txid.clear();
+
+ // Empty set can technically stay validated, but reset it to 0 anyway.
+ nic_verified_hf_version = 0;
+ }
+
+ txs_by_txid_t::iterator begin() { return txs_by_txid.begin(); }
+ txs_by_txid_t::iterator end() { return txs_by_txid.end(); }
+
+ txs_by_txid_t::const_iterator begin() const { return txs_by_txid.begin(); }
+ txs_by_txid_t::const_iterator end() const { return txs_by_txid.end(); }
+
+ txs_by_txid_t::iterator find(const crypto::hash& id) { return txs_by_txid.find(id); }
+
+ txs_by_txid_t::iterator erase(txs_by_txid_t::iterator it) { return txs_by_txid.erase(it); }
+
+ size_t size() const { return txs_by_txid.size(); }
+
+private:
+ // The only function which can set nic_verified_hf_version to non-zero value
+ friend bool ver_non_input_consensus(pool_supplement& ps, tx_verification_context& tvc, std::uint8_t hf_version);
+
// Map of supplemental tx info that we might need to validate a block
// Maps TXID -> transaction and blob
- std::unordered_map<crypto::hash, std::pair<transaction, blobdata>> txs_by_txid;
+ txs_by_txid_t txs_by_txid;
// If non-zero, then consider all the txs' non-input consensus (NIC) rules verified for this
// hard fork. User: If you add an unverified transaction to txs_by_txid, set this field to zero!
- mutable std::uint8_t nic_verified_hf_version = 0;
+ std::uint8_t nic_verified_hf_version = 0;
};
/**
@@ -124,7 +158,7 @@ struct pool_supplement
bool ver_non_input_consensus(const transaction& tx, tx_verification_context& tvc,
std::uint8_t hf_version);
-bool ver_non_input_consensus(const pool_supplement& ps, tx_verification_context& tvc,
+bool ver_non_input_consensus(pool_supplement& ps, tx_verification_context& tvc,
std::uint8_t hf_version);
} // namespace cryptonote
diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
index 599ecbe..eac1262 100644
--- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl
+++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl
@@ -91,8 +91,6 @@ namespace cryptonote
const bool allow_pruned,
cryptonote::pool_supplement& pool_supplement)
{
- pool_supplement.nic_verified_hf_version = 0;
-
if (tx_entries.size() > blk_tx_hashes.size())
{
MERROR("Failed to make pool supplement: Too many transaction blobs!");
@@ -140,7 +138,7 @@ namespace cryptonote
return false;
}
- pool_supplement.txs_by_txid.emplace(tx_hash, std::make_pair(std::move(tx), tx_entry.blob));
+ pool_supplement.add_tx(tx_hash, std::move(tx), tx_entry.blob);
}
return true;
Why this scored 46/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.