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

feat: setting protocol controlled min gas price #1326

Merged
merged 60 commits into from
Sep 8, 2022
Merged
Show file tree
Hide file tree
Changes from 48 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
d619786
adding fee stats table
robert-zaremba Sep 2, 2022
7061206
update
robert-zaremba Sep 2, 2022
8b323d0
move app constants to app/params
robert-zaremba Sep 2, 2022
9cd92d1
use appparams
robert-zaremba Sep 2, 2022
3cede55
move prefix settings to appparams
robert-zaremba Sep 2, 2022
150d566
markdown linting
robert-zaremba Sep 2, 2022
195332a
check consensus min-gas-price in app creator
robert-zaremba Sep 2, 2022
ddaf886
fix e2e tests
robert-zaremba Sep 3, 2022
895a3ad
remove MaxAddrLen
robert-zaremba Sep 3, 2022
9df787f
use consensus protected gas price
robert-zaremba Sep 6, 2022
5a75ad3
update tests
robert-zaremba Sep 6, 2022
949b0ba
formatting
robert-zaremba Sep 6, 2022
0adee69
test update
robert-zaremba Sep 6, 2022
a20feef
Merge branch 'main' into robert/tx-fees
robert-zaremba Sep 6, 2022
20e6770
don't charge fee for txs in the first block (genesis)
robert-zaremba Sep 7, 2022
3d65b32
Merge remote-tracking branch 'origin/main' into robert/tx-fees
robert-zaremba Sep 7, 2022
ebe83db
fix leverage tests
robert-zaremba Sep 7, 2022
e8ad4f5
update gas settings for leverage simtest
robert-zaremba Sep 7, 2022
751d357
add coin math util lib
robert-zaremba Sep 7, 2022
332f79a
update fee tests
robert-zaremba Sep 7, 2022
42c7643
linting
robert-zaremba Sep 7, 2022
9ddc855
adding new genmap module
robert-zaremba Sep 7, 2022
83ad415
filter sim modules
robert-zaremba Sep 7, 2022
4276147
add util/sim package
robert-zaremba Sep 7, 2022
6cc051f
refactor sim module manager
robert-zaremba Sep 7, 2022
de9709e
add account funding to umee sim tests
robert-zaremba Sep 7, 2022
a3d355f
fix leverage query test
robert-zaremba Sep 7, 2022
007f80c
comment back err check
robert-zaremba Sep 7, 2022
9ccf670
Merge remote-tracking branch 'origin/main' into robert/tx-fees
robert-zaremba Sep 7, 2022
994bf47
linting
robert-zaremba Sep 7, 2022
10436c5
lint
robert-zaremba Sep 8, 2022
225f338
fix: extra carriage-return, syntax
khoerling Sep 8, 2022
e0513c7
fix: syntax + lint
khoerling Sep 8, 2022
8797415
style
robert-zaremba Sep 8, 2022
bc5c7b5
add log check to single node - testing ci
RafilxTenfen Sep 8, 2022
742479b
set min gas price in app.toml to 0.05uumee
RafilxTenfen Sep 8, 2022
40d8eca
update e2e to use uumee
RafilxTenfen Sep 8, 2022
f9c3d71
set fee to 10000uumee as needed by the transaction
RafilxTenfen Sep 8, 2022
c7e0628
improve dockerfile to have base-builder
RafilxTenfen Sep 8, 2022
36e4274
add double check for non empty ibcStakeDenom, somehow was passing
RafilxTenfen Sep 8, 2022
539e202
revoked the max_gas to 6000000 on hermes
RafilxTenfen Sep 8, 2022
cdc5129
add one more step to docker helping to cache more packages and improv…
RafilxTenfen Sep 8, 2022
c27c732
Update util/coin/math.go
RafilxTenfen Sep 8, 2022
0fbeab0
fix lint
RafilxTenfen Sep 8, 2022
898f627
reus mingasfee variable
robert-zaremba Sep 8, 2022
fb5df4d
update free gravity msg set
robert-zaremba Sep 8, 2022
5fea4ea
update comment
robert-zaremba Sep 8, 2022
8be6c1a
Merge branch 'main' into robert/tx-fees
adamewozniak Sep 8, 2022
59e48dc
Merge branch 'main' into robert/tx-fees
RafilxTenfen Sep 8, 2022
016e223
fix tests
adamewozniak Sep 8, 2022
a86b41f
Merge branch 'main' into robert/tx-fees
adamewozniak Sep 8, 2022
c630563
removed min-gas-price flag from upgrade test single node
RafilxTenfen Sep 8, 2022
5b6d44f
add min gas price to app.toml
RafilxTenfen Sep 8, 2022
aa6ce4f
changelog
robert-zaremba Sep 8, 2022
dc3645b
removes print for CI performance
RafilxTenfen Sep 8, 2022
fc9fc23
Merge branch 'robert/tx-fees' of github.com:umee-network/umee into ro…
RafilxTenfen Sep 8, 2022
dfd339b
Merge branch 'main' into robert/tx-fees
mergify[bot] Sep 8, 2022
732515d
increased time for workflow
RafilxTenfen Sep 8, 2022
ac1b015
set fees for tx msgs
RafilxTenfen Sep 8, 2022
ae93d1c
Merge branch 'robert/tx-fees' of github.com:umee-network/umee into ro…
RafilxTenfen Sep 8, 2022
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
56 changes: 56 additions & 0 deletions ante/fee-notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Notes about Gas prices and gas used

NOTES:

- the gas used depends on the current state of the chain
- the gas records below are based on the empty state
- user is charged the gas-limit, even if tx consumed less gas
RafilxTenfen marked this conversation as resolved.
Show resolved Hide resolved

| operation | gas used |
| :--------------------------------------------- | -------: |
| x/bank send | 31'029 |
| x/group create | 68'908 |
| x/oracle MsgAggregateExchangeRateVote (3 curr) | 66'251 |
| x/oracle MsgAggregateExchangeRateVote (6 curr) | 69'726 |
| default gas limit | 200'000 |

## Target price (in USD cent) for x/bank send

| umee price (usd cent) | gas price in uumee | fee (usd cent) |
| --------------------: | -----------------: | -------------: |
| 2 | 0.2 | 0.0124 |
| 2 | 0.1 | 0.0062 |
| 2 | 0.02 | 0.00124 |
| 5 | 0.2 | 0.031 |
| 5 | 0.1 | 0.0155 |
| 5 | 0.02 | 0.0031 |

## Target price (in USD cent) for validator oracle txs (with 6 currencies) per day

There are roughly 10tx / minute and 14400 per day.
Validator has to do 2 tx (prevote and vote) every 5 blocks.
RafilxTenfen marked this conversation as resolved.
Show resolved Hide resolved
Validator will need to do `5760 = 2*14400/5` tx.
In table below we assume the same price scheme as above, but in the code most likely we will apply a fixed discount (eg 10x).

The prices are indicative. For some transactions (especially oracle) fees can be disabled.
See fee.go file for details.

| umee price (usd cent) | gas price in uumee | fee (usd cent) |
| --------------------: | -----------------: | -------------: |
| 2 | 0.2 | 161.28 |
| 2 | 0.1 | 80.64 |
| 2 | 0.02 | 16.128 |
| 5 | 0.2 | 403.2 |
| 5 | 0.1 | 201.6 |
| 5 | 0.02 | 40.32 |

## Target price (in USD) for default gas limit

| umee price (usd cent) | gas price in uumee | fee (usd cent) |
| --------------------: | -----------------: | -------------: |
| 2 | 0.2 | 0.08 |
| 2 | 0.1 | 0.04 |
| 2 | 0.02 | 0.008 |
| 5 | 0.2 | 0.2 |
| 5 | 0.1 | 0.1 |
| 5 | 0.02 | 0.02 |
87 changes: 60 additions & 27 deletions ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,13 @@ import (
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
evidencetypes "github.com/cosmos/cosmos-sdk/x/evidence/types"

appparams "github.com/umee-network/umee/v3/app/params"
leveragetypes "github.com/umee-network/umee/v3/x/leverage/types"
oracletypes "github.com/umee-network/umee/v3/x/oracle/types"
)

// MaxMsgGasUsage defines the maximum gas allowed for an oracle transaction.
const MaxMsgGasUsage = uint64(100000)
const MaxMsgGasUsage = uint64(100_000)

// FeeAndPriority ensures tx has enough fee coins to pay for the gas at the CheckTx time
// to early remove transactions from the mempool without enough attached fee.
Expand All @@ -24,37 +25,55 @@ func FeeAndPriority(ctx sdk.Context, tx sdk.Tx) (sdk.Coins, int64, error) {
return nil, 0, sdkerrors.ErrTxDecode.Wrap("Tx must be a FeeTx")
}

feeCoins := feeTx.GetFee()
gas := feeTx.GetGas()
providedFees := feeTx.GetFee()
gasLimit := feeTx.GetGas()
msgs := feeTx.GetMsgs()
isOracleOrGravity := IsOracleOrGravityTx(msgs)
chargeFees := !isOracleOrGravity || gas > uint64(len(msgs))*MaxMsgGasUsage

