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

feat(types): add MustValAddressFromBech32 function #18768

Merged
merged 7 commits into from
Dec 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Next Next commit
feat(address): add MustValAddressFromBech32 function
  • Loading branch information
luchenqun committed Dec 16, 2023
commit 2be11ed4671ee494abc2e0af5a4f687aef9236a8
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/auth/vesting) [#17810](https://github.com/cosmos/cosmos-sdk/pull/17810) Add the ability to specify a start time for continuous vesting accounts.
* (runtime) [#18475](https://github.com/cosmos/cosmos-sdk/pull/18475) Adds an implementation for core.branch.Service.
* (baseapp) [#18499](https://github.com/cosmos/cosmos-sdk/pull/18499) Add `MsgRouter` response type from message name function.
* (address) [#18768](https://github.com/cosmos/cosmos-sdk/pull/18768) Add MustValAddressFromBech32 function.
Copy link
Member

@julienrbrt julienrbrt Dec 16, 2023

Choose a reason for hiding this comment

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

nit: types

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

julienrbrt marked this conversation as resolved.
Show resolved Hide resolved

### Improvements

Expand Down
10 changes: 10 additions & 0 deletions types/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -362,6 +362,16 @@ func ValAddressFromBech32(address string) (addr ValAddress, err error) {
return ValAddress(bz), nil
}

// MustValAddressFromBech32 calls MustValAddressFromBech32 and panics on error.
Copy link
Contributor

Choose a reason for hiding this comment

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

The comment on line 365 incorrectly references MustValAddressFromBech32 instead of ValAddressFromBech32. This should be corrected to avoid confusion.

- // MustValAddressFromBech32 calls MustValAddressFromBech32 and panics on error.
+ // MustValAddressFromBech32 calls ValAddressFromBech32 and panics on error.

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
// MustValAddressFromBech32 calls MustValAddressFromBech32 and panics on error.
// MustValAddressFromBech32 calls ValAddressFromBech32 and panics on error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks rabbit. I fix it.

func MustValAddressFromBech32(address string) ValAddress {
addr, err := ValAddressFromBech32(address)
if err != nil {
panic(err)
}

return addr
}

// Returns boolean for whether two ValAddresses are Equal
func (va ValAddress) Equals(va2 Address) bool {
if va.Empty() && va2.Empty() {
Expand Down
10 changes: 10 additions & 0 deletions types/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -567,3 +567,13 @@ func (s *addressTestSuite) TestGetFromBech32() {
s.Require().Error(err)
s.Require().Equal("invalid Bech32 prefix; expected x, got cosmos", err.Error())
}

func (s *addressTestSuite) TestMustAccAddressFromBech32() {
julienrbrt marked this conversation as resolved.
Show resolved Hide resolved
address := "cosmosvaloper1780p20ce3quhzqxdq0xfeqtnzrjgdraz0kwwjp"
valAddress1, err := types.ValAddressFromBech32(address)
s.Require().Nil(err)

valAddress2 := types.MustValAddressFromBech32(address)

s.Require().Equal(valAddress1, valAddress2)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The test case provided for MustValAddressFromBech32 only covers the happy path. It would be prudent to include additional test cases to cover edge cases, such as invalid Bech32 strings, to ensure that the function behaves as expected in error scenarios.

Loading