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

Jump to bottom

refactor: allow getting address prefix from address codec #15913

Closed
wants to merge 10 commits into from
Closed

refactor: allow getting address prefix from address codec #15913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
a54ed2a
feat: allow to get bech32 prefix from address codec
julienrbrt Apr 23, 2023
c63dec0
feedback
julienrbrt Apr 23, 2023
2aa75a8
updates
julienrbrt Apr 23, 2023
ab09d08
simplify
julienrbrt Apr 23, 2023
ca61ef7
go mod tidy
julienrbrt Apr 23, 2023
01bc9a9
Merge branch 'main' into julien/codec-add-get-prefix
julienrbrt Apr 23, 2023
7e4f580
Merge branch 'main' into julien/codec-add-get-prefix
julienrbrt Apr 24, 2023
02ae046
updates
julienrbrt Apr 24, 2023
50931b5
Merge branch 'main' into julien/codec-add-get-prefix
julienrbrt Apr 24, 2023
271a70e
Merge branch 'main' into julien/codec-add-get-prefix
julienrbrt Apr 24, 2023
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
5 changes: 5 additions & 0 deletions codec/address/bech32_codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ func NewBech32Codec(prefix string) address.Codec {
return Bech32Codec{prefix}
}

// GetAddressPrefix returns the bech32 prefix
func (bc Bech32Codec) GetAddressPrefix() string {
return bc.Bech32Prefix
}

