feat(flatkv): fix zero storage pruning during flatkv migration#3577
Conversation
Use routed logical reads before pruning zero storage candidates so migrate_evm can keep pruning enabled without trusting a merged iterator view alone.
PR SummaryMedium Risk Overview
New coverage locks this in: a keeper test injects a store whose iterator lies about Reviewed by Cursor Bugbot for commit 9e7751d. 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).
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3577 +/- ##
==========================================
- Coverage 59.18% 58.35% -0.84%
==========================================
Files 2214 2144 -70
Lines 183402 175178 -8224
==========================================
- Hits 108549 102226 -6323
+ Misses 65047 63812 -1235
+ Partials 9806 9140 -666
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Summary
Make EVM zero-storage-slot pruning safe to run while a flatkv migration is in progress. The prune loop in
PruneZeroStorageSlotspreviously decided deletions from the iterator's surfaced value. During a flatkv migration the EVM store is a composite/migration store backed by two engines (memiavl + flatkv): point reads (Get/Has) are served by the authoritative routed read path, but iteration is a merged view stitched across both backends by the owning composite store. These two paths can diverge mid-migration, so trustingiterator.Value()could prune a slot that is logically non-zero (live) — corrupting state and producing a storage-digest / apphash mismatch. This change routes the prune decision through the same logical read the EVM itself trusts, so the iterator is used only as a candidate-key source.x/evm/keeper/storage_cleanup.go:PruneZeroStorageSlotsnow decides deletion fromstore.Get(key)(the routed logical read) instead ofiterator.Value(). The iterator output is treated purely as a list of candidate keys; the all-zero check is evaluated against the authoritative value. Adds aval != nilguard so keys that are logically absent (e.g. a tombstone surfaced by the merged iterator) are skipped rather than re-deleted. This guarantees a slot is pruned only when its authoritative value is all-zero, eliminating the merged-view data-loss path while keeping pruning enabled throughoutmigrate_evm.Test plan
x/evm/keeper/storage_cleanup_test.go:TestPruneZeroStorageSlots_DecidesDeletionFromRoutedGet(new): drives the realPruneZeroStorageSlotsagainst an injected store whose iterator deliberately lies aboutValue()whileGetreturns the truth, reproducing the migration merged-view divergence. Asserts a live slot (routedGetnon-zero, iterator shows zero) survives, and a dead slot (routedGetzero, iterator shows non-zero) is pruned. Verified to fail on the pre-fix code (live slot wrongly pruned) and pass on the fix, so it locks in the production behavior change. UseskvStoreOverrideMultiStore(overrides a single store key),iteratorValueLyingStore(truthful point reads, lying iterator values), andlyingIterator(per-keyValue()override).TestPruneZeroStorageSlots(existing): unchanged; continues to cover the normal (non-migration) path, batch limits, checkpoint advance, and checkpoint reset on iterator exhaustion.sei-db/state_db/sc/composite/store_migration_test.go:TestComposite_MigrateEVM_PruneZeroStorageSlotsDuringMigration(new): characterizes the composite store the fix relies on. Seeds zero/non-zero slots, reopens inMigrateEVM, prunes via routedGet, and asserts pruned keys are logically absent (viaGet) and absent from the composite iterator, while non-zero slots survive. Continues through migration completion and a finalEVMMigratedreopen, asserting version/root-hash continuity (preFlipVersion,preFlipHash) and flatkv lt-hash integrity (VerifyLtHash) at each stage.