Feat: reverting EVM messages #6
Conversation
The change to return error during msg processing if VM reverts or errors out is a simple change, but it affect many places where it was expected to emit log. Failing Txns wrap the error reason into error that could be processed by ABCIInfo. It later is marshalled into JSON and put into log field of abci.ExecTxResult. Where it can be indexed by Ethermint's custom Tx indexer. We are moving away from pulling directly from CometBFT internal indexer for all EVM txns. Ameneded the tests to cover the new error handling and forwarding. TODO: tx_info_test is originally bad, needs a rewrite.
WalkthroughThe pull request introduces significant modifications across multiple GitHub Actions workflow files, primarily by removing the Changes
Possibly related PRs
Suggested reviewers
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
No dependency changes detected. Learn more about Socket for GitHub ↗︎ 👍 No dependency changes detected in pull request |
Also adjust the tests. TODO: implement eth_getBlockReceipts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Outside diff range and nitpick comments (35)
.github/workflows/deploy-contract.yml (1)
Line range hint
22-27: LGTM: Added step ID for potential future useAdding an
idto theget-diff-actionstep is a good practice. It allows for referencing this step's outputs in subsequent steps if needed.Consider utilizing the
git_diffid in subsequent steps if you need to access any outputs from this action. For example:- name: Use diff output if: steps.git_diff.outputs.diff run: echo "Changes detected in specified patterns"indexer/kv_indexer_test.go (2)
Line range hint
60-73: LGTM! Consider adding a comment explaining the test case.The changes improve the test case by adding more comprehensive attributes, particularly the
txHash. This aligns well with the PR objectives of enhancing error handling and logging.Consider adding a brief comment above this test case to explain what specific scenario it's testing. This would improve the test's maintainability and readability.
113-113: LGTM! Consider adding a clarifying comment.The change of
expSuccesstotruefor the VM revert test case aligns with the new error handling mechanism, where failed transactions are successfully processed and indexed.Consider adding a brief comment explaining why
expSuccessistruefor a failed transaction test case. This would help prevent confusion for other developers reviewing or modifying the tests in the future.rpc/types/parsed_tx_test.go (3)
13-137: LGTM: Well-structured test cases with good coverage.The test function and test cases are well-organized, following best practices for table-driven tests. The various scenarios covered, including success and failure cases, provide good test coverage.
Consider adding a brief comment above the
testCasesslice to explain the overall purpose of these test cases and what aspects ofParseTxResultthey are testing. This would enhance the readability and maintainability of the test suite.
139-158: LGTM: Comprehensive test execution with good error handling.The test execution loop is well-structured, covering both success and error cases. The assertions are thorough, checking various aspects of the parsed result.
For error cases, consider adding more specific assertions on the error message or type. This would ensure that not only an error occurs, but it's the expected type of error. For example:
require.Error(t, err) require.Contains(t, err.Error(), "expected error message")This addition would make the tests even more robust and specific.
1-159: Great job on the comprehensive test suite forParseTxResult.This test file is well-structured and provides good coverage for the
ParseTxResultfunction. The use of table-driven tests makes it easy to maintain and extend. The various scenarios tested, including success cases, failed transactions, and error conditions, demonstrate a thorough approach to testing.For future improvements, consider:
- Adding more edge cases, such as testing with empty or malformed event attributes.
- Implementing property-based testing for generating a wider range of inputs.
- If not already present, ensure there are integration tests that use these parsed results in the context of the larger system.
These additions would further enhance the robustness of the test suite and provide even greater confidence in the
ParseTxResultfunction's behavior across a wider range of scenarios.x/evm/types/utils.go (3)
113-122: LGTM! Consider adding a comment for clarity.The changes improve error handling by returning an empty slice instead of an error when no logs are found. This aligns well with the PR objectives.
Consider adding a comment to explain why we're initializing an empty slice when no logs are found. For example:
// Initialize an empty slice if no logs are found for the specified message index if len(logs) == 0 { logs, err = TxLogsFromEvents(events, msgIndex) }
141-142: LGTM! Consider using a pre-allocated slice for consistency.The change to return an empty slice instead of an error when no logs are found is a good improvement. It simplifies error handling and is consistent with the changes in
DecodeMsgLogsFromEvents.For consistency with other parts of the codebase, consider using a pre-allocated slice:
return make([]*ethtypes.Log, 0), nilThis approach is more idiomatic in Go and consistent with how empty slices are typically returned in other parts of the codebase.
Line range hint
1-290: Summary of changes and their impactThe modifications in this file focus on improving error handling for cases where no logs are found during Ethereum transaction processing. The changes align well with the PR objectives of enhancing error handling during message processing.
Key improvements:
- Functions now return empty slices instead of errors when no logs are found.
- This approach simplifies error handling and improves code robustness.
- The changes are consistent across the modified functions.
These modifications contribute to a more resilient and predictable behavior in the EVM module, especially when dealing with transactions that don't produce logs.
Consider applying this pattern of returning empty slices instead of errors (when appropriate) to other parts of the codebase that deal with similar scenarios. This could further improve consistency and simplify error handling throughout the project.
rpc/backend/blocks.go (3)
Line range hint
257-282: Consider the implications of including failed transactionsThe function
EthMsgsFromTendermintBlockhas been simplified, but it now includes all transactions without checking their success status. This change might lead to including failed transactions in the result, which could affect downstream processing or API responses.Additionally, the log level for transaction decoding failures has been elevated from debug to warn. While this increases visibility of such failures, it might also increase log noise if decoding failures are common.
Consider the following suggestions:
- Add a comment explaining why failed transactions are now included, if this is intentional.
- If including failed transactions is not intentional, consider reintroducing a check for transaction success.
- Evaluate if warn is the appropriate log level for decoding failures, or if it should remain at debug level to reduce potential log noise.
Line range hint
1-500: Overall code review summaryThe changes in this file primarily focus on simplifying transaction processing and updating gas calculation methods. While these changes may improve efficiency, there are several points that require attention:
- The
EthMsgsFromTendermintBlockfunction now includes all transactions without checking their success status, which may have downstream implications.- Gas calculation has been updated to sum individual transaction gas usage, which appears more accurate but should be verified.
- Several TODO comments have been added, indicating pending work on transaction receipts and potentially unnecessary code.
To improve the overall quality and completeness of this implementation:
- Clearly document the rationale behind including all transactions, including failed ones, in
EthMsgsFromTendermintBlock.- Verify the accuracy and efficiency of the new gas calculation method.
- Create specific tasks or issues to address all TODO comments, ensuring that transaction receipts are properly implemented and any unnecessary code is removed.
- Consider implementing comprehensive unit tests to validate the behavior of these modified functions, especially around transaction inclusion and gas calculation.
Action Required: Address
GetGasUsedMethod Return Type and TODO Comments
- The
GetGasUsed()method currently returns astringbut is being converted touint64inrpc/backend/blocks.go. This direct conversion is invalid and needs to be corrected, potentially usingstrconv.ParseUint.- Ensure that all gas calculations consistently handle data types to prevent runtime errors.
- Address the TODO comments:
- Implement the addition of transaction receipts or create a separate issue to track this task.
- Investigate whether the referenced step is still necessary and remove the TODO comment accordingly.
🔗 Analysis chain
Line range hint
436-441: Review gas calculation and address TODO commentsThe gas calculation has been updated to sum the gas used by each transaction, which appears to be a more accurate method. However, there are two TODO comments that need attention:
- A comment indicates that transaction receipts need to be added.
- Another comment questions whether a certain step is still needed.
Please address the following:
- Verify that the new gas calculation method is correct and efficient.
- Implement the addition of transaction receipts or create a separate issue to track this task.
- Investigate whether the step mentioned in the second TODO comment is still necessary. If it is, remove the comment; if not, remove the unnecessary code.
To verify the gas calculation, run the following script:
Also applies to: 496-498
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the gas calculation method # Test: Check if GetGasUsed() method exists and is used correctly rg --type go 'func \(.*\) GetGasUsed\(\) int64' rg --type go 'txsResult\.GetGasUsed\(\)' # Test: Look for any other gas calculation methods that might be relevant rg --type go 'gas.*used'Length of output: 9317
Script:
#!/bin/bash # Description: Verify the existence and correct usage of GetGasUsed method # Search for the definition of GetGasUsed method in Go files rg --type go 'func \(.*\) GetGasUsed\(\) uint64' # Search for all usages of txsResult.GetGasUsed() in Go files rg --type go 'txsResult\.GetGasUsed\(\)' # Additionally, verify if gasUsed is utilized correctly after calculation rg --type go 'gasUsed\s*\+\='Length of output: 284
Script:
#!/bin/bash # Description: Verify the existence and correct usage of GetGasUsed method with corrected regex # Search for the definition of GetGasUsed method in Go files rg --type go 'func \(.*\) GetGasUsed\(\) uint64' # Search for all usages of txsResult.GetGasUsed() in Go files rg --type go 'txsResult\.GetGasUsed\(\)' # Additionally, verify if gasUsed is utilized correctly after calculation with corrected regex rg --type go 'gasUsed\s*\+\='Length of output: 284
Script:
#!/bin/bash # Description: Verify the existence and correct usage of GetGasUsed method with corrected regex # Search for the definition of GetGasUsed method in Go files with any return type rg --type go 'func \(.*\) GetGasUsed\(\) .*' # Search for all usages of txsResult.GetGasUsed() in Go files rg --type go 'txsResult\.GetGasUsed\(\)' # Additionally, verify if gasUsed is utilized correctly after calculation with corrected regex rg --type go 'gasUsed\s*+='Length of output: 409
server/config/config.go (1)
295-296: LGTM. Consider updating documentation.The changes to enable the indexer and allow indexer gaps by default align with the PR objectives. This modification enhances the default JSON-RPC configuration to support custom indexing capabilities.
Consider the following recommendations:
- Update any relevant documentation or user guides to reflect these new default settings.
- Ensure that performance implications of enabling the indexer by default have been considered and tested.
- Document the trade-offs of allowing indexer gaps (potential data inconsistency vs. improved performance) for users who might want to disable this feature.
server/start.go (1)
218-218: Consider the implications of enabling the JSON-RPC indexer by defaultThe change to enable the JSON-RPC indexer by default aligns with the PR objectives. However, consider the following points:
- Performance impact: Enabling the indexer by default may increase resource usage. Ensure that this change doesn't negatively impact system performance, especially for nodes with limited resources.
- Backwards compatibility: Users upgrading from previous versions might experience unexpected behavior. Consider adding a note in the changelog or upgrade guide.
- Documentation: Update any relevant documentation to reflect this change in default behavior.
- Configuration flexibility: Ensure that users can still disable the indexer easily if needed.
Consider adding a comment explaining the rationale behind enabling the indexer by default, to provide context for future developers.
testutil/tx/eth.go (2)
44-44: Define a constant for the magic number136in gas calculation.The use of the magic number
136in the gas calculation can be unclear to readers. Defining it as a constant with a descriptive name will improve code readability and maintainability.const extraGasForTest = 136 // Additional gas to account for unrefunded gas in tests // ... Gas: params.TxGasContractCreation + extraGasForTest, // accurate accounting, since test cannot refund
66-66: Define a constant or add a comment to explain the multiplier4in gas calculation.In the gas calculation, the multiplier
4is used withparams.TxDataNonZeroGasEIP2028. To enhance clarity, consider defining it as a constant or adding a comment explaining its significance.// Number of non-zero data bytes in "deadbeef" const dataByteCount = 4 // 'deadbeef' is 4 bytes // ... Gas: params.TxGas + params.TxDataNonZeroGasEIP2028*dataByteCount, // accurate accounting, since test cannot refundx/evm/keeper/msg_server.go (1)
143-143: Avoid unnecessary copying of byte slicesThe
common.CopyBytes(ret)function creates an extra copy of theretbyte slice. Sinceabi.UnpackRevertdoes not modify the input byte slice, you can passretdirectly to improve performance.Apply this diff to remove the unnecessary copy:
func unpackRevertReason(ret []byte) (string, bool) { - retBytes := common.CopyBytes(ret) - reason, errUnpack := abi.UnpackRevert(retBytes) + reason, errUnpack := abi.UnpackRevert(ret)rpc/types/parsed_tx.go (5)
154-154: Avoid informal language in commentsThe comment
"ugly hacks ahead... thanks to errors.Wrapf in BaseApp"contains informal language that may not be suitable for production code. Consider rephrasing it to maintain a professional tone.Suggested change:
-// ugly hacks ahead... thanks to errors.Wrapf in BaseApp +// Workaround due to errors.Wrapf in BaseApp
94-94: Address the TODO: Implement proper code matchingThere's a TODO comment indicating the need for proper code matching. Implement the necessary logic to handle error codes accurately to ensure robust error handling.
Would you like assistance in implementing the code matching logic or opening a GitHub issue to track this task?
100-100: Address the TODO: Verify gas usage calculationThe TODO comment suggests uncertainty about replacing
gasUsedwithgasLimit. This logic affects gas accounting and should be verified to ensure correctness.Do you need assistance in verifying this logic or opening a GitHub issue to track this task?
20-20: Consider standardizing error handling importsThe code imports both the standard
"errors"package and"errorsmod"from"cosmossdk.io/errors". For consistency, consider using a single error handling package throughout the codebase.If appropriate, refactor the code to use only
"errorsmod"for error wrapping and creation.Also applies to: 25-25
175-175: Enhance error context when unmarshalling JSONWhen wrapping the error in
errorsmod.Wrap, include the rawlogJSONor additional context to aid in debugging JSON unmarshal failures.Consider adjusting the error message for better clarity:
- err = errorsmod.Wrap(err, "failed to parse abci log as JSON") + err = errorsmod.Wrapf(err, "failed to parse abci log as JSON: %s", logJSON)indexer/kv_indexer.go (2)
64-75: Avoid shadowing the error variableThe deferred function parameter
errshadows the named return variableerrinIndexBlock, which can lead to confusion. Consider renaming the parameter to avoid shadowing and improve code clarity.Apply this diff to rename the parameter:
- defer func(err *error) { + defer func(pErr *error) {And update the references inside the function:
- *err = ee + *pErr = ee- *err = fmt.Errorf("panic during parsing block results: %v", e) + *pErr = fmt.Errorf("panic during parsing block results: %v", e)Then, pass the renamed parameter:
- }(&err) + }(&err)
125-125: Enhance error log with additional contextWhen logging the error "msg index not found in results", including the block height and transaction index can aid in debugging by providing more context about where the issue occurred.
Apply this diff to include the additional context:
- kv.logger.Error("msg index not found in results", "msgIndex", msgIndex) + kv.logger.Error( + "msg index not found in results", + "block", block.Height, + "txIndex", txIndex, + "msgIndex", msgIndex, + )x/evm/types/errors.go (1)
133-145: Clarify distinction betweencauseanderrfields invmErrorWithRetstructThe struct
vmErrorWithRetcontains bothcauseanderrfields of typeerror. It would be helpful to clarify the distinction between these two fields or consider renaming them to make their purposes clearer, to avoid confusion during error handling and debugging.x/evm/keeper/state_transition.go (1)
Line range hint
221-229: Update receipt status to 'Failed' ifPostTxProcessingfailsCurrently, if
PostTxProcessingreturns an error,receipt.Statusremains set toReceiptStatusSuccessful, which may misrepresent the transaction outcome. To accurately reflect the failure in the transaction receipt, consider settingreceipt.Statustoethtypes.ReceiptStatusFailedwhenPostTxProcessingfails.Apply this diff to update the receipt status:
if !res.Failed() { receipt.Status = ethtypes.ReceiptStatusSuccessful // Only call hooks if tx executed successfully. if err = k.PostTxProcessing(tmpCtx, msg, receipt); err != nil { // If hooks return error, revert the whole tx. res.VmError = types.ErrPostTxProcessing.Error() k.Logger(ctx).Error("tx post processing failed", "error", err) // If the tx failed in post processing hooks, we should clear the logs res.Logs = nil + receipt.Status = ethtypes.ReceiptStatusFailed } else if commit != nil { // PostTxProcessing is successful, commit the tmpCtx commit() // Since the post-processing can alter the log, we need to update the result res.Logs = types.NewLogsFromEth(receipt.Logs) } } else { receipt.Status = ethtypes.ReceiptStatusFailed }x/evm/keeper/state_transition_test.go (2)
Line range hint
468-472: Use of negative gas price in test caseIn the test case
"invalid Gas value in msg", you are using a negative gas pricebig.NewInt(-100)when creating the message. In Ethereum, gas prices are expected to be non-negative integers. While it's important to test edge cases, using a negative gas price may not produce valid or meaningful results and could lead to unexpected behavior.Consider revising the test to use a minimum gas price of zero or explicitly handle the negative gas price scenario to ensure the test behaves as intended.
702-702: Typo in comment and unresolved TODOThere's a typo in the comment:
"error matcing"should be"error matching". Additionally, theTODOcomment indicates that this test is checking for the wrong error, but it hasn't been addressed yet.Please correct the typo and resolve the
TODOby updating the test to check for the correct error. This will improve code readability and ensure the test is accurate.x/evm/handler_test.go (1)
67-67: Simplify total supply calculationCurrently, you're adding
coinsthree times tobankGenesis.Supply:bankGenesis.Supply = bankGenesis.Supply.Add(coins...).Add(coins...).Add(coins...)To improve readability and maintainability, consider summing the coins first and adding them in a single
Addcall:totalCoins := sdk.NewCoins(coins...).Add(coins...).Add(coins...) bankGenesis.Supply = bankGenesis.Supply.Add(totalCoins...)Alternatively, you can multiply the coin amounts by 3 if applicable.
rpc/backend/blocks_test.go (6)
Line range hint
87-103: Ensure Proper Error Handling in "pass - block with tx - with ShouldIgnoreGasUsed" Test CaseIn the test case
"pass - block with tx - with ShouldIgnoreGasUsed", starting at line 87, theTxsResultscontains anExecTxResultwithCode: 11andLog: "no block gas left to run tx: out of gas". This seems to simulate a scenario where the transaction runs out of gas. However, the test case expects a passing result.Consider verifying whether the test should pass when the transaction has failed due to out-of-gas error. If the intention is to handle failed transactions gracefully, ensure that the test assertions correctly reflect the expected behavior.
1160-1161: Address the TODO: Include Tests for Failed VM Execution and RevertsAt lines 1160-1161, there is a TODO comment indicating that tests for failed VM execution and reverts need to be included.
Would you like assistance in writing these test cases? I can help generate the necessary code to cover scenarios where the VM execution fails or transactions are reverted. Alternatively, I can open a GitHub issue to track this task.
Line range hint
987-1013: Improve Clarity in Error Simulation for Gas UsageIn the test case
"pass - block with tx - with ShouldIgnoreGasUsed", starting at line 987, the simulation of the transaction failure due to gas issues may need clarification.To enhance the clarity and maintainability of the test, consider the following refactor:
- Use descriptive constants or variables for error codes and log messages.
- Add comments explaining why a transaction with
Code: 11and specificLogmessage is expected to pass.This will help future developers understand the purpose of the test case and the conditions under which it should pass.
Line range hint
12-24: Consistent Use of Mock Registration FunctionsIn the test cases within the
TestBlockNumberfunction, ensure that all mock registrations consistently handle error scenarios.Some test cases register mocks using functions like
RegisterParamsInvalidHeight, while others may lack comprehensive error registrations. Ensure that each test case accurately sets up the expected state, including any necessary error conditions, to reliably test the intended functionality.
Line range hint
1057-1075: Handle Potential Nil Pointer Dereference inHeaderByNumberIn the
HeaderByNumberfunction, there is a risk of a nil pointer dereference ifexpResultBlockis nil whenexpPassis true.Ensure that
expResultBlockis properly initialized before it is used. Add checks to handle cases where the block retrieval might fail, even whenexpPassis expected to be true.
Line range hint
1164-1165: Remove Redundant Empty LinesThere are redundant empty lines at lines 1164-1165.
Consider removing unnecessary empty lines to keep the code concise and improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (26)
- .github/workflows/build.yml (0 hunks)
- .github/workflows/deploy-contract.yml (1 hunks)
- .github/workflows/test.yml (0 hunks)
- indexer/kv_indexer.go (5 hunks)
- indexer/kv_indexer_test.go (3 hunks)
- rpc/backend/blocks.go (3 hunks)
- rpc/backend/blocks_test.go (3 hunks)
- rpc/backend/tracing.go (1 hunks)
- rpc/backend/tracing_test.go (0 hunks)
- rpc/backend/tx_info.go (8 hunks)
- rpc/namespaces/ethereum/eth/api.go (1 hunks)
- rpc/types/events_test.go (0 hunks)
- rpc/types/parsed_tx.go (4 hunks)
- rpc/types/parsed_tx_test.go (1 hunks)
- server/config/config.go (1 hunks)
- server/indexer_cmd.go (0 hunks)
- server/start.go (1 hunks)
- testutil/tx/eth.go (2 hunks)
- x/evm/handler_test.go (6 hunks)
- x/evm/keeper/msg_server.go (2 hunks)
- x/evm/keeper/msg_server_test.go (5 hunks)
- x/evm/keeper/state_transition.go (2 hunks)
- x/evm/keeper/state_transition_test.go (3 hunks)
- x/evm/types/errors.go (2 hunks)
- x/evm/types/response.go (2 hunks)
- x/evm/types/utils.go (2 hunks)
💤 Files with no reviewable changes (5)
- .github/workflows/build.yml
- .github/workflows/test.yml
- rpc/backend/tracing_test.go
- rpc/types/events_test.go
- server/indexer_cmd.go
🔇 Additional comments (40)
.github/workflows/deploy-contract.yml (2)
19-19: LGTM: Consistent quote usage in YAMLThe change from single quotes to double quotes for the node version is a good practice. It improves consistency and aligns with YAML best practices.
Line range hint
28-32: Verify the source ofenv.GIT_DIFFThe condition for running the "Test contract" step has been changed from
steps.git_diff.outputs.difftoenv.GIT_DIFF. However, it's not clear whereenv.GIT_DIFFis being set.Please clarify:
- Where is
env.GIT_DIFFbeing set?- Is this change intentional?
- Does this new condition correctly determine when to run the "Test contract" step?
To help verify this, you can run the following script:
If these searches don't yield results, consider reverting to the previous condition:
if: steps.git_diff.outputs.diffx/evm/keeper/msg_server_test.go (7)
4-4: LGTM: Import of "errors" package.The addition of the "errors" package import is appropriate and aligns with the PR objectives of enhancing error handling in the test cases.
39-39: LGTM: Enhanced error handling in test cases.Changing the
expErrfield from a boolean to anerrortype allows for more precise error checking in the test cases. This modification aligns well with the PR objectives of improving error handling during message processing.
53-54: LGTM: Specific error message for insufficient gas.The change to return a specific error message "intrinsic gas too low" for the insufficient gas scenario enhances the test case's clarity and aligns with the PR objectives of providing more detailed error information.
55-69: LGTM: New test case for VM revert scenario.The addition of a new test case for handling a contract deployment transaction that results in a VM revert is a valuable enhancement to the test suite. It improves coverage of error scenarios and aligns with the PR objectives of enhancing error handling during message processing.
98-100: LGTM: Dynamic gas calculation in transfer funds test case.Updating the expected gas used to
msg.GetGas()instead of a constant value improves the accuracy of the test case. This change makes the test more robust and aligns with the PR objectives of enhancing transaction handling.
112-113: LGTM: Enhanced error checking logic.The update to use
ErrorContainsinstead of a simple boolean check for errors improves the precision of error verification in the test cases. This change aligns well with the PR objectives of enhancing error handling and providing more specific error information.
Line range hint
1-165: Overall assessment: Excellent improvements to error handling and test coverage.The changes in this file consistently enhance error handling, improve test case accuracy, and align well with the PR objectives. The modifications include:
- More specific error checking with the use of error types instead of booleans.
- Addition of a new test case for VM revert scenarios.
- Dynamic gas calculation in the transfer funds test case.
- Enhanced error checking logic using
ErrorContains.These improvements contribute to a more robust and informative test suite, which will help maintain the reliability of the EVM implementation.
indexer/kv_indexer_test.go (3)
104-113: LGTM! Verify error code and consider adding a comment for the return value.The updates to this test case provide comprehensive error reporting for VM reverts, aligning well with the PR objectives. The detailed JSON-formatted error in the Log field, including the base64-encoded return value, offers valuable debugging information.
Please confirm the following:
- The change in the error Code from 13 to 3 corresponds to the correct error code for VM reverts.
- The new Log format is consistent with the expected error reporting structure.
You can verify these by checking the relevant code:
#!/bin/bash # Search for VM revert error code definitions rg --type go "const.*ErrorCode.*= (3|13)" ./ # Search for the error log formatting function rg --type go "func.*formatErrorLog" ./Consider adding a comment explaining the significance of the base64-encoded return value in the Log field. This would improve the test's maintainability and help other developers understand the purpose of this data.
80-87: LGTM! Verify the error code change.The updates to this test case improve clarity and align with the PR objectives. The more specific error message in the Log field enhances the test's accuracy.
Please confirm that the change in the error Code from 12 to 11 is intentional and corresponds to the correct error code for exceeding the block gas limit. You can verify this by checking the error codes defined in the project:
92-100: LGTM! Verify error code and log format.The updates to this test case significantly improve the error reporting for VM errors, aligning well with the PR objectives. The detailed JSON-formatted error in the Log field provides valuable debugging information.
Please confirm the following:
- The change in the error Code from 13 to 15 corresponds to the correct error code for VM errors.
- The new Log format is consistent with the expected error reporting structure.
You can verify these by checking the relevant code:
rpc/types/parsed_tx_test.go (1)
1-11: LGTM: Appropriate package and imports.The package declaration and imports are well-structured and relevant to the test file's purpose. All imported packages are utilized in the test cases.
x/evm/types/utils.go (1)
Line range hint
168-190: Verify changes mentioned in the AI summary.The AI-generated summary mentioned changes to the
DecodeTxLogsFromEventsfunction, but there are no visible modifications in the provided code. This discrepancy might be due to the diff not showing removed lines or old code.Could you please confirm if there were any changes made to this function? If changes were made, please provide the updated code for review.
rpc/namespaces/ethereum/eth/api.go (1)
460-460: Approved: Increased visibility for transaction log parsing failuresThe change from
DebugtoWarnlog level for transaction log parsing failures is a good improvement. It will make these issues more visible, potentially helping to identify and resolve problems more quickly in production environments.However, please consider the following:
- Are these parsing failures common or expected in certain scenarios?
- Could this change lead to excessive log entries in some cases?
To help assess the impact, let's check how often this error occurs in the codebase:
This will help determine if this error handling approach is consistent across the codebase and if similar errors are logged at the Warn level.
✅ Verification successful
Verified: Consistent warning level for transaction log parsing failures
The search results confirm that "failed to parse" errors are consistently logged at the
Warnlevel across the codebase. This ensures increased visibility without causing unnecessary log clutter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for similar error logging patterns in the codebase rg --type go 'logger\.(Debug|Info|Warn|Error)\s*\(\s*"failed to parse'Length of output: 271
server/start.go (1)
218-218: Verify handling of the new default value throughout the fileWhile the change to enable the JSON-RPC indexer by default is localized to this line, it's important to ensure that the rest of the file, particularly the
startInProcessfunction, handles this new default value correctly.Please run the following script to check for any potential issues:
This will help ensure that all relevant parts of the code are consistent with the new default behavior.
✅ Verification successful
Handling of JSONRPCEnableIndexer default value is correct
No issues found with the handling of the new default value for
JSONRPCEnableIndexerthroughout theserver/start.gofile.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for potential issues with the new default value echo "Checking for potential issues with JSONRPCEnableIndexer:" rg --type go "JSONRPCEnableIndexer" server/start.go -C 5 echo "Checking if there are any conditionals that might need updating:" rg --type go "if.*config.JSONRPC.EnableIndexer" server/start.go -C 5Length of output: 2885
x/evm/types/response.go (1)
Line range hint
31-67: Ensure removal of ante event handling does not affect dependent codeThe removal of the handling of ante events simplifies the transaction data processing, aligning with the PR objectives. However, it's important to verify that no other parts of the codebase depend on these ante events for processing or logging. This will prevent any unexpected behaviors in components that might consume these events.
Run the following script to check for any references to
anteEventsor dependencies on ante events in the codebase:testutil/tx/eth.go (3)
35-77: Ensure accurate gas calculations for transaction tests.The gas calculations in
CreateRevertingContractMsgTxandCreateNoCodeCallMsgTxinclude additional amounts to account for test environments where gas refunds are not possible. Verify that these calculations accurately reflect the gas usage to prevent unexpected out-of-gas errors during testing.
Line range hint
13-77: Overall implementation is correct and follows best practices.The functions correctly construct Ethereum transactions with the intended characteristics for testing scenarios. The use of
AccessListTxand setting appropriate fields align with Ethereum transaction standards.
Line range hint
13-28: Verify all references to the renamed functionCreateUnderpricedContractMsgTx.The function
CreateContractMsgTxhas been renamed toCreateUnderpricedContractMsgTx. Please ensure that all references to the old function name have been updated throughout the codebase to prevent any broken references or build issues.Run the following script to check for any remaining references to
CreateContractMsgTx:x/evm/keeper/msg_server.go (2)
32-33: Approved: Added necessary imports for ABI and common typesThe imports
github.com/ethereum/go-ethereum/accounts/abiandgithub.com/ethereum/go-ethereum/commonare essential for handling Ethereum ABI interactions and common types, respectively.
136-136: Verify the implications of returning bothresponseandmsgErrReturning a non-nil
responsealongside an error may lead to ambiguity for the caller regarding which data to trust. Confirm that this pattern is consistent with the rest of the codebase and that callers correctly handle this scenario.Consider whether it's appropriate to return
nilfor theresponsewhen an error occurs:- return response, msgErr + return nil, msgErrRun the following script to identify how
EthereumTxresponses are handled elsewhere:✅ Verification successful
Consistent Error Handling Confirmed
The pattern of returning both
responseandmsgErris consistently handled across the codebase. Callers appropriately check for errors before utilizing the response.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for handling of non-nil responses when errors are returned. # Search for usages of EthereumTx and inspect error handling patterns. rg --type go 'EthereumTx\(' -A 5Length of output: 25135
rpc/backend/tracing.go (1)
156-156: Initialization of transaction decoder is correctThe transaction decoder is properly initialized to decode transactions efficiently.
x/evm/types/errors.go (3)
19-19: LGTMThe import of
"encoding/json"is necessary for JSON encoding used in error handling.
122-131: InterfaceVmErrordefinition looks goodThe
VmErrorinterface is well-defined and includes methods for retrieving error details, which will aid in error handling and ABCIInfo integration.
270-275: LGTMThe assignment of
err := vm.ErrExecutionRevertedin theNewExecErrorWithReasonfunction is appropriate and ensures proper error handling.rpc/backend/tx_info.go (10)
22-22: ImportingabcitypesThe addition of
abci "github.com/cometbft/cometbft/abci/types"is necessary for usingabci.CodeTypeOKin the code.
160-160: Elevating log level fromDebugtoWarnThe log level for decoding failures has been changed from
DebugtoWarn. Confirm that this escalation is intentional, as it may increase log verbosity and impact monitoring systems.
174-174: Elevating log level fromDebugtoWarnThe log level for failing to retrieve block results has been elevated from
DebugtoWarn. Please ensure this aligns with the desired logging strategy.
206-206: Elevating log level toWarnfor log parsing failuresThe log level for parsing log failures has been set to
Warn. Verify that this change is appropriate and assess its impact on logging output.
321-321: Warning added for fallback to Tendermint transaction indexerA warning has been added when falling back to the Tendermint transaction indexer, indicating that failed transactions will not be available. Ensure this message accurately reflects the system behavior.
339-339: Warning added for fallback to Tendermint transaction indexerSimilar to line 321, a warning has been added for
GetTxByTxIndex. Confirm that this warning is necessary and properly communicates any limitations to users.
367-371: Review the logic in theifconditionThe
ifcondition has been modified:if rpctypes.TxSuccessOrExceedsBlockGasLimit(&txResult.TxResult) && txResult.TxResult.Code != abci.CodeTypeOK && txResult.TxResult.Codespace != evmtypes.ModuleName {Please verify that this logic correctly captures the intended scenarios, especially with the inclusion of
txResult.TxResult.Code != abci.CodeTypeOKandtxResult.TxResult.Codespace != evmtypes.ModuleName.
394-394: Elevating log level toWarnfor invalid Ethereum transactionThe log level for invalid Ethereum transactions has been elevated to
Warn. Confirm that this change aligns with logging policies and won't cause excessive warnings.
402-402: Elevating log level toWarnfor invalid Ethereum transactionAs with line 394, the log level change to
Warnaffects another instance of invalid Ethereum transaction handling. Ensure consistency with logging guidelines.
409-409: Added warning for block transaction index out of boundsA warning log has been added when the block transaction index is out of bounds. This helps in diagnosing indexing issues.
x/evm/keeper/state_transition_test.go (1)
735-739: Method renamed; verify all references are updatedThe method
createUnderpricedContractGethMsghas been renamed fromcreateContractGethMsg. To prevent build errors or undefined references, please ensure that all instances of the old method name have been updated throughout the codebase.Run the following script to search for any remaining references to the old method name:
✅ Verification successful
All references to
createContractGethMsghave been successfully updated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old method name in Go files. # Test: Find any occurrences of 'createContractGethMsg'. Expect: No results if all references are updated. rg --type go 'createContractGethMsg'Length of output: 36
Script:
#!/bin/bash # Description: Use ast-grep to search for any references to 'createContractGethMsg' in Go files. # Test: Ensure no function calls or definitions of 'createContractGethMsg' exist. ast-grep --lang go --pattern $'function_createContractGethMsg($_)' || echo "No references found."Length of output: 67
Script:
#!/bin/bash # Description: Search for any remaining references to 'createContractGethMsg' in Go files. # Search for function declarations echo "Searching for function declarations of 'createContractGethMsg':" rg --type go 'func\s+createContractGethMsg\s*\(' # Search for function calls echo "Searching for function calls to 'createContractGethMsg':" rg --type go 'createContractGethMsg\s*\('Length of output: 366
x/evm/handler_test.go (3)
20-20: Import statement correctly addedThe import of
"github.com/ethereum/go-ethereum/core/vm"is necessary for handling Ethereum Virtual Machine (EVM) errors within the tests.
153-153: Transaction success assertion is appropriateThe check ensures that the transaction did not fail unexpectedly:
suite.Require().False(res.Failed())This assertion correctly verifies the expected successful outcome of the transaction.
302-302: Transaction success assertion is appropriateSimilarly, this line confirms that the transaction was processed without failure:
suite.Require().False(result.Failed())This is a good practice to ensure test reliability.
| // TODO(max): check if this still needed | ||
| ethBlock := ethtypes.NewBlockWithHeader(ethHeader).WithBody(ethtypes.Body{Transactions: txs}) | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Implement Transaction Receipts in blocks.go
The rpc/backend/blocks.go file currently lacks the implementation for adding transaction receipts to ethBlock:
// TODO: add tx receiptsTo ensure complete block information, please:
- Add transaction receipts to the
ethBlockconstruction. - Review the necessity of the second TODO comment:
Determine if the referenced step is required. If it is, proceed with the implementation; if not, remove the obsolete code and the TODO comment.
// TODO(max): check if this still needed
🔗 Analysis chain
Address duplicate TODO comments
The TODO comments in this function are identical to those in the RPCBlockFromTendermintBlock function:
- A comment indicates that transaction receipts need to be added.
- Another comment questions whether a certain step is still needed.
Please address the following:
- Determine if these TODOs are distinct from those in
RPCBlockFromTendermintBlock. If they are the same, consider consolidating the work. - Implement the addition of transaction receipts or create a separate issue to track this task.
- Investigate whether the step mentioned in the second TODO comment is still necessary. If it is, remove the comment; if not, remove the unnecessary code.
To check for any existing implementation of transaction receipts, run:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for existing implementation of transaction receipts
# Test: Search for any existing code related to transaction receipts
rg --type go 'tx.*receipt'
Length of output: 9317
Script:
#!/bin/bash
# Description: Retrieve lines surrounding the TODO comment in rpc/backend/blocks.go
sed -n '490,510p' rpc/backend/blocks.go
Length of output: 345
| func CreateRevertingContractMsgTx( | ||
| nonce uint64, | ||
| signer ethtypes.Signer, | ||
| gasPrice *big.Int, | ||
| from common.Address, | ||
| keyringSigner keyring.Signer, | ||
| ) (*types.MsgEthereumTx, error) { | ||
| contractCreateTx := ðtypes.AccessListTx{ | ||
| GasPrice: gasPrice, | ||
| Gas: params.TxGasContractCreation + 136, // accurate accounting, since test cannot refund | ||
| To: nil, | ||
| Data: common.Hex2Bytes("deadbeef"), | ||
| Nonce: nonce, | ||
| } | ||
| ethTx := ethtypes.NewTx(contractCreateTx) | ||
| ethMsg := &types.MsgEthereumTx{} | ||
| ethMsg.FromEthereumTx(ethTx) | ||
| ethMsg.From = from.Bytes() | ||
|
|
||
| return ethMsg, ethMsg.Sign(signer, keyringSigner) | ||
| } | ||
|
|
||
| func CreateNoCodeCallMsgTx( | ||
| nonce uint64, | ||
| signer ethtypes.Signer, | ||
| gasPrice *big.Int, | ||
| from common.Address, | ||
| keyringSigner keyring.Signer, | ||
| ) (*types.MsgEthereumTx, error) { | ||
| contractCreateTx := ðtypes.AccessListTx{ | ||
| GasPrice: gasPrice, | ||
| Gas: params.TxGas + params.TxDataNonZeroGasEIP2028*4, // accurate accounting, since test cannot refund | ||
| To: &common.Address{}, // no code exists - considered EOA | ||
| Data: common.Hex2Bytes("deadbeef"), // 4byte selector | ||
| Nonce: nonce, | ||
| } | ||
| ethTx := ethtypes.NewTx(contractCreateTx) | ||
| ethMsg := &types.MsgEthereumTx{} | ||
| ethMsg.FromEthereumTx(ethTx) | ||
| ethMsg.From = from.Bytes() | ||
|
|
||
| return ethMsg, ethMsg.Sign(signer, keyringSigner) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to eliminate duplicated code in transaction creation functions.
The functions CreateUnderpricedContractMsgTx, CreateRevertingContractMsgTx, and CreateNoCodeCallMsgTx share similar code structures when constructing Ethereum transactions. Consider refactoring to extract common logic into a helper function to improve maintainability and reduce code duplication.
For example, you can create a helper function like createEthMsgTx:
func createEthMsgTx(
txData *ethtypes.AccessListTx,
signer ethtypes.Signer,
from common.Address,
keyringSigner keyring.Signer,
) (*types.MsgEthereumTx, error) {
ethTx := ethtypes.NewTx(txData)
ethMsg := &types.MsgEthereumTx{}
ethMsg.FromEthereumTx(ethTx)
ethMsg.From = from.Bytes()
return ethMsg, ethMsg.Sign(signer, keyringSigner)
}Then refactor the existing functions to use this helper:
func CreateRevertingContractMsgTx(
nonce uint64,
signer ethtypes.Signer,
gasPrice *big.Int,
from common.Address,
keyringSigner keyring.Signer,
) (*types.MsgEthereumTx, error) {
contractCreateTx := ðtypes.AccessListTx{
GasPrice: gasPrice,
Gas: params.TxGasContractCreation + extraGasForTest, // use constant for clarity
To: nil,
Data: common.Hex2Bytes("deadbeef"),
Nonce: nonce,
}
return createEthMsgTx(contractCreateTx, signer, from, keyringSigner)
}| if len(parts) != 3 { | ||
| return errors.New("failed to locate message error in abci log") | ||
| } |
There was a problem hiding this comment.
Check for potential panic due to unexpected log format
The condition len(parts) != 3 might not cover all cases where parts could be less than expected, potentially leading to a panic when accessing parts[1] or parts[2].
Consider adding a more robust check or handling to prevent potential panics.
| return ParsedTx{MsgIndex: msgIndex, EthTxIndex: -1} | ||
| return ParsedTx{ | ||
| MsgIndex: msgIndex, | ||
| EthTxIndex: EthTxIndexUnitialized, |
There was a problem hiding this comment.
Update usage of the corrected constant name
After correcting the typo in the constant EthTxIndexUninitialized, ensure that its usage in the NewParsedTx function is updated accordingly.
Apply this diff to update the reference:
- EthTxIndex: EthTxIndexUnitialized,
+ EthTxIndex: EthTxIndexUninitialized,📝 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.
| EthTxIndex: EthTxIndexUnitialized, | |
| EthTxIndex: EthTxIndexUninitialized, |
| EthTxIndex int32 | ||
| GasUsed uint64 | ||
| Failed bool | ||
| } | ||
|
|
||
| const EthTxIndexUnitialized int32 = -1 |
There was a problem hiding this comment.
Typo in constant name 'EthTxIndexUnitialized'
The constant name EthTxIndexUnitialized contains a typo. It should be EthTxIndexUninitialized for clarity and correctness.
Apply this diff to fix the typo:
-const EthTxIndexUnitialized int32 = -1
+const EthTxIndexUninitialized int32 = -1📝 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.
| const EthTxIndexUnitialized int32 = -1 | |
| const EthTxIndexUninitialized int32 = -1 |
| // Error returns a JSON-encoded string representation of the VmErrorWithRet. | ||
| // This includes the transaction hash, gas used, revert reason (if available), | ||
| // and the VM error that occurred. For integration with ABCIInfo. | ||
| func (e *vmErrorWithRet) Error() string { | ||
| if e == nil { | ||
| return "" | ||
| } | ||
|
|
||
| return e.toJSON() | ||
| } |
There was a problem hiding this comment.
Potential PII leakage in Error() method
The Error() method returns a JSON-encoded string containing details such as the transaction hash (hash), gas used, revert reason, and VM error. Exposing this detailed information when the error is logged or returned could potentially lead to leakage of sensitive information. Please ensure that only necessary information is included in the error message, and consider sanitizing or omitting sensitive data to prevent PII leakage.
| // cause is the module-level error that can be used for ABCIInfo integration. | ||
| // vmErr is the VM error string associated with the VmError. | ||
| // ret is the revert reason bytes associated with the VmError (only if the VM error is ErrExecutionReverted). | ||
| func NewVmErrorWithRet( | ||
| vmErr string, | ||
| ret []byte, | ||
| hash string, | ||
| gasUsed uint64, | ||
| ) VmError { | ||
| e := &vmErrorWithRet{ | ||
| vmErr: vmErr, | ||
| hash: hash, | ||
| gasUsed: gasUsed, | ||
| } | ||
|
|
||
| if e.vmErr == vm.ErrExecutionReverted.Error() { | ||
| e.err = vm.ErrExecutionReverted | ||
|
|
||
| // store only if the VM error is ErrExecutionReverted | ||
| e.ret = common.CopyBytes(ret) | ||
|
|
||
| reason, errUnpack := abi.UnpackRevert(e.ret) | ||
| if errUnpack == nil { | ||
| e.err = fmt.Errorf("%s: %s", e.err.Error(), reason) | ||
| e.reason = reason | ||
| e.cause = errorsmod.Wrap(ErrExecutionReverted, e.toJSON()) | ||
| } | ||
| } else { | ||
| e.err = errors.New(e.vmErr) | ||
| e.cause = errorsmod.Wrap(ErrVMExecution, e.toJSON()) | ||
| } | ||
|
|
||
| return e | ||
| } |
There was a problem hiding this comment.
Review error wrapping for potential information exposure
In the NewVmErrorWithRet function, errors are wrapped using errorsmod.Wrap with e.toJSON() as the message. This includes detailed error information in the wrapped error message, which might expose sensitive data. Consider whether including the full JSON in the wrapped error is appropriate, and whether any sensitive information should be excluded or sanitized.
| @@ -484,7 +491,7 @@ func (suite *HandlerTestSuite) TestERC20TransferReverted() { | |||
| suite.Require().NoError(err) | |||
|
|
|||
| res, err := k.EthereumTx(suite.Ctx, tx) | |||
| suite.Require().NoError(err) | |||
| suite.Require().ErrorContains(err, tc.expErr) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using error type assertions for robustness
In line 494, you check if the error message contains a specific string:
suite.Require().ErrorContains(err, tc.expErr)Relying on error messages can be brittle. Consider using errors.Is to compare error types for more robust tests:
-suite.Require().ErrorContains(err, tc.expErr)
+suite.Require().True(errors.Is(err, expectedErrorType))| @@ -557,7 +564,7 @@ | |||
| suite.Require().NoError(db.Commit()) | |||
|
|
|||
| rsp, err := k.EthereumTx(suite.Ctx, tx) | |||
| suite.Require().NoError(err) | |||
| suite.Require().ErrorContains(err, vm.ErrOutOfGas.Error()) | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use error type comparison instead of error messages
In line 567, comparing error messages may lead to fragile tests if the message changes:
suite.Require().ErrorContains(err, vm.ErrOutOfGas.Error())Consider checking the error against the known error type using errors.Is:
-suite.Require().ErrorContains(err, vm.ErrOutOfGas.Error())
+suite.Require().True(errors.Is(err, vm.ErrOutOfGas))This approach makes the test more resilient to changes in error messages.
| { | ||
| Address: sdk.AccAddress(suite.Address.Bytes()).String(), | ||
| Coins: coins, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure consistent address encoding
In line 59, the address is converted using sdk.AccAddress(suite.Address.Bytes()).String(). For consistency and clarity, consider using suite.Address.String() directly, as suite.Address is already an sdk.AccAddress.
Apply this diff to simplify the address conversion:
{
- Address: sdk.AccAddress(suite.Address.Bytes()).String(),
+ Address: suite.Address.String(),
Coins: coins,
},📝 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.
| { | |
| Address: sdk.AccAddress(suite.Address.Bytes()).String(), | |
| Coins: coins, | |
| }, | |
| { | |
| Address: suite.Address.String(), | |
| Coins: coins, | |
| }, |
Thanks for review! Yeah I add TODO for myself as breadcrumbs, because I want to keep track the "bad" places in the codebase that need revisiting in the next PRs. Not something that is a direct dependency to have this PR valid. |
Description
The change is to return error during msg processing if VM reverts or errors out. It is a simple change, but it affects many places where it was expected to emit log.
Failing transactions wrap the error reason into error that could be processed by ABCIInfo.
It later is marshalled into JSON and put into log field of
abci.ExecTxResult. Where it canbe indexed by Ethermint's custom Tx indexer. We are moving away from pulling directly
from CometBFT internal indexer for all EVM transactions.
Amended the tests to cover the new error handling and forwarding.
Breaking changes
Logfield.Logcould be unmarshalled in a hacky PatchTxResponses that would then produce fake Ante handler events. But I decided to not implement that.trueTesting
Unit tests
Run project-wide unit tests with
make testfrom ethermint root, or individually:Note: this PR also activates an unit test workflow.
Blockscout
Results could be visible on our local blockscout.
E2E test from injective-core
Awaits implementation of interchaintest that adds E2E testing of JSON-RPC interaction.
See https://github.com/InjectiveLabs/injective-core/pull/1976
Note
Unit/integration test
tx_info_testis originally bad, needs a rewrite. Tested it manually E2E using RPCs.eth.getTransactioneth.getTransactionReceipteth.getBlockByNumberSummary by CodeRabbit
New Features
Bug Fixes
Documentation
Refactor
Chores