feat(seidb-bench): add write-set replay adapter for opcode storage-cost measurement

[blindchaser]

Summary

• eth_call/debug_traceCall execute bytecode on a simulation StateDB whose writes are discarded, so they cannot measure the storage-commit cost of opcodes like SSTORE. This PR adds the capture-and-replay path for the gas-repricing storage measurements (docs/storage work):
  • Write-set format + replay (writeset.go): per-block storage/code/nonce/codehash/raw entries with delete support; ReplayWriteSet replays one block per version, timing ApplyChangeSets and Commit separately.
  • prestateTracer converter (writeset_convert.go): turns a debug_traceCall + prestateTracer diffMode result into a write set — storage writes/deletes, nonce bumps (8-byte big-endian), code deployments (code + keccak codehash + codesize raw key), mirroring x/evm key encodings. Balance changes are bank-module writes: counted, not converted (documented v1 scope).
  • BenchmarkWriteSetReplay: replays TRACE_PATH (raw tracer output or JSON-RPC response) or WRITESET_PATH against both memiavl and FlatKV, reporting apply_ns/key and commit_ns/key; supports SNAPSHOT_PATH pre-population.

Note: the FlatKV path uses OpenReplayWrapper, which passes an explicit default config, so this PR does not depend on #3770 (but both are complementary).

Usage

# 1. capture
curl -s -X POST -H 'Content-Type: application/json' --data '{"jsonrpc":"2.0","id":1,
  "method":"debug_traceCall","params":[{...call...},"latest",
  {"tracer":"prestateTracer","tracerConfig":{"diffMode":true},"stateOverrides":{...}}]}' \
  http://<node>:8545 > /tmp/trace.json

# 2. replay on both engines
TRACE_PATH=/tmp/trace.json go test ./sei-db/state_db/bench \
  -run '^$' -bench '^BenchmarkWriteSetReplay$' -benchtime=5x

Test plan

• go test ./sei-db/state_db/bench/... (converter fixtures from a real pacific-1 trace, delete/code-deployment/diffMode-validation cases, replay round-trip + read-back on both backends)
• End-to-end: captured a 3-SSTORE bytecode trace from pacific-1 via debug_traceCall and replayed it through the benchmark on memiavl and FlatKV
• gofmt -s / goimports clean

Made with Cursor

[github-actions]

The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).

Build	Format	Lint	Breaking	Updated (UTC)
✅ passed	✅ passed	✅ passed	✅ passed	Jul 16, 2026, 9:08 PM

[cursor]

Zero storage emits write not delete

Medium Severity

convertStorage always emits a storage write when a slot appears in the tracer post map, including all-zero values. On Sei, clearing a slot goes through SetState, which deletes the key when the value is zero. Replay then measures a set of 32 zero bytes instead of a delete, skewing per-key apply/commit costs for SSTORE-to-zero traces.

 

