Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: bypass-msgs #2218

Merged
merged 18 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ jobs:
go-version: 1.18
- name: test & coverage report creation
run: |
go test -v -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... $(go list ./... | grep -v -e '/tests/e2e')
go test -v -coverprofile=coverage.txt -covermode=atomic -coverpkg=./... $(go list ./... | grep -v -e '/tests/e2e')
- name: filter non-testable files
run: |
excludelist="$(find ./ -type f -name '*.go' | xargs grep -l 'DONTCOVER')"
Expand Down Expand Up @@ -128,7 +128,7 @@ jobs:
go.sum
- name: Install GaiaV8
run: |
git checkout v8.0.0-rc3
git checkout v8.0.0
make build
cp ./build/gaiad ./build/gaiad8
if: env.GIT_DIFF
Expand All @@ -140,7 +140,7 @@ jobs:
if: env.GIT_DIFF
- name: Install Cosmovisor
run: |
go install github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor@v1.0.0
go install github.com/cosmos/cosmos-sdk/cosmovisor/cmd/cosmovisor@latest
if: env.GIT_DIFF
- name: Start GaiaV8
run: |
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ Ref: https://keepachangelog.com/en/1.0.0/
# Changelog

## [Unreleased]
*
* (feat) Add two more msg types `/ibc.core.channel.v1.MsgTimeout` and `/ibc.core.channel.v1.MsgTimeoutOnClose` to default `bypass-min-fee-msg-types`.
* (feat) Change the bypassing gas usage criteria. Instead of requiring 200,000 gas per `bypass-min-fee-msg`, we will now allow a maximum total usage of 1,000,000 gas for all bypassed messages in a transaction. Note that all messages in the transaction must be the `bypass-min-fee-msg-types` for the bypass min fee to take effect, otherwise, fee payment will still apply.

## [v9.0.1] - 2023-03-09

Expand Down
11 changes: 6 additions & 5 deletions ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -53,11 +53,12 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
sigGasConsumer = ante.DefaultSigVerificationGasConsumer
}

// maxBypassMinFeeMsgGasUsage is the maximum gas usage per message
// so that a transaction that contains only message types that can
// bypass the minimum fee can be accepted with a zero fee.
// maxTotalBypassMinFeeMsgGasUsage is the allowed maximum gas usage
// for all the bypass msgs in a transactions.
// A transaction that contains only bypass message types and the gas usage does not
// exceed maxTotalBypassMinFeeMsgGasUsage can be accepted with a zero fee.
// For details, see gaiafeeante.NewFeeDecorator()
var maxBypassMinFeeMsgGasUsage uint64 = 200_000
var maxTotalBypassMinFeeMsgGasUsage uint64 = 1_000_000
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved

anteDecorators := []sdk.AnteDecorator{
ante.NewSetUpContextDecorator(), // outermost AnteDecorator. SetUpContext must be called first
Expand All @@ -67,7 +68,7 @@ func NewAnteHandler(opts HandlerOptions) (sdk.AnteHandler, error) {
ante.NewValidateMemoDecorator(opts.AccountKeeper),
ante.NewConsumeGasForTxSizeDecorator(opts.AccountKeeper),
NewGovPreventSpamDecorator(opts.Codec, opts.GovKeeper),
gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxBypassMinFeeMsgGasUsage),
gaiafeeante.NewFeeDecorator(opts.BypassMinFeeMsgTypes, opts.GlobalFeeSubspace, opts.StakingSubspace, maxTotalBypassMinFeeMsgGasUsage),
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved

ante.NewDeductFeeDecorator(opts.AccountKeeper, opts.BankKeeper, opts.FeegrantKeeper),
ante.NewSetPubKeyDecorator(opts.AccountKeeper), // SetPubKeyDecorator must be called before all signature verification decorators
Expand Down
2 changes: 2 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,8 @@ func GetDefaultBypassFeeMessages() []string {
sdk.MsgTypeURL(&ibcchanneltypes.MsgRecvPacket{}),
sdk.MsgTypeURL(&ibcchanneltypes.MsgAcknowledgement{}),
sdk.MsgTypeURL(&ibcclienttypes.MsgUpdateClient{}),
sdk.MsgTypeURL(&ibcchanneltypes.MsgTimeout{}),
sdk.MsgTypeURL(&ibcchanneltypes.MsgTimeoutOnClose{}),
}
}

Expand Down
2 changes: 1 addition & 1 deletion contrib/scripts/run-gaia-v8.sh
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,5 @@ perl -i~ -0777 -pe 's/# Enable defines if the API server should be enabled.
enable = false/# Enable defines if the API server should be enabled.
enable = true/g' $NODE_HOME/config/app.toml

$COSMOVISOR start --home $NODE_HOME --x-crisis-skip-assert-invariants
$COSMOVISOR run start --home $NODE_HOME --x-crisis-skip-assert-invariants

2 changes: 1 addition & 1 deletion contrib/scripts/run-upgrade-commands.sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ if test -f "$BINARY"; then


