Mempool policy: reject token-preserving re-emission spends before script execution - #2447
Mempool policy: reject token-preserving re-emission spends before script execution#2447odiseusme wants to merge 6 commits into
Conversation
|
Built as specified — the (h) — a test-only Both messages go through the same probe, since Akka guarantees FIFO only per sender–receiver pair. The actor tests also assert Four mutation checks, two of which bracket the guard from both sides:
Suite is 11/11; One correction while implementing: |
…pends still validate ErgoNodeViewHolder's Declined branch discarded the pool returned by ErgoMemPool.process, so the prefilter's rejected-id cache never reached a running node. Install the returned pool when it differs by reference, which is a no-op for every pre-existing declining path. Also: single-pass input resolution at the prefilter anchor, a clearer rejection message, and an end-to-end case covering a conformant owner spend with checkReemissionRules enabled.
Invariant (h) drives the token-preserving rejection through a real ErgoNodeViewHolder, using a test-only subclass whose InjectState message reuses the existing protected updateNodeView to install a synthetic token-bearing state. It asserts the id is cached both in the published ChangedMempool reader and in a freshly queried GetDataFromCurrentView. A companion control pins the reference-inequality guard as a no-op on a declining path that returns the same pool. Removing only the guarded install makes (h) fail while the control still passes.
Send InjectState and the transaction through the same probe to avoid an Akka per-pair FIFO race that could let the tx overtake the state injection onto the missing-UTXO path. Assert the intended decline reason in both cases, and assert minimalFeeAmount == 0 as an explicit precondition, since the fee gate precedes the prefilter. Behaviour unchanged; all four mutation checks (predicate-false, delete-install, unconditional-install, d3 burn) confirmed against these tests.
9f4ed77 to
0d5037f
Compare
Mempool policy: reject token-preserving re-emission spends before script execution
Target branch:
v6.0.4Production diff: 2 files —
ErgoMemPool.scala(the policy check) andErgoNodeViewHolder.scala(a 3-line fix without which the check's caching has no effect; see below)Tests: 1 new suite, 11 cases (including an end-to-end actor test of the holder fix)
Line references are to base commit
cdde03d71unless noted — so citations into the two changed files (ErgoMemPool.scala,ErgoNodeViewHolder.scala) point at the pre-change source, not the post-change line numbers.Problem
After EIP-27 activation (mainnet height 777,217), a transaction that spends a re-emission-token-bearing box on the non-emission path while preserving that token into any output can never be included in a block.
verifyReemissionSpendingforbids it directly: once such an input is spent, every output is checked byrequire(!out.tokens.contains(reemissionTokenId))(ErgoTransaction.scala:318-319). Generic storage-rent claim builders produce exactly this shape against aged reward boxes, because recreating a box with all its tokens preserved is the only claim form they know.The shape is live traffic, not a hypothetical: roughly one fresh-transaction-id attempt per ~10 minutes, and 400+ EIP-27 rejection log lines per checking node over ~1.5 days. Each rejection event emits the phrase twice in the captured logging setup — the ERROR line and the exception line — which is the expected shape of
log.error(s"EIP-27 check failed due to ${e.getMessage} : ", e)atErgoTransaction.scala:337, where the throwable is passed alongside a message that already containse.getMessage. Halving gives ~220–250 rejection events per node against ~216 fresh attempts over the same window: very nearly 1:1. That ratio is the substantive point — each fresh id undergoes full validation approximately once, after which the existinginvalidatedTxIdscache suppresses re-requests of that id. The observed cost is therefore about one interpreter run per fresh id, and that is exactly the cost this change removes.Method: log-line counts taken from journald and file logs of two mainnet nodes running
checkReemissionRules = true, halved to account for the two matching lines each rejection event emits in the captured logging setup, with unique-id extraction used to distinguish fresh attempts from repeats of the same transaction.The effect splits by configuration.
checkReemissionRulesships asfalse(mainnet.conf:44) and is force-enabled only for mainnet miners —ErgoSettingsReader.scala:180-183hard-fails startup for a mainnet mining node that leaves it off.On a flag-on node the transaction is rejected, but late:
txReemissionis validated after the input loop the code itself labels "the most expensive check usually, so done last" (ErgoTransaction.scala:434-440), so the interpreter has already run over every input before the cheap structural rule fires. That is the per-attempt cost measured above.On a flag-off node the
txReemissionrule short-circuits, so nothing in the validation path recognises the shape at all, and the transaction's fate rests on script execution alone — which, per #2438's live-nodePOST /transactions/checkresults, the token-preserving claim passes. (Those results distinguish the two claim shapes precisely: token-preserving recreation returnsTransaction should conform EIP-27 rules, i.e. the EIP-27 rule and not a script failure; the token-dropping shape is the one that returns input script verification failure viacheckExpiredBox.) A flag-off node can therefore admit a transaction that no conforming mainnet miner can include. This is a statement about validation capability, sourced to #2438; it is not a claim about observed relay or retention behaviour.One scope qualifier: all of the above concerns UTXO-state nodes only. Digest-state nodes skip stateful validation (
ErgoMemPool.scala:277-280): that branch still callsacceptIfNoDoubleSpend, so double-spend and capacity checks run, but the prefilter and script execution do not — and, per the comment there, such nodes receive transactions from the local wallet only.This is mempool policy, not consensus
The change adds a predicate to
ErgoMemPool.processand nothing else. It alters what a node admits and caches, never what a block may contain.validateStatefulis untouched; no validation rule id, no rule ordering, and no soft-fork accounting changes.ErgoTransaction.scala,UtxoStateReader.scalaandCandidateGenerator.scalaare all untouched.The check sits ahead of the versioned-serializer round-trip in
process, so a transaction that is both token-preserving and unparseable now yieldsDeclinedrather thanInvalidated. That is deliberate: of the two,Declinedis the more lenient outcome, and it carries no peer penalty.Resolving the inputs for the predicate replaces a short-circuiting
forallwith an eagermap, so it no longer stops at the first missing input; the extra reads therefore occur only on the missing-input path, after the first missing input, and a valid transaction already required every lookup. These are LevelDB point reads — marginal, and disclosed only for completeness.The second file
ErgoMemPool.processreturns the updated pool, butErgoNodeViewHolder'sDeclinedbranch was annotated// do nothingand never calledupdateNodeView— so the returned pool, and with it the record that this transaction id was rejected, was discarded. Every pre-existing declining path returns the pool unchanged, so nothing was being lost before; this change is the first to return a pool that carries new information on aDeclinedoutcome.The fix installs the returned pool only when it is not the same object:
Reference inequality makes this a no-op for every existing declining path, so the change is behaviour-preserving apart from the new one.
DeclinedTransactionremains non-penalizing and does not represent permanent consensus invalidation. What this policy path adds is that the transaction id is installed into the mempool's expiring approximate invalidation cache (invalidatedTxIds), which the synchronizer consults to suppress future network inventory requests. That suppression outlasts the transientdeclinedmap: entries indeclinedare dropped byclearDeclined(ErgoNodeViewSynchronizer.scala:1361-1372, called fromLocalBlockAppliedat:1437andRemoteBlockAppliedat:1450), and even that only removes entries older than a 20-minute timeout rather than clearing wholesale — after which the longer-lived invalidation cache still filters the inventory. Direct local or API submissions are not blocked by that cache and may still reachprocess, since neitherprocessnorcanAcceptconsultsinvalidatedTxIds.Being a policy rule, it has a disclosed divergence rather than a hidden one:
One step further out: on a custom chain configured with a non-empty
reemissionTokenIdandcheckReemissionRules = false, the same owner-authorized spend is genuinely minable, sinceverifyReemissionSpendingis never invoked there and the mainnet-only startup guard does not apply. The filter would still decline it. The soundness property below holds regardless; the "forfeits nothing" corollary is mainnet-scoped.Soundness
If the filter fires, then some input under the 100K ERG bar carries the re-emission token past activation height — exactly the condition that sets
reemissionSpendinginverifyReemissionSpending— and some output carries that token, which is exactly what the subsequentrequireforbids. The filter's reject set is therefore a strict subset of the set EIP-27 already rejects: it can decline transactions the node would otherwise decline anyway, and never one that the EIP-27 rule function would accept.The height the filter reads is the current height, while validation later uses the upcoming height (
simplifiedUpcoming(),ErgoStateContext.scala:148). These differ only in the single block where the current height equalsactivationHeight, and there the filter abstains and ordinary validation decides — so the discrepancy can only cause the filter to under-fire, never to over-fire.Tests
verifyReemissionSpendingcurrently has no test coverage: no existing test reaches it, on any path. (Other suites match on "reemission" —ReemissionRulesSpec, an API route spec, and twoergo-walletbox-selector specs — but they exercise theReemissionRulescontracts and box selection, not this method.) Case (d3) below is the first test to reach and pass it.ErgoMemPoolReemissionPrefilterSpecpins eleven cases (a, b, c, d1, d2, d3, e, f, g, h, control):checkReemissionRulessettings, asserting the rejection reason, not merely that it was not acceptedisInvalidatedDeclined, notInvalidated, which is the property that keepsMisbehaviorPenaltyfrom being appliedAcceptedend to endcheckReemissionRulesenabled, a fully conformant owner spend — token dropped, and exactly the owed amount paid to the configured pay-to-re-emission proposition — isAcceptedend to end. This is the case that reachesverifyReemissionSpendingand satisfies its non-emission branch rather than skipping itinputs/outputsare unchanged and the same box remains spendable>, so such a box is on the non-emission path), a mixed transaction abstains,currentHeight == activationHeightabstains and the next block fires, and an emptyreemissionTokenIdis inertErgoNodeViewHolderactor, theChangedMempoolevent and a freshGetDataFromCurrentViewboth report the id cachedChangedMempool, pinning the reference-inequality guard as a no-opThe emission transaction is covered at predicate level rather than end to end: synthesising a valid emission transaction would test the fixture more than the filter.
Verification run: the new suite 11/11;
ErgoMemPoolSpecandOrderedTxPoolSpec33/33;ReemissionRulesSpec2 passed, 1 pre-existing ignored.The install is exercised end to end
An earlier revision of this branch had a defect here, worth recording because it shaped the tests.
ErgoNodeViewHolder'sDeclinedbranch discarded the mempool returned byErgoMemPool.process, so the prefilter's rejected-id cache never reached a running node — and the caching half of invariant (b) did not catch it, because it asserts againstprocess's return value rather than against the pool the node actually installs. The fix is the reference-inequality-guarded install described above.That install is now covered directly. Invariant (h) drives the rejection through a real
ErgoNodeViewHolderactor and asserts on the installed pool: a test-only subclass accepts anInjectStatemessage that reuses the existing protectedupdateNodeViewto install a synthetic token-bearing state — no production test seam, and no need to mint a token through a genesis box (which is not possible: the genesis boxes are respectively false-locked, miner-only, and 2-of-N founder-guarded). It then submits the real token-preserving transaction viaLocallyGeneratedTransaction, asserts the outcome isDeclined, and asserts that both theChangedMempoolevent's reader and a freshly queriedGetDataFromCurrentViewreportisInvalidated(tx.id). A companion control drives a pre-existing declining path that returns the same pool unchanged and asserts that noChangedMempoolis emitted — pinning the reference-inequality guard as a genuine no-op for the paths it must not disturb.Four mutation checks were run to confirm the suite is not passing vacuously. Forcing the predicate to
falsefails cases a, b, c, e, g and h while d1, d2, d3, f and the control still pass — the expected signature, since the abstention cases are by construction insensitive to under-firing (which is why (g) pins the firing direction), and the control expects no event either way. Removing only the guarded install inErgoNodeViewHolderfails (h) — itsChangedMempoolnever arrives — while the control still passes, which is what proves (h) tests the install rather than incidentally passing. Conversely, replacing the reference-inequality-guarded install with an unconditionalupdateNodeViewleaves (h) green but fails the control, which receives aChangedMempoolit should not — confirming that the control specifically pins theneguard rather than the install in general. (The two together bracket the guard: neither the install nor its condition can be removed without a test failing.) And underpaying (d3)'s burn by a single nanoErg flips it fromAcceptedtoInvalidated, establishing that it genuinely reaches the EIP-27 non-emission branch rather than passing because that branch was never evaluated.Deliberate limits
sentToReemission == toBurn,ErgoTransaction.scala:329-330). A transaction that correctly drops the token but underpays the burn still reaches script execution and is rejected there. Covering it would require the pay-to-re-emission tree and the mainnet/testnet comparison split, for a shape not implicated here.ErgoMemPool.process, so it does not apply whenCleanupWorkerre-validates transactions already in the pool. That matters only for transactions admitted before upgrading.Declinedpath's DEBUG logging (ErgoNodeViewHolder.scala:280-282). Repeated-attempt log noise is part of what this change is meant to reduce, so replacing an ERROR-per-attempt with an INFO-per-attempt would be a poor trade.Credit
checkExpiredBox's recreate branch and the re-emission burn obligation. That PR is open and unmerged; this one neither depends on it nor blocks it. Invariant (f) is an explicit commitment that the token-dropping claim shape which Storage-rent repairs (version-gated): 64-bit fee arithmetic + EIP-27 re-emission carve-out #2438 would make valid remains admissible here.