Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: fix reset commands #8270

Merged
merged 2 commits into from
Apr 7, 2022
Merged

fix: fix reset commands #8270

merged 2 commits into from
Apr 7, 2022

Conversation

alexanderbez
Copy link
Contributor

Upstream consumers of Tendermint's commands that use them directly as opposed to using the root command will fail because config is a private global. This forces the upstream user to manually parse the config or having each command do it. This PR takes the latter approach.

}

// XXX: this is totally unsafe.
// it's only suitable for testnets.
func resetPrivValidator(cmd *cobra.Command, args []string) {
func resetPrivValidator(cmd *cobra.Command, args []string) (err error) {
config, err = ParseConfig()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see resetFilePV relying on the global config value, so I'm not certain I see why we need to parse into the global var here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh? It literally calls resetFilePV with the values from config below.

Copy link
Contributor

@williambanfield williambanfield Apr 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yea, it uses the values from the variable as set here. I'm confused as to why we don't just use a local variable here for the return of ParseConfig instead of binding its result to the package-global config variable seeing as resetFilePV does not use fields from the global config variable as far as I can tell. I.e., it gets the values through args not by referencing global scope.

@creachadair creachadair merged commit 6e85f46 into v0.34.x Apr 7, 2022
@creachadair creachadair deleted the bez/fix-reset-cmds branch April 7, 2022 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants