Skip to content

Commit

Permalink
fix: gas sim (#129)
Browse files Browse the repository at this point in the history 
* small updates

* protect

* update ante

* testing

* lint fix
  • Loading branch information
@aljo242
aljo242 authored Jul 15, 2024
1 parent 9a2a3ee commit efe52f2
Show file tree
Hide file tree
Showing 7 changed files with 107 additions and 78 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ictest.yml
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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
View file
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

0 comments on commit efe52f2

Please sign in to comment.