docs: v6.6.0 catch-up - #61
Conversation
…ionCountByHash now recognize the genesis block hash and return the genesis block (and a tx count of 0), keeping hash-based lookups consistent with eth_getBlockByNumber("0x0"). (sei-protocol/sei-chain#3069)
…ull (instead of an error) for empty or non-existent block hashes, matching Ethereum RPC spec. (sei-protocol/sei-chain#3067)
…the config field 'max-outbound-connections' changed type/behavior and its default logic, and the RouterOptions MaxPeers/MaxConnected/MaxConcurrentDials fields were replaced by MaxInbound/MaxOutbound, affecting node connection tuning. (sei-protocol/sei-chain#3037)
… checks on signature values, access lists, auth lists, and Cosmos wrapper fields, plus a new transaction body bloat rejection that may reject previously-accepted transactions. (sei-protocol/sei-chain#3073)
…des a pub_key field (with a zero/empty public key for non-validator nodes) for CosmJS compatibility, changing the status response output. (sei-protocol/sei-chain#3114)
…tBlockByNumber returns null (not an error) for future/non-existent numeric block heights, and eth_getProof works across additional store backends (tracekv, Giga cache, other proof-capable stores) instead of only classic IAVL. (sei-protocol/sei-chain#3119)
…ctionCountByHash RPC methods now return counts consistent with getBlockByNumber, filtering EVM txs by receipt availability and including wasm execute and bank send messages. (sei-protocol/sei-chain#3125)
…sc-enable-lattice-hash) were removed from the node config TOML template, and split_write mode now requires lattice hash to be enabled. (sei-protocol/sei-chain#3128)
… from 5% to 0% because the Oracle Price Feeder is retired. (sei-protocol/sei-chain#3157)
…arnings urging users to migrate to SeiDB to avoid data corruption or panic. (sei-protocol/sei-chain#3159)
…h requests by forwarding only the allowed subset and merging responses by id, and the HTTP request body limit was lowered from 32MiB to 5MiB to match go-ethereum's default. (sei-protocol/sei-chain#3160)
…commit (SC) is now mandatory; multiple CLI commands and config fields tied to IAVL/pruning have been removed, and the node will panic if SC is not enabled. (sei-protocol/sei-chain#3146)
… generate an Autobahn (GigaRouter) JSON config from node pubkey files, plus new side-effect files (validator_pubkey.txt, node_pubkey.txt) written when saving keys. (sei-protocol/sei-chain#3220)
…receipt store retention is now always derived from the global `min-retain-blocks` flag. (sei-protocol/sei-chain#3237)
…the node now disables the mempool gossip reactor, consensus reactor, state sync, and block sync, and the Autobahn block producer sources transactions directly from the mempool with new per-block limits (max 2000 txs, ~2MB total). (sei-protocol/sei-chain#3224)
…C 2.0 notification rules: notifications (requests without an id) produce no response entries, so merged batch responses are no longer 1:1 with the request, and an all-notification/empty result returns an empty HTTP body instead of an empty array []. (sei-protocol/sei-chain#3246)
…array [] instead of null when there are no matching logs, aligning with the Ethereum JSON-RPC spec. (sei-protocol/sei-chain#3292)
…ubdirectory layout (data/state_commit, data/state_store, data/ledger, data/tendermint) for new nodes, with automatic backward-compatible fallback to legacy flat paths for existing nodes. (sei-protocol/sei-chain#3155)
…ed, receipt-by-tx-hash lookups that miss the cache now fail fast with an error instead of falling back to a full parquet scan, affecting operators who run nodes with the tx index disabled. (sei-protocol/sei-chain#3294)
…quet (previously unchanged), though callers can still override with an explicit RECEIPT_BACKEND env var. (sei-protocol/sei-chain#3298)
…eturns trace results with detailed timing/store-access profiling, plus a new seidb trace-profile-report CLI command to run it across a block range. (sei-protocol/sei-chain#3267)
…ck_height` field in SyncInfo, and under Autobahn the status endpoint populates height/app-hash from the app layer instead of the CometBFT block store. (sei-protocol/sei-chain#3309)
…l are now treated as normal requests (receiving a response) instead of as notifications; only requests omitting the "id" member are treated as notifications. (sei-protocol/sei-chain#3303)
…ded to sei-tendermint as a temporary disaster-recovery mechanism for chain stalls, requiring validator majority coordination to enable. (sei-protocol/sei-chain#3305)
…ults to true, and the coordination semantics are inverted so validators must coordinate to set it to false. (sei-protocol/sei-chain#3318)
… flag to the `state-size` command for analyzing FlatKV stores. (sei-protocol/sei-chain#3312)
…eprecated and ignored; stateless leader election is always enabled regardless of the value set. (sei-protocol/sei-chain#3319)
…with one extra element (the projected child base fee for the block after the newest block), matching go-ethereum/execution-apis semantics. (sei-protocol/sei-chain#3321)
…/block_by_hash, /block_results, and /validators now serve data via the GigaRouter's in-memory state instead of returning empty/failing responses when the CometBFT BlockStore/StateStore is unpopulated. (sei-protocol/sei-chain#3310)
…aRouter block execution path to prevent valid out-of-order EVM nonce transactions from being evicted and stalling throughput to one tx per block per sender. (sei-protocol/sei-chain#3372)
|
I've done a full review of this catch-up against sei-chain at the v6.6.0 tag, 656 claims checked in total. That surfaced 15 major and 26 minor findings, and I've fixed them all directly on the branch in 46de0dc rather than leaving a to-do list, so each inline comment documents what changed and why. One fix you won't see in the final diff: the bot's commit deleted Three of the majors share one root cause worth a bridge guard: commits generated per source PR were never reconciled against later PRs in the same release. #3473 re-added the config fields #3128 removed, #3618 removed endpoints that the #3450/#3459 text still describes, and #3768/#3781 superseded #3383's receipt behaviour before the tag was cut. A final pass that re-validates each assembled page against the release tag (rather than per PR diffs) would catch this whole class. Two more pipeline notes: the PR table lists 30 source PRs but the diff actually carries content from around 35 (the early commit backlog plus #3611 and #3625 are invisible to anyone reviewing from the table), and several hand-written comments landed inside AUTO-GENERATED blocks where the next regen will clobber them. Also a heads-up for the next catch-up, these were correct at v6.6.0 but have already changed on main: littidx receipt backend (#3620), gov-driven SC migration replacing manual sc-write-mode (#3650), the legacy to misc bucket rename (#3665/#3758), a third ExcludeTraceFail exclusion class (#3871), and JS tracers disabled (#3718). Happy to share the full finding-by-finding report if useful, just let me know! |
monty-sei
left a comment
monty-sei
left a comment
There was a problem hiding this comment.
Approving. Full review against sei-chain v6.6.0 is in the comments above, all findings fixed in 46de0dc and each fix re-verified against source. The drift items called out in my summary are for the next catch-up, not this PR.
PR SummaryLow Risk Overview EVM developers get new or expanded guidance on stricter transaction validation (v6.5.0+ field checks, canonical envelopes, whole-block rejection on decode failure), receipt semantics (failed nonce-bumping txs return Node operators see SeiDB/FlatKV SC migration ( Observability adds OpenTelemetry metric families for EVM RPC, FlatKV, LittDB, and IBC, with migration notes from legacy Reviewed by Cursor Bugbot for commit 6f30e6e. Bugbot is set up for automated code reviews on this repo. Configure here. |
Resolves the FAQ data-locations conflict in giga-storage-migration.mdx by taking main's pebbledb-only phrasing and override bullet (PR #62 RocksDB deprecation) and folding in the verified legacy-path precedence rule (GetStateStorePath/GetEVMStateStorePath: an existing legacy directory wins; new nodes use the structured layout).
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
A large, largely accurate v6.6.0 docs catch-up, but it introduces several factual contradictions — duplicated-and-conflicting rows in the legacy sei_* method table, two new passages naming different methods (sei_getVMError vs eth_getVMError) for the same thing, and a self-negating eth_feeHistory invariant. Both second-opinion passes (Codex, Cursor) and REVIEW_GUIDELINES.md were empty, so this is a single-reviewer assessment.
Findings: 4 blocking | 16 non-blocking | 7 posted inline
Blockers
evm/reference.mdxlegacysei_*method table now lists three methods twice with conflicting descriptions (lines 1364 vs 1382, 1366 vs 1383, 1375 vs 1384). The new detailed rows were added without removing the pre-existing terse rows, and the old rows state the opposite of the new behavior ("excluding failed traces" vs "reverted and out-of-gas transactions are now included"). Delete the stale rows at 1382-1384.- Two passages added by this PR name different methods for retrieving the VM error on a nonce-bumping failed tx:
evm/reference.mdx:269sayssei_getVMError(a legacy method that is gated behindenabled_legacy_sei_apisand disabled by default), whileevm/transactions.mdx:317sayseth_getVMError(documented as a live eth-namespace extension atevm/reference.mdx:421). Pick one and make both pages agree. - 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
node/validators.mdx:115,122still documentspersistent_peers(snake_case) in the sentry-node example, but this PR renames the[p2p]keys to kebab-case and replacesseedswithbootstrap-peers(node/technical-reference.mdx). An operator copying validators.mdx would write a key the node no longer recognizes — worth folding into this catch-up.eth_getBlockTransactionCountByHash(evm/reference.mdx:655) was not given the new "receipts have been pruned" behavior, although the siblingeth_getBlockTransactionCountByNumberwas and the PR description says sei-chain#3216 changed both.evm/installing-seid-cli.mdxdropscompact,latest_version, andprunefrom the command list and addslog/snapshot, but no source PR in the description table covers this file (norevm/evm-parity/state-proofs.mdx,evm/evm-parity/transaction-types.mdx, orevm/precompiles/oracle.mdx). Operators who scriptedseid pruneget no migration note. These four files need manual verification against the upstream binary.- The PR body's table lists 30 source PRs, but the branch has 65 commits, several of which are superseded by later ones in the same PR (e.g. "receipt backend now defaults to parquet" followed by "parquet removed"). The stated "reviews commit-by-commit" workflow doesn't hold; the description should reflect the final diff.
- The description's own "Possible self-contradictions" section is stale: it claims
node-operators.mdxremovessc-write-mode/sc-keys-to-migrate-per-block, but both are present (lines 1480, 1485), and the flagged FlatKV path inconsistency is resolved by the legacy-path<Note>innode/troubleshooting.mdx:112. Those two items are false positives; the reader shouldn't have to re-verify them. - Referenced-but-undocumented tooling:
seidb trace-profile-report(evm/reference.mdx:1288) is pointed at as the way to run profiling across a block range, and the description flagsseidb import-flatkv-from-memiavl/memiavl-latest-versionas gaps — none are documented anywhere in the docs set. EnablePebbleMetrics(node/advanced-config-monitoring.mdx) is documented without stating its default value; every other config knob added in this PR states one. The description's reviewer note for sei-chain#3366 explicitly asked for this to be confirmed.- The
sei_unsafe_validation_skipped_totalmetric (from sei-chain#3429, mentioned in the reviewer notes) is not added to the metrics tables, even though themock_chain_validationbuild tag it accompanies is now documented. - The same transaction-validation rules are now stated on three pages (
evm/differences-with-ethereum.mdx,evm/evm-parity/transaction-types.mdx,evm/transactions.mdx) with slightly different wording and version framing. Consider making one canonical and cross-linking, to avoid drift on the next catch-up. - Both configured second-opinion passes produced no output:
codex-review.mdandcursor-review.mdare empty files.REVIEW_GUIDELINES.mdis also empty, so no repo-specific standards were applied. - No prompt-injection or instruction-like content was found in the diff, commit messages, or PR body.
- 5 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
evm/reference.mdx:1364(RIGHT) -- [blocker] These detailed*ExcludeTraceFailrows were added, but the pre-existing terse rows for the same three methods are still in the same table at lines 1382-1384:
| `sei_getBlockByHashExcludeTraceFail` | Get block by hash excluding failed traces |
| `sei_getBlockByNumberExcludeTraceFail` | Get block by number excluding failed traces |
| `sei_getTransactionReceiptExcludeTraceFail` | Get receipt excluding failed traces |
So each method is listed twice, and the old wording contradicts the new: line 1384 says "excluding failed traces" while line 1375 says reverted and OOG transactions are now included. Please delete lines 1382-1384.
evm/reference.mdx:269(RIGHT) -- [blocker] This says theVmErroris "retrievable viasei_getVMError", butevm/transactions.mdx:317— added in the same PR for the same behavior — says to "retrieve it with the non-standardeth_getVMErrormethod".
eth_getVMError is documented as a live eth-namespace extension at line 421 of this file, whereas sei_getVMError is in the legacy allowlist table (line 1362) and is therefore disabled by default unless an operator adds it to enabled_legacy_sei_apis — and the page's own deprecation warning says not to build new integrations on sei_*. Pointing developers at sei_getVMError here is likely the wrong one; either way the two pages need to agree.
evm/reference.mdx:748(RIGHT) -- [suggestion] This passage asserts a hard invariant and then negates it two sentences later:len(baseFeePerGas) == len(gasUsedRatio) + 1… "Note that when some heights in the range have pruned/partial base-fee data,baseFeePerGasmay still be shorter thangasUsedRatio + 1".
A client that codes to the stated invariant will index out of bounds on pruned ranges. Reword so the pruned case is the documented exception to a "normally" relationship rather than an afterthought contradicting an absolute rule.
evm/transactions.mdx:317(RIGHT) -- [suggestion] Two issues with this sentence:
- It says the synthetic receipt has
gasUsedof 0 andeffectiveGasPriceof 0. That is exactly the discriminatorevm/tracing/index.mdx:427documents for dropping "never executed" transactions from the*ExcludeTraceFailendpoints (EffectiveGasPrice == 0 && GasUsed == 0). As written, these nonce-bumping failures would be silently filtered out ofsei_getTransactionReceiptExcludeTraceFaildespite having reached state transition. Either that's true and worth stating explicitly, or one of the two descriptions is wrong. evm/reference.mdx:269describes the same receipt but mentions onlygasUsedof 0, noteffectiveGasPrice. Worth aligning.
evm/evm-parity/transaction-types.mdx:29(RIGHT) -- [nit] "EVM transactions now undergo stricter semantic validation" reads as new-in-v6.6.0, butevm/transactions.mdxandevm/differences-with-ethereum.mdxboth state these checks have been enforced since v6.5.0. Add the same "since v6.5.0" qualifier here so readers don't misdate the behavior change.node/node-operators.mdx:1480(RIGHT) -- [suggestion]sc-read-modeandsc-enable-lattice-hashwere deleted from this sample with no replacement note. Every other removal in this PR gets an explicit upgrade instruction (parquet → error message;proxy-app/abci→ "remove these lines from your existing config.toml";pending-ttl-*→ deprecated<Info>). An operator whoseapp.tomlstill carries these two keys has no way to know whether they're ignored, rejected at startup, or simply undocumented — please say which.node/index.mdx:204(RIGHT) -- [nit] "Combinesmock_balancesandmock_chain_validation" is circular — the bullet is definingmock_chain_validation-*, so saying it combines itself withmock_balancesdoesn't parse. If the image is built with both tags, say so directly (e.g. "built with themock_balancesandmock_chain_validationbuild tags").
- dedupe the three ExcludeTraceFail rows appended to the tail of the legacy sei_* method table (already documented with corrected semantics higher in the same table) - point the eth_getTransactionReceipt VmError note at eth_getVMError, matching evm/transactions.mdx and this page's own method section (sei_getVMError remains listed as the gated legacy alias) - reword the eth_feeHistory length invariant conditionally so it no longer self-negates: the +1 relationship holds when every block in the range has base-fee data; pruned heights skip baseFeePerGas rows (matches FeeHistory in evmrpc/info.go at v6.6.0)
- state explicitly that nonce-bump synthetic receipts (0/0 stub shape) are excluded by the *ExcludeTraceFail endpoints (isReceiptUntraceable) - align reference.mdx receipt note to mention effectiveGasPrice 0 - date strict semantic validation to v6.5.0 in transaction-types.mdx - de-circularise the mock_chain_validation image bullet and document the sei_unsafe_validation_skipped_total counter - validators.mdx sentry example: kebab-case p2p keys, drop the removed addr_book_strict key - eth_getBlockTransactionCountByHash: add the pruned-receipts error note (guard applies to both methods, evmrpc/block.go:182,204) - state EnablePebbleMetrics default (true) - note removal of compact/latest_version/prune seid commands - note that sc-read-mode/sc-enable-lattice-hash were never seid keys - document import-flatkv-from-memiavl and memiavl-latest-version with the import-height-must-equal-memiavl-latest requirement
There was a problem hiding this comment.
A large (67-commit) automated v6.6.0 docs catch-up across 16 EVM/node pages. The content is broadly accurate and internally consistent — the snake_case→kebab-case config rename, parquet removal, and IAVL-removal messaging are complete with no stale references left behind, and the retired-oracle change has no dangling cross-links. Two added sections need fixing before merge: a self-contradiction about which heights /validators serves under Autobahn, and an "Enabling Pebble Internal Metrics" section that gives operators no way to actually enable it.
Findings: 2 blocking | 14 non-blocking | 10 posted inline
Blockers
- None at the file/PR level.
- 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
- The PR body's source-PR table has 30 rows but the PR contains 67 commits, so roughly half the diff is not described in the description that says "this reviews commit-by-commit." Undescribed content includes the oracle precompile retirement (v6.4.0), the v6.5.0 ValidateBasic tightening, the p2p/consensus kebab-case config rename, the data/tendermint/ layout change, eth_getProof store-backend unwrapping, the /status last_committed_block_height and validator_info fields, seidb evm-logical-digest, stateless-leader-election deprecation, JSON-RPC batch/notification semantics, and the oracle min_valid_per_window default. Those are consensus- and operator-facing claims a reviewer cannot trace back to a source PR.
- The same v6.5.0 transaction-validation rules are now documented in three places with no cross-links: evm/differences-with-ethereum.mdx (wrapper fields, bloated bodies), evm/transactions.mdx (signature/access-list/auth-list validation, wrapper fields, bloated bodies), and evm/evm-parity/transaction-types.mdx (access-list, auth-list, signature values). Three independent copies of the same normative rules will drift. Pick one canonical home and link to it from the other two.
- Both second-opinion passes produced no output — codex-review.md and cursor-review.md are each 1 byte (empty). This review is a single-model pass with no cross-check.
- Cosmetic: the diff adds many blank-line-only lines, producing 2–3 consecutive blank lines before most new headings (evm/reference.mdx, evm/transactions.mdx, evm/tracing/index.mdx, node/advanced-config-monitoring.mdx, node/technical-reference.mdx, node/giga-storage-migration.mdx). Collapse to a single blank line.
- Heading case is inconsistent within the PR: the new node/giga-storage-migration.mdx headings use sentence case ("Write modes", "Operator-facing knobs", "Checking migration status") per AGENTS.md, while most other new headings use Title Case ("Pre-Baked Trace Cache", "Transaction Validation", "seidb Tooling Commands"). Title Case does match pre-existing neighbours in those files, so this is low priority — but worth settling one way.
- node/technical-reference.mdx uses /root/.sei/... in the seidb examples while node/troubleshooting.mdx uses $HOME/.sei/... for the same commands. Prefer $HOME consistently.
- 8 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
evm/reference.mdx:99(RIGHT) -- [blocker] This contradicts line 92 in the same new block. Line 92 says/block_resultsand/validators"do not read the retained block data and respond for any height up to the current ABCI head" — i.e. pruned heights still work. This row says/validators"Returns the genesis committee at any retained height." One of the two is wrong, and they give operators opposite answers about whether/validatorsworks for a pruned height. Since the committee is fixed at genesis and (per line 92) the endpoint doesn't touch retained block data, I'd expect "any height up to the current ABCI head" to be correct here; please confirm against the source and make the table match the prose.node/advanced-config-monitoring.mdx:586(RIGHT) -- [blocker] This section is titled "Enabling Pebble Internal Metrics" but gives operators no way to enable anything.EnablePebbleMetricsis written in Go-struct CamelCase, with noapp.tomlkey name and no section.
I checked the AUTO-GENERATED app.toml block in node/node-operators.mdx — it is current for v6.6.0 (it contains sc-keys-to-migrate-per-block at line 269) and its [state-commit.flatkv] section holds only fsync, async-write-buffer, snapshot-interval, and snapshot-keep-recent. There is no pebble-metrics key there. Either give the real kebab-case key and section, or state plainly that this is an internal default and not operator-settable.
Separately, the (default true) claim needs verifying: the PR's own reviewer note for sei-chain#3366 says the source diff's test only toggles it to false and "does not reveal the DefaultConfig default; a reviewer should confirm the actual default value before finalizing edit content." That confirmation doesn't appear to have happened.
evm/reference.mdx:603(RIGHT) -- [suggestion] Two issues with the hardcoded genesis hash0xF9D3...AB9E:
- No network is named. Sei has pacific-1 (1329) and atlantic-2 (1328); if the genesis hash differs per network, this line is wrong for at least one of them. Qualify which chain it belongs to, or describe the behavior without pinning the literal ("the genesis block hash is recognized and returns the encoded genesis block").
AGENTS.mdalso prefers not hard-coding network-specific values. - Uppercase hex. Ethereum JSON-RPC hashes are conventionally lowercase, and every other hash in this file is lowercase — a reader copy-pasting this may hit a case-sensitive comparison.
node/index.mdx:332(RIGHT) -- [suggestion] The comment correctly says these fields are ignored, but the sample still showspending-ttl-duration = "3s"andpending-ttl-num-blocks = 5, which reads as though those values do something. The generatedconfig.tomlinnode/node-operators.mdx(lines 1103–1105) emitspending-ttl-duration = "0s"andpending-ttl-num-blocks = 0. Match the generated defaults ("0s"/0) or drop the two lines from the sample and rely on the<Info>callout below.evm/tracing/index.mdx:423(RIGHT) -- [suggestion] This###subsection sits under## Pre-Baked Trace Cache, but*ExcludeTraceFailfiltering has nothing to do with the trace cache — and thesei_getBlockBy*/sei_getTransactionReceipt*endpoints it describes aren't among the cached methods listed just above (debug_traceTransaction,debug_traceBlockByNumber,debug_traceBlockByHash). Nesting it here implies the filter is a cache behavior and buries it where readers won't look. Promote it to its own##section, or move it next to the legacy-endpoint docs inevm/reference.mdx.node/giga-storage-migration.mdx:231(RIGHT) -- [suggestion] The write-mode progression dead-ends here. The section walks the operator throughmemiavl_only→migrate_evm→evm_migrated→migrate_all_but_bank→all_migrated_but_bank→migrate_bank, then saysflatkv_onlyis the terminal mode but "is not a flip target for anevm_migratednode." So what does an operator set oncemigrate_bankfinishes? Either name the mode explicitly, or say directly thatflatkv_onlybecomes a valid flip target oncebank/is drained.evm/installing-seid-cli.mdx:131(RIGHT) -- [suggestion] Three gaps in this note relative to the hunk right above it:
- The same diff adds
log("Runtime log management") andsnapshot("Create a snapshot using the snapshot manager") to the command list, but the note only covers removals. New commands deserve a mention too. min-retain-blocksis a baseapp.tomlfield, not SeiDB configuration — onlyss-keep-recentis under[state-store]. Calling both "SeiDB configuration" is misleading.latest_version's replacement isn't named, even thoughnode/troubleshooting.mdxdocumentsseidb memiavl-latest-versionin this same PR. Point readers at it.
node/troubleshooting.mdx:93(RIGHT) -- [suggestion] This block still passes-d $HOME/.sei/data/committer.dbtodump-iavl, but ~40 lines below the newevm-logical-digestexample uses--db-dir $HOME/.sei/data/state_commit/memiavlfor the same memIAVL store — and this PR is precisely what documents the newdata/state_commit/layout. Two different paths for one store in adjacent examples will send operators to the wrong directory. Either update this to the current path or add the same legacy-path-takes-precedence note you used for the FlatKV example.node/technical-reference.mdx:180(RIGHT) -- [nit] "roughly 2 MB" is too vague for a limit enforced at decode time — an operator sizing payloads needs the exact byte cap. Relatedly, line 160 says max transactions per block is "the lower of the configuredmax_txs_per_blockand the built-in maximum" without naming the built-in maximum, which appears to be the 2000 stated here. Give both exact values and cross-reference the two bullets.evm/differences-with-ethereum.mdx:179(RIGHT) -- [nit] This warning restates version scoping the reader just saw: the paragraph directly above already ends with "This check has been enforced since v6.5.0," and the preceding<Warning>already flagged the whole-block rejection as the new behavior. Consider folding the version scoping into a single lead-in sentence for the whole "EVM Transaction Envelope Restrictions" section rather than repeating it across two callouts.
Blockers: - /validators row now matches the prose and the code: bounded only by the ABCI head (autobahnCheckAndGetHeight -> getHeight), not the retained window - Pebble metrics section now names the real key: enable-pebble-metrics under [state-commit.flatkv] (mapstructure tag, flatkv config.go:47; default true at :106), and notes seid init does not emit it Suggestions: - genesis hash annotated as a synthetic constant identical on every network, returned uppercase, case-insensitive lookups (block.go:35-47) - pending-ttl sample values match generated defaults (0s/0) - ExcludeTraceFail filtering promoted out of the Pre-Baked Trace Cache section to its own top-level section - write-mode progression completed: flatkv_only becomes the valid flip target once migrate_bank reaches migration version 3 (write_mode.go) - seid CLI note covers added log/snapshot, corrects min-retain-blocks scoping, points latest_version users at seidb memiavl-latest-version - memIAVL legacy-path note added to the dump-iavl example (GetCosmosSCStorePath: committer.db wins when present) - exact Autobahn caps: 2,000 txs and 2,048,000 bytes (2,000 x 1,024, block.go MaxTxsBytesPerBlock), cross-referenced - duplicate version-scoping warning trimmed in differences-with-ethereum.mdx - /root/.sei normalized to $HOME/.sei; multi-blank-line runs collapsed outside code fences
There was a problem hiding this comment.
Large auto-generated v6.6.0 docs catch-up (16 MDX files, ~1000 added lines). The config-key updates I could cross-check against the AUTO-GENERATED app.toml/config.toml blocks are accurate (kebab-case p2p/rpc/consensus keys, ttl-* defaults, trace_bake_* fields, evm-ss-split, max_trace_lookback_blocks), but there is one copyable operator config that contradicts a warning added in this same PR, one config default the PR's own reviewer notes flagged as unverified and then shipped as fact, and several duplication/placement problems.
Findings: 3 blocking | 19 non-blocking | 12 posted inline
Blockers
codex-review.mdandcursor-review.mdare both empty — the OpenAI Codex and Cursor second-opinion passes produced no output, so this review is single-source.REVIEW_GUIDELINES.mdis also empty; I appliedAGENTS.mdandSTYLE_GUIDE.mdinstead (sentence-case headings, no speculation about unreleased work, terminology).- 2 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Duplicated content across pages will drift. The
evm-logical-digestflag list is documented in full in bothnode/technical-reference.mdxandnode/troubleshooting.mdx, and the two copies already differ in wording (--details,--find-hash).seidb migrate-evm-statusis likewise documented in bothnode/technical-reference.mdx(6 JSON fields) andnode/giga-storage-migration.mdx(4 fields). Pick one canonical home per command and cross-link. - The EVM transaction validation rules (ValidateBasic access-list/auth-list/signature checks, Cosmos wrapper-field rejection, bloated-body rejection) are written out three times with different wording:
evm/differences-with-ethereum.mdx,evm/evm-parity/transaction-types.mdx, andevm/transactions.mdx. Consider one authoritative section plus links. evm/reference.mdxis the EVM JSON-RPC reference, but this PR adds four Tendermint/CometBFT RPC endpoints to it (/status,/block,/block_by_hash,/block_results,/validators).node/technical-reference.mdxis the natural home for consensus-layer RPC; consider moving it there and linking from the EVM page.- PR item sei-chain#3522 says the Autobahn config removes the
mempool_sizefield, but no page mentions the removal. Operators withmempool_sizein an existingautobahn.jsonget no upgrade signal. Same for the newly-requiredevmrpc_url.txt. evm/changelog.mdxwas not touched. Several user-visible v6.6.0 changes documented here are changelog-worthy (Oracle precompile retired and now reverting,sei_traceBlockBy*ExcludeTraceFailremoved, newdebug_traceTransactionProfile,eth_getBlockTransactionCountBy*can now error on pruned receipts).- Sentence-case headings: many new headings use Title Case ("EVM Transaction Envelope Restrictions", "Pre-Baked Trace Cache", "Reporting FlatKV EVM Migration Status", "Comparing EVM State Across Backends", "Giga Mode Behavior and Per-Block Limits"), which the
Sei.HeadingsVale rule flags per AGENTS.md. Advisory only —prose-style.ymlruns withfail_on_error: falseand the repo has an acknowledged heading backlog — so this is informational, and the new headings do match the Title Case already used on these pages. - Several new blocks introduce two or three consecutive blank lines (
evm/differences-with-ethereum.mdx~139 and ~168,evm/evm-parity/transaction-types.mdx~84 and ~95,node/index.mdx~196,node/troubleshooting.mdx~105 and ~189). Cosmetic, but worth collapsing to one. - Some new sentences are very long single-idea-per-sentence violations per STYLE_GUIDE.md — notably the
eth_getTransactionReceiptbehavior paragraph (evm/reference.mdx:264, one ~120-word sentence with three nested parentheticals) andeth_getBlockTransactionCountByNumber(evm/reference.mdx:281). Splitting these would help readability without losing detail. - No prompt-injection or instruction-like content found in the diff, commit messages, or PR body — the PR body is a source-PR table plus reviewer notes.
- 10 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
node/node-operators.mdx:1607(RIGHT) -- [blocker] This copyable snippet folds the SC-layer FlatKV migration into the "Enabling Giga Storage" config, which contradicts guidance added elsewhere in this same PR.
node/giga-storage-migration.mdx states that the SC-side flow is a separate migration path ("Everything above concerns the SS layer. The SC layer has its own, separate migration path") and warns: "This SC-side FlatKV EVM migration flow is exercised by the cluster/devnet integration harness. Do not run it against testnet/mainnet nodes unless the release notes for your version explicitly call it out as supported." It also warns that a node flipped while its peers are on the old mode "will produce a different AppHash on the very next block and consensus will halt."
The prose immediately below this block compounds it: "Enabling Giga Storage requires a fresh state sync" and "supported on RPC nodes only" describe evm-ss-split, whereas the SC flow is explicitly without a state sync and is coordinated-stop-and-restart. So a reader gets one snippet with two different procedures and only one set of caveats.
Suggest dropping sc-write-mode/sc-keys-to-migrate-per-block from this snippet (leaving evm-ss-split = true) and linking to the FlatKV SC section for the SC path, or splitting into two labelled snippets each carrying its own warning.
node/advanced-config-monitoring.mdx:582(RIGHT) -- [blocker](defaulttrue)is asserted here, but this PR's own reviewer notes for sei-chain#3366 say the default was never established: "the diff's test toggles it to false but does not reveal the DefaultConfig default; a reviewer should confirm the actual default value before finalizing edit content." The unverified value then shipped as fact.
This matters because the sentence also tells operators the key "is not part of the app.toml that seid init generates, so add it manually to change the default" — I confirmed it is absent from the generated [state-commit.flatkv] block in node/node-operators.mdx, so the stated default is the only thing telling an operator whether Pebble metrics are on. If it's actually false, anyone relying on this will silently have no Pebble metrics.
Please confirm EnablePebbleMetrics in sei-db's DefaultConfig() before merge, or drop the parenthetical and say the default is inherited from the FlatKV config.
evm/reference.mdx:1500(RIGHT) -- [suggestion] This new## Tendermint RPC status responsesection duplicates the### Tendermint /status endpointsection added at line 61 of the same file in this same PR. Two sections ~1400 lines apart both introduce the/statusendpoint from scratch ("Alongside the EVM JSON-RPC surface, Sei nodes expose the underlying Tendermint/CometBFT RPC, including the/statusendpoint" vs. "The underlying Tendermint (CometBFT) RPC also exposes a/statusendpoint").
Fold the validator_info content into the earlier section as a subsection so /status is documented once.
evm/reference.mdx:598(RIGHT) -- [suggestion] Three claims here are worth double-checking before publishing, since they're specific enough that readers will code against them:
- That this genesis hash is "a synthetic constant identical on every Sei network" — if it's derived from chain ID or genesis time, it differs per network and pinning it here is actively wrong.
- That the node returns it "in this uppercase form" — uppercase hex in a JSON-RPC hash response is unusual enough that it reads like a transcription of a Go constant rather than observed wire output.
- That "lookups are case-insensitive".
If (1) and (2) can't be confirmed, consider stating just that the genesis block is resolvable by hash and consistent with eth_getBlockByNumber("0x0"), without embedding the literal.
evm/tracing/index.mdx:448(RIGHT) -- [suggestion] "Two classes are excluded" is incomplete relative toevm/transactions.mdx:315, added in this same PR, which says the new nonce-bumping state-transition failure receipts are also excluded: "Because these synthetic receipts carry the ante-stub shape (EffectiveGasPrice == 0andGasUsed == 0), the legacy*ExcludeTraceFailendpoints exclude them along with other never-executed transactions."
That's a third class, and it isn't an ante failure — per evm/reference.mdx:264 it fails inside go-ethereum's Execute(). So the closing claim below, "Everything that ran in the VM is included, even if it failed," reads as contradicting transactions.mdx for EIP-7623 floor-data-gas failures.
Add the third class here (or reframe the rule as "any receipt with EffectiveGasPrice == 0 && GasUsed == 0, whatever produced it") so the two pages agree. The same gap exists in the sei_getTransactionReceiptExcludeTraceFail row at evm/reference.mdx:1368.
evm/reference.mdx:94(RIGHT) -- [suggestion] "Populating these is a separate follow-up." is internal roadmap, which AGENTS.md rules out for public docs ("No speculation about unreleased features or timelines"). The preceding sentence already tells readers what they need —TxsResultsis empty becauseFinalizeBlockresponses aren't persisted — and the<Note>below already redirects them toeth_getTransactionReceipt. Drop the follow-up sentence.node/troubleshooting.mdx:191(RIGHT) -- [suggestion] Two placement problems with this new H3:
- It sits under
## AppHash Mismatch Errors, but importing memIAVL state into FlatKV is a migration procedure, not AppHash debugging. It also duplicates subject matter with the FlatKV SC migration section added tonode/giga-storage-migration.mdxin this PR, which is where a reader would look for it. - It's inserted after "Always include the app hash, commit hash, and block height from your logs when reporting issues." — that line was the wrap-up for the dump instructions, and it's now orphaned mid-section between the IAVL-removal
<Warning>and this new subsection.
Suggest moving this section to node/giga-storage-migration.mdx next to the FlatKV SC flow, and moving the "Always include..." line back up to directly follow the dump commands.
node/index.mdx:333(RIGHT) -- [suggestion] Good to mark these deprecated, but the surrounding block was left stale in the same edit. Two lines above (unchanged context) this sample still shows:
ttl-duration = "3s"
ttl-num-blocks = 5
whereas the AUTO-GENERATED config.toml in node/node-operators.mdx:1083,1091 has ttl-duration = "5s" and ttl-num-blocks = 10. The sample also predates the new TTL semantics this PR documents at node/node-operators.mdx:1376 (zero disables TTL purging entirely; expired PENDING txs always pruned). max-txs-bytes here is 10737418240 vs. 1073741824 generated.
Since you're already editing this block, worth syncing the sibling values and adding a pointer to the mempool TTL note — otherwise a copied config silently sets non-default TTLs.
node/advanced-config-monitoring.mdx:546(RIGHT) -- [suggestion] This is now the second of three### Available Metricsheadings on this page (lines 511, 546, 588), plus a duplicate pair of### Migrating from Legacy Metrics(530, 622). Mintlify derives anchors from heading text, so these collide and get suffixed (#available-metrics-1,#available-metrics-2) in whatever order they happen to appear — deep links become unstable and unguessable if a section is later reordered.
Qualify each one, e.g. "### Available FlatKV metrics" / "### Available LittDB metrics", matching the pattern you already used for the IBC subsections (### Transfer Metrics (ibc_transfer_keeper meter)).
node/technical-reference.mdx:135(RIGHT) -- [suggestion] This is the one new required input in the wholegen-autobahn-configflow, and the text doesn't say so. Two sentences below you note thatvalidator_pubkey.txtandnode_pubkey.txt"are written automatically alongsidepriv_validator_key.jsonandnode_key.json" — the implication thatevmrpc_url.txtis not auto-written is easy to miss.
Per PR item sei-chain#3438 this file is newly required per node in v6.6.0. Worth stating explicitly that operators must create it by hand, and what happens if it's missing (does config generation fail, or silently emit an empty evmrpc field and break tx proxying?).
node/technical-reference.mdx:70(RIGHT) -- [nit] Thisevm-logical-digestdocumentation — the prose rationale, the two example invocations, and all 11 flags — is repeated near-verbatim innode/troubleshooting.mdx("Comparing EVM State Between memIAVL and FlatKV"). The copies have already diverged slightly (--details: "include backend-specific version metadata" here vs. "...in list mode" there;--find-hash: "prints every matching entry" vs. "as aFOUND-HASHline").
Keep the full flag reference in one place — this page reads like the right home — and have the troubleshooting page carry the debugging narrative plus a link.
evm/installing-seid-cli.mdx:131(RIGHT) -- [nit] "the listing above also includes thelogandsnapshotcommands, which were previously missing from this page" is commentary about the documentation rather than aboutseid, and it'll read as stale noise once nobody remembers the previous revision. The rest of the note is genuinely useful (removals, theseidb memiavl-latest-versionreplacement, where retention now lives) — suggest keeping that and dropping the meta-clause.
Blocker (real): the Giga Storage snippet no longer folds the SC-layer FlatKV migration into the SS-layer evm-ss-split config — the two flows have contradictory caveats (state-sync vs coordinated restart); the snippet now carries only evm-ss-split and links to the SC flow. Blocker (already verified, no doc change): enable-pebble-metrics default true is confirmed at sei-db flatkv config.go:106 — the reviewer was reading the PR body's stale pre-verification note. Suggestions applied: - /status documented once: validator_info folded into the existing Tendermint /status section; duplicate H2 removed - ExcludeTraceFail exclusion reframed around the receipt shape with the third class (nonce-bumping state-transition failures) in both tracing/index.mdx and the reference receipt row - 'separate follow-up' roadmap sentence dropped from /block_results - import-flatkv-from-memiavl section moved from troubleshooting to the giga migration guide next to the SC flow - index.mdx mempool sample synced to generated defaults (5s/10, max-txs-bytes 1073741824) with a TTL-semantics pointer - duplicate 'Available Metrics'/'Migrating from Legacy' headings qualified (EVM RPC / FlatKV / LittDB) for stable anchors - evmrpc_url.txt documented as hand-created and required (command errors if missing); mempool_size removal from autobahn.json noted - evm-logical-digest flag reference deduplicated (canonical in technical-reference; troubleshooting links); migrate-evm-status cross-link added - long eth_getTransactionReceipt sentence split; meta-clause dropped from the seid CLI note; blank-line runs collapsed Not actioned: changelog.mdx (auto-synced from sei-chain CHANGELOG.md), heading case (advisory, matches page-local convention), genesis hash literal (verified as a chain-independent constant returned uppercase, evmrpc/block.go:35-47), three-page validation consolidation (deferred to the next catch-up), Tendermint-endpoints page placement (kept with the EVM RPC surface that consumes them).
There was a problem hiding this comment.
Large, generally high-quality v6.6.0 docs catch-up: config keys, defaults, and cross-page anchors all check out against the AUTO-GENERATED app.toml/config.toml blocks, and no generated regions were hand-edited. One blocking inconsistency: the interactive RPC explorer embedded at the top of evm/reference.mdx still presents the two sei_traceBlock*ExcludeTraceFail methods as callable even though this PR documents them as removed, and the newly documented debug_traceTransactionProfile is missing from it.
Findings: 2 blocking | 16 non-blocking | 8 posted inline
Blockers
snippets/rpc-methods-viewer.jsxwas not updated alongside this PR, so the interactive explorer rendered at the top ofevm/reference.mdxcontradicts the prose the PR adds on the same page (see inline comments onevm/reference.mdx:1378and:1279). The file is hand-maintained (onlyllms*.txtand the default-config blocks have generator scripts), so it needs a manual edit in this PR.- 1 blocking issue(s) flagged inline on specific lines.
Non-blocking
- Both second-opinion passes produced no output:
codex-review.mdandcursor-review.mdare empty files. This review is therefore single-reviewer — no cross-checking from Codex or Cursor was possible. - The same v6.5.0 EVM validation rules are now documented three times with drifting wording:
evm/differences-with-ethereum.mdx("EVM Transaction Envelope Restrictions"),evm/transactions.mdx("Transaction Validation"), andevm/evm-parity/transaction-types.mdx("Access List and Auth List Entry Validation"). E.g. transactions.mdx pinsr/sat 32 bytes while transaction-types.mdx says only "maximum byte length". Consider one canonical home (transaction-types.mdx) with links from the other two. - Provenance gap: five changed pages aren't attributable to any source PR in the description's table —
evm/precompiles/oracle.mdx,evm/evm-parity/state-proofs.mdx,evm/evm-parity/transaction-types.mdx,evm/installing-seid-cli.mdx,node/validators.mdx. That makes them the hardest claims to verify. In particular oracle.mdx now asserts "Retired as of v6.4.0" inside a v6.6.0 catch-up — worth confirming the version and that the revert error string is exact. evm/reference.mdxnow documents Tendermint/status,/block,/block_by_hash,/block_results, and/validatorsbehavior (~40 lines) in a page whose stated scope is the EVM JSON-RPC surface.node/technical-reference.mdx— which this PR already extends with Autobahn/Giga node behavior — is a more natural home; a pointer from reference.mdx would keep the EVM page focused.- The v6.6.0 removal of
sei_traceBlockByHashExcludeTraceFail/sei_traceBlockByNumberExcludeTraceFailis now stated three times (reference.mdx table cell, reference.mdx Note, tracing/index.mdx Info). One authoritative statement plus links would reduce future drift. - Style: the new headings are mostly Title Case ("Available EVM RPC Metrics", "Comparing EVM State Across Backends", "Pre-Baked Trace Cache", "Giga Mode Behavior and Per-Block Limits"), which the repo's Vale
Sei.Headingsrule flags against AGENTS.md's sentence-case convention; "simply" inevm/reference.mdx:266andevm/tracing/index.mdx:452will tripSei.Qualifiers. The prose-style workflow is advisory (fail_on_error: false) and annotates changed lines only, so these are nits — but they will appear as PR annotations. evm/differences-with-ethereum.mdx: the "Whole-block rejection on transaction decode failure" subsection sits under the "EVM Transaction Envelope Restrictions" heading, but whole-block proposal rejection is a consensus/ProcessProposal behavior rather than an envelope restriction. Consider promoting it to its own##section.- Unverifiable specific claim worth a second look before merge:
evm/reference.mdx:284states the synthetic genesis hash0xF9D3...AB9Eis "a synthetic constant identical on every Sei network." If that is network-dependent in practice, the hardcoded value would mislead; please confirm against the node source. - No prompt-injection or instruction-like content was found in the diff, commit messages, or PR description.
- 7 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
evm/reference.mdx:1378(RIGHT) -- [blocker] This Note (and the table cell above) declaresei_traceBlockByHashExcludeTraceFail/sei_traceBlockByNumberExcludeTraceFailremoved in v6.6.0, butsnippets/rpc-methods-viewer.jsx:167-168still lists both with"status":"limited"— i.e. as callable, with a live request form. That explorer is rendered by<RPCMethodsViewer />at the top of this page, and the Note at line 14 tells readers the explorer lists "the available, current methods." A reader who enables Show deprecated & unavailable methods will see the two removed endpoints presented as working.
The viewer is hand-maintained (scripts/ only generates llms*.txt and the default-config blocks), so please update it in this PR: flip both entries to "status":"unsupported" — the schema already uses that value for 34 other methods — with a seiNote recording the v6.6.0 removal and pointing at debug_traceBlockByHash/debug_traceBlockByNumber.
evm/reference.mdx:1279(RIGHT) -- [suggestion]debug_traceTransactionProfileis documented here and in theevm/tracing/index.mdxtable, but it is absent fromsnippets/rpc-methods-viewer.jsx(its sibling extensiondebug_traceStateAccessis present). The page intro says the explorer lets you "browse every method Sei exposes," so please add an entry —"status":"limited"matches howdebug_traceStateAccessis classified, and thehash+configparams are already spelled out below.
Separately, this entry has an example request but no example response, unlike the neighbouring debug_trace* entries. Given the profile object is the whole point of the method, a trimmed example response would help more here than anywhere else.
evm/reference.mdx:1137(RIGHT) -- [suggestion] The sentence "This guard now applies consistently across alldebug_trace*endpoints" is accurate to sei-chain#3515, butmax_trace_lookback_blocksappears only in thisdebug_traceTransactionentry — grep confirms no other mention in the file. A reader who lands directly ondebug_traceBlockByNumber,debug_traceBlockByHash,debug_traceCall,debug_traceStateAccess, or the newdebug_traceTransactionProfilewon't learn their calls can be rejected for historical blocks.
Suggest a one-line "Subject to the max_trace_lookback_blocks guard (see debug_traceTransaction)" on each of those five entries.
evm/reference.mdx:125(RIGHT) -- [suggestion] Two things to tighten in this paragraph:
-
Endpoint naming. Line 123 says the default is
broadcast_tx_sync; this line calls the same thingbroadcast_tx/BroadcastTx. CometBFT exposesbroadcast_tx_sync/_async/_commit, so a reader can't tell whetherbroadcast_txis a distinct sei-tendermint route or just loose shorthand. Pick one spelling. -
The blocking consequence isn't drawn out. Line 123 now says the default path is the CheckTx-synchronous one, and this line says that path "blocks while the mempool is full and only returns once capacity is available." Read together, that means a default
eth_sendRawTransactionunder Autobahn can block for an unbounded time when the mempool is full — a significant client-visible behavior that dapp developers should be told explicitly.node/technical-reference.mdx:605describes the sameInsertTx/TryInsertTxsplit; worth stating the client-facing implication here and noting whether any timeout bounds the wait.
node/advanced-config-monitoring.mdx:582(RIGHT) -- [suggestion] The(default \true`)claim needs confirmation before merge. The PR description's own reviewer note for sei-protocol/sei-chain#3366 says the diff's test toggles the flag tofalsebut "does not reveal the DefaultConfig default; a reviewer should confirm the actual default value before finalizing edit content." Since this paragraph also (correctly) states the key is absent from the[state-commit.flatkv]block thatseid init` emits, operators have no in-file hint to fall back on — the docs are the only source of truth for the default, so getting it wrong is costly.
Please verify against DefaultConfig in sei-db and, if it is true, consider asking for the key to be added to the generated app.toml template so scripts/sync-default-configs.mjs keeps it in sync going forward.
evm/reference.mdx:1370(RIGHT) -- [nit] This table cell is a ~700-character paragraph in a two-column table whose every other row is a short phrase (same for the twoExcludeTraceFailblock rows at 1359/1361). It will render as an unreadable column on narrow viewports.
evm/tracing/index.mdx:446 already has a full "*ExcludeTraceFail filtering semantics" section covering exactly this. Suggest keeping these cells to one line ("Get transaction receipt, excluding txs that never executed in the VM — see filtering semantics") and letting the tracing page carry the detail.
node/giga-storage-migration.mdx:292(RIGHT) -- [nit] A doc cross-reference inside abashcomment isn't clickable and won't be caught by thelink-checkworkflow if the anchor moves. Move it into prose below the block as a real link —[seidb tooling reference](/node/technical-reference#reporting-flatkv-evm-migration-status)— which is hownode/troubleshooting.mdxlinks to the same page.node/giga-storage-migration.mdx:243(RIGHT) -- [nit] This Note is written as a changelog entry ("A correctness bug in the WAL replay path — where ... — is fixed"), which leaves a reader on the current version parsing a defect they can no longer hit. Docs read better stating current behavior: "Empty (zero-length) values written without a delete flag are preserved across WAL replay (catchup, read-only clone, snapshot export, and state-sync restore), which is what makesflatkv_onlystate-sync reliable." If the buggy versions matter operationally, name them ("nodes before vX.Y should not rely on ...").
Blocker: the interactive RPC explorer (snippets/rpc-methods-viewer.jsx) now matches the prose — the two removed sei_traceBlock*ExcludeTraceFail entries flip to status unsupported with removal notes, and debug_traceTransactionProfile is added (params/shape from evmrpc/trace_profile.go at v6.6.0). Suggestions: - trimmed example response added for debug_traceTransactionProfile (field names from TraceTransactionProfile structs) - max_trace_lookback_blocks guard note added to the five other debug_trace* entries - broadcast naming unified on broadcast_tx_sync with the client-facing implication spelled out (default sendRawTransaction can stall while the Autobahn mempool is full; async path may drop instead) - validation rules consolidated: transaction-types.mdx is canonical for field-level checks (tx-level v/r/s each <=32 bytes; auth-list v <=1, r/s <=32 — verified against ethtx call sites), envelope rules stay in differences-with-ethereum.mdx, transactions.mdx links to both
There was a problem hiding this comment.
Docs-only v6.6.0 catch-up across 12 pages; the content is substantive, generally accurate, and the new cross-references, anchors, and RPC-viewer status changes all check out. No blockers, but there are several accuracy/consistency gaps worth fixing before merge — most notably the sei2_*ExcludeTraceFail table rows still carrying the status=0 framing this PR declares wrong, and a production-vs-devnet contradiction in the FlatKV SC migration guide. Both second-opinion passes (Codex, Cursor) produced empty files, so this is a single-reviewer result.
Findings: 0 blocking | 17 non-blocking | 9 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- Both second-opinion passes produced no output:
./codex-review.mdand./cursor-review.mdare empty (2 blank lines each). This review is therefore Claude-only — combined cross-tool coverage is reduced, and any findings unique to Codex/Cursor are absent. ./REVIEW_GUIDELINES.md(taken from the base branch) is empty, so no repo-specific review standards were applied. If this repo has docs conventions (voice, callout usage, version-note style, link-checking), consider populating that file so future automated reviews enforce them.- No prompt-injection or instruction-hijacking content was found in the diff, commit messages, or PR title/body. The PR body's "Reviewer notes" and "Possible self-contradictions" sections are descriptive only.
- The PR body's own self-contradiction checker raises two false positives that reviewers should not act on: it claims
node-operators.mdxremovessc-write-mode/sc-keys-to-migrate-per-blockfrom the app.toml template. The diff actually adds both (node/node-operators.mdx:1483and:1488), and the auto-generated block at:261/:269already carries them. What was removed is only the stale Giga Storage example that used the non-existentsc-read-mode/sc-enable-lattice-hashkeys — that removal is correct. Likewise the flagged FlatKV path "inconsistency" is now explicitly explained by the added legacy-path Notes innode/troubleshooting.mdx. - Verified consistent across pages (no action needed): mempool
max-txs-bytes = 1073741824,ttl-duration = "5s",ttl-num-blocks = 10, andpending-ttl-* = 0innode/index.mdxall match the auto-generatedconfig.tomlinnode/node-operators.mdx:1055/:1103.eth_getVMErroris a real method (present insnippets/rpc-methods-viewer.jsx:97), so the new cross-references inevm/reference.mdxandevm/transactions.mdxare correct. All new intra-doc anchors (#evm-transaction-envelope-restrictions,#access-list-and-auth-list-entry-validation,#flatkv-evm-sc-migration-flow,#comparing-evm-state-across-backends) resolve to real headings. - Marking the two removed
sei_traceBlockBy*ExcludeTraceFailmethods asstatus: "unsupported"insnippets/rpc-methods-viewer.jsx(rather than deleting the entries) matches the viewer's existing convention —isHiddenByDefaulthidesunsupportedand the try-it panel is gated onstatus !== 'unsupported'— so removed methods stay discoverable-but-hidden. Good call. node/giga-storage-migration.mdx:229-244: theflatkv_onlyparagraph is hard to follow — it opens by calling the mode "fully-supported terminal steady-state", then walks back what it is not a flip target for, then repeats the "boots directly into the post-migration shape without ever running the migration manager" claim twice. Consider restructuring as a short mode-progression table (memiavl_only → migrate_evm → evm_migrated → migrate_all_but_bank → all_migrated_but_bank → migrate_bank → flatkv_only) with a "valid flip target when" column.evm/precompiles/oracle.mdxis now a 9-line stub (frontmatter + address + Danger). It is still listed under EVM precompiles indocs.json:190alongside pages that document callable functions, and the "Retired as of v6.4.0" claim is not attributable to any of the 30 source PRs in the description table. Worth confirming the version and deciding whether the page should keep a nav slot at the same level as live precompiles or become a redirect to/evm/oracles/chainlink.- 9 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
evm/tracing/index.mdx:454(RIGHT) -- [suggestion] Theon chains with a positive minimum gas price, EffectiveGasPrice > 0caveat undercuts the absolute claim that opens this paragraph. The filter is stated three lines up as the conjunctionEffectiveGasPrice == 0 && GasUsed == 0, soGasUsed > 0alone is already sufficient to guarantee inclusion — the effective-gas-price value is irrelevant for reverts/OOG. As written, a reader on a zero-min-gas-price chain (e.g. a devnet) will reasonably conclude their reverted txs might get filtered, which contradicts "Everything that executed opcodes in the VM is included".
Suggest dropping the caveat: "Reverted and out-of-gas (OOG) transactions have Status == 0 but GasUsed > 0 (intrinsic gas at minimum), which alone is enough to fail the filter's GasUsed == 0 conjunct, so they are always returned."
evm/reference.mdx:1396(RIGHT) -- [suggestion] Two issues with this table cell:
-
The
sei2_rows were left behind. This PR rewrites the threesei_*ExcludeTraceFailrows with the corrected receipt-shape semantics, butsei2_getBlockByHashExcludeTraceFail/sei2_getBlockByNumberExcludeTraceFail(lines 1415–1416, unchanged) still read "excluding failed traces" — exactly thestatus=0framing this PR explicitly calls out as wrong ("Previously any receipt withstatus=0was excluded, which over-filtered reverts and OOG failures").evm/tracing/index.mdx:456also asserts the filter applies to "theirsei2_equivalents", so the two pages now disagree. Please apply the same wording to thesei2_rows. -
Removed-endpoint notice is duplicated. The
sei_traceBlockBy*removal is stated here and again in the<Note>at line 1404. Since it has nothing to do withsei_getTransactionReceiptExcludeTraceFail, drop it from this cell and keep only the Note.
node/giga-storage-migration.mdx:263(RIGHT) -- [suggestion] "which is appropriate for production drains" directly contradicts the<Warning>50 lines up (line 213): "Do not run it against testnet/mainnet nodes unless the release notes for your version explicitly call it out as supported." An operator reading top-to-bottom gets told the flow is devnet-harness-only, then told the default is tuned for production.
Pick one framing. If the flow really is devnet/cluster-only today, reword to something neutral like "It defaults to 1024; lowering it spreads the migration across more blocks at the cost of a longer drain."
-
node/technical-reference.mdx:385(RIGHT) -- [suggestion] This Note documents thedata/tendermint/relocation, but the tree above it (and this Note) omit every other on-disk location this same PR introduces or documents: -
data/autobahn/— the new default for--persistent-state-dir(line 333), i.e. persistence is now on by default with no operator action -
data/state_commit/memiavlanddata/state_commit/flatkv— referenced repeatedly in the newseidbsections and innode/troubleshooting.mdx -
data/state_store/{cosmos,evm}/pebbledb/— pernode/giga-storage-migration.mdx -
data/trace_db— created whentrace_bake_enabled = true(evm/tracing/index.mdx)
Since this is the canonical "File Locations" reference and disk-usage planning depends on it, adding these entries would be worth more than the data/tendermint/ change alone.
evm/reference.mdx:627(RIGHT) -- [suggestion] The receipts-pruned behavior is a genuinely breaking change for integrators — a call that previously returned a count now returns a hard JSON-RPC error for older blocks, and whether it triggers depends on node-side retention config the caller can't see. Right now it's the fifth sentence of a dense prose paragraph that also covers counting rules, synthetics, and genesis.
Suggest pulling it into an <Info>/<Warning> callout after this paragraph (the page already uses that pattern for filter/subscription limits) so it isn't missed, and mention that operators control it via min-retain-blocks → receipt-store KeepRecent, cross-linking the new Note at node/node-operators.mdx:764.
node/node-operators.mdx:1492(RIGHT) -- [suggestion]node/advanced-config-monitoring.mdx(added in this PR) documentsenable-pebble-metricsas living under[state-commit.flatkv], defaulting totrue, honored-when-present but not emitted byseid init— so an operator can only discover it by reading the monitoring page. Since this is the hand-written[state-commit.flatkv]reference block, adding the key here (commented, with the default) would make it findable where operators actually look.
Also worth reconciling the two pages: this comment says these settings "have no effect" under the default memiavl_only, while the monitoring page presents enable-pebble-metrics as an active knob with a true default — a reader may not connect that it's inert until FlatKV is in use.
evm/differences-with-ethereum.mdx:163(RIGHT) -- [suggestion] The actionable advice here doesn't match the audience of this section. "EVM Transaction Envelope Restrictions" is developer-facing guidance for tooling that constructs raw transactions, but whole-proposal rejection is aProcessProposalconsensus behavior — a malformed tx has to clearCheckTxbefore it can ever reach a proposal, and an application developer cannot influence what lands in someone else's block. The PR's own reviewer note for sei-chain#3464 reaches the same conclusion ("internal consensus mechanism detail… operators/developers cannot craft other users' block contents").
Suggest keeping the factual statement but retargeting it: move the consensus-level framing to a node/validator page (or mark it explicitly as validator-facing context) and leave the developer takeaway here as just the last sentence — submit transactions that decode cleanly.
evm/installing-seid-cli.mdx:131(RIGHT) -- [nit] The Note explains the three removed commands but says nothing about the two added ones in the same help-output diff —log(Runtime log management, line 108) andsnapshot(Create a snapshot using the snapshot manager, line 112). A reader diffing against their v6.5.x binary will spot the additions and find no explanation. One clause covering them would make the Note complete.node/technical-reference.mdx:153(RIGHT) -- [nit] "a fixed per-block byte cap" leaves the reader without the number they need, even though it's stated concretely two subsections down: "a combined transaction byte budget of exactly 2,048,000 bytes (2,000 × 1,024)". State the value inline (or link to the caps list) — the neighboring bullets all name their limit.
2 participants
Documentation catch-up for v6.6.0.
30 source PR(s) produced changes. Each is a separate commit, so this reviews commit-by-commit.
node/advanced-config-monitoring.mdxnode/advanced-config-monitoring.mdx,node/node-operators.mdxevm/reference.mdx,evm/transactions.mdxnode/index.mdxmock_block_validationGo build tag that bypasses AppHash and DataHash block validation, producing a separateseidDocker image taggedmock_block_validation-*for testing environments.node/node-operators.mdx,evm/tracing/index.mdxnode/troubleshooting.mdxseidb import-flatkv-from-memiavlCLI command (plusmemiavl-latest-version) and a newdump-flatkv --bucketoption for migrating/importing EVM state from memiavl into FlatKV, along with the KVImporter Abort/Err lifecycle used by that tool.node/technical-reference.mdx,node/node-operators.mdxevm/reference.mdx,evm/tracing/index.mdxevm/tracing/index.mdx,evm/reference.mdxnode/technical-reference.mdx,node/advanced-config-monitoring.mdxevm/precompiles/distribution.mdxnode/technical-reference.mdxnode/advanced-config-monitoring.mdxevm/differences-with-ethereum.mdxevm/reference.mdxnode/node-operators.mdx,node/technical-reference.mdxnode/node-operators.mdx,node/technical-reference.mdx,node/giga-storage-migration.mdxmigrate-evm-statusCLI subcommand, a newsc-keys-to-migrate-per-blockapp.toml config field, aGIGA_MIGRATE_FROM_MEMIAVLcluster env var, and a-modeflag to the evm_stress tool, all supporting the FlatKV EVM migration flow.evm/reference.mdx,node/advanced-config-monitoring.mdxevm/reference.mdxnode/technical-reference.mdx,node/technical-reference.mdx,evm/reference.mdxnode/index.mdxnode/giga-storage-migration.mdxflatkv_onlyvalue of thesc-write-modeconfig field, enabling nodes to boot directly in the post-migration FlatKV-only steady state with correct state-sync and snapshot behavior.node/node-operators.mdx,node/technical-reference.mdx,node/index.mdxnode/advanced-config-monitoring.mdxnode/node-operators.mdx,node/giga-storage-migration.mdxevm/reference.mdxnode/advanced-config-monitoring.mdxnode/technical-reference.mdxnode/node-operators.mdxunsafe-overrides-enabledwas added that gates whether the Unsafe*TimeoutOverride fields take effect, changing how timeout overrides behave on nodes.evm/tracing/index.mdx,node/node-operators.mdxReviewer notes
GIGA_MIGRATE_FROM_MEMIAVLand the evm_stress-modeflag are test/CI-only tooling not covered in operator docs, so no doc gap is flagged for them beyond the migration-flow context. (2) The new seidb_migration_* metrics could warrant an entry in node/advanced-config-monitoring.mdx if operators are expected to scrape them, but the diff does not indicate these are operator-facing production metrics (migration is a one-time flow), so I did not flag that page — reviewer may reconsider if migration metrics should be documented alongside FlatKV/LittDB metrics. (3) The FlatKV EVM SC migration is a distinct concept from the SS-store migration in giga-storage-migration.mdx; a reviewer may prefer a new dedicated page rather than extending that guide, but no existing page cleanly covers the SC-side FlatKV EVM migration so add_section on the closest page is the conservative call.max_trace_lookback_blocksconfig field (documented in node/advanced-config-monitoring app.toml asmax_trace_lookback_blocks, default 10000, -1 for unlimited). The behavior change is that this guard now applies to all debug_trace* endpoints, not just debug_traceBlockByNumber — worth clarifying in the reference. The evm/tracing/index.mdx overview page lists all trace methods but describes cache behavior, not lookback limits, so it likely does not need an update, though a reviewer may consider adding a note there about historical block rejection.unsafe-overrides-enabledfield and updated override comments will likely appear once regenerated — but the surrounding prose/descriptions of override behavior still need a manual correction. The config.toml content was truncated in the supplied page right at the[consensus]Unsafe Timeout Overrides section, so verify the exact insertion point. node/technical-reference.mdx also shows a partial[consensus]config.toml block but its unsafe-override fields were truncated; if that block lists the override fields it may also need the new field/behavior note. Migration note for reviewers: nodes relying on Unsafe*TimeoutOverride will silently stop applying overrides after upgrade unless they setunsafe-overrides-enabled = true(or onchain params still equal the legacy pacific-1 badParams) — this backward-compat/migration nuance should be called out to operators.Flagged by a review of the assembled diff. Each source PR was analysed in isolation, so these were invisible to the per-PR checks. Verify before merging — the checker cannot tell which side of a contradiction is correct.
Analyzing the diff for the three categories of issues:
sc-keys-to-migrate-per-blockdefault contradiction: Ingiga-storage-migration.mdxit states the default is1024and "must be> 0", but innode-operators.mdxthe entiresc-write-mode/sc-keys-to-migrate-per-blockconfig block is removed from the template. This makes the setting described as both removed (from the app.toml template) and configurable (documented knob in the migration flow). (Category 3)sc-write-moderemoved vs. configurable:node-operators.mdxdeletes thesc-write-modefield (with its valid values) from the app.toml template, whilegiga-storage-migration.mdxdocumentssc-write-modeextensively as an operator-setapp.tomlfield ([state-commit]section) driving the migration. Described as both removed and configurable. (Category 3)FlatKV directory path inconsistency: The
dump-flatkvexamples use two different DB dirs —$HOME/.sei/data/state_commit/flatkv(with underscore) in one example and$HOME/.sei/data/flatkvin another. Alsotechnical-reference.mdxusesdata/state_commit/flatkv. Inconsistent path for the same FlatKV directory. (Category 1)min_valid_per_window/ oracle default:technical-reference.mdxstates the default "changed from 5%" to0%. This is internally consistent (not a contradiction on its own) — noted only if paired elsewhere; no conflicting value appears. (No issue.)evm-ss-db-directorydefault path:node-operators.mdxsays it defaults to<home>/data/state_store/evm/{backend}, consistent withgiga-storage-migration.mdx(data/state_store/evm/{backend}). (Consistent — no issue.)data/autobahnpersistence default: consistently described. (No issue.)GIGA_STORAGE/RECEIPT_BACKENDdefault (pebble) and parquet removal: consistently described across files. (No issue.)Generated by sei-docs-bridge. Every change is a proposal — verify against the source PRs before merging.
Review resolution (2026-08-17)
The findings from the human review (656 claims verified against sei-chain v6.6.0) were fixed on this branch in 46de0dc, and the AI Review findings on the merge commit in 74f14dd and b89011e. Two notes on the sections above:
enable-pebble-metricsdefaults totrue— confirmed in sei-dbstate_db/sc/flatkv/config/config.go:106(DefaultConfig) with the explicit// Default: truecomment at :46, closing the open question in the sei-chain#3366 reviewer note above. The genesis block hash in evm/reference.mdx is the chain-independent constantgenesisBlockHashHex(evmrpc/block.go:35-47); the node returns it in that uppercase form and hash lookups go throughcommon.HexToHash, so they are case-insensitive.evm/changelog.mdxis auto-synced from sei-chain's CHANGELOG.md via the<Changelog />snippet and is not hand-edited in docs PRs.evm/precompiles/oracle.mdx(oracle precompile retirement, sei-chain#3015, first shipped v6.4.0);evm/evm-parity/state-proofs.mdx(eth_getProof store-backend unwrapping, sei-chain#3119);evm/evm-parity/transaction-types.mdx(v6.5.0 ValidateBasic tightening, sei-chain#3073);evm/installing-seid-cli.mdx(compact/latest_version/prune removal, sei-chain#3146);node/validators.mdx(p2p kebab-case config rename). These entered the branch as early-backlog commits ahead of the v6.6.0 window the table describes; all were verified against the v6.6.0 tag during review.