test(evm): unit tests for evm_ante #1912
Conversation
WalkthroughThe recent changes introduce a variety of enhancements and new features focused on Ethereum transaction handling within the Cosmos SDK. These include restructuring error messages for clarity, adding new decorators for event emission, fee checking, and gas consumption, and implementing comprehensive test suites to validate these functionalities. The updates aim to improve the robustness and readability of the Ethereum Virtual Machine (EVM) integration in the application. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- app/evmante_sigverify.go (1 hunks)
- app/evmante_sigverify_test.go (1 hunks)
Additional comments not posted (5)
app/evmante_sigverify.go (2)
45-48: Improved error message formatting for invalid message types enhances clarity.
56-58: Enhanced error handling for unprotected Ethereum transactions is a good security practice.app/evmante_sigverify_test.go (2)
14-15: Introduction of global variablesInvalidChainIDandRandomAddressfor test setup is a good practice for reusability.
17-88: Comprehensive test coverage for various Ethereum transaction scenarios enhances the robustness of the signature verification process.CHANGELOG.md (1)
70-70: The changelog entry for PR #1912 is correctly formatted and placed. It succinctly captures the essence of the changes made—adding unit tests forevm_ante_sigverify.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1912 +/- ##
==========================================
+ Coverage 63.05% 64.76% +1.71%
==========================================
Files 245 250 +5
Lines 15871 15963 +92
==========================================
+ Hits 10007 10339 +332
+ Misses 5122 4871 -251
- Partials 742 753 +11
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- app/evmante_gas_wanted_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
Additional comments not posted (1)
app/evmante_gas_wanted_test.go (1)
12-106: Comprehensive and well-structured unit tests forGasWantedDecorator.The test cases are well-organized and cover a variety of scenarios, including transactions with different gas limits and block gas settings. The use of table-driven tests enhances maintainability and readability. Good job on ensuring that each test case is isolated by resetting the
TestDepsand committing the stateDB changes.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- app/evmante_emit_event.go (1 hunks)
- app/evmante_setup_ctx.go (3 hunks)
- app/evmante_setup_ctx_test.go (1 hunks)
- app/evmante_validate_basic.go (1 hunks)
Additional comments not posted (4)
app/evmante_setup_ctx_test.go (1)
12-42: Comprehensive test coverage forEthSetupContextDecorator.This test function effectively checks various critical aspects such as error handling, gas meter setup, and gas config resets, ensuring robust functionality.
app/evmante_setup_ctx.go (1)
33-36: Improved error handling for invalid transaction types.The updated error message provides clear feedback on the expected transaction type, enhancing the robustness of the transaction handling process.
app/evmante_emit_event.go (1)
35-36: Robust error handling for invalid Ethereum transaction messages.The error handling ensures that only valid Ethereum transaction messages are processed, which is crucial for the integrity of event emission.
app/evmante_validate_basic.go (1)
37-70: Comprehensive validation and error handling for Ethereum transactions.The method effectively ensures that Ethereum transactions meet the required standards and handles errors appropriately, which is essential for transaction integrity.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/evmante_emit_event.go (1 hunks)
- app/evmante_emit_event_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- app/evmante_emit_event.go
Additional comments not posted (4)
app/evmante_emit_event_test.go (4)
13-78: Comprehensive test coverage forEthEmitEventDecorator.The test function
TestEthEmitEventDecoratoreffectively covers both failure and success scenarios. It checks for the correct event type and attributes, ensuring robust functionality of the event emission process.
40-47: Proper setup and initialization for test cases.The initialization with
evmtest.NewTestDeps()and the commitment of the state database withstateDB.Commit()are correctly placed to ensure a consistent state before each test case runs.
14-38: Well-structured test cases.The use of table-driven tests enhances maintainability and clarity. Each test case is clearly defined with a setup function and expected outcomes, facilitating easy additions or modifications in the future.
49-75: Thorough assertions and error handling in tests.The assertions within the test cases are comprehensive, checking for the presence of errors, the correctness of the event type, and the presence of specific transaction attributes. This thoroughness ensures that the decorator behaves as expected under various conditions.
onikonychev
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (9)
- CHANGELOG.md (1 hunks)
- app/evmante_emit_event.go (1 hunks)
- app/evmante_emit_event_test.go (1 hunks)
- app/evmante_gas_wanted_test.go (1 hunks)
- app/evmante_setup_ctx_test.go (1 hunks)
- app/evmante_sigverify.go (1 hunks)
- app/evmante_validate_basic.go (1 hunks)
- app/evmante_validate_basic_test.go (1 hunks)
- x/evm/tx.go (1 hunks)
Files skipped from review due to trivial changes (1)
- x/evm/tx.go
Files skipped from review as they are similar to previous changes (7)
- CHANGELOG.md
- app/evmante_emit_event.go
- app/evmante_emit_event_test.go
- app/evmante_gas_wanted_test.go
- app/evmante_setup_ctx_test.go
- app/evmante_sigverify.go
- app/evmante_validate_basic.go
Additional comments not posted (1)
app/evmante_validate_basic_test.go (1)
20-244: The structure and organization ofTestEthValidateBasicDecoratorare well-executed, providing clear and comprehensive test coverage for various transaction scenarios.
app/evmante_validate_basic_test.go
Outdated
| func buildEthMsg( | ||
| chainID *big.Int, | ||
| gasLimit uint64, | ||
| from string, | ||
| to *common.Address, | ||
|
|
||
| ) *evm.MsgEthereumTx { | ||
| ethContractCreationTxParams := &evm.EvmTxArgs{ | ||
| ChainID: chainID, | ||
| Nonce: 1, | ||
| Amount: big.NewInt(10), | ||
| GasLimit: gasLimit, | ||
| GasPrice: big.NewInt(1), | ||
| To: to, | ||
| } | ||
| tx := evm.NewTx(ethContractCreationTxParams) | ||
| tx.From = from | ||
| return tx | ||
| } | ||
|
|
||
| func buildTx( | ||
| deps *evmtest.TestDeps, | ||
| ethExtentions bool, | ||
| msg sdk.Msg, | ||
| gasLimit uint64, | ||
| fees sdk.Coins, | ||
| ) sdk.Tx { | ||
| txBuilder, _ := deps.EncCfg.TxConfig.NewTxBuilder().(authtx.ExtensionOptionsTxBuilder) | ||
| if ethExtentions { | ||
| option, _ := codectypes.NewAnyWithValue(&evm.ExtensionOptionsEthereumTx{}) | ||
| txBuilder.SetExtensionOptions(option) | ||
| } | ||
| err := txBuilder.SetMsgs(msg) | ||
| if err != nil { | ||
| panic(err) | ||
| } | ||
| txBuilder.SetGasLimit(gasLimit) | ||
| txBuilder.SetFeeAmount(fees) | ||
|
|
||
| return txBuilder.GetTx() | ||
| } |
There was a problem hiding this comment.
The utility functions buildEthMsg and buildTx are well-implemented, enhancing test readability and maintainability. Consider adding error handling in buildTx instead of using panic for a more graceful error management.
- panic(err)
+ if err != nil {
+ return nil, err
+ }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.
| func buildEthMsg( | |
| chainID *big.Int, | |
| gasLimit uint64, | |
| from string, | |
| to *common.Address, | |
| ) *evm.MsgEthereumTx { | |
| ethContractCreationTxParams := &evm.EvmTxArgs{ | |
| ChainID: chainID, | |
| Nonce: 1, | |
| Amount: big.NewInt(10), | |
| GasLimit: gasLimit, | |
| GasPrice: big.NewInt(1), | |
| To: to, | |
| } | |
| tx := evm.NewTx(ethContractCreationTxParams) | |
| tx.From = from | |
| return tx | |
| } | |
| func buildTx( | |
| deps *evmtest.TestDeps, | |
| ethExtentions bool, | |
| msg sdk.Msg, | |
| gasLimit uint64, | |
| fees sdk.Coins, | |
| ) sdk.Tx { | |
| txBuilder, _ := deps.EncCfg.TxConfig.NewTxBuilder().(authtx.ExtensionOptionsTxBuilder) | |
| if ethExtentions { | |
| option, _ := codectypes.NewAnyWithValue(&evm.ExtensionOptionsEthereumTx{}) | |
| txBuilder.SetExtensionOptions(option) | |
| } | |
| err := txBuilder.SetMsgs(msg) | |
| if err != nil { | |
| panic(err) | |
| } | |
| txBuilder.SetGasLimit(gasLimit) | |
| txBuilder.SetFeeAmount(fees) | |
| return txBuilder.GetTx() | |
| } | |
| func buildEthMsg( | |
| chainID *big.Int, | |
| gasLimit uint64, | |
| from string, | |
| to *common.Address, | |
| ) *evm.MsgEthereumTx { | |
| ethContractCreationTxParams := &evm.EvmTxArgs{ | |
| ChainID: chainID, | |
| Nonce: 1, | |
| Amount: big.NewInt(10), | |
| GasLimit: gasLimit, | |
| GasPrice: big.NewInt(1), | |
| To: to, | |
| } | |
| tx := evm.NewTx(ethContractCreationTxParams) | |
| tx.From = from | |
| return tx | |
| } | |
| func buildTx( | |
| deps *evmtest.TestDeps, | |
| ethExtentions bool, | |
| msg sdk.Msg, | |
| gasLimit uint64, | |
| fees sdk.Coins, | |
| ) (sdk.Tx, error) { | |
| txBuilder, _ := deps.EncCfg.TxConfig.NewTxBuilder().(authtx.ExtensionOptionsTxBuilder) | |
| if ethExtentions { | |
| option, _ := codectypes.NewAnyWithValue(&evm.ExtensionOptionsEthereumTx{}) | |
| txBuilder.SetExtensionOptions(option) | |
| } | |
| err := txBuilder.SetMsgs(msg) | |
| if err != nil { | |
| return nil, err | |
| } | |
| txBuilder.SetGasLimit(gasLimit) | |
| txBuilder.SetFeeAmount(fees) | |
| return txBuilder.GetTx(), nil | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range and nitpick comments (1)
app/evmante_fee_checker.go (1)
29-31: Consider removing or updating the TODO comment.The TODO comment suggests that this code might be temporary or experimental. If this is not the case, consider updating or removing the comment to avoid confusion.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/evmante_fee_checker.go (4 hunks)
- app/evmante_fee_checker_test.go (1 hunks)
Additional comments not posted (2)
app/evmante_fee_checker_test.go (1)
11-97: Comprehensive and well-structured test suite forNewDynamicFeeChecker.The test cases cover various scenarios including happy and sad paths, which is crucial for ensuring robustness. The use of table-driven tests enhances readability and maintainability.
app/evmante_fee_checker.go (1)
56-59: Excellent error handling for negative priority prices.The use of
errors.Wrapfto provide detailed error messages enhances the debuggability of the system.
| 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, | ||
| ) |
There was a problem hiding this comment.
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.
| 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, | |
| ) |
| return nil, 0, errors.Wrapf( | ||
| errortypes.ErrInsufficientFee, | ||
| "insufficient fees; got: %s required: %s", feeCoins, requiredFees, | ||
| ) |
There was a problem hiding this comment.
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.
| 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, | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- app/evmante_fees.go (1 hunks)
- app/evmante_fees_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- app/evmante_fees.go
Additional comments not posted (1)
app/evmante_fees_test.go (1)
11-103: Comprehensive and well-structured unit tests forEthMinGasPriceDecorator.The test cases cover various scenarios including both happy and sad paths. The use of table-driven tests enhances maintainability and readability. Ensure that all edge cases for gas price calculations and fee validations are covered, especially with varying transaction complexities and gas price fluctuations.
There was a problem hiding this comment.
Actionable comments posted: 8
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- app/evmante_can_transfer.go (1 hunks)
- app/evmante_can_transfer_test.go (1 hunks)
- app/evmante_gas_consume.go (1 hunks)
- app/evmante_gas_consume_test.go (1 hunks)
- app/evmante_increment_sender_seq.go (1 hunks)
- app/evmante_verify_eth_acc.go (1 hunks)
- app/evmante_verify_eth_acc_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- app/evmante_verify_eth_acc_test.go
Additional comments not posted (3)
app/evmante_can_transfer.go (2)
18-22: The introduction ofCanTransferDecoratoris well-documented with comments explaining its purpose.
82-90: Check for potential integer overflow.app/evmante_gas_consume.go (1)
23-32: Initialization ofAnteDecEthGasConsumeis clear and concise.
app/evmante_increment_sender_seq.go
Outdated
| func (issd AnteDecEthIncrementSenderSequence) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler) (sdk.Context, error) { | ||
| for _, msg := range tx.GetMsgs() { | ||
| msgEthTx, ok := msg.(*evm.MsgEthereumTx) | ||
| if !ok { | ||
| return ctx, errors.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evm.MsgEthereumTx)(nil)) | ||
| } | ||
|
|
||
| txData, err := evm.UnpackTxData(msgEthTx.Data) | ||
| if err != nil { | ||
| return ctx, errors.Wrap(err, "failed to unpack tx data") | ||
| } | ||
|
|
||
| // increase sequence of sender | ||
| acc := issd.AccountKeeper.GetAccount(ctx, msgEthTx.GetFrom()) | ||
| if acc == nil { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrUnknownAddress, | ||
| "account %s is nil", gethcommon.BytesToAddress(msgEthTx.GetFrom().Bytes()), | ||
| ) | ||
| } | ||
| nonce := acc.GetSequence() | ||
|
|
||
| // we merged the nonce verification to nonce increment, so when tx includes multiple messages | ||
| // with same sender, they'll be accepted. | ||
| if txData.GetNonce() != nonce { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrInvalidSequence, | ||
| "invalid nonce; got %d, expected %d", txData.GetNonce(), nonce, | ||
| ) | ||
| } | ||
|
|
||
| if err := acc.SetSequence(nonce + 1); err != nil { | ||
| return ctx, errors.Wrapf(err, "failed to set sequence to %d", acc.GetSequence()+1) | ||
| } | ||
|
|
||
| issd.AccountKeeper.SetAccount(ctx, acc) | ||
| } | ||
|
|
||
| return next(ctx, tx, simulate) | ||
| } |
There was a problem hiding this comment.
Optimize error handling for clarity and maintainability.
The error messages could be more descriptive and specific to the context of the failure. This would improve debugging and maintainability.
Consider handling potential panics from type assertions and method calls.
The function assumes that certain type assertions and method calls will not fail (e.g., msg.(*evm.MsgEthereumTx)). It's safer to handle these potential panics by checking the returned values before usage.
app/evmante_can_transfer_test.go
Outdated
| 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) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance test coverage for edge cases.
The current tests cover basic scenarios. Consider adding more edge cases, such as boundary values for fees and gas prices, to ensure robustness.
app/evmante_can_transfer.go
Outdated
| // AnteHandle creates an EVM from the message and calls the BlockContext CanTransfer function to | ||
| // see if the address can execute the transaction. | ||
| func (ctd CanTransferDecorator) AnteHandle( | ||
| ctx sdk.Context, tx sdk.Tx, simulate bool, next sdk.AnteHandler, | ||
| ) (sdk.Context, error) { | ||
| params := ctd.EvmKeeper.GetParams(ctx) | ||
| ethCfg := evm.EthereumConfig(ctd.EvmKeeper.EthChainID(ctx)) | ||
| signer := gethcore.MakeSigner(ethCfg, big.NewInt(ctx.BlockHeight())) | ||
|
|
||
| for _, msg := range tx.GetMsgs() { | ||
| msgEthTx, ok := msg.(*evm.MsgEthereumTx) | ||
| if !ok { | ||
| return ctx, errors.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evm.MsgEthereumTx)(nil)) | ||
| } | ||
|
|
||
| baseFee := ctd.EvmKeeper.GetBaseFee(ctx) | ||
|
|
||
| coreMsg, err := msgEthTx.AsMessage(signer, baseFee) | ||
| if err != nil { | ||
| return ctx, errors.Wrapf( | ||
| err, | ||
| "failed to create an ethereum core.Message from signer %T", signer, | ||
| ) | ||
| } | ||
|
|
||
| if baseFee == nil { | ||
| return ctx, errors.Wrap( | ||
| evm.ErrInvalidBaseFee, | ||
| "base fee is supported but evm block context value is nil", | ||
| ) | ||
| } | ||
| if coreMsg.GasFeeCap().Cmp(baseFee) < 0 { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrInsufficientFee, | ||
| "max fee per gas less than block base fee (%s < %s)", | ||
| coreMsg.GasFeeCap(), baseFee, | ||
| ) | ||
| } | ||
|
|
||
| // NOTE: pass in an empty coinbase address and nil tracer as we don't need them for the check below | ||
| cfg := &statedb.EVMConfig{ | ||
| ChainConfig: ethCfg, | ||
| Params: params, | ||
| CoinBase: gethcommon.Address{}, | ||
| BaseFee: baseFee, | ||
| } | ||
|
|
||
| stateDB := statedb.New(ctx, &ctd.EvmKeeper, statedb.NewEmptyTxConfig(gethcommon.BytesToHash(ctx.HeaderHash().Bytes()))) | ||
| evm := ctd.EvmKeeper.NewEVM(ctx, coreMsg, cfg, evm.NewNoOpTracer(), stateDB) | ||
|
|
||
| // check that caller has enough balance to cover asset transfer for **topmost** call | ||
| // NOTE: here the gas consumed is from the context with the infinite gas meter | ||
| if coreMsg.Value().Sign() > 0 && !evm.Context.CanTransfer(stateDB, coreMsg.From(), coreMsg.Value()) { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrInsufficientFunds, | ||
| "failed to transfer %s from address %s using the EVM block context transfer function", | ||
| coreMsg.Value(), | ||
| coreMsg.From(), | ||
| ) | ||
| } | ||
| } | ||
|
|
||
| return next(ctx, tx, simulate) |
There was a problem hiding this comment.
Ensure robust error handling and clear error messages.
The error messages are clear, but consider adding more specific logging for debugging purposes, especially around the Ethereum transaction handling in lines 48-54 and 82-90.
| // there's no need to verify it again during ReCheckTx | ||
| // | ||
| // Use new context with gasWanted = 0 | ||
| // Otherwise, there's an error on txmempool.postCheck (tendermint) | ||
| // that is not bubbled up. Thus, the Tx never runs on DeliverMode | ||
| // Error: "gas wanted -1 is negative" | ||
| newCtx := ctx.WithGasMeter(eth.NewInfiniteGasMeterWithLimit(gasWanted)) | ||
| return next(newCtx, tx, simulate) | ||
| } | ||
|
|
||
| evmParams := anteDec.EvmKeeper.GetParams(ctx) | ||
| evmDenom := evmParams.GetEvmDenom() | ||
|
|
||
| var events sdk.Events | ||
|
|
||
| // Use the lowest priority of all the messages as the final one. | ||
| minPriority := int64(math.MaxInt64) | ||
| baseFee := anteDec.EvmKeeper.GetBaseFee(ctx) | ||
|
|
||
| for _, msg := range tx.GetMsgs() { | ||
| msgEthTx, ok := msg.(*evm.MsgEthereumTx) | ||
| if !ok { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrUnknownRequest, | ||
| "invalid message type %T, expected %T", | ||
| msg, (*evm.MsgEthereumTx)(nil), | ||
| ) | ||
| } | ||
| from := msgEthTx.GetFrom() | ||
|
|
||
| txData, err := evm.UnpackTxData(msgEthTx.Data) | ||
| if err != nil { | ||
| return ctx, errors.Wrap(err, "failed to unpack tx data") | ||
| } | ||
|
|
||
| if ctx.IsCheckTx() && anteDec.maxGasWanted != 0 { | ||
| // We can't trust the tx gas limit, because we'll refund the unused gas. | ||
| if txData.GetGas() > anteDec.maxGasWanted { | ||
| gasWanted += anteDec.maxGasWanted | ||
| } else { | ||
| gasWanted += txData.GetGas() | ||
| } | ||
| } else { | ||
| gasWanted += txData.GetGas() | ||
| } | ||
|
|
||
| fees, err := keeper.VerifyFee(txData, evmDenom, baseFee, ctx.IsCheckTx()) | ||
| if err != nil { | ||
| return ctx, errors.Wrapf(err, "failed to verify the fees") | ||
| } | ||
|
|
||
| if err = anteDec.deductFee(ctx, fees, from); err != nil { | ||
| return ctx, err | ||
| } | ||
|
|
||
| events = append(events, | ||
| sdk.NewEvent( | ||
| sdk.EventTypeTx, | ||
| sdk.NewAttribute(sdk.AttributeKeyFee, fees.String()), | ||
| ), | ||
| ) | ||
|
|
||
| priority := evm.GetTxPriority(txData, baseFee) | ||
|
|
||
| if priority < minPriority { | ||
| minPriority = priority | ||
| } | ||
| } | ||
|
|
||
| ctx.EventManager().EmitEvents(events) | ||
|
|
||
| blockGasLimit := eth.BlockGasLimit(ctx) | ||
|
|
||
| // return error if the tx gas is greater than the block limit (max gas) | ||
|
|
||
| // NOTE: it's important here to use the gas wanted instead of the gas consumed | ||
| // from the tx gas pool. The latter only has the value so far since the | ||
| // EthSetupContextDecorator, so it will never exceed the block gas limit. | ||
| if gasWanted > blockGasLimit { | ||
| return ctx, errors.Wrapf( | ||
| errortypes.ErrOutOfGas, | ||
| "tx gas (%d) exceeds block gas limit (%d)", | ||
| gasWanted, | ||
| blockGasLimit, | ||
| ) | ||
| } | ||
|
|
||
| // Set tx GasMeter with a limit of GasWanted (i.e. gas limit from the Ethereum tx). | ||
| // The gas consumed will be then reset to the gas used by the state transition | ||
| // in the EVM. | ||
|
|
||
| // FIXME: use a custom gas configuration that doesn't add any additional gas and only | ||
| // takes into account the gas consumed at the end of the EVM transaction. | ||
| newCtx := ctx. | ||
| WithGasMeter(eth.NewInfiniteGasMeterWithLimit(gasWanted)). | ||
| WithPriority(minPriority) | ||
|
|
||
| // we know that we have enough gas on the pool to cover the intrinsic gas | ||
| return next(newCtx, tx, simulate) | ||
| } |
There was a problem hiding this comment.
Review the logic for gas consumption and error handling.
The logic for handling gas consumption in lines 103-112 seems complex and could be prone to errors. Consider simplifying this logic or adding more comments to explain the reasoning behind these calculations.
| // FIXME: use a custom gas configuration that doesn't add any additional gas and only | ||
| // takes into account the gas consumed at the end of the EVM transaction. | ||
| newCtx := ctx. | ||
| WithGasMeter(eth.NewInfiniteGasMeterWithLimit(gasWanted)). | ||
| WithPriority(minPriority) | ||
|
|
||
| // we know that we have enough gas on the pool to cover the intrinsic gas | ||
| return next(newCtx, tx, simulate) | ||
| } |
There was a problem hiding this comment.
Potential issue with custom gas configuration.
The comment on line 159 suggests a FIXME for using a custom gas configuration. This needs to be addressed to ensure that the gas consumed is accurately accounted for at the end of the EVM transaction.
app/evmante_gas_consume_test.go
Outdated
| func (s *TestSuite) TestGasWantedDecorator() { | ||
| testCases := []struct { | ||
| name string | ||
| ctxSetup func(deps *evmtest.TestDeps) | ||
| txSetup func(deps *evmtest.TestDeps) sdk.Tx | ||
| wantErr string | ||
| }{ | ||
| { | ||
| name: "happy: non fee tx type", | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return happyCreateContractTx(deps) | ||
| }, | ||
| wantErr: "", | ||
| }, | ||
| { | ||
| name: "happy: tx without gas, block gas limit 1000", | ||
| ctxSetup: func(deps *evmtest.TestDeps) { | ||
| cp := &tmproto.ConsensusParams{ | ||
| Block: &tmproto.BlockParams{MaxGas: 1000}, | ||
| } | ||
| deps.Ctx = deps.Ctx.WithConsensusParams(cp) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return legacytx.StdTx{ | ||
| Msgs: []sdk.Msg{ | ||
| happyCreateContractTx(deps), | ||
| }, | ||
| } | ||
| }, | ||
| wantErr: "", | ||
| }, | ||
| { | ||
| name: "happy: tx with gas wanted 500, block gas limit 1000", | ||
| ctxSetup: func(deps *evmtest.TestDeps) { | ||
| cp := &tmproto.ConsensusParams{ | ||
| Block: &tmproto.BlockParams{MaxGas: 1000}, | ||
| } | ||
| deps.Ctx = deps.Ctx.WithConsensusParams(cp) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return legacytx.StdTx{ | ||
| Msgs: []sdk.Msg{ | ||
| happyCreateContractTx(deps), | ||
| }, | ||
| Fee: legacytx.StdFee{Gas: 500}, | ||
| } | ||
| }, | ||
| wantErr: "", | ||
| }, | ||
| { | ||
| name: "sad: tx with gas wanted 1000, block gas limit 500", | ||
| ctxSetup: func(deps *evmtest.TestDeps) { | ||
| cp := &tmproto.ConsensusParams{ | ||
| Block: &tmproto.BlockParams{ | ||
| MaxGas: 500, | ||
| }, | ||
| } | ||
| deps.Ctx = deps.Ctx.WithConsensusParams(cp) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return legacytx.StdTx{ | ||
| Msgs: []sdk.Msg{ | ||
| happyCreateContractTx(deps), | ||
| }, | ||
| Fee: legacytx.StdFee{Gas: 1000}, | ||
| } | ||
| }, | ||
| wantErr: "exceeds block gas limit", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| s.Run(tc.name, func() { | ||
| deps := evmtest.NewTestDeps() | ||
| stateDB := deps.StateDB() | ||
| anteDec := app.AnteDecoratorGasWanted{} | ||
|
|
||
| tx := tc.txSetup(&deps) | ||
| s.Require().NoError(stateDB.Commit()) | ||
|
|
||
| deps.Ctx = deps.Ctx.WithIsCheckTx(true) | ||
| if tc.ctxSetup != nil { | ||
| tc.ctxSetup(&deps) | ||
| } | ||
| _, err := anteDec.AnteHandle( | ||
| deps.Ctx, tx, false, NextNoOpAnteHandler, | ||
| ) | ||
| if tc.wantErr != "" { | ||
| s.Require().ErrorContains(err, tc.wantErr) | ||
| return | ||
| } | ||
| s.Require().NoError(err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Refactor to improve test readability and maintainability.
The test cases could be refactored to use a more structured approach, such as table-driven tests, to improve readability and maintainability.
| // AnteHandle validates checks that the sender balance is greater than the total transaction cost. | ||
| // The account will be set to store if it doesn't exist, i.e. cannot be found on store. | ||
| // This AnteHandler decorator will fail if: | ||
| // - any of the msgs is not a MsgEthereumTx | ||
| // - from address is empty | ||
| // - account balance is lower than the transaction cost | ||
| func (anteDec AnteDecVerifyEthAcc) AnteHandle( | ||
| ctx sdk.Context, | ||
| tx sdk.Tx, | ||
| simulate bool, | ||
| next sdk.AnteHandler, | ||
| ) (newCtx sdk.Context, err error) { | ||
| if !ctx.IsCheckTx() { | ||
| return next(ctx, tx, simulate) | ||
| } | ||
|
|
||
| for i, msg := range tx.GetMsgs() { | ||
| msgEthTx, ok := msg.(*evm.MsgEthereumTx) | ||
| if !ok { | ||
| return ctx, errors.Wrapf(errortypes.ErrUnknownRequest, "invalid message type %T, expected %T", msg, (*evm.MsgEthereumTx)(nil)) | ||
| } | ||
|
|
||
| txData, err := evm.UnpackTxData(msgEthTx.Data) | ||
| if err != nil { | ||
| return ctx, errors.Wrapf(err, "failed to unpack tx data any for tx %d", i) | ||
| } | ||
|
|
||
| // sender address should be in the tx cache from the previous AnteHandle call | ||
| from := msgEthTx.GetFrom() | ||
| if from.Empty() { | ||
| return ctx, errors.Wrap(errortypes.ErrInvalidAddress, "from address cannot be empty") | ||
| } | ||
|
|
||
| // check whether the sender address is EOA | ||
| fromAddr := gethcommon.BytesToAddress(from) | ||
| acct := anteDec.EvmKeeper.GetAccount(ctx, fromAddr) | ||
|
|
||
| if acct == nil { | ||
| acc := anteDec.AccountKeeper.NewAccountWithAddress(ctx, from) | ||
| anteDec.AccountKeeper.SetAccount(ctx, acc) | ||
| acct = statedb.NewEmptyAccount() | ||
| } else if acct.IsContract() { | ||
| return ctx, errors.Wrapf(errortypes.ErrInvalidType, | ||
| "the sender is not EOA: address %s, codeHash <%s>", fromAddr, acct.CodeHash) | ||
| } | ||
|
|
||
| if err := keeper.CheckSenderBalance(sdkmath.NewIntFromBigInt(acct.Balance), txData); err != nil { | ||
| return ctx, errors.Wrap(err, "failed to check sender balance") | ||
| } | ||
| } | ||
| return next(ctx, tx, simulate) | ||
| } |
There was a problem hiding this comment.
Ensure consistent error handling and improve error messages.
The error messages could be more consistent and provide more context about the failure, which would aid in debugging and user understanding.
… reject_msgs, can_transfer
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range and nitpick comments (1)
app/evmante_verify_eth_acc_test.go (1)
123-123: Consider renaminghappyTransfertTxtohappyTransferTxto correct the typo and improve readability.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (11)
- app/evmante_can_transfer.go (1 hunks)
- app/evmante_can_transfer_test.go (1 hunks)
- app/evmante_fee_checker_test.go (1 hunks)
- app/evmante_gas_consume.go (1 hunks)
- app/evmante_gas_consume_test.go (1 hunks)
- app/evmante_handler_test.go (1 hunks)
- app/evmante_increment_sender_seq.go (1 hunks)
- app/evmante_increment_sender_seq_test.go (1 hunks)
- app/evmante_reject_msgs_test.go (1 hunks)
- app/evmante_validate_basic_test.go (1 hunks)
- app/evmante_verify_eth_acc_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (5)
- app/evmante_can_transfer.go
- app/evmante_fee_checker_test.go
- app/evmante_gas_consume.go
- app/evmante_gas_consume_test.go
- app/evmante_increment_sender_seq.go
Additional comments not posted (3)
app/evmante_can_transfer_test.go (1)
14-109: Enhance test coverage for edge cases as previously suggested. Consider adding more edge cases, such as boundary values for fees and gas prices, to ensure robustness.app/evmante_verify_eth_acc_test.go (1)
138-146: The implementation ofnonEvmMsgTxcorrectly constructs a non-EVM transaction using the Cosmos SDK's bank module. Good use of utility functions for clarity and maintainability.app/evmante_validate_basic_test.go (1)
20-243: TheTestEthValidateBasicDecoratorfunction comprehensively tests the basic validation of Ethereum transactions with a variety of scenarios. Excellent structuring of test cases and effective use of utility functions.
| func (s *TestSuite) TestAnteDecoratorPreventEtheruemTxMsgs() { | ||
| testCases := []struct { | ||
| name string | ||
| txSetup func(deps *evmtest.TestDeps) sdk.Tx | ||
| wantErr string | ||
| }{ | ||
| { | ||
| name: "sad: evm message", | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return happyTransfertTx(deps, 0) | ||
| }, | ||
| wantErr: "invalid type", | ||
| }, | ||
| { | ||
| name: "happy: non evm message", | ||
| txSetup: nonEvmMsgTx, | ||
| wantErr: "", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| s.Run(tc.name, func() { | ||
| deps := evmtest.NewTestDeps() | ||
| anteDec := app.AnteDecoratorPreventEtheruemTxMsgs{} | ||
| tx := tc.txSetup(&deps) | ||
|
|
||
| _, err := anteDec.AnteHandle( | ||
| deps.Ctx, tx, false, NextNoOpAnteHandler, | ||
| ) | ||
| if tc.wantErr != "" { | ||
| s.Require().ErrorContains(err, tc.wantErr) | ||
| return | ||
| } | ||
| s.Require().NoError(err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Ensure correct spelling in function and decorator names to maintain professionalism and avoid confusion.
- func (s *TestSuite) TestAnteDecoratorPreventEtheruemTxMsgs() {
+ func (s *TestSuite) TestAnteDecoratorPreventEthereumTxMsgs() {
- anteDec := app.AnteDecoratorPreventEtheruemTxMsgs{}
+ anteDec := app.AnteDecoratorPreventEthereumTxMsgs{}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.
| func (s *TestSuite) TestAnteDecoratorPreventEtheruemTxMsgs() { | |
| testCases := []struct { | |
| name string | |
| txSetup func(deps *evmtest.TestDeps) sdk.Tx | |
| wantErr string | |
| }{ | |
| { | |
| name: "sad: evm message", | |
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | |
| return happyTransfertTx(deps, 0) | |
| }, | |
| wantErr: "invalid type", | |
| }, | |
| { | |
| name: "happy: non evm message", | |
| txSetup: nonEvmMsgTx, | |
| wantErr: "", | |
| }, | |
| } | |
| for _, tc := range testCases { | |
| s.Run(tc.name, func() { | |
| deps := evmtest.NewTestDeps() | |
| anteDec := app.AnteDecoratorPreventEtheruemTxMsgs{} | |
| tx := tc.txSetup(&deps) | |
| _, err := anteDec.AnteHandle( | |
| deps.Ctx, tx, false, NextNoOpAnteHandler, | |
| ) | |
| if tc.wantErr != "" { | |
| s.Require().ErrorContains(err, tc.wantErr) | |
| return | |
| } | |
| s.Require().NoError(err) | |
| }) | |
| } | |
| } | |
| func (s *TestSuite) TestAnteDecoratorPreventEthereumTxMsgs() { | |
| testCases := []struct { | |
| name string | |
| txSetup func(deps *evmtest.TestDeps) sdk.Tx | |
| wantErr string | |
| }{ | |
| { | |
| name: "sad: evm message", | |
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | |
| return happyTransfertTx(deps, 0) | |
| }, | |
| wantErr: "invalid type", | |
| }, | |
| { | |
| name: "happy: non evm message", | |
| txSetup: nonEvmMsgTx, | |
| wantErr: "", | |
| }, | |
| } | |
| for _, tc := range testCases { | |
| s.Run(tc.name, func() { | |
| deps := evmtest.NewTestDeps() | |
| anteDec := app.AnteDecoratorPreventEthereumTxMsgs{} | |
| tx := tc.txSetup(&deps) | |
| _, err := anteDec.AnteHandle( | |
| deps.Ctx, tx, false, NextNoOpAnteHandler, | |
| ) | |
| if tc.wantErr != "" { | |
| s.Require().ErrorContains(err, tc.wantErr) | |
| return | |
| } | |
| s.Require().NoError(err) | |
| }) | |
| } | |
| } |
| func (s *TestSuite) TestAnteDecEthIncrementSenderSequence() { | ||
| testCases := []struct { | ||
| name string | ||
| beforeTxSetup func(deps *evmtest.TestDeps, sdb *statedb.StateDB) | ||
| txSetup func(deps *evmtest.TestDeps) sdk.Tx | ||
| wantErr string | ||
| wantSeq uint64 | ||
| }{ | ||
| { | ||
| name: "happy: single message", | ||
| beforeTxSetup: func(deps *evmtest.TestDeps, sdb *statedb.StateDB) { | ||
| balance := big.NewInt(100) | ||
| sdb.AddBalance(deps.Sender.EthAddr, balance) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return happyTransfertTx(deps, 0) | ||
| }, | ||
| wantErr: "", | ||
| wantSeq: 1, | ||
| }, | ||
| { | ||
| name: "happy: two messages", | ||
| beforeTxSetup: func(deps *evmtest.TestDeps, sdb *statedb.StateDB) { | ||
| balance := big.NewInt(100) | ||
| sdb.AddBalance(deps.Sender.EthAddr, balance) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| txMsgOne := happyTransfertTx(deps, 0) | ||
| txMsgTwo := happyTransfertTx(deps, 1) | ||
|
|
||
| txBuilder := deps.EncCfg.TxConfig.NewTxBuilder() | ||
| s.Require().NoError(txBuilder.SetMsgs(txMsgOne, txMsgTwo)) | ||
|
|
||
| tx := txBuilder.GetTx() | ||
| return tx | ||
| }, | ||
| wantErr: "", | ||
| wantSeq: 2, | ||
| }, | ||
| { | ||
| name: "sad: account does not exists", | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.Tx { | ||
| return happyTransfertTx(deps, 0) | ||
| }, | ||
| wantErr: "unknown address", | ||
| }, | ||
| { | ||
| name: "sad: tx with non evm message", | ||
| txSetup: nonEvmMsgTx, | ||
| wantErr: "invalid message", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| s.Run(tc.name, func() { | ||
| deps := evmtest.NewTestDeps() | ||
| stateDB := deps.StateDB() | ||
| anteDec := app.NewAnteDecEthIncrementSenderSequence(deps.Chain.AppKeepers) | ||
|
|
||
| if tc.beforeTxSetup != nil { | ||
| tc.beforeTxSetup(&deps, stateDB) | ||
| s.Require().NoError(stateDB.Commit()) | ||
| } | ||
| tx := tc.txSetup(&deps) | ||
|
|
||
| _, err := anteDec.AnteHandle( | ||
| deps.Ctx, tx, false, NextNoOpAnteHandler, | ||
| ) | ||
| if tc.wantErr != "" { | ||
| s.Require().ErrorContains(err, tc.wantErr) | ||
| return | ||
| } | ||
| s.Require().NoError(err) | ||
|
|
||
| if tc.wantSeq > 0 { | ||
| seq := deps.Chain.AccountKeeper.GetAccount(deps.Ctx, deps.Sender.NibiruAddr).GetSequence() | ||
| s.Require().Equal(tc.wantSeq, seq) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding a test case to verify the behavior when the sequence increment fails due to database errors.
Would you like me to help draft this additional test case?
| func (s *TestSuite) TestAnteHandlerEVM() { | ||
| testCases := []struct { | ||
| name string | ||
| txSetup func(deps *evmtest.TestDeps) sdk.FeeTx | ||
| ctxSetup func(deps *evmtest.TestDeps) | ||
| beforeTxSetup func(deps *evmtest.TestDeps, sdb *statedb.StateDB) | ||
| wantErr string | ||
| }{ | ||
| { | ||
| name: "happy: signed tx, sufficient funds", | ||
| beforeTxSetup: func(deps *evmtest.TestDeps, sdb *statedb.StateDB) { | ||
| sdb.AddBalance( | ||
| deps.Sender.EthAddr, | ||
| new(big.Int).Add(gasLimitCreateContract(), big.NewInt(100)), | ||
| ) | ||
| }, | ||
| ctxSetup: func(deps *evmtest.TestDeps) { | ||
| gasPrice := sdk.NewInt64Coin("unibi", 1) | ||
| cp := &tmproto.ConsensusParams{ | ||
| Block: &tmproto.BlockParams{ | ||
| MaxGas: new(big.Int).Add(gasLimitCreateContract(), big.NewInt(100)).Int64(), | ||
| }, | ||
| } | ||
| deps.Ctx = deps.Ctx. | ||
| WithMinGasPrices( | ||
| sdk.NewDecCoins(sdk.NewDecCoinFromCoin(gasPrice)), | ||
| ). | ||
| WithIsCheckTx(true). | ||
| WithConsensusParams(cp) | ||
| }, | ||
| txSetup: func(deps *evmtest.TestDeps) sdk.FeeTx { | ||
| txMsg := happyTransfertTx(deps, 0) | ||
| txBuilder := deps.EncCfg.TxConfig.NewTxBuilder() | ||
|
|
||
| gethSigner := deps.Sender.GethSigner(deps.Chain.EvmKeeper.EthChainID(deps.Ctx)) | ||
| keyringSigner := deps.Sender.KeyringSigner | ||
| err := txMsg.Sign(gethSigner, keyringSigner) | ||
| s.Require().NoError(err) | ||
|
|
||
| tx, err := txMsg.BuildTx(txBuilder, eth.EthBaseDenom) | ||
| s.Require().NoError(err) | ||
|
|
||
| return tx | ||
| }, | ||
| wantErr: "", | ||
| }, | ||
| } | ||
|
|
||
| for _, tc := range testCases { | ||
| s.Run(tc.name, func() { | ||
| deps := evmtest.NewTestDeps() | ||
| stateDB := deps.StateDB() | ||
|
|
||
| anteHandlerEVM := app.NewAnteHandlerEVM( | ||
| deps.Chain.AppKeepers, ante.AnteHandlerOptions{ | ||
| HandlerOptions: authante.HandlerOptions{ | ||
| AccountKeeper: deps.Chain.AccountKeeper, | ||
| BankKeeper: deps.Chain.BankKeeper, | ||
| FeegrantKeeper: deps.Chain.FeeGrantKeeper, | ||
| SignModeHandler: deps.EncCfg.TxConfig.SignModeHandler(), | ||
| SigGasConsumer: authante.DefaultSigVerificationGasConsumer, | ||
| ExtensionOptionChecker: func(*codectypes.Any) bool { return true }, | ||
| }, | ||
| }) | ||
|
|
||
| tx := tc.txSetup(&deps) | ||
|
|
||
| if tc.ctxSetup != nil { | ||
| tc.ctxSetup(&deps) | ||
| } | ||
| if tc.beforeTxSetup != nil { | ||
| tc.beforeTxSetup(&deps, stateDB) | ||
| err := stateDB.Commit() | ||
| s.Require().NoError(err) | ||
| } | ||
|
|
||
| _, err := anteHandlerEVM( | ||
| deps.Ctx, tx, false, | ||
| ) | ||
| if tc.wantErr != "" { | ||
| s.Require().ErrorContains(err, tc.wantErr) | ||
| return | ||
| } | ||
| s.Require().NoError(err) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider adding more comprehensive tests for the AnteHandlerEVM, especially focusing on scenarios where multiple handlers interact and on error propagation.
Would you like assistance in drafting these additional test scenarios?
Summary by CodeRabbit
New Features
EthEmitEventDecoratorto handle event emission for Ethereum transactions.EthValidateBasicDecoratorfor basic validation of Ethereum transactions.CanTransferDecoratorto check sender's transfer eligibility based on EVM rules.AnteDecEthGasConsumefor validating intrinsic gas and managing gas consumption.Bug Fixes
Tests
Chores
-vflag intest-coverage-integrationtarget for verbose output in test coverage reports.