// StringToBytes encodes text to bytes
func (bc Bech32Codec) StringToBytes(text string) ([]byte, error) {
hrp, bz, err := bech32.DecodeAndConvert(text)
Expand Down
6 changes: 4 additions & 2 deletions core/address/codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@ package address

// Codec defines an interface to convert addresses from and to string/bytes.
type Codec interface {
// StringToBytes decodes text to bytes
// GetAddressPrefix returns the address prefix (if any).
GetAddressPrefix() string
Copy link
Member

Choose a reason for hiding this comment

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

We can't assume the codec is always bech32. I'd rather find another way to deal with this

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes (#15913 (comment)), so do you find this naming not fitting if the address has no prefix?

I do not know if you've seen the autocli PR but this come handy there.

Copy link
Member Author

@julienrbrt julienrbrt Apr 23, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Is that fine for autocli to depend directly on auth then?
Personally, I don't like it, but that would solve the issue I have there.
Another way I thought would be autocli to depend on auth config, but this was still less clean than with this PR.

Copy link
Member

Choose a reason for hiding this comment

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

since autocli is built off of grpc why not use

cosmos-sdk/proto/cosmos/auth/v1beta1/query.proto

Line 64 in be081b1

rpc Bech32Prefix(Bech32PrefixRequest) returns (Bech32PrefixResponse) {
. This way autocli/hubl will only work with bech32 addresses for now, but there can be a fallback or a way for a user to load their own

Copy link
Member Author

@julienrbrt julienrbrt Apr 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It is almost what it already uses there, and it works fine for hubl, which will build the command after having queried an endpoint. However, for normal commands, like simd, gaiad, etc.. When we enrich the root command, we have no endpoint to query. The information is however available, and imho this was the cleanest way to get it.

We could however:

  • Add an argument in EnhanceRootCommand for the bech32 prefix (easiest but most ugly because for hubl we'll need to pass an empty string)
  • Inject address codec and add api to get address prefix (this was my preferred solution and what is done in this PR)
  • Inject auth configuration and use config.Bech32Prefix for the address prefix, which is fine for app wiring app, but it makes it weird when you have to construct AppOptions manually
  • Inject auth, then we have a hard dependency on auth for autocli

I am open to other suggestions however.

Copy link
Member

Choose a reason for hiding this comment

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

Why don't we just change the flag.Builder argument to take address.Codec instead of bech32 prefix. I think that would resolve this without needing to change address.Codec. Also if we do need the actual prefix let's use an extension interface instead of changing address.Codec

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't think of the problem this way. This would indeed work too, and only complexify the flag builder
I will close this then and re-open it shortly with only the clean-ups that were done here.

Copy link
Contributor

Choose a reason for hiding this comment

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

We can't assume the codec is always bech32. I'd rather find another way to deal with this

While we can't assume this, this is pretty baked into the SDK and I personally can't see this changing in the future.

// StringToBytes decodes text to bytes.
StringToBytes(text string) ([]byte, error)
// BytesToString encodes bytes to text
// BytesToString encodes bytes to text.
BytesToString(bz []byte) (string, error)
}
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ module github.com/cosmos/cosmos-sdk
require (
cosmossdk.io/api v0.4.1
cosmossdk.io/collections v0.1.0
cosmossdk.io/core v0.6.1
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab
cosmossdk.io/depinject v1.0.0-alpha.3
cosmossdk.io/errors v1.0.0-beta.7
cosmossdk.io/log v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ cosmossdk.io/api v0.4.1 h1:0ikaYM6GyxTYYcfBiyR8YnLCfhNnhKpEFnaSepCTmqg=
cosmossdk.io/api v0.4.1/go.mod h1:jR7k5ok90LxW2lFUXvd8Vpo/dr4PpiyVegxdm7b1ZdE=
cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8=
cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo=
cosmossdk.io/core v0.6.1 h1:OBy7TI2W+/gyn2z40vVvruK3di+cAluinA6cybFbE7s=
cosmossdk.io/core v0.6.1/go.mod h1:g3MMBCBXtxbDWBURDVnJE7XML4BG5qENhs0gzkcpuFA=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab h1:T5M7ZlsIoOi+ystyBuEzk19kh72bXbtfMqq6NEIkub8=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab/go.mod h1:beJu7PuC+A7Os512U3gdsBrc3Qcax9XJnVE7jtiA+Gc=
cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=
cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w=
Expand Down
2 changes: 1 addition & 1 deletion simapp/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ go 1.20
require (
cosmossdk.io/api v0.4.1
cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba
cosmossdk.io/core v0.6.1
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab
cosmossdk.io/depinject v1.0.0-alpha.3
cosmossdk.io/log v1.0.0
cosmossdk.io/math v1.0.0
Expand Down
4 changes: 2 additions & 2 deletions simapp/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba h1:LuPHCncU2KLMNPItFEC
cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba/go.mod h1:SXdwqO7cN5htalh/lhXWP8V4zKtBrhhcSTU+ytuEtmM=
cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8=
cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo=
cosmossdk.io/core v0.6.1 h1:OBy7TI2W+/gyn2z40vVvruK3di+cAluinA6cybFbE7s=
cosmossdk.io/core v0.6.1/go.mod h1:g3MMBCBXtxbDWBURDVnJE7XML4BG5qENhs0gzkcpuFA=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab h1:T5M7ZlsIoOi+ystyBuEzk19kh72bXbtfMqq6NEIkub8=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab/go.mod h1:beJu7PuC+A7Os512U3gdsBrc3Qcax9XJnVE7jtiA+Gc=
cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=
cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w=
Expand Down
2 changes: 1 addition & 1 deletion tests/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ require (
cloud.google.com/go/storage v1.30.0 // indirect
cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba // indirect
cosmossdk.io/collections v0.1.0 // indirect
cosmossdk.io/core v0.6.1 // indirect
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab // indirect
filippo.io/edwards25519 v1.0.0 // indirect
github.com/99designs/go-keychain v0.0.0-20191008050251-8e49817e8af4 // indirect
github.com/99designs/keyring v1.2.1 // indirect
Expand Down
4 changes: 2 additions & 2 deletions tests/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba h1:LuPHCncU2KLMNPItFEC
cosmossdk.io/client/v2 v2.0.0-20230309163709-87da587416ba/go.mod h1:SXdwqO7cN5htalh/lhXWP8V4zKtBrhhcSTU+ytuEtmM=
cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8=
cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo=
cosmossdk.io/core v0.6.1 h1:OBy7TI2W+/gyn2z40vVvruK3di+cAluinA6cybFbE7s=
cosmossdk.io/core v0.6.1/go.mod h1:g3MMBCBXtxbDWBURDVnJE7XML4BG5qENhs0gzkcpuFA=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab h1:T5M7ZlsIoOi+ystyBuEzk19kh72bXbtfMqq6NEIkub8=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab/go.mod h1:beJu7PuC+A7Os512U3gdsBrc3Qcax9XJnVE7jtiA+Gc=
cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=
cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w=
Expand Down
2 changes: 1 addition & 1 deletion tools/rosetta/go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ require (
require (
cosmossdk.io/api v0.4.1 // indirect
cosmossdk.io/collections v0.1.0 // indirect
cosmossdk.io/core v0.6.1 // indirect
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab // indirect
cosmossdk.io/depinject v1.0.0-alpha.3 // indirect
cosmossdk.io/errors v1.0.0-beta.7 // indirect
cosmossdk.io/store v0.1.0-alpha.1.0.20230328185921-37ba88872dbc // indirect
Expand Down
4 changes: 2 additions & 2 deletions tools/rosetta/go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ cosmossdk.io/api v0.4.1 h1:0ikaYM6GyxTYYcfBiyR8YnLCfhNnhKpEFnaSepCTmqg=
cosmossdk.io/api v0.4.1/go.mod h1:jR7k5ok90LxW2lFUXvd8Vpo/dr4PpiyVegxdm7b1ZdE=
cosmossdk.io/collections v0.1.0 h1:nzJGeiq32KnZroSrhB6rPifw4I85Cgmzw/YAmr4luv8=
cosmossdk.io/collections v0.1.0/go.mod h1:xbauc0YsbUF8qKMVeBZl0pFCunxBIhKN/WlxpZ3lBuo=
cosmossdk.io/core v0.6.1 h1:OBy7TI2W+/gyn2z40vVvruK3di+cAluinA6cybFbE7s=
cosmossdk.io/core v0.6.1/go.mod h1:g3MMBCBXtxbDWBURDVnJE7XML4BG5qENhs0gzkcpuFA=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab h1:T5M7ZlsIoOi+ystyBuEzk19kh72bXbtfMqq6NEIkub8=
cosmossdk.io/core v0.6.2-0.20230423164157-c63dec0f7aab/go.mod h1:beJu7PuC+A7Os512U3gdsBrc3Qcax9XJnVE7jtiA+Gc=
cosmossdk.io/depinject v1.0.0-alpha.3 h1:6evFIgj//Y3w09bqOUOzEpFj5tsxBqdc5CfkO7z+zfw=
cosmossdk.io/depinject v1.0.0-alpha.3/go.mod h1:eRbcdQ7MRpIPEM5YUJh8k97nxHpYbc3sMUnEtt8HPWU=
cosmossdk.io/errors v1.0.0-beta.7 h1:gypHW76pTQGVnHKo6QBkb4yFOJjC+sUGRc5Al3Odj1w=
Expand Down
5 changes: 5 additions & 0 deletions x/auth/keeper/bech32_codec.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ func NewBech32Codec(prefix string) address.Codec {
return bech32Codec{prefix}
}

// GetAddressPrefix returns the bech32 prefix
func (bc bech32Codec) GetAddressPrefix() string {
return bc.bech32Prefix
}

// StringToBytes encodes text to bytes
func (bc bech32Codec) StringToBytes(text string) ([]byte, error) {
if len(strings.TrimSpace(text)) == 0 {
Expand Down
28 changes: 8 additions & 20 deletions x/auth/keeper/grpc_query.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ func (ak AccountKeeper) Account(c context.Context, req *types.QueryAccountReques
}

ctx := sdk.UnwrapSDKContext(c)
addr, err := ak.addressCdc.StringToBytes(req.Address)
addr, err := ak.StringToBytes(req.Address)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -165,12 +165,12 @@ func (ak AccountKeeper) ModuleAccountByName(c context.Context, req *types.QueryM

// Bech32Prefix returns the keeper internally stored bech32 prefix.
func (ak AccountKeeper) Bech32Prefix(ctx context.Context, req *types.Bech32PrefixRequest) (*types.Bech32PrefixResponse, error) {
bech32Prefix, err := ak.getBech32Prefix()
if err != nil {
return nil, err
addressPrefix := ak.GetAddressPrefix()
if addressPrefix == "" {
return nil, status.Error(codes.NotFound, "bech32 prefix is not set")
}

return &types.Bech32PrefixResponse{Bech32Prefix: bech32Prefix}, nil
return &types.Bech32PrefixResponse{Bech32Prefix: addressPrefix}, nil
}

// AddressBytesToString converts an address from bytes to string, using the
Expand All @@ -184,7 +184,7 @@ func (ak AccountKeeper) AddressBytesToString(ctx context.Context, req *types.Add
return nil, errors.New("empty address bytes is not allowed")
}

text, err := ak.addressCdc.BytesToString(req.AddressBytes)
text, err := ak.BytesToString(req.AddressBytes)
if err != nil {
return nil, err
}
Expand All @@ -203,7 +203,7 @@ func (ak AccountKeeper) AddressStringToBytes(ctx context.Context, req *types.Add
return nil, errors.New("empty address string is not allowed")
}

bz, err := ak.addressCdc.StringToBytes(req.AddressString)
bz, err := ak.StringToBytes(req.AddressString)
if err != nil {
return nil, err
}
Expand All @@ -222,7 +222,7 @@ func (ak AccountKeeper) AccountInfo(goCtx context.Context, req *types.QueryAccou
}

ctx := sdk.UnwrapSDKContext(goCtx)
addr, err := ak.addressCdc.StringToBytes(req.Address)
addr, err := ak.StringToBytes(req.Address)
if err != nil {
return nil, err
}
Expand All @@ -246,15 +246,3 @@ func (ak AccountKeeper) AccountInfo(goCtx context.Context, req *types.QueryAccou
},
}, nil
}

// BytesToString converts an address from bytes to string, using the
// keeper's bech32 prefix.
func (ak AccountKeeper) BytesToString(address []byte) (string, error) {
return ak.addressCdc.BytesToString(address)
}

// StringToBytes converts an address from string to bytes, using the
// keeper's bech32 prefix.
func (ak AccountKeeper) StringToBytes(address string) ([]byte, error) {
return ak.addressCdc.StringToBytes(address)
}
24 changes: 7 additions & 17 deletions x/auth/keeper/keeper.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import (

// AccountKeeperI is the interface contract that x/auth's keeper implements.
type AccountKeeperI interface {
address.Codec

// Return a new account with the next account number and the specified address. Does not save the new account to the store.
NewAccountWithAddress(context.Context, sdk.AccAddress) sdk.AccountI

Expand Down Expand Up @@ -58,20 +60,20 @@ type AccountKeeperI interface {
// AccountKeeper encodes/decodes accounts using the go-amino (binary)
// encoding/decoding library.
type AccountKeeper struct {
address.Codec

storeService store.KVStoreService
cdc codec.BinaryCodec
permAddrs map[string]types.PermissionsForAddress

// The prototypical AccountI constructor.
proto func() sdk.AccountI
addressCdc address.Codec
proto func() sdk.AccountI

// the address capable of executing a MsgUpdateParams message. Typically, this
// should be the x/gov module account.
authority string

// State

ParamsState collections.Item[types.Params] // NOTE: name is this because it conflicts with the Params gRPC method impl
AccountNumber collections.Sequence
}
Expand All @@ -93,16 +95,14 @@ func NewAccountKeeper(
permAddrs[name] = types.NewPermissionsForAddress(name, perms)
}

bech32Codec := NewBech32Codec(bech32Prefix)

sb := collections.NewSchemaBuilder(storeService)

return AccountKeeper{
Codec: NewBech32Codec(bech32Prefix),
storeService: storeService,
proto: proto,
cdc: cdc,
permAddrs: permAddrs,
addressCdc: bech32Codec,
authority: authority,
ParamsState: collections.NewItem(sb, types.ParamsKey, "params", codec.CollValue[types.Params](cdc)),
AccountNumber: collections.NewSequence(sb, types.GlobalAccountNumberKey, "account_number"),
Expand All @@ -117,7 +117,7 @@ func (ak AccountKeeper) GetAuthority() string {
// GetAddressCodec returns the x/auth module's address.
// x/auth is tied to bech32 encoded user accounts
func (ak AccountKeeper) GetAddressCodec() address.Codec {
return ak.addressCdc
return ak.Codec
}

// Logger returns a module-specific logger.
Expand Down Expand Up @@ -254,16 +254,6 @@ func (ak AccountKeeper) UnmarshalAccount(bz []byte) (sdk.AccountI, error) {
// GetCodec return codec.Codec object used by the keeper
func (ak AccountKeeper) GetCodec() codec.BinaryCodec { return ak.cdc }

// add getter for bech32Prefix
func (ak AccountKeeper) getBech32Prefix() (string, error) {
bech32Codec, ok := ak.addressCdc.(bech32Codec)
if !ok {
return "", fmt.Errorf("unable cast addressCdc to bech32Codec; expected %T got %T", bech32Codec, ak.addressCdc)
}

return bech32Codec.bech32Prefix, nil
}

// SetParams sets the auth module's parameters.
// CONTRACT: This method performs no validation of the parameters.
func (ak AccountKeeper) SetParams(ctx context.Context, params types.Params) error {
Expand Down
14 changes: 14 additions & 0 deletions x/authz/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions x/distribution/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions x/feegrant/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions x/gov/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions x/group/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 14 additions & 0 deletions x/nft/testutil/expected_keepers_mocks.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading