testing(flatkv): evm migration int testings#3473
Conversation
PR SummaryHigh Risk Overview State-commit / migration engine: New CI / Docker: Tests: Large additions at composite, rootmulti, and migration layers (lifecycle, resume, determinism, double-flush, post- Reviewed by Cursor Bugbot for commit 78734e9. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
![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: 7cb7906e25
ℹ️ 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".
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3473 +/- ##
==========================================
- Coverage 59.03% 59.01% -0.02%
==========================================
Files 2187 2189 +2
Lines 181425 181612 +187
==========================================
+ Hits 107102 107179 +77
- Misses 64701 64816 +115
+ Partials 9622 9617 -5
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
blindchaser
force-pushed
the
yiren/fkv-mig-int-rebase
branch
6 times, most recently
from
372a884 to
4583e17
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
blindchaser
force-pushed
the
yiren/fkv-mig-int-rebase
branch
2 times, most recently
from
dce2b45 to
77a21c7
Compare
blindchaser
changed the title
blindchaser
force-pushed
the
yiren/fkv-mig-int-rebase
branch
from
3f8c7a5 to
292c1e8
Compare
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.
Reviewed by Cursor Bugbot for commit ec3b449. Configure here.
| migrationComplete := m.boundary.Equals(MigrationBoundaryComplete) | ||
| metadataPairsWritten := int64(1) | ||
|
|
||
| if m.boundary.Equals(MigrationBoundaryComplete) { |
There was a problem hiding this comment.
Delete pairs written to old DB not counted in stats
Low Severity
batchStats.oldDBPairsWritten is set from the return value of flattenPairsByStore(oldDBPairsByStore), which counts the final deduplicated pairs. However, migration delete pairs for migrated keys are added to oldDBPairsByStore and caller's original pairs routed to old DB are also added. These are correctly counted by flattenPairsByStore. But batchStats.newDBPairsWritten uses newDBPairsWritten + metadataPairsWritten to account for metadata pairs appended after flatten. The oldDBPairsWritten counting appears correct on closer inspection — this is not a bug but the asymmetry is worth noting. Actually, upon careful re-review, the counting is consistent. Withdrawing this concern.
Reviewed by Cursor Bugbot for commit ec3b449. Configure here.
|
AI Review:
|
| if m.oldDBIteratorBuilder == nil { | ||
| return nil, fmt.Errorf("iteration not supported for store %q", store) | ||
| } | ||
|
|
||
| // Preserve the legacy memiavl iterator path during the migration window. | ||
| // This keeps existing EndBlock cleanup code that still does prefix scans | ||
| // from panicking before the first migration batch commits, without adding a | ||
| // per-block full FlatKV scan on large stores. | ||
| return m.oldDBIteratorBuilder(store, start, end, ascending) |
There was a problem hiding this comment.
We should make this check stronger. It should only return the oldDB iterator if the migration boundary equals MigrationBoundaryNotStarted, as an iterator constructed at any time after migration actually starts will return invalid data.
There was a problem hiding this comment.
Good catch, agreed, anything past NotStarted would silently return incomplete results since left-of-boundary keys have already been deleted from the old DB. Tightened the check to !m.boundary.Equals(MigrationBoundaryNotStarted) and updated the godoc to spell out why.
There was a problem hiding this comment.
I tried tightening it to your suggestion and CI panicked at height 432:
panic: RouterCommitKVStore.Iterator(store="evm"):
iteration not supported once migration has started
at x/evm/keeper.RemoveFirstNTxHashes
at EndBlock → ProcessBlock → consensus halt
EVM EndBlock does a per-block prefix scan to GC old tx hashes. The error becomes a panic because CommitKVStore.Iterator() has no error return.
The proper fix is a merged iterator (old + new DB, with tombstone masking), which we don't have yet. The only known caller (RemoveFirstNTxHashes) is best-effort GC, incomplete results just leave a few stale hashes, no consensus impact — but panicking out of EndBlock does kill consensus.
Reverting for now and keeping the original Complete-only check. Updated the godoc to flag the caveat so future callers don't assume completeness during InProgress. Merged iterator tracked as follow-up
- Iterator: refuse once boundary leaves NotStarted (returning an old-DB iterator after the first batch silently drops migrated keys). - ApplyChangeSets: add firstBatchInBlock to Router/DBWriter so the one-migration-batch-per-block invariant lives in the contract instead of composite's empty-changeset hack; second non-empty flushes now forward writes only. - Rename shouldWriteOriginalPairToNewDB -> shouldForwardWriteToNewDB with godoc; record routing stats on every flush, not just the first.
3 participants
Summary
This branch adds the FlatKV EVM Migrate path and the first reusable Docker migration test flow, using EVM state as the initial scenario. Validators start with EVM state committed through memIAVL, restart in
migrate_evm, drain EVM account/code/storage keys into FlatKV incrementally as the restarted chain continues producing blocks, and verify that the post-migration FlatKV state is deterministic across validators. The core abstractions are a migration-aware composite store, explicit write-mode routing, a per-block migration manager, FlatKV initial-version seeding, and Docker-level migration orchestration.Migration hardening added on top of
main:Preserves pre-migration AppHash replay by keeping FlatKV's seeded lattice hash out of
LastCommitInfountil FlatKV participates in committed state.Ensures migration progresses on empty/live blocks and keeps the migration tail bounded by routing live writes with boundary plus old-DB existence: existing not-yet-migrated keys stay in memIAVL, while brand-new keys go to FlatKV.
Keeps legacy EVM prefix scans available during active migration by serving iterators from memIAVL, while exact reads remain migration-aware across both backends.
Makes FlatKV
SetInitialVersiondurable for read-only tooling by writing seeded metadata and a seeded snapshot.Stabilizes Docker migration runs by requiring validators to stop from one committed height before flipping
sc-write-mode, with retry instead of mixed-height restart.Reduces CI fixture OOG risk by splitting bulk EVM storage deployment into smaller batches.
Hardens unrelated upgrade-log CI checks around height/log-format races so migration failures stay visible.
sei-db/config/toml.go,app/seidb.go,docker/localnode/config/app.toml: addsc-keys-to-migrate-per-blockso operators and tests can control per-block migration batch size. The config path enforces a single source of truth from TOML throughStateCommitConfig.sei-db/state_db/sc/composite/store.go: builds routers after both backends are loaded, seeds a fresh FlatKV to the next memIAVL version on latest-version migration entry, and keeps targeted historical loads strict. It guarantees AppHash neutrality at the already-committed pre-migration height by only appending the FlatKV lattice hash after FlatKV participates in committed state.sei-db/state_db/sc/composite/store.go: routes child-store reads, writes, iteration, and proofs throughRouterCommitKVStore. Empty blocks still call the router in active migration modes so migration progress is not tied to user writes.sei-db/state_db/sc/migration/router_builder.go: defines write-mode routing formemiavl_only,migrate_evm,evm_migrated, later migration modes,flatkv_only, and dual-write testing. The routing table makes EVM migration explicit and rejects unsupported routes loudly. Each router builder now passes the memIAVL iterator builder straight intoNewMigrationManagerinstead of mutating the manager after construction.sei-db/state_db/sc/migration/migration_manager.go: manages the migration boundary, copies bounded batches from old DB to new DB, writes migrated keys to FlatKV, deletes copied old keys, and atomically writesmigration-versionwhile clearing the boundary on completion.NewMigrationManagernow takesoldDBIteratorBuilderas a constructor parameter so the manager is fully wired before any caller can use it.sei-db/state_db/sc/migration/migration_manager.go: preserves live-block correctness by routing writes with both migration boundary and old-DB existence. Existing not-yet-migrated keys stay in old DB so their latest value is copied when the iterator reaches them; brand-new keys go to new DB so continuously-created EVM keys do not extend the migration tail forever. Exact reads fall back from old DB to new DB for not-yet-migrated ranges.sei-db/state_db/sc/migration/migration_manager.go: keeps legacy EVM prefix-scan paths available during the migration window via the old DB iterator, while rejecting iteration after completion and rejecting access to the reserved migration metadata store.sei-db/state_db/sc/migration/migration_metrics.go: encapsulates the per-run aggregator insideMigrationMetrics.RecordBatchnow drives both OTel emission and the in-process completion summary, and exposes new counters for batches, original-pair routing (old/new DB), and per-DB pairs written. A nil metrics argument toNewMigrationManageris replaced with a local-only metrics so the completion log keeps working without an OTel exporter.sei-db/state_db/sc/flatkv/store_meta.go: adds durableSetInitialVersionseeding for FlatKV global and per-DB metadata, plus a seeded snapshot. This makes the next FlatKV commit land at the intended version and gives read-only tooling a stable seeded baseline.sei-db/state_db/sc/flatkv/store_meta.go: adds FlatKV latest-version helpers for startup sanity checks and recovery paths without requiring a full writable open first.sei-db/tools/cmd/seidb/main.go,sei-db/tools/cmd/seidb/operations/migrate_evm_status.go: addseidb migrate-evm-status, which opens FlatKV read-only through a snapshot/WAL clone and emits JSON status forversion_at,migration_version, completion, and boundary presence.Makefile,docker/docker-compose.yml,docker/localnode/scripts/step4_config_override.sh: propagateGIGA_MIGRATE_FROM_MEMIAVLinto validator containers and boot nodes inmemiavl_onlyfor migration CI. TOML edits are anchored to real keys to avoid matching comments..github/workflows/integration-test.yml: adds theFlatKV EVM Migrateintegration row. It boots a 4-validator cluster inmemiavl_only, deploys an EVM fixture, runs the migration script, and verifies post-migration EVM state. This establishes the CI shape future FlatKV migration scenarios can reuse: prepare fixture, run coordinated mode flip, poll status, and verify post-migration state.integration_test/contracts/deploy_flatkv_evm_fixture.sh: creates a deterministic EVM fixture before migration, including account balance, contract code/storage, and a configurable bulk-storage workload split into small deploy batches to stay below EVM gas limits.integration_test/contracts/verify_flatkv_evm_migrate.sh: enforces the operator sequence for the Docker migration test: verify all nodes are inmemiavl_only, wait for fixture blocks to settle, freeze validators at a common committed height, stop them cleanly, flipsc-write-modetomigrate_evm, restart, pollseidb migrate-evm-status, and compare FlatKV bucket digests at a shared height.integration_test/contracts/verify_flatkv_evm_migrate.sh: rejects unsafe stop conditions before flipping config. If validators commit an extra block during shutdown, the script restarts them inmemiavl_only, waits for convergence, and retries rather than allowing mixed-height migration.integration_test/contracts/verify_flatkv_evm_store.sh: verifies that the pre-migration EVM fixture remains readable after migration and improves diagnostics around FlatKV bucket dumps.scripts/evm_stress/main.go: addscontract-storageworkload mode, which deploys one-storage-slot contracts from many funded accounts to generate account, code, and storage rows for migration stress.integration_test/upgrade_module/scripts/verify_upgrade_needed_log.sh: hardens the upgrade log check against observed height/log-format races so unrelated upgrade CI does not hide migration signal.Test plan
sei-db/state_db/sc/composite/store_migration_test.go: covers happy-path MemiavlOnly to MigrateEVM lifecycle, crash-and-resume, deterministic roots across independent stores, and post-completionevm_migratedmode flip. These tests enforce read transparency, persisted boundary recovery, and FlatKV root invariants.sei-db/state_db/sc/composite/store_test.go: covers AppHash-neutral migration entry, idempotent FlatKV seeding across restarts, child-store routing, write-mode behavior, and commit-info/lattice invariants.sei-cosmos/storev2/rootmulti/flatkv_migration_test.go: covers rootmulti-level migration lifecycle, empty-block progress, legacy EVM iteration during migration, AppHash determinism across runs, migration-version persistence, and post-completion mode flip.sei-cosmos/storev2/rootmulti/flatkv_helpers_test.go: adds shared rootmulti migration helpers for configs, restart, rollback, historical hash verification, and FlatKV offline inspection.sei-db/state_db/sc/migration/migration_manager_test.go: covers persisted boundary resume, per-block batching, completion atomicity, writer failure paths, randomized migration equivalence, new-key tail handling, existing-key old-DB routing, reserved migration-store protection, target-version acceptance after completion, and the constructor-suppliedoldDBIteratorBuilderpath used by post-completion iteration assertions. Run-stat assertions now read throughMigrationMetrics.RunStats().sei-db/state_db/sc/migration/migration_transitions_test.go: updates the direct migration transition test for the FlatKV EVM Migrate (0 -> 1) path.sei-db/state_db/sc/flatkv/store_meta_test.go: coversSetInitialVersionhappy path, seeded snapshot creation, invalid version rejection, read-only rejection, post-commit rejection, reopen persistence, and rollback around seeded metadata.sei-db/tools/cmd/seidb/operations/flatkv_open_test.go: verifies read-only FlatKV tooling can open a store immediately afterSetInitialVersioncreates the seeded snapshot.integration_test/contracts/verify_flatkv_evm_migrate.sh: exercises the full Docker migration sequence, including common-height freeze, clean shutdown, restart inmigrate_evm, status polling, completion summaries, and cross-validator FlatKV digest agreement.integration_test/contracts/deploy_flatkv_evm_fixture.shandintegration_test/contracts/verify_flatkv_evm_store.sh: validate fixture generation and post-migration EVM read transparency for balances, code, storage, and missing-account cases.bash -n integration_test/contracts/verify_flatkv_evm_migrate.sh, focused Go tests for./sei-db/state_db/sc/migration,./sei-db/state_db/sc/composite,./sei-cosmos/storev2/rootmulti,./sei-db/tools/cmd/seidb/operations, and./scripts/evm_stress.