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.

Sign up for GitHub

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

feat(client): add consensus address for debug cmd #20328

Merged
merged 5 commits into from
May 13, 2024
Merged

feat(client): add consensus address for debug cmd #20328

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
500c5c3
feat: add consensus address for debug cmd
mmsqe May 9, 2024
83a1883
update doc
mmsqe May 9, 2024
5e2b905
Update client/debug/main.go
mmsqe May 10, 2024
de0d610
Update client/debug/main.go
mmsqe May 10, 2024
af2b1e7
Merge remote-tracking branch 'origin/main' into con_addr
mmsqe May 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
* (client) [#19870](https://github.com/cosmos/cosmos-sdk/pull/19870) Add new query command `wait-tx`. Alias `event-query-tx-for` to `wait-tx` for backward compatibility.
* (crypto/keyring) [#20212](https://github.com/cosmos/cosmos-sdk/pull/20212) Expose the db keyring used in the keystore.
* (genutil) [#19971](https://github.com/cosmos/cosmos-sdk/pull/19971) Allow manually setting the consensus key type in genesis
* (debug) [#20328](https://github.com/cosmos/cosmos-sdk/pull/20328) Add consensus address for debug cmd.

### Improvements

Expand Down
34 changes: 25 additions & 9 deletions client/debug/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -260,26 +260,42 @@ func AddrCmd() *cobra.Command {
addr []byte
err error
)
addr, err = hex.DecodeString(addrString)
if err != nil {
var err2 error
addr, err2 = clientCtx.AddressCodec.StringToBytes(addrString)
if err2 != nil {
var err3 error
addr, err3 = clientCtx.ValidatorAddressCodec.StringToBytes(addrString)
if err3 != nil {
return fmt.Errorf("expected hex or bech32. Got errors: hex: %w, bech32 acc: %w, bech32 val: %w", err, err2, err3)
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
mmsqe marked this conversation as resolved.
Show resolved Hide resolved
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should return err directly here instead of loop because we assume the array has 4 elements anyway. St like
fmt.Errorf("expected hex or bech32. Got errors: hex: %w, bech32 acc: %w, bech32 val: %w", bech32 cons: %w", errs[0], errs[1], errs[2], errs[3])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was try to avoid that way.

"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}

acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)

cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)
Comment on lines +263 to +298
Copy link
Contributor

Choose a reason for hiding this comment

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

Refactor error handling in AddrCmd to avoid ignoring errors.

- con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
+ con, err := clientCtx.ConsensusAddressCodec.BytesToString(addr)
+ if err != nil {
+     return fmt.Errorf("failed to convert consensus address: %w", err)
+ }

This change ensures that errors during the conversion of consensus addresses are not silently ignored, improving the robustness of the command.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, err := clientCtx.ConsensusAddressCodec.BytesToString(addr)
if err != nil {
return fmt.Errorf("failed to convert consensus address: %w", err)
}
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)

Consider using a switch-case for address decoding in AddrCmd.

- for _, fn := range decodeFns {
-     if addr, err = fn(addrString); err == nil {
-         break
-     }
-     errs = append(errs, err)
- }
+ for _, fn := range decodeFns {
+     addr, err = fn(addrString)
+     if err == nil {
+         break
+     }
+     errs = append(errs, err)
+ }

Using a switch-case can make the code cleaner and more readable by explicitly handling each decoding function, and it aligns better with the Uber Golang style guide which favors readability and explicit error handling.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
addr, err = fn(addrString)
if err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)

Optimize error formatting in AddrCmd.

- format += errTags[i] + ": %w"
+ format += fmt.Sprintf("%s: %%w", errTags[i])

This change ensures that the error tags are correctly formatted into the error string, improving the clarity of error messages.

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += errTags[i] + ": %w"
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)
decodeFns := []func(text string) ([]byte, error){
hex.DecodeString,
clientCtx.AddressCodec.StringToBytes,
clientCtx.ValidatorAddressCodec.StringToBytes,
clientCtx.ConsensusAddressCodec.StringToBytes,
}
errs := make([]any, 0, len(decodeFns))
for _, fn := range decodeFns {
if addr, err = fn(addrString); err == nil {
break
}
errs = append(errs, err)
}
if len(errs) == len(decodeFns) {
errTags := []string{
"hex", "bech32 acc", "bech32 val", "bech32 con",
}
format := ""
for i := range errs {
if format != "" {
format += ", "
}
format += fmt.Sprintf("%s: %%w", errTags[i])
}
return fmt.Errorf("expected hex or bech32. Got errors: "+format, errs...)
}
acc, _ := clientCtx.AddressCodec.BytesToString(addr)
val, _ := clientCtx.ValidatorAddressCodec.BytesToString(addr)
con, _ := clientCtx.ConsensusAddressCodec.BytesToString(addr)
cmd.Println("Address:", addr)
cmd.Printf("Address (hex): %X\n", addr)
cmd.Printf("Bech32 Acc: %s\n", acc)
cmd.Printf("Bech32 Val: %s\n", val)
cmd.Printf("Bech32 Con: %s\n", con)

return nil
},
}
Expand Down
Loading