-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Conversation
WalkthroughThe codebase has been updated with a new function, Changes
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 X ? TipsChat with CodeRabbit Bot (
|
types/address.go
Outdated
@@ -362,6 +362,16 @@ func ValAddressFromBech32(address string) (addr ValAddress, err error) { | |||
return ValAddress(bz), nil | |||
} | |||
|
|||
// MustValAddressFromBech32 calls MustValAddressFromBech32 and panics on error. |
There was a problem hiding this comment.
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.
// MustValAddressFromBech32 calls MustValAddressFromBech32 and panics on error. | |
// MustValAddressFromBech32 calls ValAddressFromBech32 and panics on error. |
There was a problem hiding this comment.
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.
types/address_test.go
Outdated
func (s *addressTestSuite) TestMustAccAddressFromBech32() { | ||
address := "cosmosvaloper1780p20ce3quhzqxdq0xfeqtnzrjgdraz0kwwjp" | ||
valAddress1, err := types.ValAddressFromBech32(address) | ||
s.Require().Nil(err) | ||
|
||
valAddress2 := types.MustValAddressFromBech32(address) | ||
|
||
s.Require().Equal(valAddress1, valAddress2) | ||
} |
There was a problem hiding this comment.
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.
CHANGELOG.md
Outdated
@@ -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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: types
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utACK, modulo updating the changelog correctly
Description
I noticed that
MustAccAddressFromBech32
was implemented, butMustValAddressFromBech32
was not. I need to use it in actual coding scenarios. For example, in evmos I need to convert a string ValAddress into an evm hex string.The pseudo code is as follows: