From c3bc5c82eb43f4cca91144c375f104b03b333f5a Mon Sep 17 00:00:00 2001 From: Julien Robert Date: Thu, 12 Oct 2023 12:13:04 +0200 Subject: [PATCH] refactor(x/authz): add exec as autocli cmd (#18039) --- x/authz/client/cli/tx.go | 88 ++++---- x/authz/client/cli/tx_test.go | 316 ----------------------------- x/authz/client/testutil/helpers.go | 3 +- x/authz/module/autocli.go | 21 +- x/authz/module/module.go | 4 +- 5 files changed, 59 insertions(+), 373 deletions(-) diff --git a/x/authz/client/cli/tx.go b/x/authz/client/cli/tx.go index 0ca6c50452fa..3e4f795a8f50 100644 --- a/x/authz/client/cli/tx.go +++ b/x/authz/client/cli/tx.go @@ -34,7 +34,7 @@ const ( // GetTxCmd returns the transaction commands for this module func GetTxCmd() *cobra.Command { - AuthorizationTxCmd := &cobra.Command{ + authorizationTxCmd := &cobra.Command{ Use: authz.ModuleName, Short: "Authorization transactions subcommands", Long: "Authorize and revoke access to execute transactions on behalf of your address", @@ -43,12 +43,48 @@ func GetTxCmd() *cobra.Command { RunE: client.ValidateCmd, } - AuthorizationTxCmd.AddCommand( + authorizationTxCmd.AddCommand( NewCmdGrantAuthorization(), NewCmdExecAuthorization(), ) - return AuthorizationTxCmd + return authorizationTxCmd +} + +// NewCmdExecAuthorization returns a CLI command handler for creating a MsgExec transaction. +// Deprecated: This command is deprecated in favor for the AutoCLI exec command. +// It stays here for backward compatibility, as the AutoCLI command has a small breaking change, +// but it will be removed in future versions. +func NewCmdExecAuthorization() *cobra.Command { + cmd := &cobra.Command{ + Use: "legacy-exec [tx-json-file] --from [grantee]", + Short: "Execute tx on behalf of granter account. Deprecated, use exec instead.", + Example: fmt.Sprintf("$ %s tx authz exec tx.json --from grantee\n $ %[1]s tx bank send [granter] [recipient] [amount] --generate-only tx.json && %[1]s tx authz exec tx.json --from grantee", version.AppName), + Args: cobra.ExactArgs(1), + RunE: func(cmd *cobra.Command, args []string) error { + clientCtx, err := client.GetClientTxContext(cmd) + if err != nil { + return err + } + grantee := clientCtx.GetFromAddress() + + if offline, _ := cmd.Flags().GetBool(flags.FlagOffline); offline { + return errors.New("cannot broadcast tx during offline mode") + } + + theTx, err := authclient.ReadTxFromFile(clientCtx, args[0]) + if err != nil { + return err + } + msg := authz.NewMsgExec(grantee, theTx.GetMsgs()) + + return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), &msg) + }, + } + + flags.AddTxFlagsToCmd(cmd) + + return cmd } // NewCmdGrantAuthorization returns a CLI command handler for creating a MsgGrant transaction. @@ -57,12 +93,11 @@ func NewCmdGrantAuthorization() *cobra.Command { cmd := &cobra.Command{ Use: "grant [grantee] --from [granter]", Short: "Grant authorization to an address", - Long: strings.TrimSpace( - fmt.Sprintf(`create a new grant authorization to an address to execute a transaction on your behalf: + Long: fmt.Sprintf(`create a new grant authorization to an address to execute a transaction on your behalf: Examples: $ %[1]s tx authz grant cosmos1skjw.. send --spend-limit=1000stake --from=cosmos1skl.. $ %[1]s tx authz grant cosmos1skjw.. generic --msg-type=/cosmos.gov.v1.MsgVote --from=cosmos1sk.. - `, version.AppName)), + `, version.AppName), Args: cobra.ExactArgs(2), RunE: func(cmd *cobra.Command, args []string) error { clientCtx, err := client.GetClientTxContext(cmd) @@ -224,47 +259,6 @@ func getExpireTime(cmd *cobra.Command) (*time.Time, error) { return &e, nil } -// NewCmdExecAuthorization returns a CLI command handler for creating a MsgExec transaction. -// -// cannot give autocli support, can be CLI breaking -func NewCmdExecAuthorization() *cobra.Command { - cmd := &cobra.Command{ - Use: "exec [tx-json-file] --from [grantee]", - Short: "execute tx on behalf of granter account", - Long: strings.TrimSpace( - fmt.Sprintf(`execute tx on behalf of granter account: -Example: - $ %s tx %s exec tx.json --from grantee - $ %s tx bank send --from --chain-id --generate-only > tx.json && %s tx %s exec tx.json --from grantee - `, version.AppName, authz.ModuleName, version.AppName, version.AppName, authz.ModuleName), - ), - Args: cobra.ExactArgs(1), - RunE: func(cmd *cobra.Command, args []string) error { - clientCtx, err := client.GetClientTxContext(cmd) - if err != nil { - return err - } - grantee := clientCtx.GetFromAddress() - - if offline, _ := cmd.Flags().GetBool(flags.FlagOffline); offline { - return errors.New("cannot broadcast tx during offline mode") - } - - theTx, err := authclient.ReadTxFromFile(clientCtx, args[0]) - if err != nil { - return err - } - msg := authz.NewMsgExec(grantee, theTx.GetMsgs()) - - return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), &msg) - }, - } - - flags.AddTxFlagsToCmd(cmd) - - return cmd -} - // bech32toValAddresses returns []ValAddress from a list of Bech32 string addresses. func bech32toValAddresses(clientCtx client.Context, validators []string) ([]sdk.ValAddress, error) { vals := make([]sdk.ValAddress, len(validators)) diff --git a/x/authz/client/cli/tx_test.go b/x/authz/client/cli/tx_test.go index 24ecefc4e873..2768d557c612 100644 --- a/x/authz/client/cli/tx_test.go +++ b/x/authz/client/cli/tx_test.go @@ -8,7 +8,6 @@ import ( abci "github.com/cometbft/cometbft/abci/types" rpcclientmock "github.com/cometbft/cometbft/rpc/client/mock" - "github.com/cosmos/gogoproto/proto" "github.com/stretchr/testify/suite" _ "cosmossdk.io/api/cosmos/authz/v1beta1" @@ -470,318 +469,3 @@ func (s *CLITestSuite) TestCLITxGrantAuthorization() { }) } } - -func (s *CLITestSuite) TestExecAuthorizationWithExpiration() { - val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1) - - grantee := s.grantee[0] - tenSeconds := time.Now().Add(time.Second * time.Duration(10)).Unix() - - _, err := authzclitestutil.CreateGrant(s.clientCtx, - []string{ - grantee.String(), - "generic", - fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%d", cli.FlagExpiration, tenSeconds), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - }, - ) - s.Require().NoError(err) - // msg vote - voteTx := fmt.Sprintf(`{"body":{"messages":[{"@type":"/cosmos.gov.v1.MsgVote","proposal_id":"1","voter":"%s","option":"VOTE_OPTION_YES"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""}},"signatures":[]}`, val[0].Address.String()) - execMsg := testutil.WriteToNewTempFile(s.T(), voteTx) - defer execMsg.Close() - - // waiting for authorization to expire - time.Sleep(12 * time.Second) - - cmd := cli.NewCmdExecAuthorization() - - out, err := clitestutil.ExecTestCLICmd(s.clientCtx, cmd, []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - }) - s.Require().NoError(err) - var response sdk.TxResponse - s.Require().NoError(s.clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String()) -} - -func (s *CLITestSuite) TestNewExecGenericAuthorized() { - val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1) - grantee := s.grantee[0] - twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix() - - _, err := authzclitestutil.CreateGrant(s.clientCtx, - []string{ - grantee.String(), - "generic", - fmt.Sprintf("--%s=%s", cli.FlagMsgType, typeMsgVote), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - }, - ) - s.Require().NoError(err) - - // msg vote - voteTx := fmt.Sprintf(`{"body":{"messages":[{"@type":"/cosmos.gov.v1.MsgVote","proposal_id":"1","voter":"%s","option":"VOTE_OPTION_YES"}],"memo":"","timeout_height":"0","extension_options":[],"non_critical_extension_options":[]},"auth_info":{"signer_infos":[],"fee":{"amount":[],"gas_limit":"200000","payer":"","granter":""}},"signatures":[]}`, val[0].Address.String()) - execMsg := testutil.WriteToNewTempFile(s.T(), voteTx) - defer execMsg.Close() - - testCases := []struct { - name string - args []string - respType proto.Message - expectErr bool - }{ - { - "fail invalid grantee", - []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, "grantee"), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=true", flags.FlagGenerateOnly), - }, - nil, - true, - }, - { - "fail invalid json path", - []string{ - "/invalid/file.txt", - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - }, - nil, - true, - }, - { - "valid txn", - []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - }, - &sdk.TxResponse{}, - false, - }, - { - "valid tx with amino", - []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagSignMode, flags.SignModeLegacyAminoJSON), - }, - &sdk.TxResponse{}, - false, - }, - } - - for _, tc := range testCases { - tc := tc - s.Run(tc.name, func() { - cmd := cli.NewCmdExecAuthorization() - - out, err := clitestutil.ExecTestCLICmd(s.clientCtx, cmd, tc.args) - if tc.expectErr { - s.Require().Error(err) - } else { - s.Require().NoError(err) - s.Require().NoError(s.clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) - } - }) - } -} - -func (s *CLITestSuite) TestNewExecGrantAuthorized() { - val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1) - grantee := s.grantee[0] - twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix() - - _, err := authzclitestutil.CreateGrant(s.clientCtx, - []string{ - grantee.String(), - "send", - fmt.Sprintf("--%s=12testtoken", cli.FlagSpendLimit), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - }, - ) - s.Require().NoError(err) - - from := val[0].Address - tokens := sdk.NewCoins( - sdk.NewCoin("testtoken", sdkmath.NewInt(12)), - ) - msgSend := &banktypes.MsgSend{ - FromAddress: from.String(), - ToAddress: grantee.String(), - Amount: tokens, - } - normalGeneratedTx, err := clitestutil.SubmitTestTx( - s.clientCtx, - msgSend, - from, - clitestutil.TestTxConfig{GenOnly: true}, - ) - s.Require().NoError(err) - execMsg := testutil.WriteToNewTempFile(s.T(), normalGeneratedTx.String()) - defer execMsg.Close() - - testCases := []struct { - name string - args []string - expectErr bool - expectErrMsg string - }{ - { - "valid txn", - []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - }, - false, - "", - }, - { - "error over spent", - []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - }, - false, - "", - }, - } - - for _, tc := range testCases { - tc := tc - s.Run(tc.name, func() { - cmd := cli.NewCmdExecAuthorization() - clientCtx := s.clientCtx - - var response sdk.TxResponse - out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args) - switch { - case tc.expectErrMsg != "": - s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String()) - s.Require().Contains(response.RawLog, tc.expectErrMsg) - - case tc.expectErr: - s.Require().Error(err) - - default: - s.Require().NoError(err) - s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String()) - } - }) - } -} - -func (s *CLITestSuite) TestExecSendAuthzWithAllowList() { - val := testutil.CreateKeyringAccounts(s.T(), s.kr, 1) - grantee := s.grantee[3] - - allowedAddr := s.grantee[4] - notAllowedAddr := s.grantee[5] - twoHours := time.Now().Add(time.Minute * time.Duration(120)).Unix() - - _, err := authzclitestutil.CreateGrant(s.clientCtx, - []string{ - grantee.String(), - "send", - fmt.Sprintf("--%s=100stake", cli.FlagSpendLimit), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - fmt.Sprintf("--%s=%s", flags.FlagFrom, val[0].Address.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%d", cli.FlagExpiration, twoHours), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=%s", cli.FlagAllowList, allowedAddr), - }, - ) - s.Require().NoError(err) - - from := val[0].Address - tokens := sdk.NewCoins( - sdk.NewCoin("stake", sdkmath.NewInt(12)), - ) - msgSend := &banktypes.MsgSend{ - FromAddress: from.String(), - ToAddress: grantee.String(), - Amount: tokens, - } - validGeneratedTx, err := clitestutil.SubmitTestTx( - s.clientCtx, - msgSend, - from, - clitestutil.TestTxConfig{GenOnly: true}, - ) - - s.Require().NoError(err) - execMsg := testutil.WriteToNewTempFile(s.T(), validGeneratedTx.String()) - defer execMsg.Close() - - msgSend1 := &banktypes.MsgSend{ - FromAddress: from.String(), - ToAddress: notAllowedAddr.String(), - Amount: tokens, - } - invalidGeneratedTx, err := clitestutil.SubmitTestTx( - s.clientCtx, - msgSend1, - from, - clitestutil.TestTxConfig{GenOnly: true}, - ) - - s.Require().NoError(err) - execMsg1 := testutil.WriteToNewTempFile(s.T(), invalidGeneratedTx.String()) - defer execMsg1.Close() - - // test sending to allowed address - args := []string{ - execMsg.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - } - var response sdk.TxResponse - cmd := cli.NewCmdExecAuthorization() - out, err := clitestutil.ExecTestCLICmd(s.clientCtx, cmd, args) - s.Require().NoError(err) - s.Require().NoError(s.clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String()) - - // test sending to not allowed address - args = []string{ - execMsg1.Name(), - fmt.Sprintf("--%s=%s", flags.FlagFrom, grantee.String()), - fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastSync), - fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin("stake", sdkmath.NewInt(10))).String()), - fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation), - } - out, err = clitestutil.ExecTestCLICmd(s.clientCtx, cmd, args) - s.Require().NoError(err) - s.Require().NoError(s.clientCtx.Codec.UnmarshalJSON(out.Bytes(), &response), out.String()) -} diff --git a/x/authz/client/testutil/helpers.go b/x/authz/client/testutil/helpers.go index 231cf3bba033..f00e6b938081 100644 --- a/x/authz/client/testutil/helpers.go +++ b/x/authz/client/testutil/helpers.go @@ -8,6 +8,5 @@ import ( ) func CreateGrant(clientCtx client.Context, args []string) (testutil.BufferWriter, error) { - cmd := cli.NewCmdGrantAuthorization() - return clitestutil.ExecTestCLICmd(clientCtx, cmd, args) + return clitestutil.ExecTestCLICmd(clientCtx, cli.NewCmdGrantAuthorization(), args) } diff --git a/x/authz/module/autocli.go b/x/authz/module/autocli.go index 21d5d6338cc6..93057d5f960c 100644 --- a/x/authz/module/autocli.go +++ b/x/authz/module/autocli.go @@ -7,7 +7,6 @@ import ( autocliv1 "cosmossdk.io/api/cosmos/autocli/v1" "github.com/cosmos/cosmos-sdk/version" - "github.com/cosmos/cosmos-sdk/x/authz" bank "github.com/cosmos/cosmos-sdk/x/bank/types" ) @@ -51,13 +50,25 @@ func (am AppModule) AutoCLIOptions() *autocliv1.ModuleOptions { Service: authzv1beta1.Msg_ServiceDesc.ServiceName, EnhanceCustomCommand: true, RpcCommandOptions: []*autocliv1.RpcCommandOptions{ + { + RpcMethod: "Exec", + Use: "exec [msg-json-file] --from [grantee]", + Short: "Execute tx on behalf of granter account", + Example: fmt.Sprintf("$ %s tx authz exec msg.json --from grantee\n $ %[1]s tx bank send [granter] [recipient] [amount] --generate-only | jq .body.messages > msg.json && %[1]s tx authz exec msg.json --from grantee", version.AppName), + PositionalArgs: []*autocliv1.PositionalArgDescriptor{ + {ProtoField: "msgs", Varargs: true}, + }, + }, { RpcMethod: "Revoke", - Use: "revoke [grantee] [msg-type-url] --from=[granter]", + Use: "revoke [grantee] [msg-type-url] --from [granter]", Short: `Revoke authorization from a granter to a grantee`, - Example: fmt.Sprintf(`%s tx %s revoke cosmos1skj.. %s --from=cosmos1skj..`, - version.AppName, authz.ModuleName, bank.SendAuthorization{}.MsgTypeURL()), - PositionalArgs: []*autocliv1.PositionalArgDescriptor{{ProtoField: "grantee"}, {ProtoField: "msg_type_url"}}, + Example: fmt.Sprintf(`%s tx authz revoke cosmos1skj.. %s --from=cosmos1skj..`, + version.AppName, bank.SendAuthorization{}.MsgTypeURL()), + PositionalArgs: []*autocliv1.PositionalArgDescriptor{ + {ProtoField: "grantee"}, + {ProtoField: "msg_type_url"}, + }, }, }, }, diff --git a/x/authz/module/module.go b/x/authz/module/module.go index 0c60bbf272bf..48fe4d7fada0 100644 --- a/x/authz/module/module.go +++ b/x/authz/module/module.go @@ -9,7 +9,6 @@ import ( "github.com/spf13/cobra" modulev1 "cosmossdk.io/api/cosmos/authz/module/v1" - "cosmossdk.io/core/address" "cosmossdk.io/core/appmodule" "cosmossdk.io/core/store" "cosmossdk.io/depinject" @@ -40,7 +39,6 @@ var ( // AppModuleBasic defines the basic application module used by the authz module. type AppModuleBasic struct { cdc codec.Codec - ac address.Codec } // Name returns the authz module's name. @@ -111,7 +109,7 @@ type AppModule struct { // NewAppModule creates a new AppModule object func NewAppModule(cdc codec.Codec, keeper keeper.Keeper, ak authz.AccountKeeper, bk authz.BankKeeper, registry cdctypes.InterfaceRegistry) AppModule { return AppModule{ - AppModuleBasic: AppModuleBasic{cdc: cdc, ac: ak.AddressCodec()}, + AppModuleBasic: AppModuleBasic{cdc: cdc}, keeper: keeper, accountKeeper: ak, bankKeeper: bk,