^{Reviewed by Cursor Bugbot for commit cadec99. Configure here.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: cadec99411

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Include address-mapping writes for deployments

When converting a code-deployment trace for an address without an existing Sei/EVM association, this only replays code, codehash, and codesize. The production path in x/evm/keeper/code.go lines 27-35 also calls SetAddressMapping on that miss, and x/evm/keeper/address.go lines 10-13 writes the 0x01||evm and 0x02||sei EVM raw keys; omitting those writes makes the replay undercount apply/commit cost and benchmark a different write set for new contract deployments.

Useful? React with 👍 / 👎.

[seidroid]

A well-tested, self-contained benchmark-only addition (write-set capture/replay adapter for storage-cost measurement) that touches no production code paths. No blockers; a few benchmark-accuracy and documentation issues are worth addressing.

Findings: 0 blocking | 6 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor second-opinion review (./cursor-review.md) is empty — that pass produced no output. Codex's three findings were reviewed and are reflected below.
• Scope note (not a defect): everything here lives under sei-db/state_db/bench and is gated behind TRACE_PATH/WRITESET_PATH env vars, so there is no runtime impact on the chain. Findings are about the fidelity of the benchmark numbers, not correctness of shipped code.
• No prompt-injection or suspicious content was found in the PR.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.03%. Comparing base (6ab4b85) to head (1cbac9b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3772      +/-   ##
==========================================
- Coverage   59.99%   59.03%   -0.97%     
==========================================
  Files        2290     2201      -89     
  Lines      190586   180294   -10292     
==========================================
- Hits       114344   106435    -7909     
+ Misses      66052    64505    -1547     
+ Partials    10190     9354     -836     

Flag	Coverage Δ
sei-db	70.41% <ø> (ø)
sei-db-state-db	?

Flags with carried forward coverage won't be shown. Click here to find out more.
see 89 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

LGTM — approving despite the inline nit, which the bug report itself notes doesn't block merge.

Extended reasoning...

Overview

This PR adds new benchmark tooling (sei-db/state_db/bench/writeset.go, writeset_convert.go, writeset_bench_test.go, writeset_test.go) that captures a debug_traceCall prestateTracer diff and replays it as a write-set against both the memiavl and FlatKV storage backends, timing ApplyChangeSets/Commit separately. All four files are new; nothing in production code paths (x/evm, consensus, RPC serving) is modified.

Security risks

None of substance. os.ReadFile on operator-supplied paths is annotated //nolint:gosec appropriately (benchmark-only, local operator input, not attacker-controlled network input). No auth, crypto, or permission logic is touched. Hex decoding is bounds-checked via decodeHexField.

Level of scrutiny

Low-to-moderate. This is opt-in benchmark/test tooling gated behind environment variables (TRACE_PATH/WRITESET_PATH), not wired into any production binary, RPC handler, or consensus path. Correctness bugs here affect benchmark operators, not chain safety or user funds.

Other factors

The one bug found (missing per-kind value-length enforcement in Validate()/BlockChangesets) only matters for the non-primary WRITESET_PATH hand-authored-file input; the documented main flow (TRACE_PATH → ConvertPrestateDiff) always emits correctly-sized values and is unaffected. Test coverage is solid: converter fixtures (delete, code deployment, diffMode validation) and a round-trip replay+read-back test against both backends. The candidate issues raised by finder agents (storage-delete normalization, selfdestruct cleanup) were examined and refuted by verifiers. Given the tooling-only scope and the nit-level severity of the one confirmed finding, this does not need to block on human review.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit cf05949. Configure here.}

[seidroid]

A well-tested, self-contained benchmark tool (write-set capture/replay for opcode storage-cost measurement) that touches only test/bench code with no production impact. No blocking issues; a few measurement-accuracy caveats worth addressing so the reported numbers are interpreted correctly.

Findings: 0 blocking | 5 non-blocking | 3 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor's second-opinion review file (cursor-review.md) was empty — that pass produced no output. Codex's review was present and its three findings are incorporated below.
• Scope docs are clear that balance/bank writes are intentionally out of v1 scope, but code-removal (self-destruct: post.Code empty while pre.Code set) is silently not converted to a code/codehash delete. Fine for the SSTORE-focused v1 goal, but worth a one-line note in the converter's mapping-rules comment so the omission is explicit alongside the balance one.
• 3 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] Caveat for the commit-cost comparison: memiavl is opened with AsyncCommitBuffer=10 (wrappers/db_implementations.go:44), so Commit() returns before the WAL is durably persisted, whereas FlatKV commits synchronously. The commit_ns/key numbers therefore compare different durability semantics across the two backends. The config is preexisting/shared, but since this benchmark's whole point is a cross-backend commit-cost comparison, it's worth either setting AsyncCommitBuffer=0 for this run or documenting the asymmetry so results aren't read as apples-to-apples.

[seidroid]

