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

fix: gas sim #129

Merged
merged 5 commits into from
Jul 15, 2024
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
2 changes: 1 addition & 1 deletion .github/workflows/ictest.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: 1.22.3
go-version: 1.22.5
cache: true
cache-dependency-path: go.sum
- uses: technote-space/[email protected]
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ jobs:
steps:
- uses: actions/setup-go@v4
with:
go-version: 1.22.3
go-version: 1.22.5
- uses: actions/checkout@v4
- name: golangci-lint
uses: golangci/golangci-lint-action@v3
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: 1.22.3
go-version: 1.22.5
cache: true
cache-dependency-path: go.sum
- uses: technote-space/[email protected]
Expand All @@ -40,7 +40,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: 1.22.3
go-version: 1.22.5
cache: true
cache-dependency-path: go.sum
- uses: technote-space/[email protected]
Expand All @@ -60,7 +60,7 @@ jobs:
- uses: actions/checkout@v4
- uses: actions/setup-go@v4
with:
go-version: 1.22.3
go-version: 1.22.5
cache: true
cache-dependency-path: go.sum
- uses: technote-space/[email protected]
Expand Down
34 changes: 23 additions & 11 deletions x/feemarket/ante/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula

var feeCoin sdk.Coin
if simulate && len(feeCoins) == 0 {
feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.ZeroInt())
// if simulating and user did not provider a fee - create a dummy value for them
feeCoin = sdk.NewCoin(params.FeeDenom, sdkmath.OneInt())
} else {
feeCoin = feeCoins[0]
}
Expand All @@ -128,19 +129,20 @@ func (dfd feeMarketCheckDecorator) anteHandle(ctx sdk.Context, tx sdk.Tx, simula
if err != nil {
return ctx, errorsmod.Wrapf(err, "error checking fee")
}
}

priorityFee, err := dfd.resolveTxPriorityCoins(ctx, feeCoin, params.FeeDenom)
if err != nil {
return ctx, errorsmod.Wrapf(err, "error resolving fee priority")
}

baseGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, params.FeeDenom)
if err != nil {
return ctx, err
}
priorityFee, err := dfd.resolveTxPriorityCoins(ctx, feeCoin, params.FeeDenom)
if err != nil {
return ctx, errorsmod.Wrapf(err, "error resolving fee priority")
}

ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice))
baseGasPrice, err := dfd.feemarketKeeper.GetMinGasPrice(ctx, params.FeeDenom)
if err != nil {
return ctx, err
}

ctx = ctx.WithPriority(GetTxPriority(priorityFee, int64(gas), baseGasPrice))

return next(ctx, tx, simulate)
}

Expand Down Expand Up @@ -220,6 +222,16 @@ const (
// normalizedGasPrice = effectiveGasPrice / currentGasPrice (floor is 1. The minimum effective gas price can ever be is current gas price)
// scaledGasPrice = normalizedGasPrice * 10 ^ gasPricePrecision (amount of decimal places in the normalized gas price to consider when converting to int64).
func GetTxPriority(fee sdk.Coin, gasLimit int64, currentGasPrice sdk.DecCoin) int64 {
// protections from dividing by 0
if gasLimit == 0 {
return 0
}

// if the gas price is 0, just use a raw amount
if currentGasPrice.IsZero() {
return fee.Amount.Int64()
}

effectiveGasPrice := fee.Amount.ToLegacyDec().QuoInt64(gasLimit)
normalizedGasPrice := effectiveGasPrice.Quo(currentGasPrice.Amount)
scaledGasPrice := normalizedGasPrice.MulInt64(int64(math.Pow10(gasPricePrecision)))
Expand Down
20 changes: 13 additions & 7 deletions x/feemarket/ante/suite/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,13 +133,14 @@ func (s *TestSuite) SetupHandlers(mock bool) {

// TestCase represents a test case used in test tables.
type TestCase struct {
Name string
Malleate func(*TestSuite) TestCaseArgs
RunAnte bool
RunPost bool
Simulate bool
ExpPass bool
ExpErr error
Name string
Malleate func(*TestSuite) TestCaseArgs
RunAnte bool
RunPost bool
Simulate bool
ExpPass bool
ExpErr error
ExpectConsumedGas uint64
}

type TestCaseArgs struct {
Expand Down Expand Up @@ -193,6 +194,11 @@ func (s *TestSuite) RunTestCase(t *testing.T, tc TestCase, args TestCaseArgs) {
require.NotNil(t, newCtx)

s.Ctx = newCtx
if tc.RunPost {
consumedGas := newCtx.GasMeter().GasConsumed()
require.Equal(t, tc.ExpectConsumedGas, consumedGas)
}

} else {
switch {
case txErr != nil:
Expand Down
3 changes: 2 additions & 1 deletion x/feemarket/post/fee.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ func (dfd FeeMarketDeductDecorator) PostHandle(ctx sdk.Context, tx sdk.Tx, simul

var feeCoin sdk.Coin
if simulate && len(feeCoins) == 0 {
feeCoin = sdk.NewCoin(params.FeeDenom, math.ZeroInt())
// if simulating and user did not provider a fee - create a dummy value for them
feeCoin = sdk.NewCoin(params.FeeDenom, math.OneInt())
} else {
feeCoin = feeCoins[0]
}
Expand Down
118 changes: 64 additions & 54 deletions x/feemarket/post/fee_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,12 +124,12 @@ func TestSendTip(t *testing.T) {
func TestPostHandle(t *testing.T) {
// Same data for every test case
const (
baseDenom = "stake"
resolvableDenom = "atom"
baseDenom = "stake"
resolvableDenom = "atom"
expectedConsumedGas = 33339
gasLimit = expectedConsumedGas
)

// exact cost of transaction
gasLimit := uint64(27284)
validFeeAmount := types.DefaultMinBaseGasPrice.MulInt64(int64(gasLimit))
validFeeAmountWithTip := validFeeAmount.Add(math.LegacyNewDec(100))
validFee := sdk.NewCoins(sdk.NewCoin(baseDenom, validFeeAmount.TruncateInt()))
Expand Down Expand Up @@ -204,11 +204,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validFee,
}
},
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass, no tip",
Expand All @@ -223,11 +224,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validFee,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass with tip",
Expand All @@ -242,11 +244,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validFeeWithTip,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass with tip - simulate",
Expand All @@ -261,11 +264,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validFeeWithTip,
}
},
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass, no tip - resolvable denom",
Expand All @@ -280,11 +284,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validResolvableFee,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass, no tip - resolvable denom - simulate",
Expand All @@ -299,11 +304,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validResolvableFee,
}
},
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass with tip - resolvable denom",
Expand All @@ -318,11 +324,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validResolvableFeeWithTip,
}
},
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: false,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "signer has enough funds, should pass with tip - resolvable denom - simulate",
Expand All @@ -337,11 +344,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validResolvableFeeWithTip,
}
},
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: true,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "0 gas given should pass in simulate - no fee",
Expand All @@ -354,11 +362,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: nil,
}
},
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "0 gas given should pass in simulate - fee",
Expand All @@ -371,11 +380,12 @@ func TestPostHandle(t *testing.T) {
FeeAmount: validFee,
}
},
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
RunAnte: true,
RunPost: false,
Simulate: true,
ExpPass: true,
ExpErr: nil,
ExpectConsumedGas: expectedConsumedGas,
},
{
Name: "no fee - fail",
Expand Down
Loading