Skip to content

Commit

Permalink
refactor(auth): remove bech32 global from auth (#15826)
Browse files Browse the repository at this point in the history
## Description

ref #13140 

on top of removing bech32 from auth this also removes the get address cmd from root command as it was duplicated

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

* [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
* [ ] added `!` to the type prefix if API or client breaking change
* [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#pr-targeting))
* [ ] provided a link to the relevant issue or specification
* [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules)
* [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/main/CONTRIBUTING.md#testing)
* [ ] added a changelog entry to `CHANGELOG.md`
* [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
* [ ] updated the relevant documentation or specification
* [ ] reviewed "Files changed" and left comments if necessary
* [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

* [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
* [ ] confirmed `!` in the type prefix if API or client breaking change
* [ ] confirmed all author checklist items have been addressed 
* [ ] reviewed state machine logic
* [ ] reviewed API design and naming
* [ ] reviewed documentation is accurate
* [ ] reviewed tests and test coverage
* [ ] manually tested (if applicable)
  • Loading branch information
tac0turtle authored Apr 14, 2023
1 parent 875e29e commit 722bea5
Show file tree
Hide file tree
Showing 9 changed files with 34 additions and 23 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (crypto) [#15070](https://github.com/cosmos/cosmos-sdk/pull/15070) `GenerateFromPassword` and `Cost` from `bcrypt.go` now take a `uint32` instead of a `int` type.
* (x/capability) [#15344](https://github.com/cosmos/cosmos-sdk/pull/15344) Capability module was removed and is now housed in [IBC-GO](https://github.com/cosmos/ibc-go).
* [#15299](https://github.com/cosmos/cosmos-sdk/pull/15299) remove `StdTx` transaction and signing APIs. No SDK version has actually supported `StdTx` since before Stargate.
* [#15600](https://github.com/cosmos/cosmos-sdk/pull/15600) add support for getting signers to `codec.Codec` and protoregistry support to `InterfaceRegistry`:
* (codec) [#15600](https://github.com/cosmos/cosmos-sdk/pull/15600) add support for getting signers to `codec.Codec` and protoregistry support to `InterfaceRegistry`:
* `Codec` is now a private interface and has the methods `InterfaceRegistry`, `GetMsgAnySigners`, `GetMsgV1Signers`, and `GetMsgV2Signers` which will fail when using `AminoCodec`.
All implementations of `Codec` by other users must now embed an official implementation from the `codec` package.
* `InterfaceRegistry` is now a private interface and implements `protodesc.Resolver` plus the `RangeFiles` method
Expand All @@ -167,6 +167,7 @@ Ref: https://keepachangelog.com/en/1.0.0/

### CLI Breaking Changes

* (cli) [#15826](https://github.com/cosmos/cosmos-sdk/pull/15826) Remove `<appd> q account` command. Use `<appd> q auth account` instead.
* (x/staking) [#14864](https://github.com/cosmos/cosmos-sdk/pull/14864) `create-validator` CLI command now takes a json file as an arg instead of having a bunch of required flags to it.
* (cli) [#14659](https://github.com/cosmos/cosmos-sdk/pull/14659) `<app> q block <height>` is removed as it just output json. The new command allows either height/hash and is `<app> q block --type=height|hash <height|hash>`.
* (x/gov) [#14880](https://github.com/cosmos/cosmos-sdk/pull/14880) Remove `<app> tx gov submit-legacy-proposal cancel-software-upgrade` and `software-upgrade` commands. These commands are now in the `x/upgrade` module and using gov v1. Use `tx upgrade software-upgrade` instead.
Expand Down
1 change: 0 additions & 1 deletion simapp/simd/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,6 @@ func queryCommand() *cobra.Command {
}

cmd.AddCommand(
authcmd.GetAccountCmd(),
rpc.ValidatorCommand(),
server.QueryBlockCmd(),
authcmd.QueryTxsByEventsCmd(),
Expand Down
5 changes: 3 additions & 2 deletions tests/e2e/auth/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
Expand Down Expand Up @@ -1251,7 +1252,7 @@ func (s *E2ETestSuite) TestMultisignBatch() {
defer filename.Close()
val.ClientCtx.HomeDir = strings.Replace(val.ClientCtx.HomeDir, "simd", "simcli", 1)

queryResJSON, err := authclitestutil.QueryAccountExec(val.ClientCtx, addr)
queryResJSON, err := authclitestutil.QueryAccountExec(val.ClientCtx, addr, address.NewBech32Codec("cosmos"))
s.Require().NoError(err)
var account sdk.AccountI
s.Require().NoError(val.ClientCtx.Codec.UnmarshalInterfaceJSON(queryResJSON.Bytes(), &account))
Expand Down Expand Up @@ -1319,7 +1320,7 @@ func (s *E2ETestSuite) TestGetAccountCmd() {
s.Run(tc.name, func() {
clientCtx := val.ClientCtx

out, err := authclitestutil.QueryAccountExec(clientCtx, tc.address)
out, err := authclitestutil.QueryAccountExec(clientCtx, tc.address, address.NewBech32Codec("cosmos"))
if tc.expectErr {
s.Require().Error(err)
s.Require().NotEqual("internal", err.Error())
Expand Down
11 changes: 6 additions & 5 deletions x/auth/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strconv"
"strings"

"cosmossdk.io/core/address"
errorsmod "cosmossdk.io/errors"
"github.com/spf13/cobra"

Expand Down Expand Up @@ -34,7 +35,7 @@ const (
)

// GetQueryCmd returns the transaction commands for this module
func GetQueryCmd() *cobra.Command {
func GetQueryCmd(ac address.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: types.ModuleName,
Short: "Querying commands for the auth module",
Expand All @@ -44,7 +45,7 @@ func GetQueryCmd() *cobra.Command {
}

cmd.AddCommand(
GetAccountCmd(),
GetAccountCmd(ac),
GetAccountAddressByIDCmd(),
GetAccountsCmd(),
QueryParamsCmd(),
Expand Down Expand Up @@ -88,7 +89,7 @@ $ <appd> query auth params

// GetAccountCmd returns a query account that will display the state of the
// account at a given address.
func GetAccountCmd() *cobra.Command {
func GetAccountCmd(ac address.Codec) *cobra.Command {
cmd := &cobra.Command{
Use: "account [address]",
Short: "Query for account by address",
Expand All @@ -98,13 +99,13 @@ func GetAccountCmd() *cobra.Command {
if err != nil {
return err
}
key, err := sdk.AccAddressFromBech32(args[0])
_, err = ac.StringToBytes(args[0])
if err != nil {
return err
}

queryClient := types.NewQueryClient(clientCtx)
res, err := queryClient.Account(cmd.Context(), &types.QueryAccountRequest{Address: key.String()})
res, err := queryClient.Account(cmd.Context(), &types.QueryAccountRequest{Address: args[0]})
if err != nil {
node, err2 := clientCtx.GetNode()
if err2 != nil {
Expand Down
14 changes: 10 additions & 4 deletions x/auth/client/cli/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,15 @@ import (
"strings"
"testing"

"cosmossdk.io/core/address"
"cosmossdk.io/math"
abci "github.com/cometbft/cometbft/abci/types"
rpcclientmock "github.com/cometbft/cometbft/rpc/client/mock"
"github.com/stretchr/testify/suite"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
addresscodec "github.com/cosmos/cosmos-sdk/codec/address"
"github.com/cosmos/cosmos-sdk/crypto/hd"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
kmultisig "github.com/cosmos/cosmos-sdk/crypto/keys/multisig"
Expand Down Expand Up @@ -46,6 +48,8 @@ type CLITestSuite struct {
clientCtx client.Context
val sdk.AccAddress
val1 sdk.AccAddress

ac address.Codec
}

func TestCLITestSuite(t *testing.T) {
Expand Down Expand Up @@ -99,6 +103,8 @@ func (s *CLITestSuite) SetupSuite() {
multi := kmultisig.NewLegacyAminoPubKey(2, []cryptotypes.PubKey{pub1, pub2})
_, err = kb.SaveMultisig("multi", multi)
s.Require().NoError(err)

s.ac = addresscodec.NewBech32Codec("cosmos")
}

func (s *CLITestSuite) TestCLIValidateSignatures() {
Expand Down Expand Up @@ -606,7 +612,7 @@ func (s *CLITestSuite) TestSignWithMultisig() {

// Create an address that is not in the keyring, will be used to simulate `--multisig`
multisig := "cosmos1hd6fsrvnz6qkp87s3u86ludegq97agxsdkwzyh"
multisigAddr, err := sdk.AccAddressFromBech32(multisig)
_, err = s.ac.StringToBytes(multisig)
s.Require().NoError(err)

// Generate a transaction for testing --multisig with an address not in the keyring.
Expand All @@ -632,7 +638,7 @@ func (s *CLITestSuite) TestSignWithMultisig() {
// even though the tx signer is NOT the multisig address. This is fine though,
// as the main point of this test is to test the `--multisig` flag with an address
// that is not in the keyring.
_, err = authtestutil.TxSignExec(s.clientCtx, addr1, multiGeneratedTx2File.Name(), "--multisig", multisigAddr.String())
_, err = authtestutil.TxSignExec(s.clientCtx, addr1, multiGeneratedTx2File.Name(), "--multisig", multisig)
s.Require().Contains(err.Error(), "error getting account from keybase")
}

Expand Down Expand Up @@ -788,9 +794,9 @@ func (s *CLITestSuite) TestGetBroadcastCommandWithoutOfflineFlag() {
// Create new file with tx
builder := txCfg.NewTxBuilder()
builder.SetGasLimit(200000)
from, err := sdk.AccAddressFromBech32("cosmos1cxlt8kznps92fwu3j6npahx4mjfutydyene2qw")
from, err := s.ac.StringToBytes("cosmos1cxlt8kznps92fwu3j6npahx4mjfutydyene2qw")
s.Require().NoError(err)
to, err := sdk.AccAddressFromBech32("cosmos1cxlt8kznps92fwu3j6npahx4mjfutydyene2qw")
to, err := s.ac.StringToBytes("cosmos1cxlt8kznps92fwu3j6npahx4mjfutydyene2qw")
s.Require().NoError(err)
err = builder.SetMsgs(banktypes.NewMsgSend(from, to, sdk.Coins{sdk.NewInt64Coin("stake", 10000)}))
s.Require().NoError(err)
Expand Down
5 changes: 3 additions & 2 deletions x/auth/client/testutil/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"strings"

"cosmossdk.io/core/address"
"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
Expand Down Expand Up @@ -89,10 +90,10 @@ func TxAuxToFeeExec(clientCtx client.Context, filename string, extraArgs ...stri
return clitestutil.ExecTestCLICmd(clientCtx, cli.GetAuxToFeeCommand(), append(args, extraArgs...))
}

func QueryAccountExec(clientCtx client.Context, address fmt.Stringer, extraArgs ...string) (testutil.BufferWriter, error) {
func QueryAccountExec(clientCtx client.Context, address fmt.Stringer, ac address.Codec, extraArgs ...string) (testutil.BufferWriter, error) {
args := []string{address.String(), fmt.Sprintf("--%s=json", flags.FlagOutput)}

return clitestutil.ExecTestCLICmd(clientCtx, cli.GetAccountCmd(), append(args, extraArgs...))
return clitestutil.ExecTestCLICmd(clientCtx, cli.GetAccountCmd(ac), append(args, extraArgs...))
}

func TxMultiSignBatchExec(clientCtx client.Context, filename, from, sigFile1, sigFile2 string, extraArgs ...string) (testutil.BufferWriter, error) {
Expand Down
2 changes: 1 addition & 1 deletion x/auth/keeper/deterministic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ func (suite *DeterministicTestSuite) TestGRPCQueryAccounts() {
})

// Regression test
addr1, err := sdk.AccAddressFromBech32("cosmos1892yr6fzlj7ud0kfkah2ctrav3a4p4n060ze8f")
addr1, err := suite.accountKeeper.GetAddressCodec().StringToBytes("cosmos1892yr6fzlj7ud0kfkah2ctrav3a4p4n060ze8f")
suite.Require().NoError(err)
pub1, err := hex.DecodeString("D1002E1B019000010BB7034500E71F011F1CA90D5B000E134BFB0F3603030D0303")
suite.Require().NoError(err)
Expand Down
6 changes: 3 additions & 3 deletions x/auth/keeper/grpc_query.go
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 := sdk.AccAddressFromBech32(req.Address)
addr, err := ak.addressCdc.StringToBytes(req.Address)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -222,7 +222,7 @@ func (ak AccountKeeper) AccountInfo(goCtx context.Context, req *types.QueryAccou
}

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

return &types.QueryAccountInfoResponse{
Info: &types.BaseAccount{
Address: addr.String(),
Address: req.Address,
PubKey: pkAny,
AccountNumber: account.GetAccountNumber(),
Sequence: account.GetSequence(),
Expand Down
10 changes: 6 additions & 4 deletions x/auth/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ var (
)

// AppModuleBasic defines the basic application module used by the auth module.
type AppModuleBasic struct{}
type AppModuleBasic struct {
ac address.Codec
}

// Name returns the auth module's name.
func (AppModuleBasic) Name() string {
Expand Down Expand Up @@ -83,8 +85,8 @@ func (AppModuleBasic) GetTxCmd() *cobra.Command {
}

// GetQueryCmd returns the root query command for the auth module.
func (AppModuleBasic) GetQueryCmd() *cobra.Command {
return cli.GetQueryCmd()
func (ab AppModuleBasic) GetQueryCmd() *cobra.Command {
return cli.GetQueryCmd(ab.ac)
}

// RegisterInterfaces registers interfaces and implementations of the auth module.
Expand Down Expand Up @@ -114,7 +116,7 @@ func (am AppModule) IsAppModule() {}
// NewAppModule creates a new AppModule object
func NewAppModule(cdc codec.Codec, accountKeeper keeper.AccountKeeper, randGenAccountsFn types.RandomGenesisAccountsFn, ss exported.Subspace) AppModule {
return AppModule{
AppModuleBasic: AppModuleBasic{},
AppModuleBasic: AppModuleBasic{ac: accountKeeper.GetAddressCodec()},
accountKeeper: accountKeeper,
randGenAccountsFn: randGenAccountsFn,
legacySubspace: ss,
Expand Down

0 comments on commit 722bea5

Please sign in to comment.