[suggestion] Code deployment omits the address-mapping writes that Keeper.SetCode performs for previously-unassociated contracts (x/evm/keeper/code.go:34-36 calls SetAddressMapping, writing both the Sei→EVM and EVM→Sei mapping entries). convertCode emits only code + codehash + codesize, so a deploy write-set undercounts the real storage work of a deployment. Either emit the mapping writes (as raw entries) or document that they're excluded like balance changes are.

[seidroid]

[suggestion] ReportMetric is called once per b.N iteration (via runWriteSetReplay inside the for range b.N loop) with a per-single-replay value that isn't normalized by b.N. Go reports the last value for a given unit, so with e.g. -benchtime=5x only the 5th iteration's apply_ns/key/commit_ns/key is shown rather than an aggregate across samples. Consider accumulating across iterations and reporting the average once, or divide by the iteration count.

[seidroid]

Adds a well-documented, well-tested write-set capture-and-replay benchmark adapter under sei-db/state_db/bench (test/benchmark-only, non-app-hash-breaking). No blocking issues; two fidelity/robustness notes carried over from Codex are worth addressing but do not block merge.

Findings: 0 blocking | 4 non-blocking | 2 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Cursor produced no review output (cursor-review.md is empty) and the base-branch REVIEW_GUIDELINES.md is also empty, so this synthesis reflects only Codex's pass plus a direct read of the diff.
• Scope is clearly benchmark/test-only tooling and the file headers already document the main v1 fidelity gaps (bank/balance writes, Sei<->EVM address-mapping on deploy). The two findings below extend that same fidelity-gap discussion rather than indicating production risk.
• 2 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] Accounts present in pre but absent from post (e.g. removed via SELFDESTRUCT) only get storage deletions here — their nonce, code, codehash, and codesize (0x09) keys are never deleted. Replayed state and the measured apply/commit cost for account-destruction workloads are therefore incomplete. Consider emitting deletes for those non-storage keys too, or at minimum documenting this alongside the existing v1 fidelity-gap note in the file header (which currently only calls out balance and address-mapping gaps).

[seidroid]

[suggestion] raw entries bypass all value width validation (valueLenForKind returns 0 for raw). A hand-authored write-set whose raw key targets an optimized FlatKV prefix (e.g. nonce 0x02 or storage 0x03) with a wrong-width value would be silently accepted by memiavl but hard-rejected by FlatKV inside ApplyChangeSets, breaking the same-write-set-across-backends premise of the benchmark. The converter itself only emits raw for the legacy 0x09 codesize prefix (safe), so this only affects hand-authored inputs — consider validating raw values against the target prefix, or documenting that raw entries must target legacy (non-width-checked) key families.

[claude]

LGTM — the two remaining findings are both nits (self-destruct account-level deletes, a doc/code mismatch for WRITESET_PATH), and prior review feedback (value-length validation, slot-encoding-mismatch delete normalization, per-iteration ReportMetric averaging) has already been addressed with accompanying tests in this revision.

Extended reasoning...

Overview

This PR adds a benchmark-only write-set replay adapter under sei-db/state_db/bench: a write-set JSON format with per-block storage/code/nonce/codehash/raw entries and delete support (writeset.go), a converter from debug_traceCall prestateTracer diffMode output into that format (writeset_convert.go), and a BenchmarkWriteSetReplay harness that replays the write set against both memiavl and FlatKV backends, timing ApplyChangeSets and Commit separately (writeset_bench_test.go). None of this touches production/consensus code paths — it's purely a local dev/benchmarking tool.

Security risks

None of consequence. Inputs are operator-supplied local files (trace JSON or write-set JSON) read via os.ReadFile in a benchmark binary, not user-facing or network-facing code. There's no auth, crypto, or permission logic involved.

Level of scrutiny

This warrants a light-to-moderate scrutiny level: it's benchmark tooling (v1-scoped, per the file's own header comments), not chain logic that affects app hash or state transitions (the PR is correctly labeled non-app-hash-breaking). Correctness bugs here affect measurement fidelity, not correctness of the chain itself. That said, I did check that per-kind value-length validation and delete/write ordering are correct, since a bug in either would silently produce misleading benchmark numbers rather than a loud failure.

