Skip to content

execution/state: follow-ups from the typed-vio refactor review (#21536) #22259

Description

@yperbasis

Follow-up work items from the review of #21536. None are blockers there — the retype verified as faithful to main's logic. Distinct from the cache-stack work in #22154 and the main-side bug in #22204.

Efficiency

  • Write-only WriteCell pools (versionmap.go): the per-path cell pools only get cells back via the rare Delete/DeleteAll exception paths; each block builds a fresh NewVersionMap and drops the old map wholesale, so in steady state putCell's getCell() hits an empty pool and allocates anyway — per-write pool bookkeeping without the recycling the comment ("a freed cell from block N is recycled into block N+1's first write") promises. Either add a block-end walk that releases cells, or drop the pools and allocate plainly as before.
  • ApplyVersionedWrites header materialize + re-probe (intra_block_state.go): the apply loop copies every WriteHeader into a slice, sorts it, then re-probes the typed per-path maps (GetStorage/GetBalance/…) to recover each value it just iterated past. Collect (header, typed value) pairs in the same single map walk before sorting. Runs per finalized tx.
  • AsBlockAccessList per-tx addrs() map (versionedio.go): writes.addrs() allocates a throwaway dedup map per transaction purely to feed the idempotent ensureAccountState; iterating forEachAddr directly is alloc-free and identical (the BAL is sorted afterwards). addrs() itself duplicates forEachAddr's ten per-path loops and can be a 3-line wrapper over it — or deleted once the call site iterates directly.

Structure

  • Empty-code→delete rule in one place: the len(code)==0 → DomainDel(CodeDomain) rule is hand-rolled at three sites — Writer.UpdateAccountCode, applyVersionedWrites, and BlockStateCache.Flush (bcOpPutCode). This exact drift already happened once in the other direction (main had the rule only at the Flush site — the bug execution/state: typed-vio refactor (L2 — typed AddressEntry + WriteCell[T] + sync.Pool) #21536 fixes). A single helper over kv.TemporalPutDel (SharedDomains.AsPutDel adapts the non-Writer sites) makes the next tweak land everywhere.
  • Enforce accounts.Code's invariant structurally (execution/types/accounts/code.go): Hash == Keccak256(Bytes) is comment-only — both fields are exported, and several sites in execution/state build accounts.Code{...} literals bypassing NewCode, while the parallel commitment calculator (calc_state.go) feeds vw.Val.Hash straight into the state root. Unexport the fields behind NewCode plus a trusted-pair constructor (which must canonicalize the empty-hash variants EmptyCodeHash/ZeroCodeHash/NilCodeHash). Related vestige: applyVersionedWrites erases the typed value to []byte and re-keccaks at apply time (accounts.NewCode(d.code).Hash) even though every producer already carries a construction-time hash — keep the typed Code and use its hash.
  • Generic typed pool accessors: twenty near-identical getVW*/releaseVW* (versionedio.go) and getCell*/releaseCell* (versionmap.go) pairs. A generic pool wrapper with an optional clear-func (same shape as vwMapPool[T]) collapses ~40 declarations and centralizes the easy-to-miss unpin-on-release special cases (Address, Code).
  • AccountView is a single-implementation interface (versionedio.go): only concreteAccountView exists (the versionMap-cell backing is future work), and it forces the double r.Val != nil && !r.Val.IsNil() check at every consumer — forgetting either half at a future call site is a nil-deref or wrong-existence bug. VersionedRead[*accounts.Account] gives one nil-check with identical semantics; introduce the interface when the second backing lands.

Minor

  • SetAccountFieldZero is behavior-identical to SetAccountFieldFromAccount(out, addr, path, ver, nil) — delegate or inline.
  • Several tests hand-roll accounts.Code{Hash: accounts.InternCodeHash(crypto.HashData(x)), Bytes: x} where accounts.NewCode(x) does exactly that (state_test.go, parallel_fixes_test.go, exec3_finalize_test.go).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions