Repository navigation
Backport release/v6.7: feat(seidb): Add JSON output to evm-logical-digest and inspect a FlatKV migration in flight - #4255
Conversation
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-4166-to-release/v6.7
git worktree add --checkout .worktree/backport-4166-to-release/v6.7 backport-4166-to-release/v6.7
cd .worktree/backport-4166-to-release/v6.7
git reset --hard HEAD^
git cherry-pick -x 29139f27297d719a0732d413eae2ca727d52b8e0
git push --force-with-lease |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release/v6.7 #4255 +/- ##
================================================
- Coverage 61.34% 60.33% -1.02%
================================================
Files 2163 2064 -99
Lines 188780 177335 -11445
================================================
- Hits 115812 106993 -8819
+ Misses 62250 60554 -1696
+ Partials 10718 9788 -930
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…KV migration in flight (#4166) Adds a `--json` flag to `seidb evm-logical-digest`, and extends inspect mode to work during a FlatKV migration. Getting JSON out required splitting rendering from accumulating: both forms now render from one report struct per run (`evmDigestJSON` / `evmInspectJSON`), so no number is computed twice and the prose and the JSON cannot drift. A package-level `digestSink` holds the two destinations, so one assignment redirects every line the scan helpers emit. - `sei-db/tools/cmd/seidb/operations/evm_logical_digest.go`: - `--json` emits the report as one JSON line on stdout and moves the narration to stderr. Storage-layer logging is raised to error level, because `seilog` writes to stdout and fixes its destination at process start. A warning fires when `SEI_LOG_OUTPUT` still points at stdout; it goes to the narration, never into the report. - `marker_adjustments` names the migration marker rows XORed out of the misc bucket. That list is the only thing distinguishing a mid-migration reading from a completed one, since the misc digest is adjusted in both. - A new zero-value census counts the populations FlatKV normalization can change. It is one nullable field shared by the row-level and account-level counters, so a half-counted census is unreachable. `nil` reads as "not measured" in both forms, not as all-zero. - `semanticAccountDigestState` records whether a code-hash row was present, so a stored all-zero row is distinguished from an absent one. - Inspect mode accepts `--backend composite` (FlatKV rows plus memiavl rows past the migration boundary) and `--memiavl-open-mode=replay`, reusing the existing no-repair read-only open. Translator normalization and `--details` storage inspect still require snapshot mode and now reject replay with their own messages. - The composite and semantic scans take a consumer, an optional account-key filter, and an optional progress callback, so digest and inspect share one scan. The filter drops out-of-shard account fragments before buffering. - The composite digest takes no census: its accounts are partly rebuilt from FlatKV rows, which cannot observe whether memiavl held a code-hash row. `sei-db/tools/cmd/seidb/operations/evm_logical_digest_test.go` - Output-form agreement: every bucket count and digest, the final digest, and the run context match between prose and JSON, for both digest and inspect reports. JSON output is one line and carries no prose. - Census: all six counters over a crafted leaf set; all-or-nothing across both counter levels (table over nil and non-nil); an untaken census omitted from both forms. - Marker adjustments: empty for a clean digest, named for one that consumed a boundary row, misc bucket equal in both, and empty encodes as `[]` not `null`. - JSON-mode warning: present when `SEI_LOG_OUTPUT` is unset, absent when it is redirected, and never in the report buffer. - Composite and filtered inspect: the memiavl tail counts only unmigrated rows, matching a reference accumulator; an account prefix filter skips out-of-range addresses. Existing tests take the new census argument and otherwise assert unchanged behavior. --------- Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 29139f2)
d2e8157 to
8d333ab
Compare
|
Rebuilt this branch: re-ran the cherry-pick of 29139f2 and resolved the two conflicts by hand. Force-pushed
Verified locally:
|
… release/v6.7 goconst is enabled on this branch and disabled on main, so the third "composite" literal this backport adds fails lint here only.
|
Pushed Fixed the way this file already handles the same linter — it has a const block for the Reproduced and verified with the same linter version CI uses (v2.8.0): fails on the parent commit with |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
PR SummaryLow Risk Overview Inspect during FlatKV migration is expanded: Memiavl semantic digests gain an optional zero-value census (zero accounts, all-zero code-hash rows, empty code, zero storage) to debug FlatKV vs memiavl normalization differences; composite digest paths intentionally skip census. JSON mode raises default log level to error and warns when Reviewed by Cursor Bugbot for commit 34fd677. Bugbot is set up for automated code reviews on this repo. Configure here. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2bdf15e830
ℹ️ 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".
| // enterJSONMode redirects this command's narration to stderr so stdout carries | ||
| // only the encoded report, and raises the storage layer's log level, which | ||
| // writes to stdout and would otherwise put several lines in front of the object. |
There was a problem hiding this comment.
Rewrite this Godoc to state only what enterJSONMode does; its multi-paragraph discussion of logger limitations, implementation choices, and operational rationale is exactly the mechanism-heavy form the repository guide prohibits, so the rationale should be moved to the specific lines that require it or omitted.
AGENTS.md reference: AGENTS.md:L61-L68
Useful? React with 👍 / 👎.
| if meta != "" { | ||
| fmt.Printf("key=%X logical=%X %s\n", physKey, logical, meta) | ||
| } else { | ||
| fmt.Printf("key=%X logical=%X\n", physKey, logical) | ||
| entry.Meta = meta | ||
| } | ||
| a.entries = append(a.entries, entry) |
There was a problem hiding this comment.
When inspect mode runs with --list --list-limit <=0 (or a large limit) without --json, every matching row is now both printed immediately and appended to a.entries, even though the text report never reads that slice. Large storage inspections can therefore retain hex-encoded copies of the entire result set and exhaust memory, whereas the previous text path streamed entries with bounded memory; only accumulate entries when generating JSON.
Useful? React with 👍 / 👎.
| if acc.inspectBucket == flatkvBucketAccount { | ||
| accounts = make(map[string]*semanticAccountDigestState) | ||
| } | ||
| if err := consumeCompositeFlatKV(source.opened, acc.addLogical, accounts, acc.matchesAccountPhysicalKey, nil); err != nil { |
There was a problem hiding this comment.
For --backend composite --inspect-bucket storage --list --details, this path routes FlatKV rows through acc.addLogical, which discards the raw value and always supplies an empty metadata string; the memiavl half does the same below. The accepted --details flag consequently produces no block_height or leaf_version metadata, unlike the existing backend-specific storage paths, so composite mode should preserve that metadata or explicitly reject this unsupported combination.
Useful? React with 👍 / 👎.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want higher recall? High effort reviews run extra passes and find more bugs. A team admin can switch effort levels in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2bdf15e. Configure here.
| digestOut.sayf("key=%X logical=%X %s\n", physKey, logical, meta) | ||
| } else { | ||
| digestOut.sayf("key=%X logical=%X\n", physKey, logical) | ||
| } |
There was a problem hiding this comment.
Inspect list buffers all matching entries
Medium Severity
--list now appends every matching pair onto entries before the report is emitted, including in text mode where those rows are already printed and then ignored. With --list-limit at 0 (unlimited), a full-bucket inspect holds every key and logical value as hex in memory instead of streaming them.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2bdf15e. Configure here.
![seidroid[bot]](https://avatars.githubusercontent.com/in/2745135?s=60&v=4){kind=small}
There was a problem hiding this comment.
Clean backport of #4166 — the code hunks match the merged original, the text/JSON reports are rendered from one report() so they cannot drift, and isZeroAccount is refactored without changing semantics. Two small behavioural gaps (unbounded entry buffering in text list mode, --details silently ignored on the new composite inspect) and a missing CHANGELOG entry for the backport.
Findings: 0 blocking | 4 non-blocking | 2 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- [suggestion] The original PR #4166 added a CHANGELOG.md entry under
## Unreleased/### Improvements; this backport carries no CHANGELOG change. The base branch records backports explicitly (CHANGELOG.md:49-56, e.g. "#4095 Backportrelease/v6.7: ..."), so v6.7's changelog will not mention this tool change. - [suggestion] The
EvmLogicalDigestCmdgodoc's Usage block documents every other mode with an example but says nothing about--json, nor about the two new inspect capabilities this PR adds (--backend compositeinspect and--memiavl-open-mode replayinspect). A line in Usage for each would keep the block a complete map of the command's modes. - 2 suggestion(s)/nit(s) flagged inline on specific lines.
Inline comments (could not post inline; listed here)
sei-db/tools/cmd/seidb/operations/evm_logical_digest.go:1113(RIGHT) -- [suggestion]a.entriesis appended unconditionally, including in text mode where the entry is printed and immediately discarded. Before this PR the text--listpath streamed; now it retains every listed key/value as two hex strings. With--list-limit <=0(documented as unlimited) on a large storage or code bucket that is unbounded growth — roughly 200 B per row, so tens of GB on a full EVM storage scan — where the old code used constant memory.
Since report() only reads a.entries when a JSON report is being written, gating the append on digestOut.jsonReport != nil keeps text mode streaming:
if digestOut.jsonReport != nil {
a.entries = append(a.entries, entry)
} else if meta != "" {
digestOut.sayf("key=%X logical=%X %s\n", physKey, logical, meta)
} else {
digestOut.sayf("key=%X logical=%X\n", physKey, logical)
}sei-db/tools/cmd/seidb/operations/evm_logical_digest.go:1248(RIGHT) -- [suggestion]inspectCompositeMigrateEVMnever readsacc.details: both the FlatKV side (consumeCompositeFlatKV→acc.addLogical) and the memiavl side passmetaas"", so--inspect-bucket storage --list --details --backend compositesilently produces listings with noleaf_version/version metadata rather than failing. That is inconsistent with the other unsupported combinations this PR guards explicitly (inspectMemIAVLrejects translator+replay,inspectMemIAVLSemanticrejects details+storage+replay). Either wire the meta through or return an error naming the unsupported combination, so an operator does not read a detail-less listing as "no version metadata exists".
Adds the `release/v6.7` entries merged since the rc1 changelog (#4110), in prep to cut **v6.7.0-rc2**: - [#4292](#4292) — Flush MemIAVL changelog before exiting on an upgrade panic - [#4285](#4285) — fix(seidb): refuse a corrupted changelog in digest replay instead of repairing it - [#4255](#4255) — feat(seidb): Add JSON output to evm-logical-digest and inspect a FlatKV migration in flight - [#4116](#4116) — rc1 version bump - [#4113](#4113) — rc1 changelog backport Regenerated with `./scripts/generate-changelog.sh release/v6.6 release/v6.7`; only the `## v6.7` PR list changes, so the `backport release/v6.7` cherry-pick applies cleanly (verified with `git apply --check` against `origin/release/v6.7`). Docs-only; no code change. Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
## Summary - Bump `version.json` from `v6.7.0-rc1` to `v6.7.0-rc2` to cut the second `v6.7` release candidate. Contents since rc1: #4285, #4255, #4292 (all `sei-db` fixes/tooling), plus the rc2 changelog update (#4294). Merge after #4294 so the tag cut by `uci-release-publish` on this `version.json` change includes the updated changelog. ## Test plan - [x] `git diff --check` Made with [Cursor](https://cursor.com) Co-authored-by: Cursor <cursoragent@cursor.com>
3 participants
Backport of #4166 to
release/v6.7.