if ctx.IsCheckTx() && chargeFees {
minGasPrices := ctx.MinGasPrices()
if !minGasPrices.IsZero() {
requiredFees := make(sdk.Coins, len(minGasPrices))

// Determine the required fees by multiplying each required minimum gas
// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
glDec := sdk.NewDec(int64(gas))
for i, gp := range minGasPrices {
fee := gp.Amount.Mul(glDec)
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}
priority := getTxPriority(isOracleOrGravity, msgs)
chargeFees := !isOracleOrGravity || gasLimit > uint64(len(msgs))*MaxMsgGasUsage
// we also don't charge transaction fees for the first block, for the genesis transactions.
if !chargeFees || ctx.BlockHeight() == 0 {
return sdk.Coins{}, priority, nil
}

if !feeCoins.IsAnyGTE(requiredFees) {
return nil, 0, sdkerrors.ErrInsufficientFee.Wrapf(
"insufficient fees; got: %s required: %s", feeCoins, requiredFees)
}
var err error
if ctx.IsCheckTx() {
err = checkFees(ctx.MinGasPrices(), providedFees, gasLimit)
} else {
err = checkFees(nil, providedFees, gasLimit)
}
if err != nil {
err = sdkerrors.Wrap(err, msgs[0].String())
}
return providedFees, priority, err
}

func checkFees(minGasPrices sdk.DecCoins, fees sdk.Coins, gasLimit uint64) error {
if minGasPrices != nil {
// check minGasPrices set by validator
if err := AssertMinProtocolGasPrice(minGasPrices); err != nil {
return err
}
} else {
// in deliverTx = use protocol min gas price
minGasPrices = sdk.DecCoins{appparams.MinMinGasPrice}
}

priority := getTxPriority(isOracleOrGravity, msgs)
if !chargeFees {
return sdk.Coins{}, priority, nil
requiredFees := make(sdk.Coins, len(minGasPrices))

// Determine the required fees by multiplying each required minimum gas
// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
glDec := sdk.NewDec(int64(gasLimit))
for i, gp := range minGasPrices {
fee := gp.Amount.Mul(glDec)
requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
}
return feeCoins, priority, nil

if !fees.IsAnyGTE(requiredFees) {
return sdkerrors.ErrInsufficientFee.Wrapf(
"insufficient fees; got: %s required: %s", fees, requiredFees)
}
return nil
}

// IsOracleOrGravityTx checks if all messages are oracle messages
Expand All @@ -68,9 +87,8 @@ func IsOracleOrGravityTx(msgs []sdk.Msg) bool {
*oracletypes.MsgAggregateExchangeRateVote:
continue

// TODO: remove messages which should not be "free":
// TODO: revisit free gravity msg set
case *gbtypes.MsgValsetConfirm,
*gbtypes.MsgRequestBatch,
*gbtypes.MsgConfirmBatch,
*gbtypes.MsgERC20DeployedClaim,
*gbtypes.MsgConfirmLogicCall,
Expand All @@ -90,6 +108,21 @@ func IsOracleOrGravityTx(msgs []sdk.Msg) bool {
return true
}

// AssertMinProtocolGasPrice returns an error if the provided gasPrices are lower then
// the required by protocol.
func AssertMinProtocolGasPrice(gasPrices sdk.DecCoins) error {
RafilxTenfen marked this conversation as resolved.
Show resolved Hide resolved
for _, c := range gasPrices {
if c.Denom == appparams.MinMinGasPrice.Denom {
if c.Amount.LT(appparams.MinMinGasPrice.Amount) {
break // go to error below
}
return nil
}
}
return sdkerrors.ErrInsufficientFee.Wrapf(
"gas price too small; got: %v required min: %v", gasPrices, appparams.MinMinGasPrice)
}

// getTxPriority returns naive tx priority based on the lowest fee amount (regardless of the
// denom) and oracle tx check.
// Dirty optimization: since we already check if msgs are oracle or gravity messages, then we
Expand Down
87 changes: 68 additions & 19 deletions ante/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import (
"github.com/cosmos/cosmos-sdk/testutil/testdata"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
signing "github.com/cosmos/cosmos-sdk/x/auth/signing"

"github.com/umee-network/umee/v3/ante"
appparams "github.com/umee-network/umee/v3/app/params"
"github.com/umee-network/umee/v3/util/coin"
oracletypes "github.com/umee-network/umee/v3/x/oracle/types"
)

Expand All @@ -18,32 +21,73 @@ func (suite *IntegrationTestSuite) TestFeeAndPriority() {

msgs := testdata.NewTestMsg(addr1)
require.NoError(suite.txBuilder.SetMsgs(msgs))
fee := sdk.NewCoins(sdk.NewInt64Coin("atom", 150))
gasLimit := 200000
suite.txBuilder.SetFeeAmount(fee)
suite.txBuilder.SetGasLimit(uint64(gasLimit))

// Test1: validator min gas price check
// Ante should fail when validator min gas price is above the transaction gas limit
minGasPrice := sdk.NewDecCoinFromDec("atom", sdk.NewDecFromInt(fee[0].Amount).QuoInt64(int64(gasLimit/2)))
minGas := appparams.MinMinGasPrice
mkFee := func(factor string) sdk.Coins {
return coin.NewDecBld(minGas).Scale(int64(appparams.DefaultGasLimit)).ScaleStr(factor).ToCoins()
}
mkGas := func(denom, factor string) sdk.DecCoins {
if denom == "" {
denom = minGas.Denom
}
f := sdk.MustNewDecFromStr(factor)
return sdk.DecCoins{sdk.NewDecCoinFromDec(denom, minGas.Amount.Mul(f))}
}
mkTx := func(fee sdk.Coins) signing.Tx {
suite.txBuilder.SetFeeAmount(fee)
tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
require.NoError(err)
return tx
}

suite.txBuilder.SetGasLimit(appparams.DefaultGasLimit)
// we set fee to 2*gasLimit*minGasPrice
fee := mkFee("2")
tx := mkTx(fee)

//
// Test CheckTX
//
ctx := suite.ctx.
WithMinGasPrices([]sdk.DecCoin{minGasPrice}).
WithMinGasPrices(sdk.DecCoins{minGas}).
WithIsCheckTx(true)
tx, err := suite.CreateTestTx(privs, accNums, accSeqs, suite.ctx.ChainID())
require.NoError(err)
_, _, err = ante.FeeAndPriority(ctx, tx)
require.ErrorIs(sdkerrors.ErrInsufficientFee, err)

// Test2: min gas price not checked in DeliverTx
ctx = suite.ctx.WithIsCheckTx(false)
// min-gas-settings should work
suite.checkFeeAnte(tx, fee, ctx)

// Test3: should not error when min gas price is same or lower than the fee
ctx = ctx.WithMinGasPrices(sdk.NewDecCoinsFromCoins(fee...))
suite.checkFeeAnte(tx, fee, ctx)
// should work when exact fee is provided
suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("", "2")))

// should fail when not enough fee is provided
suite.checkFeeFailed(tx, ctx.WithMinGasPrices(mkGas("", "3")))

ctx = ctx.WithMinGasPrices([]sdk.DecCoin{sdk.NewDecCoin(fee[0].Denom, fee[0].Amount.QuoRaw(2))})
// should fail when other denom is required
suite.checkFeeFailed(tx, ctx.WithMinGasPrices(mkGas("other", "1")))

// should fail when some fee doesn't include all gas denoms
ctx = ctx.WithMinGasPrices(sdk.DecCoins{minGas,
sdk.NewDecCoinFromDec("other", sdk.NewDec(10))})
suite.checkFeeFailed(tx, ctx)

//
// Test DeliverTx
//
ctx = suite.ctx.
WithMinGasPrices(sdk.DecCoins{minGas}).
WithIsCheckTx(false)

// ctx.MinGasPrice shouldn't matter
suite.checkFeeAnte(tx, fee, ctx)
suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("", "3")))
suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(mkGas("other", "1")))
suite.checkFeeAnte(tx, fee, ctx.WithMinGasPrices(sdk.DecCoins{}))