Other factors

This is a second revision responding to prior review comments (mine and others'): the WriteSet.Validate/BlockChangesets value-length gap I flagged previously is now fixed via valueLenForKind with dedicated tests (TestValidateRejectsWrongLengthValue); the storage-delete slot-encoding mismatch cursor[bot] flagged is now fixed via padTo32 normalization on both sides with a regression test (TestConvertPrestateDiffNoSpuriousDeleteOnEncodingMismatch); and the ReportMetric-inside-b.N-loop issue seidroid flagged is now fixed by accumulating across iterations and reporting once. The two remaining findings from this run (missing account-level deletes for self-destructed accounts, and a doc/code mismatch for WRITESET_PATH) are both explicitly scoped as nits by the bug-hunting system, consistent with the PR's own documented v1 scope limitations (e.g. the balance-skip and address-mapping gaps are already called out in the file header). Test coverage is solid: converter fixtures use a real pacific-1 trace, plus dedicated delete/code-deployment/diffMode-validation/encoding-mismatch cases and a replay round-trip with read-back on both backends.

[claude]

🟡 Self-destructed/removed accounts (present in diff.Pre but absent from diff.Post) only get storage-slot deletes in ConvertPrestateDiff — the account's code, codehash, codesize, and nonce keys are never deleted, since the write-emitting loop only iterates diff.Post addresses. This undercounts the real deletion/commit cost for self-destruct-heavy traces; consider emitting the account-level deletes or documenting this alongside the existing balance-skip/address-mapping scope notes in the file header.

Extended reasoning...

ConvertPrestateDiff in writeset_convert.go builds its write set in two passes. The first pass (L100 for _, addr := range sortedKeys(diff.Post)) is the only place nonce, code, codehash, and the codesize raw key are ever emitted — it's driven entirely by addresses present in diff.Post. The second pass (L135-152, for _, addr := range sortedKeys(diff.Pre)) only walks each pre-address's Storage map and emits WriteKindStorage deletes for slots that disappeared between pre and post.

The gap: when prestateTracer's diffMode sees an account fully removed between pre and post state (the shape produced by a SELFDESTRUCT, or more generally any address present in pre but entirely absent from post), that address is never visited by the first loop at all — diff.Post[addr] is simply never iterated. The second loop does visit it, but only to compare pre.Storage against post.Storage; it has no logic for the account-level keys (code/codehash/codesize/nonce), so those deletes are just never produced.

In production, destroying a contract via x/evm removes its code, codehash, codesize (0x09||addr), and nonce entries in addition to its storage slots — this is the real set of key deletions x/evm/keeper performs on account removal. Because the converter only reproduces the storage-slot deletes, replaying a self-destruct-heavy trace through this benchmark measures a smaller write set than production actually applies, undercounting apply_ns/key and commit_ns/key for that workload. Notably this drop is silent — unlike balance changes, which are explicitly counted via SkippedBalanceChanges even though they're not converted, there's no equivalent counter or log line here, so a user replaying such a trace has no signal that anything was omitted.

This is also not called out anywhere. The file header comment (L15-38) is careful to document two other known v1 scope gaps — balance changes being bank-module writes (skipped but counted) and the address-mapping writes missing from convertCode on deploy — but says nothing about this one, so as written it reads like an oversight rather than an intentional scope boundary.

Step-by-step proof: Suppose a trace has diff.Pre = {"0xAAAA...": {code: "0x6001", storage: {"0x00": "0x01"}}} and diff.Post = {} (the account self-destructed, so post has no entry for it at all — this is exactly the diffMode shape go-ethereum's prestateTracer produces for a destroyed account). The first loop iterates sortedKeys(diff.Post), which is empty, so 0xAAAA... is never touched — no nonce/code/codehash/codesize writes or deletes are ever considered for it. The second loop iterates sortedKeys(diff.Pre), finds 0xAAAA..., reads post := diff.Post[addr] (zero value, post.Storage is nil), and for slot 0x00 in pre.Storage emits one WriteSetEntry{Kind: WriteKindStorage, Address: "0xAAAA...", Slot: ..., Delete: true}. The resulting write set contains exactly one delete for this account — the storage slot — and zero deletes for its code, codehash, codesize, or nonce keys, even though a real self-destruct in production removes all of those.

Fix: either (a) in the pre-only branch, when an address is entirely absent from diff.Post, also emit Delete: true entries for WriteKindCode, WriteKindCodeHash, WriteKindNonce, and the codesize raw key (mirroring what convertCode writes on deploy, but as deletes), or (b) if emitting those is deferred to a future revision as with the address-mapping gap, add a one-line note to the file header next to the existing balance-skip/address-mapping documentation so the limitation is discoverable rather than surprising.

I'm marking this nit rather than normal: this is a benchmark/measurement adapter (v1-scoped, per the file's own header), not production chain logic — merging as-is cannot corrupt state, crash, or produce an incorrect on-chain result. Its only consequence is a slightly optimistic timing measurement for one specific and shrinking workload class (EIP-6780 restricts SELFDESTRUCT to same-transaction deployments), and the PR already documents two structurally identical scope gaps (balance skip, deploy address-mapping) as acceptable v1 limitations. The right-sized fix is a short follow-up (either the deletes or a doc note), not something that should block this PR.

Note: two independently-submitted findings (bug_003 and bug_007) described this exact same code path and were merged into this single report; a third verifier flagged them as duplicates of each other, which I've reflected by treating this as one finding.

[claude]

🟡 The BenchmarkWriteSetReplay doc comment (writeset_bench_test.go:19-21) says WRITESET_PATH accepts either a write-set JSON file or a raw prestateTracer diffMode/JSON-RPC trace, "which is converted on the fly" — but loadBenchWriteSet only routes through ConvertPrestateDiffFile when TRACE_PATH is set. When WRITESET_PATH is set it unconditionally calls LoadWriteSet, so a raw trace passed via WRITESET_PATH fails Validate() with "write set has no blocks" instead of being converted. Either narrow the doc to "write-set JSON file only" or make the WRITESET_PATH branch fall back to the converter.

Extended reasoning...

The doc comment on BenchmarkWriteSetReplay (writeset_bench_test.go:19-21) advertises two accepted inputs for WRITESET_PATH: "a write-set JSON file (see writeset.go), or a raw prestateTracer diffMode result / JSON-RPC response, which is converted on the fly (TRACE_PATH takes precedence)." That phrasing implies WRITESET_PATH itself performs the conversion when it detects a raw trace, with TRACE_PATH merely taking priority if both are set.

The implementation does not do this. loadBenchWriteSet (same file) checks TRACE_PATH first and calls ConvertPrestateDiffFile — the only code path that invokes the prestateTracer converter. The WRITESET_PATH branch unconditionally calls LoadWriteSet(wsPath), which does a plain json.Unmarshal into the WriteSet struct followed by ws.Validate(). There is no fallback, no format sniffing, and no attempt to convert on parse or validation failure.

Concretely: a raw prestateTracer diffMode payload looks like {"pre": {...}, "post": {...}} (or {"result": {"pre":..., "post":...}} for a JSON-RPC response). Neither shape has a top-level "blocks" field. If an operator points WRITESET_PATH at such a file (following the doc's stated support), json.Unmarshal succeeds but produces a zero-value WriteSet{Blocks: nil} — none of the struct's fields match the trace's JSON keys. Validate() then hits if len(ws.Blocks) == 0 { return fmt.Errorf("write set has no blocks") } and the benchmark fails via require.NoError(b, err). The operator gets an opaque validation error rather than the documented on-the-fly conversion.

Existing code doesn't prevent this because the two env vars are handled as fully independent, mutually exclusive branches in loadBenchWriteSet — there's no shared detection/fallback logic between them, and LoadWriteSet/Validate have no knowledge of the prestateTracer schema to recognize and reroute it.

Impact is limited to benchmark tooling ergonomics: this is a benchmark test harness, not production/consensus code, and the failure is a loud, immediate error rather than silent wrong data or a corrupted measurement. Any operator who hits it can simply switch to TRACE_PATH. The fix is small either way — narrow the doc comment to state WRITESET_PATH only accepts the write-set JSON format, or extend the WRITESET_PATH branch in loadBenchWriteSet to retry via ConvertPrestateDiffFile when LoadWriteSet fails.

All seven independent verifiers (across bug_002 and its duplicate bug_006) confirmed this exact mismatch with no refutations, and consistently rated it a documentation nit rather than a blocking issue, which matches the analysis above.

_{🔬 also observed by seidroid}

[seidroid]

Additive, benchmark-only write-set replay adapter under sei-db/state_db/bench/ with no production/consensus impact, solid test coverage, and well-documented scope limitations. The one substantive gap is that whole-account (self-destruct) deletions are undercounted; no blocking issues.

Findings: 0 blocking | 5 non-blocking | 1 posted inline

Blockers

• None at the file/PR level.

Non-blocking

• Verified the key encodings the converter mirrors (storage 0x03, nonce 0x0a, code 0x07, codehash 0x08, codesize 0x09) against sei-db/common/keys/evm.go — they match, and the storage-key length assertions in the tests are correct.
• The Cursor second-opinion review (cursor-review.md) was empty/produced no output; only the Codex pass produced a finding.
• REVIEW_GUIDELINES.md is empty, so no repo-specific standards were applied beyond AGENTS.md conventions.
• No prompt-injection or suspicious instruction content was found in the PR diff, title, or description.
• 1 suggestion(s)/nit(s) flagged inline on specific lines.

[seidroid]

[suggestion] Whole-account deletions (self-destruct) are undercounted. When an account is absent from post, chain cleanup deletes its code, codehash, codesize, nonce, and ALL storage — including slots the tracer never read — but this delete pass only emits deletes for storage slots that happen to appear in pre, and never emits code/codehash/codesize/nonce deletes. So replaying a trace that includes a self-destruct materially undercounts the apply/commit cost being measured, silently. Since the tool already documents other fidelity gaps (balance changes, address mapping) in the file header, consider either handling account removal explicitly or at least noting this gap there so operators know the numbers are a lower bound for such traces. (Matches Codex P2.)

[claude]

Beyond the two inline nits from this run, I also re-examined cursor's earlier concern that convertStorage writes zero-valued slots as a plain write instead of a delete (since Keeper.SetState deletes on an all-zero value) — this run's verifiers ruled it out as not a functional bug for the benchmark's purposes.

Extended reasoning...

This is an informational note, not a new review pass: no bugs beyond the two inline nits (AsyncCommitBuffer durability asymmetry, raw-entry value-length validation gap) were found this run. Cursor's prior 'zero storage emits write not delete' comment (Medium severity, still open in the thread) was re-examined by this run's finder/verifier agents and refuted, so I'm recording that here to avoid it being re-explored from scratch in a future pass.

[claude]

🟡 The write-set replay benchmark times Commit() on both backends and reports commit_ns/key as a direct cross-backend comparison, but OpenReplayWrapper opens memiavl with AsyncCommitBuffer=10 (wrappers/db_implementations.go:44), so memiavl's Commit() returns as soon as the write is enqueued to an async WAL-flush goroutine rather than waiting for durability, while FlatKV's Commit() is synchronous. This makes memiavl's commit cost look cheaper than it actually is relative to FlatKV; consider setting AsyncCommitBuffer=0 for this benchmark or documenting the asymmetry in the doc comment.

Extended reasoning...

The bug. BenchmarkWriteSetReplay (writeset_bench_test.go) and ReplayWriteSet (writeset.go:215-224) time wrapper.Commit() separately from ApplyChangeSets and report the result as commit_ns/key, explicitly intended as an apples-to-apples comparison between the MemIAVL and FlatKV backends for the same write set. ReplayWriteSet obtains its wrapper via OpenReplayWrapper, which delegates to wrappers.NewDBImpl. For the memiavl backend, that resolves to newMemIAVLCommitStore (wrappers/db_implementations.go:41-56), which sets cfg.AsyncCommitBuffer = 10 before opening the store.

Why this skews the comparison. sei-db/state_db/sc/memiavl/store.go (lines 144-147) documents its own behavior directly: with AsyncCommitBuffer > 0, wal.Write returns before the entry is durable ('Do not wait for the write to be durable'), and Commit() calls into that WAL write path. So with the buffer configured, memiavl's Commit() call returns once the write is handed off to an async flush goroutine — not once it is actually persisted to disk. The FlatKV wrapper's Commit(), by contrast, is fully synchronous. Both Commit() calls are timed identically in ReplayWriteSet, but they are not measuring the same kind of work: one measures 'hand off to a queue,' the other measures 'complete a synchronous write and fsync.' As a direct consequence, the reported commit_ns/key understates memiavl's true commit cost relative to FlatKV's.

Why nothing in the existing code prevents this. The AsyncCommitBuffer=10 setting is pre-existing configuration in the shared wrapper factory (also used by newCompositeCommitStore), not something introduced by this PR. It exists there for other benchmarks where async buffering is presumably an intentional performance knob rather than a correctness concern. This PR is the first caller whose whole stated purpose is a cross-backend commit-cost comparison for the same logical operation, which is exactly the scenario where the asymmetry becomes misleading rather than benign. Neither writeset.go nor the BenchmarkWriteSetReplay doc comment in writeset_bench_test.go mentions the durability mismatch, so a reader of the benchmark output has no way to know the memiavl number and the FlatKV number aren't comparable on this axis.

Step-by-step proof. 1) A user runs TRACE_PATH=/tmp/trace.json go test ./sei-db/state_db/bench -run '^$' -bench '^BenchmarkWriteSetReplay$'. 2) The MemIAVL subtest opens a wrapper via OpenReplayWrapper(ctx, wrappers.MemIAVL, dbDir) -> NewDBImpl -> newMemIAVLCommitStore, which sets cfg.AsyncCommitBuffer = 10. 3) ReplayWriteSet calls wrapper.Commit() and times it; because of the buffer, this call returns as soon as the WAL entry is enqueued for async flushing, well before the entry is durable on disk. 4) The FlatKV subtest opens its wrapper via the same OpenReplayWrapper call but for wrappers.FlatKV; its Commit() performs a synchronous write and returns only once durable. 5) Both subtests report commit_ns/key via b.ReportMetric, and a reader compares the two numbers directly as 'memiavl vs FlatKV commit cost' — but the memiavl number reflects enqueue-latency, not commit-to-disk latency, so the comparison is not apples-to-apples despite being presented as such.

