Skip to content

Commit

Permalink
fix!: Add --fee-payer CLI flag, rename --fee-account to `--fee-gr…
Browse files Browse the repository at this point in the history
…anter` (cosmos#10625)

<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

Closes: cosmos#10626

<!-- Add a description of the changes that this PR introduces and the files that
are the most critical to review. -->

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)
  • Loading branch information
askolesov authored and blewater committed Dec 8, 2021
1 parent 7c26bb7 commit c709c43
Show file tree
Hide file tree
Showing 8 changed files with 42 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* [\#9695](https://github.com/cosmos/cosmos-sdk/pull/9695) `<app> 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

Expand All @@ -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

Expand Down
17 changes: 15 additions & 2 deletions client/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 8 additions & 0 deletions client/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ type Context struct {
TxConfig TxConfig
AccountRetriever AccountRetriever
NodeURI string
FeePayer sdk.AccAddress
FeeGranter sdk.AccAddress
Viper *viper.Viper

Expand Down Expand Up @@ -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 {
Expand Down
6 changes: 4 additions & 2 deletions client/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
5 changes: 5 additions & 0 deletions client/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions client/tx/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions x/feegrant/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{},
Expand All @@ -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{},
Expand All @@ -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...,
)
Expand Down
6 changes: 3 additions & 3 deletions x/feegrant/spec/01_concepts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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
Expand Down

0 comments on commit c709c43

Please sign in to comment.