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

test(evm): unit tests for evm_ante #1912

Merged
merged 16 commits into from
Jun 11, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- [#1907](https://github.com/NibiruChain/nibiru/pull/1907) - test(evm): grpc_query full coverage
- [#1909](https://github.com/NibiruChain/nibiru/pull/1909) - chore(evm): set is_london true by default and removed from config
- [#1911](https://github.com/NibiruChain/nibiru/pull/1911) - chore(evm): simplified config by removing old eth forks
- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler
- [#1912](https://github.com/NibiruChain/nibiru/pull/1912) - test(evm): unit tests for evm_ante
- [#1914](https://github.com/NibiruChain/nibiru/pull/1914) - refactor(evm): Remove dead code and document non-EVM ante handler- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.
- [#1917](https://github.com/NibiruChain/nibiru/pull/1917) - test(e2e-evm): TypeScript support. Type generation from compiled contracts. Formatter for TS code.

#### Dapp modules: perp, spot, oracle, etc
Expand Down
59 changes: 59 additions & 0 deletions app/evmante_emit_event.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
// Copyright (c) 2023-2024 Nibi, Inc.
package app

import (
"strconv"

errorsmod "cosmossdk.io/errors"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/NibiruChain/nibiru/x/evm"
)

// EthEmitEventDecorator emit events in ante handler in case of tx execution failed (out of block gas limit).
type EthEmitEventDecorator struct {
AppKeepers
}

// NewEthEmitEventDecorator creates a new EthEmitEventDecorator
func NewEthEmitEventDecorator(k AppKeepers) EthEmitEventDecorator {
return EthEmitEventDecorator{AppKeepers: k}
}

// AnteHandle emits some basic events for the eth messages
func (eeed EthEmitEventDecorator) AnteHandle(
ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler,
) (newCtx sdk.Context, err error) {
// After eth tx passed ante handler, the fee is deducted and nonce increased,
// it shouldn't be ignored by json-rpc. We need to emit some events at the
// very end of ante handler to be indexed by the consensus engine.
txIndex := eeed.EvmKeeper.EVMState().BlockTxIndex.GetOr(ctx, 0)

for i, msg := range tx.GetMsgs() {
msgEthTx, ok := msg.(*evm.MsgEthereumTx)
if !ok {
return ctx, errorsmod.Wrapf(
errortypes.ErrUnknownRequest,
"invalid message type %T, expected %T",
msg, (*evm.MsgEthereumTx)(nil),
)
}

// emit ethereum tx hash as an event so that it can be indexed by
// Tendermint for query purposes it's emitted in ante handler, so we can
// query failed transaction (out of block gas limit).
ctx.EventManager().EmitEvent(
sdk.NewEvent(
evm.EventTypeEthereumTx,
sdk.NewAttribute(evm.AttributeKeyEthereumTxHash, msgEthTx.Hash),
sdk.NewAttribute(
evm.AttributeKeyTxIndex, strconv.FormatUint(txIndex+uint64(i),
10,
),
), // #nosec G701
))
}

return next(ctx, tx, simulate)
}
79 changes: 79 additions & 0 deletions app/evmante_emit_event_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package app_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/x/auth/migrations/legacytx"

"github.com/NibiruChain/nibiru/x/evm"

"github.com/NibiruChain/nibiru/app"
"github.com/NibiruChain/nibiru/x/evm/evmtest"
tf "github.com/NibiruChain/nibiru/x/tokenfactory/types"
)

func (s *TestSuite) TestEthEmitEventDecorator() {
testCases := []struct {
name string
txSetup func(deps *evmtest.TestDeps) sdk.Tx
wantErr string
}{
{
name: "sad: non ethereum tx",
txSetup: func(deps *evmtest.TestDeps) sdk.Tx {
return legacytx.StdTx{
Msgs: []sdk.Msg{
&tf.MsgMint{},
},
}
},
wantErr: "invalid message",
},
{
name: "happy: eth tx emitted event",
txSetup: func(deps *evmtest.TestDeps) sdk.Tx {
tx := happyCreateContractTx(deps)
return tx
},
wantErr: "",
},
}

for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
stateDB := deps.StateDB()
anteDec := app.NewEthEmitEventDecorator(deps.Chain.AppKeepers)

tx := tc.txSetup(&deps)
s.Require().NoError(stateDB.Commit())

_, err := anteDec.AnteHandle(
deps.Ctx, tx, false, NextNoOpAnteHandler,
)
if tc.wantErr != "" {
s.Require().ErrorContains(err, tc.wantErr)
return
}
s.Require().NoError(err)
events := deps.Ctx.EventManager().Events()

s.Require().Greater(len(events), 0)
event := events[len(events)-1]
s.Require().Equal(evm.EventTypeEthereumTx, event.Type)

// Convert tx to msg to get hash
txMsg, ok := tx.GetMsgs()[0].(*evm.MsgEthereumTx)
s.Require().True(ok)

// TX hash attr must present
attr, ok := event.GetAttribute(evm.AttributeKeyEthereumTxHash)
s.Require().True(ok, "tx hash attribute not found")
s.Require().Equal(txMsg.Hash, attr.Value)

// TX index attr must present
attr, ok = event.GetAttribute(evm.AttributeKeyTxIndex)
s.Require().True(ok, "tx index attribute not found")
s.Require().Equal("0", attr.Value)
})
}
}
28 changes: 23 additions & 5 deletions app/evmante_fee_checker.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ import (
// - Tx priority is set to `effectiveGasPrice / DefaultPriorityReduction`.
func NewDynamicFeeChecker(k evmkeeper.Keeper) ante.TxFeeChecker {
return func(ctx sdk.Context, feeTx sdk.FeeTx) (sdk.Coins, int64, error) {
// TODO: in the e2e test, if the fee in the genesis transaction meet the baseFee and minGasPrice in the feemarket, we can remove this code
// TODO: in the e2e test,
// if the fee in the genesis transaction meet the baseFee and minGasPrice in the feemarket,
// we can remove this code
if ctx.BlockHeight() == 0 {
// genesis transactions: fallback to min-gas-price logic
return checkTxFeeWithValidatorMinGasPrices(ctx, feeTx)
Expand All @@ -51,7 +53,10 @@ func NewDynamicFeeChecker(k evmkeeper.Keeper) ante.TxFeeChecker {

// priority fee cannot be negative
if maxPriorityPrice.IsNegative() {
return nil, 0, errors.Wrapf(errortypes.ErrInsufficientFee, "max priority price cannot be negative")
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"max priority price cannot be negative",
)
}

gas := feeTx.GetGas()
Expand All @@ -62,11 +67,21 @@ func NewDynamicFeeChecker(k evmkeeper.Keeper) ante.TxFeeChecker {
baseFeeInt := sdkmath.NewIntFromBigInt(baseFee)

if feeCap.LT(baseFeeInt) {
return nil, 0, errors.Wrapf(errortypes.ErrInsufficientFee, "gas prices too low, got: %s%s required: %s%s. Please retry using a higher gas price or a higher fee", feeCap, denom, baseFeeInt, denom)
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"gas prices too low, got: %s%s required: %s%s. "+
"Please retry using a higher gas price or a higher fee",
feeCap,
denom,
baseFeeInt,
denom,
)
Comment on lines +70 to +78
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure error messages are clear and actionable.

The error message could be more user-friendly by specifying the actual and required values more clearly. Consider rephrasing to enhance clarity.

- "gas prices too low, got: %s%s required: %s%s. Please retry using a higher gas price or a higher fee",
+ "Gas prices too low. Current: %s%s, required: %s%s. Please increase the gas price or fee and try again.",
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"gas prices too low, got: %s%s required: %s%s. "+
"Please retry using a higher gas price or a higher fee",
feeCap,
denom,
baseFeeInt,
denom,
)
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"Gas prices too low. Current: %s%s, required: %s%s. Please increase the gas price or fee and try again.",
feeCap,
denom,
baseFeeInt,
denom,
)

}

// calculate the effective gas price using the EIP-1559 logic.
effectivePrice := sdkmath.NewIntFromBigInt(evm.EffectiveGasPrice(baseFeeInt.BigInt(), feeCap.BigInt(), maxPriorityPrice.BigInt()))
effectivePrice := sdkmath.NewIntFromBigInt(
evm.EffectiveGasPrice(baseFeeInt.BigInt(), feeCap.BigInt(), maxPriorityPrice.BigInt()),
)

// NOTE: create a new coins slice without having to validate the denom
effectiveFee := sdk.Coins{
Expand Down Expand Up @@ -109,7 +124,10 @@ func checkTxFeeWithValidatorMinGasPrices(ctx sdk.Context, tx sdk.FeeTx) (sdk.Coi
}

if !feeCoins.IsAnyGTE(requiredFees) {
return nil, 0, errors.Wrapf(errortypes.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees)
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"insufficient fees; got: %s required: %s", feeCoins, requiredFees,
)
Comment on lines +127 to +130
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ensure consistency in error messaging.

The error message format should be consistent with other parts of the application. Consider using a similar structure as used in other error messages for uniformity.

- "insufficient fees; got: %s required: %s",
+ "Insufficient fees. Provided: %s, required: %s.",
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"insufficient fees; got: %s required: %s", feeCoins, requiredFees,
)
return nil, 0, errors.Wrapf(
errortypes.ErrInsufficientFee,
"Insufficient fees. Provided: %s, required: %s.", feeCoins, requiredFees,
)

}
}

Expand Down
97 changes: 97 additions & 0 deletions app/evmante_fee_checker_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package app_test

import (
sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/NibiruChain/nibiru/app"
"github.com/NibiruChain/nibiru/eth"
"github.com/NibiruChain/nibiru/x/evm/evmtest"
)

func (s *TestSuite) TestNewDynamicFeeChecker() {
testCases := []struct {
name string
txSetup func(deps *evmtest.TestDeps) sdk.FeeTx
ctxSetup func(deps *evmtest.TestDeps)
wantErr string
wantFee int64
wantPriority int64
}{
{
name: "happy: genesis tx with sufficient fee",
ctxSetup: func(deps *evmtest.TestDeps) {
gasPrice := sdk.NewInt64Coin("unibi", 1)
deps.Ctx = deps.Ctx.
WithBlockHeight(0).
WithMinGasPrices(
sdk.NewDecCoins(sdk.NewDecCoinFromCoin(gasPrice)),
).
WithIsCheckTx(true)
},
txSetup: func(deps *evmtest.TestDeps) sdk.FeeTx {
txMsg := happyCreateContractTx(deps)
txBuilder := deps.EncCfg.TxConfig.NewTxBuilder()
tx, err := txMsg.BuildTx(txBuilder, eth.EthBaseDenom)
s.Require().NoError(err)
return tx
},
wantErr: "",
wantFee: gasLimitCreateContract().Int64(),
wantPriority: 0,
},
{
name: "sad: genesis tx insufficient fee",
ctxSetup: func(deps *evmtest.TestDeps) {
gasPrice := sdk.NewInt64Coin("unibi", 2)
deps.Ctx = deps.Ctx.
WithBlockHeight(0).
WithMinGasPrices(
sdk.NewDecCoins(sdk.NewDecCoinFromCoin(gasPrice)),
).
WithIsCheckTx(true)
},
txSetup: func(deps *evmtest.TestDeps) sdk.FeeTx {
txMsg := happyCreateContractTx(deps)
txBuilder := deps.EncCfg.TxConfig.NewTxBuilder()
tx, err := txMsg.BuildTx(txBuilder, eth.EthBaseDenom)
s.Require().NoError(err)
return tx
},
wantErr: "insufficient fee",
},
{
name: "happy: tx with sufficient fee",
txSetup: func(deps *evmtest.TestDeps) sdk.FeeTx {
txMsg := happyCreateContractTx(deps)
txBuilder := deps.EncCfg.TxConfig.NewTxBuilder()
tx, err := txMsg.BuildTx(txBuilder, eth.EthBaseDenom)
s.Require().NoError(err)
return tx
},
wantErr: "",
wantFee: gasLimitCreateContract().Int64(),
wantPriority: 0,
},
}

for _, tc := range testCases {
s.Run(tc.name, func() {
deps := evmtest.NewTestDeps()
checker := app.NewDynamicFeeChecker(deps.K)

if tc.ctxSetup != nil {
tc.ctxSetup(&deps)
}

fee, priority, err := checker(deps.Ctx, tc.txSetup(&deps))

if tc.wantErr != "" {
s.Require().ErrorContains(err, tc.wantErr)
return
}
s.Require().NoError(err)
s.Require().Equal(tc.wantFee, fee.AmountOf("unibi").Int64())
s.Require().Equal(tc.wantPriority, priority)
})
}
}
File renamed without changes.
Loading
Loading