fix: let the create-validator command retrieve the loaded configuration. #2128
Conversation
Upon CLI bootstrapping, a PersistentPreRunE handler is added to the command in order to, among other things, load the configuration in a viper instance and store it into the command's context with a ViperContextKey key. The create-validator command was using the wrong, but homonymous, ViperContextKey for retrieving the viper configuration from the command context. As a result, the command implementation was not able to find some configuration values which were silently used as empty strings that, eventually, would halt the execution with unexpected errors. This commit replaces the source package of the ViperContextKey key from github.com/cosmos/cosmos-sdk/client to github.com/berachain/beacon-kit/mod/cli/pkg/context
WalkthroughThe changes in this pull request focus on the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2128 +/- ##
=======================================
Coverage 23.48% 23.48%
=======================================
Files 357 357
Lines 16064 16064
Branches 12 12
=======================================
Hits 3772 3772
Misses 12121 12121
Partials 171 171
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
📒 Files selected for processing (1)
mod/cli/pkg/commands/deposit/create.go(2 hunks)
🔇 Additional comments (2)
mod/cli/pkg/commands/deposit/create.go (2)
27-27: LGTM: Import change aligns with PR objectives.
The switch to the project's own context package (clicontext) is the correct approach to fix the configuration retrieval issue.
Line range hint 39-163: Verify command behavior remains unchanged.
The core command functionality is preserved, which is correct. However, let's verify that the command still works with the new context implementation.
✅ Verification successful
Based on the verification results, I can now provide the final response:
Command implementation is correctly context-independent
The create-validator command implementation is properly isolated from context dependencies:
- The command uses cobra's native command structure without relying on external context
- No direct dependencies on cosmos-sdk client context are present
- The command only depends on local configuration through cobra flags and the chainSpec parameter
- BLS signer implementation is self-contained and works with local flag values
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the create-validator command is properly integrated with the new context
# Expected: The command should be registered and use the new context package
# Check if the command is properly registered and its configuration
rg -A 2 "create-validator.*RunE:"
# Verify there are no remaining references to the old cosmos-sdk client context
rg "github.com/cosmos/cosmos-sdk/client.*ContextKey"
Length of output: 90
Script:
#!/bin/bash
# Let's try a broader search to understand the command registration and context usage
# Check for create-validator command registration
rg -l "create-validator"
# Check imports in the create.go file to verify context dependencies
rg --type go -l "import \(" -A 20 "mod/cli/pkg/commands/deposit/create.go"
# Look for any cosmos-sdk context usage
rg -l "cosmos-sdk/client"
# Check for any context-related types or interfaces being used
ast-grep --pattern 'type $_ interface {
$$$
Context($$$) $$$
$$$
}'
Length of output: 1102
Script:
#!/bin/bash
# Let's examine the create validator command implementation more closely
# Check the actual implementation of create validator command
rg -A 5 "func createValidatorCmd" mod/cli/pkg/commands/deposit/create.go
# Check if there are any context-related parameters or usage in the file
rg "context\.|Context" mod/cli/pkg/commands/deposit/create.go
# Check for any dependencies on client context in the deposit package
rg -l "client\.Context|ClientContext" mod/cli/pkg/commands/deposit/
# Look for any BLS signer implementation to verify context independence
ast-grep --pattern 'func getBLSSigner($$$) {
$$$
}'
Length of output: 525
| components.BlsSignerInput{ | ||
| AppOpts: client.GetViperFromCmd(cmd), | ||
| AppOpts: clicontext.GetViperFromCmd(cmd), | ||
| PrivKey: legacyKey, | ||
| }, |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
LGTM: Configuration retrieval fix implemented correctly.
The switch to clicontext.GetViperFromCmd properly fixes the configuration access issue. However, consider adding tests to verify the configuration retrieval behavior.
Would you like me to help create test cases for the configuration retrieval functionality? This would help ensure the fix remains robust against future changes.
Upon CLI bootstrapping, a
PersistentPreRunEhandler is added to the command in order to, among other things, load the configuration in a viper instance and store it into the command's context with aViperContextKeykey.The
create-validatorcommand was using the wrong, but homonymous,ViperContextKeyfor retrieving the viper configuration from the command context.As a result, the command implementation was not able to find some configuration values which were silently used as empty strings that, eventually, would halt the execution with unexpected errors.
This commit replaces the source package of the
ViperContextKeykey fromgithub.com/cosmos/cosmos-sdk/clientto
github.com/berachain/beacon-kit/mod/cli/pkg/contextThis should also fix the issue #2122 .
Summary by CodeRabbit