tx_pool: do expensive verify after potential no-drop offenses
What the developer wrote
Authored by j-berman
tx_pool: do expensive verify after potential no-drop offenses
What changed, and why it matters
This commit changes the order in which Monero checks incoming transactions. Instead of running an expensive consensus check first, it now checks cheaper 'no-drop' rules (like whether the transaction pays a valid fee and isn't already known) before doing the heavy validation. This is a performance and resource-usage optimization, likely to reduce denial-of-service risk from spam transactions. A second small change makes the spent-key-image check skip non-standard input types instead of asserting they never occur. The commit message and diff do not describe this as a security fix, and no independent researcher is credited.
Treat as a hardening/performance patch rather than an urgent security fix. Review whether the reordered checks preserve all existing consensus invariants, especially that no transaction can bypass ver_non_input_consensus before being accepted. Monitor for related follow-up commits or disclosures.
Security signals we found
Reordering validation to perform cheaper checks before expensive consensus verification can mitigate CPU-exhaustion DoS from crafted transactions.
The spent-key-image change removes a 'should never fail' assertion and replaces it with a defensive skip, reducing the chance of a node crash or consensus split if an unexpected input type reaches that path.
No explicit security framing, CVE, or researcher attribution is present in the commit or supplied references.
Evidence from the diff
In tx_pool.cpp::add_tx(), the call to cryptonote::ver_non_input_consensus() is moved after the fee/weight and already-in-pool checks. This means a transaction that fails cheap no-drop checks (e.g., insufficient fee, duplicate) is rejected before the more expensive non-input consensus verification runs. The get_tx_fee() call is hoisted out of the try block and made const. In have_tx_keyimg_as_spent(), the code replaces a CHECKED_GET_SPECIFIC_VARIANT macro (which previously asserted the variant type) with an explicit type check that continues on unexpected input types. This makes the spent-key-image scan more defensive.
Changed components
src/cryptonote_core/tx_pool.cpptransaction pool validation path (add_tx)spent key image check (have_tx_keyimg_as_spent)Inspect captured patch +12 / −12
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index 1aa228d..f94fe2f 100644
--- a/src/cryptonote_core/tx_pool.cpp
+++ b/src/cryptonote_core/tx_pool.cpp
@@ -157,20 +157,10 @@ namespace cryptonote
return false;
}
- if (version != nic_verified_hf_version && !cryptonote::ver_non_input_consensus(tx, tvc, version))
- {
- LOG_PRINT_L1("transaction " << id << " failed non-input consensus rule checks");
- tvc.m_verifivation_failed = true; // should already be set, but just in case
- return false;
- }
-
- uint64_t fee;
+ const uint64_t fee = get_tx_fee(tx);
bool fee_good = false;
try
{
- // get_tx_fee() can throw. It shouldn't throw because we check preconditions in
- // ver_non_input_consensus(), but let's put it in a try block just in case.
- fee = get_tx_fee(tx);
fee_good = kept_by_block || m_blockchain.check_fee(tx_weight, fee);
}
catch(...) {}
@@ -217,6 +207,14 @@ namespace cryptonote
}
}
+ // Do more expensive verification after plausible no-drop offenses
+ if (version != nic_verified_hf_version && !cryptonote::ver_non_input_consensus(tx, tvc, version))
+ {
+ LOG_PRINT_L1("transaction " << id << " failed non-input consensus rule checks");
+ tvc.m_verifivation_failed = true; // should already be set, but just in case
+ return false;
+ }
+
// assume failure during verification steps until success is certain
tvc.m_verifivation_failed = true;
@@ -1388,7 +1386,9 @@ namespace cryptonote
CRITICAL_REGION_LOCAL1(m_blockchain);
for(const auto& in: tx.vin)
{
- CHECKED_GET_SPECIFIC_VARIANT(in, const txin_to_key, tokey_in, true);//should never fail
+ if (in.type() != typeid(txin_to_key))
+ continue;
+ const auto &tokey_in = boost::get<txin_to_key>(in);
if(have_tx_keyimg_as_spent(tokey_in.k_image, txid))
return true;
}
Why this scored 48/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.