Map cosmos-sdk pruning config to ev-node format and remove sonic dependency#345
Conversation
- Rename clearConflictingViperKeys to mapCosmosPruningToEvNode - Map cosmos-sdk pruning keys to ev-node format with proper value conversion - Add comprehensive tests for pruning mapping function - Remove sonic installation from all Dockerfiles Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
- Simplify switch statement logic in mapCosmosPruningToEvNode - Add test case for verifying cosmos settings override existing evnode settings Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
server/start.go
Outdated
| case "default", "everything", "custom": | ||
| evnodePruningMode = "all" | ||
| default: | ||
| // Unknown non-empty values default to "all", empty values remain empty |
There was a problem hiding this comment.
Unknown values default to disabled.
There was a problem hiding this comment.
Fixed in 387da22. Unknown pruning values now default to "disabled" instead of "all".
server/start.go
Outdated
| v.Set("evnode.pruning.pruning_keep_recent", cosmosKeepRecent) | ||
| } | ||
| if cosmosInterval != "" { | ||
| v.Set("evnode.pruning.pruning_interval", cosmosInterval) |
There was a problem hiding this comment.
Make sure we can set the interval like this. In ev-node intervals are time, while in the SDK I believe it is number of block. If that's the case, make sure to do nblock * blockTime
There was a problem hiding this comment.
Fixed in 387da22. The interval is now properly converted from blocks to time duration by multiplying the block count by the block time (from evnode.node.block_time, defaulting to 1s if not set).
server/start_test.go
Outdated
| for _, tc := range testCases { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| v := viper.New() | ||
| v.Set("pruning", tc.cosmosPruning) |
There was a problem hiding this comment.
Improve the test by using server DefaultConfig() and read it to viper, to make sure this works.
There was a problem hiding this comment.
Fixed in 387da22. Added a new test TestMapCosmosPruningToEvNode_WithDefaultConfig that uses serverconfig.DefaultConfig() to test with realistic cosmos-sdk configuration values.
|
@copilot check my comments |
- Change unknown pruning values to default to "disabled" instead of "all" - Convert cosmos-sdk pruning-interval from blocks to time duration - Add evnode.node.block_time to tests and use DefaultConfig for realistic testing - Update all test expectations to match the block-to-duration conversion Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
| evnodePruningMode = "disabled" | ||
| case "default", "everything", "custom": | ||
| evnodePruningMode = "all" | ||
| default: |
server/start.go
Outdated
| func mapCosmosPruningToEvNode(v *viper.Viper) { | ||
| // Get cosmos-sdk pruning configuration | ||
| cosmosPruning := v.GetString("pruning") | ||
| cosmosKeepRecent := v.GetString("pruning-keep-recent") |
There was a problem hiding this comment.
Use the flags from the SDK package instead of directly putting them here (client/flags or server most likely)
There was a problem hiding this comment.
Fixed in 3608be4. Now using cosmos-sdk server flag constants (sdkserver.FlagPruning, sdkserver.FlagPruningKeepRecent, sdkserver.FlagPruningInterval) instead of hardcoded string literals.
- Change default case to map unknown pruning values to "metadata" instead of "disabled" - Use cosmos-sdk server flag constants (FlagPruning, FlagPruningKeepRecent, FlagPruningInterval) instead of hardcoded strings - Update imports across server package to use qualified sdkserver import to avoid name collision - Update all tests to use the SDK flag constants - Update test expectations for unknown value mapping to metadata Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
* prunning * changed to batch delete * Map cosmos-sdk pruning config to ev-node format and remove sonic dependency (#345) * Initial plan * Implement pruning config mapping and remove sonic dependency - Rename clearConflictingViperKeys to mapCosmosPruningToEvNode - Map cosmos-sdk pruning keys to ev-node format with proper value conversion - Add comprehensive tests for pruning mapping function - Remove sonic installation from all Dockerfiles Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Fix comment clarity in mapCosmosPruningToEvNode Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Simplify logic and add test for override behavior - Simplify switch statement logic in mapCosmosPruningToEvNode - Add test case for verifying cosmos settings override existing evnode settings Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Remove redundant empty string case in switch statement Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address PR review feedback on pruning mapping - Change unknown pruning values to default to "disabled" instead of "all" - Convert cosmos-sdk pruning-interval from blocks to time duration - Add evnode.node.block_time to tests and use DefaultConfig for realistic testing - Update all test expectations to match the block-to-duration conversion Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * Address PR review: use SDK flag constants and map unknown to metadata - Change default case to map unknown pruning values to "metadata" instead of "disabled" - Use cosmos-sdk server flag constants (FlagPruning, FlagPruningKeepRecent, FlagPruningInterval) instead of hardcoded strings - Update imports across server package to use qualified sdkserver import to avoid name collision - Update all tests to use the SDK flag constants - Update test expectations for unknown value mapping to metadata Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com> * fixes * updates --------- Co-authored-by: julienrbrt <julien@rbrt.fr> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com> Co-authored-by: julienrbrt <29894366+julienrbrt@users.noreply.github.com>
Implementation Plan
1. Update pruning configuration mapping in
server/start.goclearConflictingViperKeystomapCosmosPruningToEvNodepruning→evnode.pruning.pruning_modewith value conversion:pruning-keep-recent→evnode.pruning.pruning_keep_recentpruning-interval→evnode.pruning.pruning_interval(with block-to-duration conversion)FlagPruning,FlagPruningKeepRecent,FlagPruningInterval) instead of hardcoded strings2. Remove sonic dependency from Dockerfiles
Dockerfile(line 31)Dockerfile.cosmos-sdk(line 32)tests/integration/docker/Dockerfile.gm(line 37)3. Testing and Validation
Technical Details
The implementation now uses the official cosmos-sdk server flag constants to ensure consistency with the SDK's configuration system. Unknown pruning mode values are mapped to "metadata" mode, which provides a conservative middle-ground that prunes block data but preserves metadata.
The cosmos-sdk import is qualified as
sdkserverthroughout the server package to avoid name collisions with the localserverpackage.Original prompt
This pull request was created from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.