refac: collapse ExecutionKind/EvmKind into orthogonal Job + StateView types - #2721
gventino-cw wants to merge 1 commit into
Conversation
![cloudwalk-review-agent[bot]](https://avatars.githubusercontent.com/u/6581007?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary
Refactor looks behavior-preserving and internally consistent: ExecutionKind/EvmKind responsibilities are cleanly split into ExecutionContext { job, at } and worker-pool Lane, and callsites were updated coherently across executor, storage, RPC, follower, and metrics.
I checked the highest-risk paths (state resolution, EVM policy toggles, and routing):
- Storage read resolution still keys off state view (
PendingvsLatest(Some)vsPast) in a way that matches prior semantics. - EVM policy flags (
tx_chain_id_check, nonce/EIP-3607 checks) now depend onJob::is_transaction(), which preserves the old tx-vs-call behavior. - Worker-pool busy metrics moved to
Lanelabels with unchanged label values.
No concrete correctness/security/deploy-blocking issues found in the provided diff context.
|
Failed to generate code suggestions for PR |
db9fa08 to
d53381e
Compare
… types
Replace the fused ExecutionKind enum (CallLatest/CallPast/Transaction/RPC/AccessList)
with an orthogonal pair:
- Job {Transaction, Call, Rpc, AccessList}: what triggers the execution (EVM policy,
cache eligibility)
- StateView {Pending, Latest(Option<BlockNumber>), Past(BlockNumber)}: which state
storage reads resolve to
- ExecutionContext {job, at}: the combination, carried by EVM inputs and storage reads
EvmKind is replaced by Lane {Transaction, CallPresent, CallPast, Inspector}, which
keeps only what the worker pool genuinely needs (admission lane + busy-gauge metric
label), with identical label strings.
Closes #2720
d53381e to
ca791a0
Compare
There was a problem hiding this comment.
Summary
Refactor appears behavior-preserving in the provided diff: the split into ExecutionContext { job, at } plus worker-pool Lane is consistently propagated through executor, EVM/session reset, storage read resolution, RPC entrypoints, and metrics label emission.
I specifically checked the high-risk paths:
- EVM policy toggles still separate tx vs non-tx behavior via
Job::is_transaction()(chain-id/nonce/EIP-3607). - Storage pending/latest/past resolution semantics are preserved via
StateView(Pendingtemp-first,Latest(Some)staleness-aware downgrade path,Pastfixed block). - Worker busy gauges keep the same label strings after
EvmKind→Lanerename.
No concrete correctness/security/concurrency issue was found in the supplied context.
|
Failed to generate code suggestions for PR |
This branch has not been deployed
1 participant
Motivation
Review of #2710 (discussion r4138469433) pointed out that
ExecutionKindandEvmKindencode overlapping, redundant information:ExecutionKind::CallLatestandExecutionKind::CallPastfuse what triggers the execution with when the state is read, whileEvmKindre-derives the same distinction for the worker pool.This collapses both into orthogonal types, with no behavior change.
Changes
New:
Job+StateView+ExecutionContext(src/eth/types/execution_context.rs, replacessrc/eth/types/execution_kind.rs):Job { Transaction, Call, Rpc, AccessList }— what triggers the execution. Drives EVM policy (nonce/EIP-3607 checks, chain-id check) and storage cache eligibility (Job::Call | Job::AccessListreads fromFoundAt::PermLatestcache latest, exactly the oldCallLatest/CallPast/AccessListset).StateView { Pending, Latest(Option<BlockNumber>), Past(BlockNumber) }— which state storage reads resolve to.Pendingreads transient state;Past(n)reads a mined block;Latest(n)resolves through the block cache.ExecutionContext { job, at }— the combination, carried end-to-end by EVM inputs, the executor, and storageread/read_account/read_slot.EvmKind→Lane(src/eth/executor/evm/types/mod.rs):Lane { Transaction, CallPresent, CallPast, Inspector }keeps only what the worker pool genuinely needs — admission lane and busy-gauge metric label — with identical label strings ("transaction","call_present","call_past","inspector").Metric labels now come from a single
ExecutionContext::metrics_label()instead of per-enumAsRefStrimpls; EVM execution labels (transaction,call_latest,call_past,access_list) are byte-identical to before.Verification
cargo check --all-targetscleanExecutionKind/EvmKindremainjust lint-checkfails only on the pre-existingfetch_updatedeprecation atsrc/eth/follower/importer/mod.rs:60(same onmain)Closes #2720