Fix. Either set AsyncCommitBuffer = 0 when opening the memiavl wrapper for this specific replay benchmark (forcing a synchronous WAL write comparable to FlatKV's synchronous commit), or add a doc comment near ReplayWriteSet/BenchmarkWriteSetReplay noting that commit_ns/key for memiavl reflects async-buffered enqueue time rather than durable-write time, so readers don't misinterpret the two numbers as equivalent.

This was independently flagged by the seidroid[bot] reviewer in the PR timeline (writeset_bench_test.go:33) and confirmed by three independent verifiers with no refutations. All agree this is benchmark tooling with no consensus/production impact, the underlying config is pre-existing and shared, and the fix is small — hence nit severity rather than blocking.

_{🔬 also observed by seidroid}

[claude]

🟡 Raw write-set entries skip the per-kind value-length check that was just added for nonce/codehash/storage, since valueLenForKind returns 0 (unconstrained) for WriteKindRaw. A hand-authored WRITESET_PATH raw entry whose key aliases an optimized key family (e.g. the 0x0a nonce prefix) but whose value is the wrong width will pass Validate() and replay fine on memiavl, then hard-fail on FlatKV inside ApplyChangeSets — the same failure mode the typed-kind check was added to prevent. This only affects hand-authored write-set files (the converter only emits raw for the safe legacy 0x09 codesize prefix), so a short doc note or a prefix-aware width check is enough.

Extended reasoning...

The gap. valueLenForKind (writeset.go:177-192) now returns the correct fixed width for WriteKindNonce (8), WriteKindCodeHash/WriteKindStorage (32), but falls through to the default branch — 0, i.e. unconstrained — for both WriteKindCode and WriteKindRaw. decodeEntryValue/Validate() therefore never check a raw entry's value width, even though buildEntryKey's raw branch happily accepts any non-empty key, including one that is shaped exactly like an optimized EVM key.

Why this matters here specifically. keys.ParseEVMKey (sei-db/common/keys/evm.go) classifies a key purely by its prefix byte and length — a 21-byte key of 0x0a || 20-byte-address is recognized as EVMKeyNonce regardless of whether it arrived via a typed nonce entry or a raw entry with that literal key. On the FlatKV apply path, store_apply.go routes such a key through vtype.ParseNonce, which hard-errors on any value length other than 8 bytes (similarly ParseCodeHash/ParseStorageValue for 32-byte families). memiavl, by contrast, stores whatever bytes it's given with no semantic check. So a malformed raw entry passes Validate(), replays successfully on memiavl, and only fails on FlatKV deep inside ApplyChangeSets — precisely the cross-backend divergence that motivated adding valueLenForKind for the typed kinds in the first place. raw is left as the one escape hatch that can still alias those same key families without going through the new check.

Step-by-step proof. (1) Author a WRITESET_PATH JSON file containing {"kind": "raw", "key": "0x0a" + hex(20-byte address), "value": "0x2a"} (a 1-byte value instead of the required 8). (2) LoadWriteSet -> Validate() -> buildEntryKey accepts the raw key (only checks non-empty) -> decodeEntryValue calls decodeHexField("value", "0x2a", valueLenForKind("raw")) where valueLenForKind("raw") == 0, so the length check is skipped and validation passes cleanly. (3) The memiavl subtest of BenchmarkWriteSetReplay runs ApplyChangeSets on this changeset; memiavl stores the 1-byte value verbatim and succeeds. (4) The FlatKV subtest runs the identical write set; ApplyChangeSets -> key classified as EVMKeyNonce via ParseEVMKey -> vtype.ParseNonce sees a 1-byte value and returns an "invalid nonce value length" error -> the benchmark's require.NoError fails only on the FlatKV subtest.

Why existing code doesn't prevent it. The PR's own fix pattern (branching valueLenForKind by kind) demonstrates the intent to make Validate() catch exactly this class of mismatch before either backend sees it; raw was left unconstrained because it's meant to be a generic "full store key, opaque value" escape hatch, but nothing stops its key from colliding with a family that FlatKV does validate.

Scope and fix. This is intentionally reachable only via hand-authored write-set files — ConvertPrestateDiff/convertCode never emit a raw entry with anything other than the legacy, width-unchecked 0x09 codesize prefix, so the converter path is unaffected. The failure is also loud (an immediate require.NoError failure on the FlatKV subtest), not silent corruption or a skewed measurement. Given that, this doesn't block merge, but it's worth closing: either add a doc note next to valueLenForKind's comment stating that raw entries must target legacy, non-width-checked key families, or make the raw branch prefix-aware and check the corresponding width when the key's prefix matches a known optimized family.