FlatKV refactor for state sync import + export#3250
Conversation
|
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 @@
## main #3250 +/- ##
==========================================
+ Coverage 59.29% 59.32% +0.02%
==========================================
Files 2070 2076 +6
Lines 169782 170033 +251
==========================================
+ Hits 100670 100865 +195
- Misses 60327 60357 +30
- Partials 8785 8811 +26
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| // buffer reaches importBatchSize. If done fires, the worker abandons | ||
| // remaining work and exits immediately. | ||
| func (w *dbWorker) run(done <-chan struct{}) error { | ||
| for { |
There was a problem hiding this comment.
consider to add
defer func() {
if w.batch != nil {
_ = w.batch.Close()
}()
to avoid resource leak in some edge cases
| return nil, fmt.Errorf("convertFlatKVNodes account: %w", err) | ||
| } | ||
| var nodes []types.SnapshotNode | ||
| if nonce := acct.GetNonce(); nonce != 0 { |
There was a problem hiding this comment.
when nonce == 0 and codeHash is zero (e.g., an EOA that has received Sei but never sent a transaction), the SS store should still keep the account right?
There was a problem hiding this comment.
What shall we store in SS in this case? Just account and nonce=0? Agreed it's a little bit tricky here
There was a problem hiding this comment.
i'm thinking we should use vtype/account_data.go/IsDelete() to distinguish existing but nonce==0 vs non-existing/deleted, which decide if we put it in ss.
There was a problem hiding this comment.
make sense, will make a change
| return nil | ||
| } | ||
|
|
||
| newHash, _ := lthash.ComputeLtHash(w.ltHash, w.ltPairs) |
There was a problem hiding this comment.
In theory, we could offload lattice hash calculation to a work pool and get parallelism between DB operations and hash calculations. Cryptosim performance makes me think we could probably get a 2-3x speedup from this, assuming receiving data from the network isn't the bottleneck.
Writing this so we remember it in the future. Not a blocker for this PR (let's not prematurely optimize until we have benchmarks and actionable data).
There was a problem hiding this comment.
That's true, good point, let me either add a todo or just optimize in this PR
|
|
||
| // sequentialIterator iterates through a slice of DBs one at a time. | ||
| // It fully drains the current DB before moving to the next. | ||
| type sequentialIterator struct { |
There was a problem hiding this comment.
Nit, I think putting these before structs that implement an interface makes the code easier to grok.
| // sequentialIterator iterates through a slice of DBs one at a time. | |
| // It fully drains the current DB before moving to the next. | |
| type sequentialIterator struct { | |
| var _ Iterator = (*sequentialIterator)(nil) | |
| // sequentialIterator iterates through a slice of DBs one at a time. | |
| // It fully drains the current DB before moving to the next. | |
| type sequentialIterator struct { |
| default: | ||
| return nil, nil |
There was a problem hiding this comment.
Should this return an error? If we get an unrecognized key type here, that's a bug, right?
There was a problem hiding this comment.
Agreed, returning an error now
* main: Fix buffer offset in ProposerPriorityHash (CON-200) (#3255) Handle error case in light client divergence detector (#3254) perf(evmrpc): eliminate redundant block fetches in simulate backend (#3208) fix(evmrpc): omit notifications from legacy JSON-RPC batch responses per spec (#3246) fix: deduplicate block fetch in getTransactionReceipt (#3244)
Refactor the FlatKV export/import pipeline to use raw physical keys and vtype-serialized values end-to-end, and teach the SS (State Store) composite importer to handle FlatKV snapshot data. **FlatKV Exporter — rewritten to raw export** - Replaced the old parsing/conversion exporter with a raw iterator approach: `KVExporter` now uses `RawGlobalIterator()` to emit physical key/value pairs directly from the DBs without any transformation - Removed all per-type conversion functions (`accountToNodes`, `codeToNodes`, `storageToNodes`, `legacyToNodes`, `pendingNodes`) **FlatKV Importer — channel-based concurrent pipeline** - Rewrote `KVImporter` as a concurrent pipeline with per-DB worker goroutines (`dbWorker`) writing batches and computing LtHash in parallel - `AddNode` sends to a shared `ingestCh`; a `dispatch` goroutine routes keys to the correct DB worker via `routePhysicalKey` - Fail-fast error propagation using `atomic.Pointer[error]` and a `done` channel — `AddNode` and `dispatch` exit immediately on first error - New `FinalizeImport(version)` method persists LtHash and version metadata after the entire import completes - Fsync is always disabled during import for performance **FlatKV RawGlobalIterator — simplified** - Reimplemented as a `sequentialIterator` that iterates DBs in fixed order (EVM sub-DBs then legacyDB) using `dataDBs()` - Iterator errors are now checked and propagated (previously silently swallowed) - Removed `EmptyIterator` (dead code) **FlatKV Store refactoring** - Added `dataDBs()` and `namedDataDBs()` helpers in `store.go` to centralize the list of data DBs, replacing inline DB lists throughout the codebase - Added `routePhysicalKey(key)` to route a physical key to the correct DB - Removed all memiavl references from FlatKV code and comments **Composite SC Store** - Fixed `flatkvCommiter` typo → `flatkvCommitter` (used ~10 times) - Moved `FlatKVExportModuleName` to `common/keys/keys.go` as `FlatKVStoreKey` - Fixed composite importer routing bug: FlatKV nodes now return early if `flatkvImporter` is nil instead of falling through to Cosmos importer - Fixed error message: "failed to create evm importer" → "failed to create flatkv importer" **SS Composite Import — rewritten and simplified** - Rewrote `Import` from ~135 lines with two separate code paths into a unified ~75-line function: - Single routing loop for all write modes (DualWrite, SplitWrite, CosmosOnlyWrite, no-EVM) - `evmCh` is only created when EVM store is available and active - Lightweight `done` channel + `sync.Once` pattern replaces complex `drainImportErr`/`sendNode` retry closures - Added `convertFlatKVNodes()` function that transforms FlatKV physical-key snapshot nodes into SS-compatible format: - Strips module prefix (`ktype.StripModulePrefix`) and parses key kind (`keys.ParseEVMKey`) - **Account keys**: deserializes `vtype.AccountData`, emits separate nonce (8-byte BE) and codeHash (32-byte) nodes; skips zero-nonce and zero-codeHash (EOA accounts) - **Storage/Code keys**: deserializes the corresponding vtype and emits raw value with `StoreKey = "evm"` - **Legacy keys**: deserializes `vtype.LegacyData` and preserves the original module name as `StoreKey` so keys route back to the correct Cosmos SS module (e.g. `"bank"`, `"staking"`) - Updated import routing: when EVM SS is enabled, EVM keys go exclusively to EVM store, non-EVM keys go to Cosmos store - Removed unused `normalizeSnapshotNode` function and `splitWrite` variable from Import - Fixed config validation error messages: `ReaderConstantThreadCount` / `ReaderPoolQueueSize` / `MiscConstantThreadCount` now say "must not be negative" (previously said "must be greater than 0" while allowing 0) - Fixed duplicate "for" typo in `api.go` comment - **`ss/composite/store_test.go`**: Updated `TestImport_OnlyEvmModule` to verify EVM-only routing (no cosmos duplication); rewrote `TestImport_OnlyEvmFlatkvModule` and `TestImport_BothEvmAndEvmFlatkv` with proper FlatKV physical keys and vtype-serialized values; rewrote `TestImport_CosmosOnlyWrite_ConvertsFlatkvToCosmos` to verify conversion in no-EVM path - **`sc/flatkv/import_export_test.go`**: Updated round-trip tests, exporter tests for raw key/value output, importer tests for physical key routing, corrupt data propagation tests - **`sc/composite/store_test.go`**: Added `RawGlobalIterator()` to mock, updated all `flatkvCommitter` references, replaced `FlatKVExportModuleName` with `keys.FlatKVStoreKey` - **`sc/flatkv/store_write_test.go`**, **`store_read_test.go`**, **`snapshot_test.go`**, **`perdb_lthash_test.go`**, **`lthash_correctness_test.go`**: Fixed compile errors, updated to use centralized DB helpers and physical key formats
Two changes that together eliminate the need for SS-path iteration on EVM data: 1. composite.Import now sends evm snapshot nodes to BOTH cosmos and evm under DualWrite (previously mutually-exclusive after #3250). This restores the state-sync-time safety net that made mainnet DualWrite state-synced nodes surface pointer lookups via cosmos even while the iterator routing bug was present. 2. Pointer registry collapsed to single-key: one record per pointee, with the uint16 version packed into the value instead of the key suffix. GetPointerInfo/GetAnyPointerInfo become single Gets — the only hot-path EVM iterator is gone. Migration handlers updated to read the version-prefixed value and drop the now-unneeded dedup map. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Describe your changes and provide context
Refactor the FlatKV export/import pipeline to use raw physical keys and vtype-serialized values end-to-end, and teach the SS (State Store) composite importer to handle FlatKV snapshot data.
SC (State Commitment) Layer
FlatKV Exporter — rewritten to raw export
KVExporternow usesRawGlobalIterator()to emit physical key/value pairs directly from the DBs without any transformationaccountToNodes,codeToNodes,storageToNodes,legacyToNodes,pendingNodes)FlatKV Importer — channel-based concurrent pipeline
KVImporteras a concurrent pipeline with per-DB worker goroutines (dbWorker) writing batches and computing LtHash in parallelAddNodesends to a sharedingestCh; adispatchgoroutine routes keys to the correct DB worker viaroutePhysicalKeyatomic.Pointer[error]and adonechannel —AddNodeanddispatchexit immediately on first errorFinalizeImport(version)method persists LtHash and version metadata after the entire import completesFlatKV RawGlobalIterator — simplified
sequentialIteratorthat iterates DBs in fixed order (EVM sub-DBs then legacyDB) usingdataDBs()EmptyIterator(dead code)FlatKV Store refactoring
dataDBs()andnamedDataDBs()helpers instore.goto centralize the list of data DBs, replacing inline DB lists throughout the codebaseroutePhysicalKey(key)to route a physical key to the correct DBComposite SC Store
flatkvCommitertypo →flatkvCommitter(used ~10 times)FlatKVExportModuleNametocommon/keys/keys.goasFlatKVStoreKeyflatkvImporteris nil instead of falling through to Cosmos importerSS (State Store) Layer
SS Composite Import — rewritten and simplified
Importfrom ~135 lines with two separate code paths into a unified ~75-line function:evmChis only created when EVM store is available and activedonechannel +sync.Oncepattern replaces complexdrainImportErr/sendNoderetry closuresconvertFlatKVNodes()function that transforms FlatKV physical-key snapshot nodes into SS-compatible format:ktype.StripModulePrefix) and parses key kind (keys.ParseEVMKey)vtype.AccountData, emits separate nonce (8-byte BE) and codeHash (32-byte) nodes; skips zero-nonce and zero-codeHash (EOA accounts)StoreKey = "evm"vtype.LegacyDataand preserves the original module name asStoreKeyso keys route back to the correct Cosmos SS module (e.g."bank","staking")normalizeSnapshotNodefunction andsplitWritevariable from ImportConfig & Misc
ReaderConstantThreadCount/ReaderPoolQueueSize/MiscConstantThreadCountnow say "must not be negative" (previously said "must be greater than 0" while allowing 0)api.gocommentTesting performed to validate your change
ss/composite/store_test.go: UpdatedTestImport_OnlyEvmModuleto verify EVM-only routing (no cosmos duplication); rewroteTestImport_OnlyEvmFlatkvModuleandTestImport_BothEvmAndEvmFlatkvwith proper FlatKV physical keys and vtype-serialized values; rewroteTestImport_CosmosOnlyWrite_ConvertsFlatkvToCosmosto verify conversion in no-EVM pathsc/flatkv/import_export_test.go: Updated round-trip tests, exporter tests for raw key/value output, importer tests for physical key routing, corrupt data propagation testssc/composite/store_test.go: AddedRawGlobalIterator()to mock, updated allflatkvCommitterreferences, replacedFlatKVExportModuleNamewithkeys.FlatKVStoreKeysc/flatkv/store_write_test.go,store_read_test.go,snapshot_test.go,perdb_lthash_test.go,lthash_correctness_test.go: Fixed compile errors, updated to use centralized DB helpers and physical key formats