feat(evm): Disable create and reserve balance in delegate context - #114
Merged
Conversation
Richard1048576
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at d0cedf4 (base f7dacd16). Adversarial pass over the two delegated-safety policies + the scheduler refactor, focused on reserve-accounting correctness, Block-STM determinism, the delegated-CREATE ban, and refactor regressions.
Verdict: LGTM — safe to merge
No correctness, determinism, or regression defect found. The only surviving items are Minor test-coverage gaps (logic verified correct by reading, just not exercised). Opt-in and default-false, so existing behavior is unchanged unless enabled.
Verified sound
- Reserve accounting — Overflow is saturating (
max_balance_spending().unwrap_or(U256::MAX)+saturating_add). The "strictly-later, excluding self" horizon is correct (partition_point(|c| *c <= txid)). The pre-debit baseline is reconstructed purely from this tx's own journal by reverse-applying every surviving entry from the first-debit index (all three balance-affectingJournalEntryvariants handled; self-transfers are no-ops). Reverted inner-frame debits are correctly ignored (drained bycheckpoint_revert); post-CancunSELFDESTRUCT(EIP-6780) is covered; the roottx.valuetransfer is excluded exactly once. The revert path preserves nonce / EIP-7702 auth effects+refund / charged gas while discarding execution-state refunds, and recomputes refund caps + the EIP-7623 floor. - Parallel determinism —
required_after/build_scheduleread only the static ordered tx list andTxEnv::max_balance_spending()— no execution results, no MV/DB reads — so the suffix sums are a pure deterministic function, andOnceLock/DashMapinit is worker-independent. The whole reserve decision is a pure function of (this tx's own journal + the static tx list); it reads no unregistered speculative state, so Block-STM re-execution recomputes it consistently. The candidate set is OR-reduced into one boolean, so its ahash iteration order is irrelevant. - Delegated CREATE/CREATE2 ban — no issues.
- Scheduler/lib refactor — no behavior regression; the split is a faithful move. (
concurrency_level == 0now asserts instead of hanging — an improvement;from_envre-reading per construction is cosmetic.)
Minor — test-coverage gaps (non-blocking)
The following behaviors are implemented correctly but have no test exercising them:
- Execution-refund discard on a reserve revert — every reserve-path delegate uses CALL+SELFBALANCE or SELFDESTRUCT, neither of which earns an EIP-3529 refund on PRAGUE, so
set_refund(0)is a no-op in all current tests. Add a delegate that does anSSTOREnonzero→zero clear before the debit, so the discarded execution refund actually matters. - EIP-7623 calldata floor binding on the revert path — no reserve-reverting tx makes the floor the binding gas. Add a type-4 delegated tx with several KB of calldata whose floor dominates.
- Stale-read determinism, other direction —
reserve_retry_reexecutes_after_a_stale_speculative_balance_readonly covers violation→success. Add the mirror (a stale low read that first passes, then re-executes into a violation). - Multi-candidate reserve — no test has a single tx produce surviving debits from two distinct delegated accounts; the per-candidate loop in
has_reserve_violationis unexercised end-to-end.
Considered and dismissed
- "A later same-sender tx that will be skipped/invalid still inflates the reserve." Real behavior, but intentional and documented conservative over-reservation (only ever causes more reverts, never under-protection), and opt-in. Not a defect — at most a test-hardening opportunity.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds two independent, opt-in safety policies for EIP-7702 delegated execution:
CREATE/CREATE2: reject contract creation only when the current execution context is an EIP-7702 delegated account. OrdinaryCREATEandCREATE2remain unchanged.Both policies default to
false, so the existing Grevm/revm execution behavior is preserved unless they are explicitly enabled throughDelegatedSafetyConfig.Reserve-balance semantics
For a delegated account
Adebited by transactionTi:The guard:
tx.value;CALLvalue transfers and post-CancunSELFDESTRUCTbalance movement;TxIdin both parallel execution and sequential suffix fallback.If the final balance is below the reserve, Grevm returns a top-level
REVERT. Execution state, output, and execution-generated refunds are rolled back, while the transaction nonce, EIP-7702 authorization effects/refund, and gas charged for the attempted execution are preserved. Refund caps and the EIP-7623 gas floor are recomputed from the pre-refund gas state.Comparison with Monad
The Grevm policy follows Monad's core failure semantics but intentionally uses a narrower reserve model for Gravity Reth's skipped-invalid-transaction risk.
min(balance before the first surviving delegated debit, sum of later same-sender transactions' max balance spending)min(original transaction balance, 10 MON), with a sender adjustment for upfront gastx.valuemax_balance_spending()is still included, so Grevm may conservatively over-reserveSELFDESTRUCT; reverted inner-frame debits are ignoredSELFDESTRUCT; rollback hooks remove reverted-frame violationsREVERT, preserving unused gasEVMC_MONAD_RESERVE_BALANCE_VIOLATION; top-level contract creation consequently has different remaining-gas behaviorDelegatedSafetyConfig; disabled by defaultMONAD_FOURNeither policy is uniformly stricter: Grevm requires no reserve when the account has no later transaction, but it can reserve more than 10 native tokens when the later maximum-spending sum is larger.
Parallel correctness and determinism
Refactoring
The PR also splits the previous monolithic scheduler/library implementation into focused modules:
This keeps the policy-specific logic out of the core scheduling path and makes parallel and sequential execution share the same configuration and transaction handler.
Testing
cargo test --features test-utils— all 70 tests passed.cargo test --release --features test-utils --test delegated_safety— all 13 delegated-safety tests passed.cargo clippy --all-features --all-targets -- -D warnings— passed.cargo +nightly fmt --all -- --check— passed.GREVM_MIN_PARALLEL_TXS=0 GREVM_MAINNET_BLOCKS=test_data/spec_coverage cargo test --features test-utils --test mainnet replay_mainnet_blocks -- --nocapture— all 18 hardfork-boundary fixtures from Tangerine through Prague passed.CI now also builds every feature and target under Clippy with warnings denied.