testing(flatkv): evm migration int testings#3473
Conversation
PR SummaryHigh Risk Overview Introduces a new Hardens the migration implementation: Improves FlatKV lifecycle correctness by writing a seeded snapshot during Reviewed by Cursor Bugbot for commit 134e164. 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.11% 59.08% -0.04%
==========================================
Files 2187 2188 +1
Lines 182236 182393 +157
==========================================
+ Hits 107724 107761 +37
- Misses 64864 64981 +117
- Partials 9648 9651 +3
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
There was a problem hiding this comment.
These tests duplicate tests I'm working on in a branch. But I think it's worth merging both. It's nice validation to have multiple test suites written independently, as it's less likely that we've both made a bad assumption as to how something should work.
| type migrationRunStats struct { | ||
| startedAt time.Time | ||
|
|
||
| batches int64 | ||
| keysMigrated int64 | ||
| keyBytesMigrated int64 | ||
| valueBytesMigrated int64 | ||
| originalPairsRoutedOldDB int64 | ||
| originalPairsRoutedNewDB int64 | ||
| oldDBPairsWritten int64 | ||
| newDBPairsWritten int64 | ||
| } | ||
|
|
||
| type migrationBatchStats struct { | ||
| keysMigrated int64 | ||
| keyBytesMigrated int64 | ||
| valueBytesMigrated int64 | ||
| originalPairsRoutedOldDB int64 | ||
| originalPairsRoutedNewDB int64 | ||
| oldDBPairsWritten int64 | ||
| newDBPairsWritten int64 | ||
| } |
There was a problem hiding this comment.
Would it make sense to encapsulate this stuff inside MigrationMetrics? We could then expose metrics from these values, which might end up being useful if we encounter problems when we deploy this code. The unit tests could continue to access these values after the move.
| // At the end of this migration, all evm/ data lives in flatkv, and all other data remains in memIAVL. | ||
| // | ||
| // This test evaluates the 0->1 migration path. | ||
| // This test evaluates the FlatKV EVM migrate path. |
There was a problem hiding this comment.
Nit, could we add (0 -> 1) to this godoc? The migration versions are referenced in a bunch of other places, and I think it's a useful way of thinking about things sometimes.
| } | ||
|
|
||
| /* Data flow: MigrateEVM (0 -> 1) | ||
| /* Data flow: FlatKV EVM migrate |
There was a problem hiding this comment.
Can we retain the (0 -> 1) part?
| if err != nil { | ||
| return nil, fmt.Errorf("NewMigrationManager: %w", err) | ||
| } | ||
| migrationManager.oldDBIteratorBuilder = buildMemIAVLIteratorBuilder(memIAVL) |
There was a problem hiding this comment.
Ok to add this, but we should probably make it a constructor parameter in the NewMigrationManager() method instead of directly setting it.
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:
|
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.