From 001be0fd5437236400cd6d6e3dcdd8bb82167844 Mon Sep 17 00:00:00 2001 From: Alexander Kolesov Date: Wed, 1 Dec 2021 14:47:40 +0300 Subject: [PATCH] fix!: Add `--fee-payer` CLI flag, rename `--fee-account` to `--fee-granter` (#10625) ## Description Closes: #10626 Changes: - Added `--fee-payer` CLI flag: - The flag is used to populate the corresponding `context` property. - Context in its turn is used to set `fee-payer` during transaction building. - `--fee-account` CLI flag is renamed to `--fee-granter`: - With the flag added it becomes unclear whether `--fee-account` stands for `payer` or `granter`. - Renaming to `--fee-granter` makes things more explicit. - This is CLI breaking change. --- ### 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... - [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title - [x] added `!` to the type prefix if API or client breaking change - [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting)) - [x] provided a link to the relevant issue or specification - [x] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules) - [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing) - [x] added a changelog entry to `CHANGELOG.md` - [x] included comments for [documenting Go code](https://blog.golang.org/godoc) - [x] updated the relevant documentation or specification - [x] reviewed "Files changed" and left comments if necessary - [x] 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) --- CHANGELOG.md | 2 ++ client/cmd.go | 17 +++++++++++++++-- client/context.go | 8 ++++++++ client/flags/flags.go | 6 ++++-- client/query.go | 5 +++++ client/tx/tx.go | 1 + x/feegrant/client/testutil/suite.go | 8 ++++---- x/feegrant/spec/01_concepts.md | 6 +++--- 8 files changed, 42 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9251e3418cf5..ef67961d2a50 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,6 +123,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) ` keys migrate` CLI command now takes no arguments * [\#9246](https://github.com/cosmos/cosmos-sdk/pull/9246) Removed the CLI flag `--setup-config-only` from the `testnet` command and added the subcommand `init-files`. * [\#9780](https://github.com/cosmos/cosmos-sdk/pull/9780) Use sigs.k8s.io for yaml, which might lead to minor YAML output changes +* [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Rename `--fee-account` CLI flag to `--fee-granter` ### Improvements @@ -138,6 +139,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (genesis) [\#9697](https://github.com/cosmos/cosmos-sdk/pull/9697) Ensure `InitGenesis` returns with non-empty validator set. * [\#10341](https://github.com/cosmos/cosmos-sdk/pull/10341) Move from `io/ioutil` to `io` and `os` packages. * [\#10468](https://github.com/cosmos/cosmos-sdk/pull/10468) Allow futureOps to queue additional operations in simulations +* [\#10625](https://github.com/cosmos/cosmos-sdk/pull/10625) Add `--fee-payer` CLI flag ### Bug Fixes diff --git a/client/cmd.go b/client/cmd.go index 0e401a9250f6..70f354bc2ddf 100644 --- a/client/cmd.go +++ b/client/cmd.go @@ -220,8 +220,21 @@ func readTxCommandFlags(clientCtx Context, flagSet *pflag.FlagSet) (Context, err clientCtx = clientCtx.WithSignModeStr(signModeStr) } - if clientCtx.FeeGranter == nil || flagSet.Changed(flags.FlagFeeAccount) { - granter, _ := flagSet.GetString(flags.FlagFeeAccount) + if clientCtx.FeePayer == nil || flagSet.Changed(flags.FlagFeePayer) { + payer, _ := flagSet.GetString(flags.FlagFeePayer) + + if payer != "" { + payerAcc, err := sdk.AccAddressFromBech32(payer) + if err != nil { + return clientCtx, err + } + + clientCtx = clientCtx.WithFeePayerAddress(payerAcc) + } + } + + if clientCtx.FeeGranter == nil || flagSet.Changed(flags.FlagFeeGranter) { + granter, _ := flagSet.GetString(flags.FlagFeeGranter) if granter != "" { granterAcc, err := sdk.AccAddressFromBech32(granter) diff --git a/client/context.go b/client/context.go index 67096c33a310..56ef46ff0a88 100644 --- a/client/context.go +++ b/client/context.go @@ -46,6 +46,7 @@ type Context struct { TxConfig TxConfig AccountRetriever AccountRetriever NodeURI string + FeePayer sdk.AccAddress FeeGranter sdk.AccAddress Viper *viper.Viper @@ -181,6 +182,13 @@ func (ctx Context) WithFromAddress(addr sdk.AccAddress) Context { return ctx } +// WithFeePayerAddress returns a copy of the context with an updated fee payer account +// address. +func (ctx Context) WithFeePayerAddress(addr sdk.AccAddress) Context { + ctx.FeePayer = addr + return ctx +} + // WithFeeGranterAddress returns a copy of the context with an updated fee granter account // address. func (ctx Context) WithFeeGranterAddress(addr sdk.AccAddress) Context { diff --git a/client/flags/flags.go b/client/flags/flags.go index 49b805534efd..5ddffd453c57 100644 --- a/client/flags/flags.go +++ b/client/flags/flags.go @@ -70,7 +70,8 @@ const ( FlagCountTotal = "count-total" FlagTimeoutHeight = "timeout-height" FlagKeyAlgorithm = "algo" - FlagFeeAccount = "fee-account" + FlagFeePayer = "fee-payer" + FlagFeeGranter = "fee-granter" FlagReverse = "reverse" // Tendermint logging flags @@ -112,7 +113,8 @@ func AddTxFlagsToCmd(cmd *cobra.Command) { cmd.Flags().String(FlagKeyringBackend, DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test|memory)") cmd.Flags().String(FlagSignMode, "", "Choose sign mode (direct|amino-json), this is an advanced feature") cmd.Flags().Uint64(FlagTimeoutHeight, 0, "Set a block timeout height to prevent the tx from being committed past a certain height") - cmd.Flags().String(FlagFeeAccount, "", "Fee account pays fees for the transaction instead of deducting from the signer") + cmd.Flags().String(FlagFeePayer, "", "Fee payer pays fees for the transaction instead of deducting from the signer") + cmd.Flags().String(FlagFeeGranter, "", "Fee granter grants fees for the transaction") // --gas can accept integers and "auto" cmd.Flags().String(FlagGas, "", fmt.Sprintf("gas limit to set per-transaction; set to %q to calculate sufficient gas automatically (default %d)", GasFlagAuto, DefaultGasLimit)) diff --git a/client/query.go b/client/query.go index be603dfbf6e2..b10a0e348026 100644 --- a/client/query.go +++ b/client/query.go @@ -62,6 +62,11 @@ func (ctx Context) GetFromAddress() sdk.AccAddress { return ctx.FromAddress } +// GetFeePayerAddress returns the fee granter address from the context +func (ctx Context) GetFeePayerAddress() sdk.AccAddress { + return ctx.FeePayer +} + // GetFeeGranterAddress returns the fee granter address from the context func (ctx Context) GetFeeGranterAddress() sdk.AccAddress { return ctx.FeeGranter diff --git a/client/tx/tx.go b/client/tx/tx.go index 737a528cff0e..db787a774c51 100644 --- a/client/tx/tx.go +++ b/client/tx/tx.go @@ -93,6 +93,7 @@ func BroadcastTx(clientCtx client.Context, txf Factory, msgs ...sdk.Msg) error { } tx.SetFeeGranter(clientCtx.GetFeeGranterAddress()) + tx.SetFeePayer(clientCtx.GetFeePayerAddress()) err = Sign(txf, clientCtx.GetFromName(), tx, true) if err != nil { return err diff --git a/x/feegrant/client/testutil/suite.go b/x/feegrant/client/testutil/suite.go index a12a473a209b..74380ab14fba 100644 --- a/x/feegrant/client/testutil/suite.go +++ b/x/feegrant/client/testutil/suite.go @@ -697,7 +697,7 @@ func (s *IntegrationTestSuite) TestTxWithFeeGrant() { // any tx with it by using --fee-account shouldn't fail out, err := govtestutil.MsgSubmitProposal(val.ClientCtx, grantee.String(), "Text Proposal", "No desc", govtypes.ProposalTypeText, - fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()), + fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()), ) s.Require().NoError(err) @@ -837,7 +837,7 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { func() (testutil.BufferWriter, error) { return govtestutil.MsgSubmitProposal(val.ClientCtx, grantee.String(), "Text Proposal", "No desc", govtypes.ProposalTypeText, - fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()), + fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()), ) }, &sdk.TxResponse{}, @@ -847,7 +847,7 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { "valid weighted_vote tx", func() (testutil.BufferWriter, error) { return govtestutil.MsgVote(val.ClientCtx, grantee.String(), "0", "yes", - fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter.String()), + fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter.String()), ) }, &sdk.TxResponse{}, @@ -864,7 +864,7 @@ func (s *IntegrationTestSuite) TestFilteredFeeAllowance() { grantee.String(), "cosmos14cm33pvnrv2497tyt8sp9yavhmw83nwej3m0e8", fmt.Sprintf("--%s=%s", cli.FlagSpendLimit, "100stake"), - fmt.Sprintf("--%s=%s", flags.FlagFeeAccount, granter), + fmt.Sprintf("--%s=%s", flags.FlagFeeGranter, granter), }, commonFlags..., ) diff --git a/x/feegrant/spec/01_concepts.md b/x/feegrant/spec/01_concepts.md index e880c5e76ce0..6b22d46b68c4 100644 --- a/x/feegrant/spec/01_concepts.md +++ b/x/feegrant/spec/01_concepts.md @@ -49,9 +49,9 @@ There are two types of fee allowances present at the moment: - `period_reset` keeps track of when a next period reset should happen. -## FeeAccount flag +## FeeGranter flag -`feegrant` module introduces a `FeeAccount` flag for CLI for the sake of executing transactions with fee granter. When this flag is set, `clientCtx` will append the granter account address for transactions generated through CLI. +`feegrant` module introduces a `FeeGranter` flag for CLI for the sake of executing transactions with fee granter. When this flag is set, `clientCtx` will append the granter account address for transactions generated through CLI. +++ https://github.com/cosmos/cosmos-sdk/blob/d97e7907f176777ed8a464006d360bb3e1a223e4/client/cmd.go#L224-L235 @@ -64,7 +64,7 @@ There are two types of fee allowances present at the moment: Example cmd: ```go -./simd tx gov submit-proposal --title="Test Proposal" --description="My awesome proposal" --type="Text" --from validator-key --fee-account=cosmos1xh44hxt7spr67hqaa7nyx5gnutrz5fraw6grxn --chain-id=testnet --fees="10stake" +./simd tx gov submit-proposal --title="Test Proposal" --description="My awesome proposal" --type="Text" --from validator-key --fee-granter=cosmos1xh44hxt7spr67hqaa7nyx5gnutrz5fraw6grxn --chain-id=testnet --fees="10stake" ``` ## Granted Fee Deductions