Consolidate SC configurations and interface for Giga #2786
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 #2786 +/- ##
==========================================
- Coverage 56.70% 56.69% -0.02%
==========================================
Files 2016 2023 +7
Lines 165448 165575 +127
==========================================
+ Hits 93813 93865 +52
- Misses 63420 63487 +67
- Partials 8215 8223 +8
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
blindchaser
left a comment
blindchaser
left a comment
There was a problem hiding this comment.
Configs: StateCommitConfig no longer has AsyncCommitBuffer, SnapshotKeepRecent, etc. (they moved under MemIAVLConfig), but the template still renders .StateCommit.*. WriteConfigFile will panic at runtime when rendering.
need to update the toml.go, app/seidb.go / sei-cosmos/server/config/config.go etc
|
|
||
| // SplitRead reads EVM data from EVM backend and non-EVM data from Cosmos. | ||
| // Use when migration is complete and backends are fully separated. | ||
| SplitRead ReadMode = "split_read" |
There was a problem hiding this comment.
how about naming it SplitByKeyRead.
and do we still have split read use case if we choose migrating ALL EVM state over to Giga DB?
There was a problem hiding this comment.
Still need split read, it means EVM goes from EVM and cosmos goes to cosmos
sei-db/config/write_mode.go
Outdated
| m := WriteMode(s) | ||
| if !m.IsValid() { | ||
| return "", fmt.Errorf("invalid write mode: %q, valid modes: %s, %s, %s, %s", | ||
| s, CosmosOnlyWrite, DualWrite, SplitWrite) |
There was a problem hiding this comment.
this s a good catch from ai
There was a problem hiding this comment.
True, should be fixed
| Config: config, // Embed the config directly | ||
| Dir: commitDBPath, | ||
| CreateIfMissing: true, | ||
| ZeroCopy: true, |
There was a problem hiding this comment.
confirm that we want the ZeroCopy always to be true?
| return nil, fmt.Errorf("failed to load cosmos version: %w", err) | ||
| } | ||
|
|
||
| return cosmosSC, nil |
There was a problem hiding this comment.
the return type of func LoadVersion is types.Committer, and we always return a cosmosCommitter here?
There was a problem hiding this comment.
You are right, here we should return a composite store
* main: Composite State Store part 1: EVM config and type definitions (#2754) Add scenario capability to benchmark script (#2779) emit rewards withdrawn events for redelegate/undelegate (#2781) add original cachekv as base layer (#2780) Add Ethereum state test runner for Giga executor validation (#2707) Add changelog for 6.2 and 6.3 (#2751) Fix typo in backport CI workflow name (#2784) Upgrade to latest UCI workflows (#2783) Configure self-hosted runners for Go tests (#2715)
That's a good catch! Will fix the toml config name |
Describe your changes and provide context
This PR unifies and consolidate the configuration and interface for committer (CommitStore) by:
Testing performed to validate your change