key=$($BINARY keys show val --home $NODE_HOME)
if [ key == "" ]; then
if [ -z "$key" ]; then
echo $USER_MNEMONIC | $BINARY --home $NODE_HOME keys add val --recover --keyring-backend=test
fi

Expand Down
11 changes: 6 additions & 5 deletions docs/modules/globalfee.md
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,15 @@ The `minimum-gas-prices` config parameter allows node operators to impose additi
Bypass messages are messages that are exempt from paying fees. The above global fees and `minimum-gas-prices` checks do not apply for transactions that satisfy the following conditions:

- Contains only bypass message types, i.e., bypass transactions.
- The total gas used is less than or equal to `len(messages) * MaxBypassMinFeeMsgGasUsage`. Note: the current `MaxBypassMinFeeMsgGasUsage` is set to `200,000`.
- The total gas used is less than or equal to `MaxTotalBypassMinFeeMsgGasUsage`. Note: the current `MaxTotalBypassMinFeeMsgGasUsage` is set to `1,000,000`.
- In case of non-zero transaction fees, the denom has to be a subset of denoms defined in the global fees list.

Node operators can configure `bypass-min-fee-msg-types` in `config/app.toml`.

Nodes created using Gaiad `v7.0.2` or later use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"]` as defaults. Nodes with `bypass-min-fee-msg-types = []` or missing this field in `app.toml` also use default bypass message types.

Nodes created using Gaiad `v7.0.1` or earlier do not have `bypass-min-fee-msg-types` configured in `config/app.toml` - they are also using default values. The `bypass-min-fee-msg-types` config option can be added to `config/app.toml` before the `[telemetry]` field.
- Nodes created using Gaiad `v7.0.2` or `v9.0.x` use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"]` as defaults.
- Nodes created using Gaiad `v10.0.x` or later use `["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer", "/ibc.core.channel.v1.MsgTimeout", "/ibc.core.channel.v1.MsgTimeoutOnClose"]` as defaults.
- Node Nodes with `bypass-min-fee-msg-types = []` or missing this field in `app.toml` also use default bypass message types.
- Nodes created using Gaiad `v7.0.1` and `v7.0.0` do not have `bypass-min-fee-msg-types` configured in `config/app.toml` - they are also using same default values as in `v7.0.2`. The `bypass-min-fee-msg-types` config option can be added to `config/app.toml` before the `[telemetry]` field.

An example of `bypass-min-fee-msg-types` in `app.toml`:

