Fix SS migration doc - #4027
Conversation
PR SummaryLow Risk Overview Paths and legacy behavior: Cosmos SS is documented as Config and verification: The Migration paths: Prerequisites and Step 2 distinguish P2P state-sync (layout-agnostic on the wire) from layout-sensitive Reviewed by Cursor Bugbot for commit 58ef462. 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 #4027 +/- ##
==========================================
- Coverage 61.24% 60.24% -1.00%
==========================================
Files 2153 2054 -99
Lines 188393 176643 -11750
==========================================
- Hits 115373 106419 -8954
+ Misses 62289 60462 -1827
+ Partials 10731 9762 -969
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Docs-only update to the Giga SS migration guide; the new path layout (data/state_store/{cosmos,evm}/{backend}), the legacy data/evm_ss fallback, the evm-ss-separate-dbs key and the sample startup log lines all check out against sei-db/common/utils/path.go, app/seidb.go and sei-db/state_db/ss/composite/store.go. One gap: the new "restore a data-directory snapshot" path omits the config that must match the snapshot.
Findings: 0 blocking | 4 non-blocking | 1 posted inline
Blockers
- None at the file/PR level.
Non-blocking
- None at the file/PR level.
- 1 suggestion(s)/nit(s) flagged inline on specific lines.
- 3 non-blocking pre-existing issue(s) listed below under pre-existing issues.
Pre-existing issues
- [suggestion]
docs/migration/giga_store_migration.md"What's different about EVM SS" (lines 32-36, unchanged by this PR) says EVM SS is "point-query only by design" and "iteration is explicitly disabled on the EVM backend". The code supports it:CompositeStateStore.Iterator/ReverseIteratorroute EVM store keys toEVMStateStore, which serves them from the unified DB whenseparateDBs=false(sei-db/state_db/ss/evm/store.go:121). The section's rationale ("cross-bucket scans would defeat the per-type sub-DB layout") only applies to theevm-ss-separate-dbs = truemode this PR now documents as non-default and experimental, so the section reads as more restrictive than reality. - [suggestion]
sei-db/config/toml.go:157still documentsevm-ss-db-directoryas defaulting to<home>/data/evm_ss, whilesei-db/config/ss_config.go:124andutils.GetEVMStateStorePathuse<home>/data/state_store/evm/{backend}(withdata/evm_ssonly as a legacy fallback). This PR fixes exactly that staleness in the migration guide; the generated app.toml comment operators actually read has the same issue. - [suggestion] The "Rollback Steps" section (lines 189-198) lists only the config flip and directory cleanup, without pointing at the FAQ entry (lines 222-227) that explains EVM state written while split was enabled is absent from Cosmos SS, so historical EVM queries stay broken until another state sync. An operator following Rollback Steps alone will not see that caveat.
| **Data-directory snapshots** (a tar of `~/.sei/data`) *are* layout-sensitive. A | ||
| tarball taken from a Giga SS node contains `data/state_store/evm/` (and | ||
| `data/state_store/cosmos/`) instead of a single mixed Cosmos SS directory. You can | ||
| enable Giga SS Store by restoring such a snapshot and setting `evm-ss-split = true`; |
There was a problem hiding this comment.
[suggestion] Restoring a Giga data/ tarball needs more than evm-ss-split = true: the restoring node's ss-backend (and ss-db-directory / evm-ss-db-directory if set) must match the layout inside the tarball, because both SS paths are backend-qualified (utils.GetStateStorePath / GetEVMStateStorePath → data/state_store/{cosmos,evm}/{backend}).
The backend mismatch is the dangerous case and it fails silently: a pebbledb tarball restored on a node configured with ss-backend = "rocksdb" resolves both SS dirs to non-existent .../rocksdb paths, so Cosmos SS opens empty — and because the safety checks in NewCompositeStateStore only fire when "Cosmos SS already has history", none of them trip. The node starts against a restored SC with an empty SS instead of refusing to launch. (A mismatched evm-ss-separate-dbs is caught: the EVM dir is non-empty but its unified DB is, so validateEVMSSPreRecovery aborts startup.)
Suggest adding a line here — and to the matching FAQ answer at line 243 — that the restoring node's ss-backend, evm-ss-separate-dbs, and any SS directory overrides must match the node the snapshot was taken from.
Describe your changes and provide context
Fix some minor outdated text for SS migration guide.
Testing performed to validate your change