// should fail when not enough fee is provided
suite.checkFeeFailed(mkTx(mkFee("0.5")), ctx)
suite.checkFeeFailed(mkTx(sdk.Coins{}), ctx)

// should work when more fees are applied
fee = append(fee, sdk.NewInt64Coin("other", 10))
suite.checkFeeAnte(mkTx(fee), fee, ctx)

// Test4: ensure no fees for oracle msgs
require.NoError(suite.txBuilder.SetMsgs(
Expand All @@ -62,6 +106,11 @@ func (suite *IntegrationTestSuite) TestFeeAndPriority() {
suite.checkFeeAnte(oracleTx, sdk.Coins{}, suite.ctx.WithIsCheckTx(false))
}

func (suite *IntegrationTestSuite) checkFeeFailed(tx sdk.Tx, ctx sdk.Context) {
_, _, err := ante.FeeAndPriority(ctx, tx)
suite.Require().ErrorIs(sdkerrors.ErrInsufficientFee, err)
}

func (suite *IntegrationTestSuite) checkFeeAnte(tx sdk.Tx, feeExpected sdk.Coins, ctx sdk.Context) {
require := suite.Require()
fee, _, err := ante.FeeAndPriority(ctx, tx)
Expand Down
1 change: 1 addition & 0 deletions ante/spam_prevention.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ func (spd *SpamPreventionDecorator) CheckOracleSpam(ctx sdk.Context, msgs []sdk.
err = spd.validate(ctx, msg.Feeder, msg.Validator, spd.oracleVoteMap, curHeight, "vote")
default:
// non oracle msg: stop validation!
// NOTE: only tx which contains only oracle Msgs are considered oracle-prioritized
return nil
}
if err != nil {
Expand Down
Loading