Expand All @@ -72,7 +73,7 @@ An example of `bypass-min-fee-msg-types` in `app.toml`:
#
# Example:
# ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement", ...]
bypass-min-fee-msg-types = ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer"]
bypass-min-fee-msg-types = ["/ibc.core.channel.v1.MsgRecvPacket", "/ibc.core.channel.v1.MsgAcknowledgement","/ibc.applications.transfer.v1.MsgTransfer", "/ibc.core.channel.v1.MsgTimeout", "/ibc.core.channel.v1.MsgTimeoutOnClose"]
```


Expand Down
50 changes: 46 additions & 4 deletions x/globalfee/ante/antetest/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -468,8 +468,7 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
txCheck: true,
yaruwangway marked this conversation as resolved.
Show resolved Hide resolved
expErr: false,
},
// test bypass msg
"msg type ibc, zero fee in globalfee denom": {
"bypass msg type: ibc.core.channel.v1.MsgRecvPacket": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())),
Expand All @@ -479,6 +478,46 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
txCheck: true,
expErr: false,
},
"bypass msg type: ibc.core.channel.v1.MsgTimeout": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())),
gasLimit: newTestGasLimit(),
txMsg: ibcchanneltypes.NewMsgTimeout(
ibcchanneltypes.Packet{}, 1, nil, ibcclienttypes.Height{}, ""),
txCheck: true,
expErr: false,
},
"bypass msg type: ibc.core.channel.v1.MsgTimeoutOnClose": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())),
gasLimit: newTestGasLimit(),
txMsg: ibcchanneltypes.NewMsgTimeout(
ibcchanneltypes.Packet{}, 2, nil, ibcclienttypes.Height{}, ""),
txCheck: true,
expErr: false,
},
"bypass msg gas usage exceeds maxTotalBypassMinFeeMsgGasUsage": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())),
gasLimit: 2 * newTestMaxTotalBypassMinFeeMsgGasUsage(),
txMsg: ibcchanneltypes.NewMsgTimeout(
ibcchanneltypes.Packet{}, 2, nil, ibcclienttypes.Height{}, ""),
txCheck: true,
expErr: true,
},
"bypass msg gas usage equals to maxTotalBypassMinFeeMsgGasUsage": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
gasPrice: sdk.NewCoins(sdk.NewCoin("uatom", sdk.ZeroInt())),
gasLimit: newTestMaxTotalBypassMinFeeMsgGasUsage(),
txMsg: ibcchanneltypes.NewMsgTimeout(
ibcchanneltypes.Packet{}, 3, nil, ibcclienttypes.Height{}, ""),
txCheck: true,
expErr: false,
},
"msg type ibc, zero fee not in globalfee denom": {
minGasPrice: minGasPrice,
globalFeeParams: globalfeeParamsLow,
Expand Down Expand Up @@ -573,9 +612,8 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
stakingParam.BondDenom = "uatom"
stakingSubspace := s.SetupTestStakingSubspace(stakingParam)
// setup antehandler
mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace, newTestGasLimit())
mfd := gaiafeeante.NewFeeDecorator(gaiaapp.GetDefaultBypassFeeMessages(), globalfeeSubspace, stakingSubspace, newTestMaxTotalBypassMinFeeMsgGasUsage())
antehandler := sdk.ChainAnteDecorators(mfd)

s.Require().NoError(s.txBuilder.SetMsgs(testCase.txMsg))
s.txBuilder.SetFeeAmount(testCase.gasPrice)
s.txBuilder.SetGasLimit(testCase.gasLimit)
Expand All @@ -597,3 +635,7 @@ func (s *IntegrationTestSuite) TestGlobalFeeMinimumGasFeeAnteHandler() {
func newTestGasLimit() uint64 {
return 200000
}

func newTestMaxTotalBypassMinFeeMsgGasUsage() uint64 {
return 1000000
}
27 changes: 13 additions & 14 deletions x/globalfee/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ import (
var _ sdk.AnteDecorator = FeeDecorator{}

type FeeDecorator struct {
BypassMinFeeMsgTypes []string
GlobalMinFee globalfee.ParamSource
StakingSubspace paramtypes.Subspace
MaxBypassMinFeeMsgGasUsage uint64
BypassMinFeeMsgTypes []string
GlobalMinFee globalfee.ParamSource
StakingSubspace paramtypes.Subspace
MaxTotalBypassMinFeeMsgGasUsage uint64
}

func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxBypassMinFeeMsgGasUsage uint64) FeeDecorator {
func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace paramtypes.Subspace, maxTotalBypassMinFeeMsgGasUsage uint64) FeeDecorator {
if !globalfeeSubspace.HasKeyTable() {
panic("global fee paramspace was not set up via module")
}
Expand All @@ -42,10 +42,10 @@ func NewFeeDecorator(bypassMsgTypes []string, globalfeeSubspace, stakingSubspace
}

return FeeDecorator{
BypassMinFeeMsgTypes: bypassMsgTypes,
GlobalMinFee: globalfeeSubspace,
StakingSubspace: stakingSubspace,
MaxBypassMinFeeMsgGasUsage: maxBypassMinFeeMsgGasUsage,
BypassMinFeeMsgTypes: bypassMsgTypes,
GlobalMinFee: globalfeeSubspace,
StakingSubspace: stakingSubspace,
MaxTotalBypassMinFeeMsgGasUsage: maxTotalBypassMinFeeMsgGasUsage,
}
}

Expand All @@ -62,12 +62,11 @@ func (mfd FeeDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, ne
// Accept zero fee transactions only if both of the following statements are true:
// - the tx contains only message types that can bypass the minimum fee,
// see BypassMinFeeMsgTypes;
// - the total gas limit per message does not exceed MaxBypassMinFeeMsgGasUsage,
// i.e., totalGas <= len(msgs) * MaxBypassMinFeeMsgGasUsage
// - the total gas limit per message does not exceed MaxTotalBypassMinFeeMsgGasUsage,
// i.e., totalGas <= MaxTotalBypassMinFeeMsgGasUsage
// Otherwise, minimum fees and global fees are checked to prevent spam.
containsOnlyBypassMinFeeMsgs := mfd.bypassMinFeeMsgs(msgs)
doesNotExceedMaxGasUsage := gas <= uint64(len(msgs))*mfd.MaxBypassMinFeeMsgGasUsage
allowedToBypassMinFee := containsOnlyBypassMinFeeMsgs && doesNotExceedMaxGasUsage
doesNotExceedMaxGasUsage := gas <= mfd.MaxTotalBypassMinFeeMsgGasUsage
allowedToBypassMinFee := mfd.containsOnlyBypassMinFeeMsgs(msgs) && doesNotExceedMaxGasUsage

var allFees sdk.Coins
requiredGlobalFees, err := mfd.getGlobalFee(ctx, feeTx)
Expand Down
2 changes: 1 addition & 1 deletion x/globalfee/ante/fee_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ func getMinGasPrice(ctx sdk.Context, feeTx sdk.FeeTx) sdk.Coins {
return requiredFees.Sort()
}

func (mfd FeeDecorator) bypassMinFeeMsgs(msgs []sdk.Msg) bool {
func (mfd FeeDecorator) containsOnlyBypassMinFeeMsgs(msgs []sdk.Msg) bool {
for _, msg := range msgs {
if tmstrings.StringInSlice(sdk.MsgTypeURL(msg), mfd.BypassMinFeeMsgTypes) {
continue
Expand Down