Skip to content

[#3662] Stop endowing the handler at genesis, and report the value a self-destruct destroys - #43

Merged
isaacdoidge merged 5 commits into
feature/evm_automationfrom
issue-3662
Aug 25, 2026
Merged

[#3662] Stop endowing the handler at genesis, and report the value a self-destruct destroys#43
isaacdoidge merged 5 commits into
feature/evm_automationfrom
issue-3662

Conversation

@isaacdoidge

@isaacdoidge isaacdoidge commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Two changes to the Supra revm fork, both required by the supply-mirroring work in Entropy-Foundation/smr-moonshot#3662: the EVM must stop being endowed with value it cannot mint, and every path that destroys native value must be observable so the node can account for it.

Part of Entropy-Foundation/smr-moonshot#3662. No Closes keyword: GitHub only auto-closes issues in the same repository, so a keyword here would not fire and would misrepresent what merging this does. The issue is closed by the smr-moonshot half, which merges last.

1. The handler deploys without a genesis endowment

Today's design endows ERC20SupraHandler with the entire SUPRA supply cap at genesis. Under the new model EVM native supply starts at zero and every unit arrives by crossing from the Move side, so there is nothing for genesis to endow.

Two independent reasons to remove it rather than shrink it:

  • The handler's own test suite already asserts the invariant it breaks. test/ERC20SupraHandler.t.sol asserts address(erc20SupraHandler).balance == token.totalSupply() in three places. That holds only while every unit of native the handler holds was deposited in exchange for tokens. A genesis endowment violates it in real genesis state; the tests pass only because they deploy the handler unendowed.
  • The endowment is unreachable anyway. ERC20Supra.initialize never mints, so total supply at genesis is zero; the handler is the only authorised minter; its only mint path requires sending native in; and withdraw requires burning tokens you already hold. The endowed native can only be reached by exercising the owner's unbounded mint authority or a UUPS upgrade — the powers smr-moonshot#3661 removes.

initial_native_token and the whole parameter chain that carried it are removed rather than set to zero, so the concept cannot return by accident. The proxy deploys via GenesisTransaction::create — byte-identical, same constructor args, same precomputed address, same nonce accounting, no value — and check_multisig_setup asserts the deployed handler holds nothing, kept deliberately as a regression guard.

2. The value an EIP-6780 self-destruct destroys is now reported

JournalInner::selfdestruct has a balance-transfer block gated on address != target, so its first branch zeroes the balance with no credit anywhere exactly when a contract created in the same transaction self-destructs to itself. That is a genuine burn of native value, and revm reported only had_value: bool; the destroyed account simply vanished from the final state. A node that cannot see it cannot account for it.

Recognised at the branch that causes it, not reconstructed from a state diff — a diff cannot distinguish a burn from a transfer without re-deriving that condition, and would be wrong the moment it changed. The pre-Cancun self-target case comes free, since it takes the same branch.

Revert safety is structural. The amount is read from the AccountDestroyed journal entry itself, and both checkpoint_revert and discard_tx subtract what they drain. So a reverted frame un-reports its burn, an outer frame reverting a nested one un-reports the nested burn too, and a discarded transaction reports zero. The amount cannot survive the entry that records the destruction — a stronger guarantee than two counters kept in step. This mattered most of anything here: an over-reported burn would let the node retire escrow for value that was never destroyed.

SelfDestructResult is untouched — it is upstream surface, and the amount has to be summed and revert-adjusted after the opcode returns, which a per-call return value cannot do. Nothing was added to JournalTr; Journal derefs to JournalInner and the consumer names the concrete type, so the workspace's other JournalTr impl is unaffected. JournalEntryTr did need a method, to read the concrete entry from the generic revert loops.

One lifetime property the consumer must honour: the accumulator is not cleared in commit_tx, which is what makes it readable after transact(). A host that never drains will attribute an earlier transaction's burn to a later one. The per-batch total is correct regardless; smr-moonshot drains once per block and tests that a second block does not inherit the first, so this is enforced at the consumer rather than by moving the total into the execution result and touching output surface.

Verification

smr-moonshot pins this branch directly (issue-3662). The parallel issue-3662-rc4 branch, which carried the same payload against the older release candidate while smr-moonshot#3674 blocked a tag, has been deleted now that bluealloy#3674 is resolved and this branch has been merged up to feature/evm_automation. This must become a released tag before the smr-moonshot half can merge.

cargo build --workspace and the crate tests pass; revm-context goes from 46 to 55 tests. Each of the three burn decision points was mutation-tested and turns a named test red: the address == target condition, the checkpoint_revert subtraction, and the discard_tx subtraction.

Two conditions inherited rather than introduced, both confirmed by stashing and re-running: two pre-existing hard compile errors in lib-test targets (precompile_provider.rs E0277, eip3155.rs E0283 — a winnow-versus-core AsRef<[u8]> ambiguity), and a red clippy baseline of 14 unique diagnostics, identical before and after, so this adds none. Note that an unqualified clippy run aborts at revm-handler and never lints supra-extension at all; --keep-going is needed to see the real set.

Untested: all burn tests drive JournalInner directly, so nothing runs real SELFDESTRUCT bytecode through transact() and reads the total off the host. The opcode-to-journal wiring is unchanged, but that path is verified from the node side rather than here. There is also no rust-toolchain.toml in this repo (smr-moonshot#3675), so cargo +1.97.1 was used to match CI's current stable.

isaacdoidge and others added 2 commits August 21, 2026 13:33
…wment

The Supra EVM cannot mint, so native SUPRA on the EVM side is becoming a
mirror of value escrowed on the Move side, arriving only by crossing.
Endowing the handler at genesis has no place in that model, and the
handler's own test suite already asserts the invariant an endowment
breaks: its balance equals ERC20Supra's total supply, which holds only
while every unit of native it holds was deposited in exchange for tokens.

The initial_native_token configuration field and the parameter chain that
carried it to the proxy deployment are removed rather than set to zero, so
the concept cannot return by accident. The proxy now deploys through
GenesisTransaction::create, which is byte-identical apart from carrying no
value, and check_multisig_setup asserts the deployed handler holds nothing.

Refs Entropy-Foundation/smr-moonshot#3662, Entropy-Foundation/smr-moonshot#3474
…stroys

A `SELFDESTRUCT` that names the destroyed account as its own beneficiary
zeroes the balance without crediting anyone, so that value leaves
circulation. Supra's EVM native balance mirrors value escrowed outside the
EVM and cannot be minted, so the host needs the exact amount in order to
account for it; `SelfDestructResult::had_value` only says whether the
balance was non-zero.

Accumulate the destroyed amount on `JournalInner` at the point where the
journal decides to zero the balance, and subtract it again whenever the
journal entry that recorded it is reverted, so a reverted frame or a
discarded transaction reports nothing. The total is not cleared at the
transaction boundary, so it is still readable after execution; the host
drains it with `take_selfdestruct_burn` once per transaction.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isaacdoidge

Copy link
Copy Markdown
Collaborator Author

Pushed 0021d020: the value an EIP-6780 self-destruct destroys is now observable. First piece of B4 (burn accounting) for Entropy-Foundation/smr-moonshot#3662 — the node cannot account for a burn it cannot see.

The invisible burn

JournalInner::selfdestruct has a balance-transfer block gated on address != target, so its first branch — is_created_locally() || !is_cancun_enabled — zeroes the balance with no credit anywhere exactly when a contract self-destructs to itself. That is a genuine burn of native value, and revm reported only had_value: bool; the destroyed account simply vanished from the final state.

Recognised at the branch that causes it, not reconstructed from a state diff — a diff cannot distinguish a burn from a transfer without re-deriving that condition, and would be wrong the moment it changed. The pre-Cancun self-target case comes free, since it takes the same branch.

Revert safety is structural, not bookkeeping

This was the property that mattered most: an over-reported burn would let the node account for value that was never destroyed.

A burn is only ever recorded alongside an AccountDestroyed journal entry, and the amount is read from that entry. Both revert paths drain those entries and subtract what they drain — checkpoint_revert over the drained range, discard_tx over everything. So a reverted frame un-reports its burn, an outer frame reverting a nested one un-reports the nested burn too, and a discarded transaction reports zero. The amount cannot survive the entry that records the destruction, which is a stronger guarantee than parallel bookkeeping kept in step.

Three tests cover it, and each of the three decision points was mutation-tested: dropping the address == target condition, the checkpoint_revert subtraction, or the discard_tx subtraction each turns a named test red.

Deliberately not done

SelfDestructResult is untouched — it is upstream surface, and the amount has to be summed and revert-adjusted after the opcode returns, which a per-call return value cannot do. Nothing was added to JournalTr either: Journal derefs to JournalInner and the downstream consumer names the concrete type, so the trait stays as it is and the workspace's other JournalTr impl is unaffected. JournalEntryTr did need a method, to read the concrete entry from the generic revert loops; that trait has one in-tree impl and already carries a Supra-local addition.

Baseline, and a correction to how it has been described

Two pre-existing hard compile errors sit in lib-test targets — precompile_provider.rs (E0277) and eip3155.rs (E0283), both a winnow-vs-core AsRef<[u8]> ambiguity — confirmed pre-existing by stashing and re-running. Clippy is red at baseline with 14 unique diagnostics, identical before and after, so this change adds none.

Worth recording for anyone measuring this repo again: an unqualified clippy run aborts at revm-handler and never lints supra-extension at all, so the "roughly 25 supra-extension lints" figure quoted previously is not what a plain run produces. --keep-going is needed to see the real set.

Two things left open rather than papered over

A lifetime judgement call for review. The accumulator is not cleared in commit_tx/finalize, which is what makes it readable after transact() — but a host that never drains it will attribute an earlier transaction's burn to a later one. The per-batch total stays correct regardless; per-transaction attribution depends on the host draining. The consumer aggregates per block, so this is sufficient, and the contract will be enforced and tested on the node side rather than by moving the total into the execution result, which would mean touching output surface for a property the consumer can guarantee itself.

An unexplained build result. The very first baseline cargo build --workspace --all-targets on issue-3662-rc4 returned 0; a later identical run returned 101. That is unexplained rather than dismissed, and the green build signal on that branch is cargo build --workspace only.

Also untested: all nine tests drive JournalInner directly. Nothing runs real SELFDESTRUCT bytecode through transact() and reads the total off the host — the opcode-to-journal wiring is unchanged, but that path is not verified here. It will be, from the node side, as B4 continues.

b62bb058 on issue-3662-rc4 carries the identical change, since smr-moonshot pins that branch by ref while smr-moonshot#3674 blocks a tag.

@isaacdoidge isaacdoidge changed the title [#3662] Deploy the ERC20Supra handler without a genesis endowment [#3662] Stop endowing the handler at genesis, and report the value a self-destruct destroys Aug 22, 2026

@isaacdoidge isaacdoidge left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed both commits, with the accumulator's placement question examined explicitly.

Is JournalInner the right place? Yes — and here is the reasoning worth recording.

Everything else block-cumulative in this stack is aggregated outside revm, in the executor, over per-transaction outputs: receipts, cumulative gas, the settlement records, and now the fee side of the destroyed total all live in EvmExecutionAggregatedResults. Upstream revm keeps the same discipline — the journal's per-tx metadata is cleared by commit_tx/discard_tx, and the only thing that outlives a transaction is state itself. So this field is genuinely the first of its kind, and the burden of proof is on it.

It carries that burden for two reasons:

  1. The fact exists only at journal-entry granularity. A self-targeting destruction removes the account from the final state, and ExecutionResult carries no balance delta, so a host-side aggregation has nothing to aggregate from. Something inside revm must surface it; the only alternatives are a new field threaded through ResultAndState/handler/inspector types (a much larger upstream-merge surface, per-tx attribution we don't need) or this.
  2. The number must obey frame-revert semantics exactly, and the implementation makes that coupling structural rather than reconstructed: the revert paths re-derive the reverted amount from the very entries being reverted (entry.selfdestruct_burn()), so the counter and the state cannot disagree by construction. The nested-revert and discard tests pin the property.

Two contracts keep it sound, both worth keeping visible: the drain discipline (take between transactions only — the saturating_sub in the revert paths silently tolerates a mid-tx drain rather than failing it, so the field doc's "drain between transactions" is load-bearing), and the lifetime coupling — the block total falls out of Supra's one-journal-per-block usage; an embedder that finalizes per transaction gets per-tx totals instead, which still composes via take, so nothing breaks, but the "block" in the doc is a property of the caller, not of the journal.

Correctness

The branch analysis is exact: only the destroying branch with a self-targeting beneficiary counts (a distinct beneficiary transfers, the post-Cancun not-created-locally self-target is a no-op that writes no entry and counts nothing), a repeated destruction sees a zero balance and cannot double-count, and the pre-Cancun behaviour matches the destroying branch's actual semantics. commit_tx/discard_tx/finalize all destructure the field explicitly — the existing destructuring pattern did its job of forcing every lifecycle path to take a position. Test coverage hits every case I could construct, including the cross-tx no-op and finalize survival.

Nits

  • JournalEntryTr::selfdestruct_burn is a required method with no default. A U256::ZERO default impl would let other ENTRY implementors (and future upstream syncs) compile without caring, at no cost to JournalEntry.
  • The genesis commit (deploy the handler unendowed, drop initial_native_token end to end) pairs correctly with the smr-side removal and the value() == 0 assertion; the config-field removal is a breaking config change, fine under the pre-production policy.

isaacdoidge and others added 2 commits August 22, 2026 12:40
…rain contract

`JournalEntryTr::selfdestruct_burn` reports a Supra-local concept, so give
it a zero default: an entry type with no notion of a destroyed balance, or
one arriving from an upstream sync, then needs no change. `JournalEntry`
overrides it, so nothing is lost.

Document the two contracts the accumulator places on its caller rather than
on the journal. The revert paths subtract with `saturating_sub`, which makes
a mid-transaction drain quiet instead of rejected, so the field is only
correct if it is drained between transactions; say so where a caller will
read it. And the journal has no notion of a block, so record that the total
covers whatever span the caller chooses to drain over, which for Supra is a
block because it runs one journal per block.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…urn total

A revert that tries to take back more than the accumulator holds can only
mean the total was drained while the destruction it covers was still
revertible, so the saturation is itself the symptom. Assert against it at
both revert sites, which turns the drain contract from a documented
requirement into one that fails loudly wherever an embedder would introduce
the violation.

Release behaviour is unchanged: the subtraction still saturates, because on
a consensus-critical path a caller that breaks the contract should cost
accounting accuracy rather than halt the node.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@isaacdoidge

Copy link
Copy Markdown
Collaborator Author

e755573a / c8c6506b — the review's nit is applied and the drain contract is now enforced rather than only documented.

The framing correction is worth recording, because I had it wrong too

"The counter and the state cannot disagree by construction" holds inside the drain discipline, not at its boundary. A caller who drains mid-transaction leaves a later revert with nothing to take back, so the drained total keeps a burn that never happened, and saturating_sub is exactly what makes that quiet. The gap was closed by contract, not by construction — and I repeated the stronger claim before noticing the difference.

Most of that contract is now construction again

A mid-transaction drain is detectable at the moment it bites: a revert path trying to subtract more than the accumulator holds is the symptom. Both subtraction sites now carry a debug_assert! naming the cause, so the violation is loud in every test and debug build, while release behaviour is byte-identical — saturating_sub still tolerates it rather than panicking a validator, which is the right call on a consensus-critical path.

The spurious-fire question was worked before adding it, and the enumeration is the reason to trust it: every burn is accumulated in the same branch that pushes its AccountDestroyed entry, so nothing counts without pushing or pushes without counting; entries leave only three ways, and the two that drain also subtract while commit_tx/finalize clear burns that are permanent, so the accumulator is always at least the burns still live in the journal; a repeated checkpoint_revert is guarded by journal_i < journal.len() and drains nothing; and a custom ENTRY that under-reports drifts the accumulator up and can never saturate, so only an over-reporting implementation fires — which is a broken implementation worth catching.

"Release behaviour unchanged" was evidenced rather than asserted: the release binary reports 9 burn tests against debug's 10, which is the cfg(debug_assertions) test compiling out and nothing else moving.

Two doc contracts, both caller obligations

Drain between transactions only — with the reason stated, that the saturation is what makes a violation quiet, so the discipline is a requirement and not advice. Mirrored onto take_selfdestruct_burn itself, since a caller reaches for take first.

Scope is whatever the caller drains — the journal has no notion of a block. The block total falls out of Supra running one journal per block; an embedder finalizing per transaction gets per-transaction totals from the same field, and they still compose, because each drain covers exactly the destructions since the last.

The end-to-end wiring is covered, contrary to my earlier note

I recorded a residual that nothing downstream read the accumulator and that no test drove real SELFDESTRUCT bytecode through transact(). Both are now false, and I have verified it rather than assuming: smr-moonshot drains at consensus/execution/src/evm/executor.rs:475, and its a_self_destruct_is_reported_once_and_never_inherited_by_the_next_block deploys a real self-destructing constructor with TxKind::Create and asserts both halves — that the first block's total contains the destruction, and that the second block's contains only its own.

That second half is the one this design needs: without the drain, block one's endowment would be counted again and the escrow asked to retire it twice.

@isaacdoidge
isaacdoidge requested a review from aregng August 23, 2026 07:08
@isaacdoidge
isaacdoidge merged commit dacb2c8 into feature/evm_automation Aug 25, 2026
1 check failed
@isaacdoidge
isaacdoidge deleted the issue-3662 branch August 25, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants