From 5385116abac4e7b47d1af38cb25d0828cf37f1f9 Mon Sep 17 00:00:00 2001 From: Matt Kocubinski Date: Thu, 15 Jun 2023 12:35:48 -0500 Subject: [PATCH] refactor(x/auth): rm dependency on x/bank (#16548) --- x/auth/ante/sigverify_test.go | 4 +- x/auth/ante/testutil_test.go | 3 +- x/auth/migrations/v2/store.go | 21 ++++++---- x/auth/tx/config/config.go | 25 +---------- x/auth/tx/config/expected_keepers.go | 4 +- x/auth/tx/legacy_amino_json_test.go | 6 +-- x/auth/tx/testutil/expected_keepers_mocks.go | 16 +++---- x/bank/keeper/grpc_query.go | 44 ++++++++++++++++++++ 8 files changed, 76 insertions(+), 47 deletions(-) diff --git a/x/auth/ante/sigverify_test.go b/x/auth/ante/sigverify_test.go index cd803f3fac..ee2acb3093 100644 --- a/x/auth/ante/sigverify_test.go +++ b/x/auth/ante/sigverify_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" "github.com/golang/mock/gomock" "github.com/stretchr/testify/require" @@ -26,7 +27,6 @@ import ( authtx "github.com/cosmos/cosmos-sdk/x/auth/tx" txmodule "github.com/cosmos/cosmos-sdk/x/auth/tx/config" "github.com/cosmos/cosmos-sdk/x/auth/types" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) func TestSetPubKey(t *testing.T) { @@ -128,7 +128,7 @@ func TestConsumeSignatureVerificationGas(t *testing.T) { func TestSigVerification(t *testing.T) { suite := SetupTestSuite(t, true) - suite.txBankKeeper.EXPECT().DenomMetadata(gomock.Any(), gomock.Any()).Return(&banktypes.QueryDenomMetadataResponse{}, nil).AnyTimes() + suite.txBankKeeper.EXPECT().DenomMetadataV2(gomock.Any(), gomock.Any()).Return(&bankv1beta1.QueryDenomMetadataResponse{}, nil).AnyTimes() enabledSignModes := []signing.SignMode{signing.SignMode_SIGN_MODE_DIRECT, signing.SignMode_SIGN_MODE_TEXTUAL, signing.SignMode_SIGN_MODE_LEGACY_AMINO_JSON} // Since TEXTUAL is not enabled by default, we create a custom TxConfig diff --git a/x/auth/ante/testutil_test.go b/x/auth/ante/testutil_test.go index c22c4b7bd7..2561634113 100644 --- a/x/auth/ante/testutil_test.go +++ b/x/auth/ante/testutil_test.go @@ -34,7 +34,6 @@ import ( authtestutil "github.com/cosmos/cosmos-sdk/x/auth/testutil" txtestutil "github.com/cosmos/cosmos-sdk/x/auth/tx/testutil" "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/cosmos/cosmos-sdk/x/bank" ) // TestAccount represents an account used in the tests in x/auth/ante. @@ -67,7 +66,7 @@ func SetupTestSuite(t *testing.T, isCheckTx bool) *AnteTestSuite { key := storetypes.NewKVStoreKey(types.StoreKey) testCtx := testutil.DefaultContextWithDB(t, key, storetypes.NewTransientStoreKey("transient_test")) suite.ctx = testCtx.Ctx.WithIsCheckTx(isCheckTx).WithBlockHeight(1) // app.BaseApp.NewContext(isCheckTx, cmtproto.Header{}).WithBlockHeight(1) - suite.encCfg = moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}, bank.AppModuleBasic{}) + suite.encCfg = moduletestutil.MakeTestEncodingConfig(auth.AppModuleBasic{}) maccPerms := map[string][]string{ "fee_collector": nil, diff --git a/x/auth/migrations/v2/store.go b/x/auth/migrations/v2/store.go index ea40cd0932..a408f5ccad 100644 --- a/x/auth/migrations/v2/store.go +++ b/x/auth/migrations/v2/store.go @@ -22,10 +22,10 @@ import ( "fmt" "strconv" + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" stakingv1beta1 "cosmossdk.io/api/cosmos/staking/v1beta1" abci "github.com/cometbft/cometbft/abci/types" "github.com/cosmos/gogoproto/grpc" - gogoproto "github.com/cosmos/gogoproto/proto" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" @@ -35,7 +35,6 @@ import ( sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" "github.com/cosmos/cosmos-sdk/x/auth/vesting/exported" vestingtypes "github.com/cosmos/cosmos-sdk/x/auth/vesting/types" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" ) const ( @@ -236,11 +235,11 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.C queryFn := querier.Route(balancesPath) - q := &banktypes.QueryAllBalancesRequest{ + q := &bankv1beta1.QueryAllBalancesRequest{ Address: address, Pagination: nil, } - b, err := gogoproto.Marshal(q) + b, err := proto.Marshal(q) if err != nil { return nil, fmt.Errorf("cannot marshal bank type query request, %w", err) } @@ -253,11 +252,19 @@ func getBalance(ctx sdk.Context, address string, queryServer grpc.Server) (sdk.C if err != nil { return nil, fmt.Errorf("bank query error, %w", err) } - balance := new(banktypes.QueryAllBalancesResponse) - if err := gogoproto.Unmarshal(resp.Value, balance); err != nil { + balance := new(bankv1beta1.QueryAllBalancesResponse) + if err := proto.Unmarshal(resp.Value, balance); err != nil { return nil, fmt.Errorf("unable to unmarshal bank balance response: %w", err) } - return balance.Balances, nil + coins := make(sdk.Coins, len(balance.Balances)) + for i, b := range balance.Balances { + amount, err := strconv.Atoi(b.Amount) + if err != nil { + return nil, fmt.Errorf("cannot convert balance amount to int, %w", err) + } + coins[i] = sdk.NewCoin(b.Denom, sdk.NewInt(int64(amount))) + } + return coins, nil } // We use the baseapp.QueryRouter here to do inter-module state querying. diff --git a/x/auth/tx/config/config.go b/x/auth/tx/config/config.go index b249dbf943..27cf26e08e 100644 --- a/x/auth/tx/config/config.go +++ b/x/auth/tx/config/config.go @@ -30,7 +30,6 @@ import ( "github.com/cosmos/cosmos-sdk/x/auth/posthandler" "github.com/cosmos/cosmos-sdk/x/auth/tx" authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" - "github.com/cosmos/cosmos-sdk/x/bank/types" ) func init() { @@ -167,32 +166,12 @@ func newAnteHandler(txConfig client.TxConfig, in ModuleInputs) (sdk.AnteHandler, // `NewTextualWithGRPCConn`. func NewBankKeeperCoinMetadataQueryFn(bk BankKeeper) textual.CoinMetadataQueryFn { return func(ctx context.Context, denom string) (*bankv1beta1.Metadata, error) { - res, err := bk.DenomMetadata(ctx, &types.QueryDenomMetadataRequest{Denom: denom}) + res, err := bk.DenomMetadataV2(ctx, &bankv1beta1.QueryDenomMetadataRequest{Denom: denom}) if err != nil { return nil, metadataExists(err) } - m := &bankv1beta1.Metadata{ - Base: res.Metadata.Base, - Display: res.Metadata.Display, - // fields below are not strictly needed by Textual - // but added here for completeness. - Description: res.Metadata.Description, - Name: res.Metadata.Name, - Symbol: res.Metadata.Symbol, - Uri: res.Metadata.URI, - UriHash: res.Metadata.URIHash, - } - m.DenomUnits = make([]*bankv1beta1.DenomUnit, len(res.Metadata.DenomUnits)) - for i, d := range res.Metadata.DenomUnits { - m.DenomUnits[i] = &bankv1beta1.DenomUnit{ - Denom: d.Denom, - Exponent: d.Exponent, - Aliases: d.Aliases, - } - } - - return m, nil + return res.Metadata, nil } } diff --git a/x/auth/tx/config/expected_keepers.go b/x/auth/tx/config/expected_keepers.go index 581e71ac19..7a8f5f8f0a 100644 --- a/x/auth/tx/config/expected_keepers.go +++ b/x/auth/tx/config/expected_keepers.go @@ -3,10 +3,10 @@ package tx import ( "context" - "github.com/cosmos/cosmos-sdk/x/bank/types" + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" ) // BankKeeper defines the contract needed for tx-related APIs type BankKeeper interface { - DenomMetadata(c context.Context, req *types.QueryDenomMetadataRequest) (*types.QueryDenomMetadataResponse, error) + DenomMetadataV2(c context.Context, req *bankv1beta1.QueryDenomMetadataRequest) (*bankv1beta1.QueryDenomMetadataResponse, error) } diff --git a/x/auth/tx/legacy_amino_json_test.go b/x/auth/tx/legacy_amino_json_test.go index cdb7ee284d..6d1de39103 100644 --- a/x/auth/tx/legacy_amino_json_test.go +++ b/x/auth/tx/legacy_amino_json_test.go @@ -17,7 +17,7 @@ import ( signingtypes "github.com/cosmos/cosmos-sdk/types/tx/signing" "github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx" "github.com/cosmos/cosmos-sdk/x/auth/signing" - banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + "github.com/cosmos/cosmos-sdk/x/auth/types" ) var ( @@ -26,7 +26,7 @@ var ( coins = sdk.Coins{sdk.NewInt64Coin("foocoin", 10)} gas = uint64(10000) - msg = banktypes.NewMsgSend(addr1, addr2, coins) + msg = &types.MsgUpdateParams{Authority: addr1.String()} memo = "foo" timeout = uint64(10) ) @@ -156,8 +156,8 @@ func TestLegacyAminoJSONHandler_AllGetSignBytesComparison(t *testing.T) { modeHandler := aminojson.NewSignModeHandler(aminojson.SignModeHandlerOptions{}) mode, _ := signing.APISignModeToInternal(modeHandler.Mode()) legacyAmino := codec.NewLegacyAmino() - legacy.RegisterAminoMsg(legacyAmino, &banktypes.MsgSend{}, "cosmos-sdk/MsgSend") legacytx.RegressionTestingAminoCodec = legacyAmino + legacy.RegisterAminoMsg(legacyAmino, &types.MsgUpdateParams{}, "cosmos-sdk/x/auth/MsgUpdateParams") testcases := []struct { name string diff --git a/x/auth/tx/testutil/expected_keepers_mocks.go b/x/auth/tx/testutil/expected_keepers_mocks.go index fada0426cf..4a9256aebe 100644 --- a/x/auth/tx/testutil/expected_keepers_mocks.go +++ b/x/auth/tx/testutil/expected_keepers_mocks.go @@ -8,7 +8,7 @@ import ( context "context" reflect "reflect" - types "github.com/cosmos/cosmos-sdk/x/bank/types" + bankv1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" gomock "github.com/golang/mock/gomock" ) @@ -35,17 +35,17 @@ func (m *MockBankKeeper) EXPECT() *MockBankKeeperMockRecorder { return m.recorder } -// DenomMetadata mocks base method. -func (m *MockBankKeeper) DenomMetadata(c context.Context, req *types.QueryDenomMetadataRequest) (*types.QueryDenomMetadataResponse, error) { +// DenomMetadataV2 mocks base method. +func (m *MockBankKeeper) DenomMetadataV2(c context.Context, req *bankv1beta1.QueryDenomMetadataRequest) (*bankv1beta1.QueryDenomMetadataResponse, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "DenomMetadata", c, req) - ret0, _ := ret[0].(*types.QueryDenomMetadataResponse) + ret := m.ctrl.Call(m, "DenomMetadataV2", c, req) + ret0, _ := ret[0].(*bankv1beta1.QueryDenomMetadataResponse) ret1, _ := ret[1].(error) return ret0, ret1 } -// DenomMetadata indicates an expected call of DenomMetadata. -func (mr *MockBankKeeperMockRecorder) DenomMetadata(c, req interface{}) *gomock.Call { +// DenomMetadataV2 indicates an expected call of DenomMetadataV2. +func (mr *MockBankKeeperMockRecorder) DenomMetadataV2(c, req interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DenomMetadata", reflect.TypeOf((*MockBankKeeper)(nil).DenomMetadata), c, req) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DenomMetadataV2", reflect.TypeOf((*MockBankKeeper)(nil).DenomMetadataV2), c, req) } diff --git a/x/bank/keeper/grpc_query.go b/x/bank/keeper/grpc_query.go index ae2e678276..982995b975 100644 --- a/x/bank/keeper/grpc_query.go +++ b/x/bank/keeper/grpc_query.go @@ -3,6 +3,7 @@ package keeper import ( "context" + v1beta1 "cosmossdk.io/api/cosmos/bank/v1beta1" "cosmossdk.io/collections" "cosmossdk.io/math" "google.golang.org/grpc/codes" @@ -224,6 +225,49 @@ func (k BaseKeeper) DenomMetadata(c context.Context, req *types.QueryDenomMetada }, nil } +// DenomMetadataV2 is identical to DenomMetadata but receives protoreflect types instead of gogo types. It exists to +// resolve a cyclic dependency existent between x/auth and x/bank, so that x/auth may call this keeper without +// depending on x/bank. +func (k BaseKeeper) DenomMetadataV2(c context.Context, req *v1beta1.QueryDenomMetadataRequest) (*v1beta1.QueryDenomMetadataResponse, error) { + if req == nil { + return nil, status.Errorf(codes.InvalidArgument, "empty request") + } + + if err := sdk.ValidateDenom(req.Denom); err != nil { + return nil, status.Error(codes.InvalidArgument, err.Error()) + } + + ctx := sdk.UnwrapSDKContext(c) + + metadata, found := k.GetDenomMetaData(ctx, req.Denom) + if !found { + return nil, status.Errorf(codes.NotFound, "client metadata for denom %s", req.Denom) + } + + denomUnits := make([]*v1beta1.DenomUnit, len(metadata.DenomUnits)) + for i, unit := range metadata.DenomUnits { + denomUnits[i] = &v1beta1.DenomUnit{ + Denom: unit.Denom, + Exponent: unit.Exponent, + Aliases: unit.Aliases, + } + } + metadataV2 := &v1beta1.Metadata{ + Description: metadata.Description, + DenomUnits: denomUnits, + Base: metadata.Base, + Display: metadata.Display, + Name: metadata.Name, + Symbol: metadata.Symbol, + Uri: metadata.URI, + UriHash: metadata.URIHash, + } + + return &v1beta1.QueryDenomMetadataResponse{ + Metadata: metadataV2, + }, nil +} + func (k BaseKeeper) DenomOwners( goCtx context.Context, req *types.QueryDenomOwnersRequest,