fix(evm): tx receipt proper marshalling #2101
Conversation
WalkthroughThe pull request introduces significant updates across various files in the Nibiru EVM project. Key changes include enhancements to the 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2101 +/- ##
==========================================
- Coverage 64.58% 64.57% -0.02%
==========================================
Files 271 271
Lines 21237 21246 +9
==========================================
+ Hits 13716 13719 +3
- Misses 6574 6575 +1
- Partials 947 952 +5
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (13)
evm-e2e/contracts/EventsEmitter.sol (3)
1-2: Consider pinning the exact Solidity version.Using
^0.8.24allows minor version updates which could potentially introduce subtle differences in behavior. For test contracts, it's safer to pin the exact version to ensure consistent behavior across different environments.-pragma solidity ^0.8.24; +pragma solidity 0.8.24;
4-5: LGTM! Consider a more specific event name.The event structure is well-designed with proper indexing of the sender address. However, since this is a test contract, consider renaming
TestEventto something more specific likeTransactionReceiptTestEventto better indicate its purpose in testing transaction receipt marshalling.- event TestEvent(address indexed sender, uint256 value); + event TransactionReceiptTestEvent(address indexed sender, uint256 value);
4-10: Well-designed test contract for transaction receipt verification.This contract is well-suited for testing the transaction receipt marshalling fix. Its simple event emission allows for easy verification of receipt fields like
fromaddress, while keeping the test focused and isolated.evm-e2e/hardhat.config.js (1)
6-6: Consider pinning the exact Solidity version.While using "0.8.24" works, it's recommended to use the exact version format (^0.8.24) to ensure consistent behavior across different environments.
- solidity: "0.8.24", + solidity: "^0.8.24",evm-e2e/contracts/TransactionReverter.sol (3)
1-5: Add NatSpec documentation for the contract and state variable.Consider adding comprehensive NatSpec documentation to improve code clarity and maintainability.
// SPDX-License-Identifier: MIT pragma solidity ^0.8.20; +/// @title TransactionReverter +/// @notice Contract for testing various transaction reversion scenarios +/// @dev Used for testing transaction receipt handling in the EVM contract TransactionReverter { + /// @notice Public state variable that gets modified before reverts uint256 public value;
7-7: Add a descriptive parameter name to the custom error.The custom error parameter should have a descriptive name for better clarity.
- error CustomRevertReason(uint256 providedValue); + error CustomRevertReason(uint256 attemptedValue);
16-19: LGTM! Consider adding input validation if needed.The implementation correctly uses custom errors for gas efficiency.
Consider adding input validation if there are any constraints on the
newValueparameter.evm-e2e/package.json (1)
Line range hint
14-30: Consider adding peer dependency versionsWhile the direct dependencies are specified with version constraints, you might want to add
peerDependenciesfor critical relationships, especially between Hardhat and its plugins.Add this section to ensure version compatibility:
"devDependencies": { ... }, + "peerDependencies": { + "hardhat": "^2.22.15", + "ethers": "^6.12.1" + }, "scripts": {evm-e2e/test/utils.ts (1)
57-68: Review production readiness of the utility function.A few concerns with the
sendTestNibifunction:
- Console logging should be removed or wrapped in a debug flag
- The 10s confirmation timeout might be too short for some networks
- await txResponse.wait(1, 10e3); - console.log(txResponse); + await txResponse.wait(1, 30e3); // Increase timeout to 30s return txResponse;evm-e2e/test/tx_receipt.test.ts (2)
25-37: Consider adding effectiveGasPrice validation.While this test effectively validates the
contractAddressfield (one of the PR objectives), consider adding validation for theeffectiveGasPricefield, which was also mentioned as a missing field in the PR objectives.assertBaseReceiptFields(receipt); expect(receipt.to).toBeNull(); // Contract creation has no 'to' address expect(receipt.contractAddress).toBeDefined(); + expect(receipt.effectiveGasPrice).toBeDefined(); + expect(receipt.effectiveGasPrice).toBeGreaterThan(0n);
66-99: Add effectiveGasPrice validation to base assertions.The helper function provides comprehensive validation of receipt fields, but it should also validate the
effectiveGasPricefield since it was one of the missing fields mentioned in the PR objectives.// Gas fields expect(typeof receipt.gasUsed).toBe('bigint'); expect(receipt.gasUsed).toBeGreaterThan(0n); expect(typeof receipt.cumulativeGasUsed).toBe('bigint'); expect(receipt.cumulativeGasUsed).toBeGreaterThanOrEqual(receipt.gasUsed); + expect(typeof receipt.effectiveGasPrice).toBe('bigint'); + expect(receipt.effectiveGasPrice).toBeGreaterThan(0n);eth/rpc/backend/tx_info_test.go (1)
Line range hint
194-220: Enhance test coverage for receipt marshalling.While the test verifies basic marshalling/unmarshalling, consider adding these improvements to align with the PR objectives:
- Verify the JSON output contains all required fields:
- Add assertions to check the presence of
to,from,effectiveGasPrice, andcontractAddress- Verify field values are preserved after unmarshalling:
- Add assertions comparing the original and unmarshalled values
Here's a suggested enhancement:
jsonBz, err := tr.MarshalJSON() s.Require().NoError(err) +// Verify JSON contains all required fields +var jsonMap map[string]interface{} +s.Require().NoError(json.Unmarshal(jsonBz, &jsonMap)) +s.Require().Contains(jsonMap, "to") +s.Require().Contains(jsonMap, "from") +s.Require().Contains(jsonMap, "effectiveGasPrice") +s.Require().Contains(jsonMap, "contractAddress") gethReceipt := new(gethcore.Receipt) err = json.Unmarshal(jsonBz, gethReceipt) s.Require().NoError(err) receipt := new(backend.TransactionReceipt) err = json.Unmarshal(jsonBz, receipt) s.Require().NoError(err) + +// Verify field values are preserved +s.Require().Equal(tr.From, receipt.From) +s.Require().Equal(tr.To, receipt.To) +s.Require().Equal(tr.EffectiveGasPrice, receipt.EffectiveGasPrice) +s.Require().Equal(tr.ContractAddress, receipt.ContractAddress)CHANGELOG.md (1)
87-87: Enhance the changelog entry for PR #2101.The current entry "fix(evm): tx receipt proper marshalling" is too terse. Consider expanding it to describe what was fixed and the impact:
- - [#2101](https://github.com/NibiruChain/nibiru/pull/2101) - fix(evm): tx receipt proper marshalling + - [#2101](https://github.com/NibiruChain/nibiru/pull/2101) - fix(evm): Add missing fields in transaction receipt marshalling (`to`, `from`, `effectiveGasPrice`, `contractAddress`). This ensures complete transaction data is returned to clients.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
evm-e2e/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (33)
CHANGELOG.md(1 hunks)eth/rpc/backend/tx_info.go(3 hunks)eth/rpc/backend/tx_info_test.go(1 hunks)evm-e2e/.prettierrc(1 hunks)evm-e2e/contracts/EventsEmitter.sol(1 hunks)evm-e2e/contracts/InfiniteLoopGasCompiled.json(0 hunks)evm-e2e/contracts/ReceiveNibiCompiled.json(0 hunks)evm-e2e/contracts/SendNibiCompiled.json(0 hunks)evm-e2e/contracts/TestERC20Compiled.json(0 hunks)evm-e2e/contracts/TransactionReverter.sol(1 hunks)evm-e2e/hardhat.config.js(1 hunks)evm-e2e/justfile(1 hunks)evm-e2e/package.json(2 hunks)evm-e2e/test/contract_infinite_loop_gas.test.ts(1 hunks)evm-e2e/test/contract_send_nibi.test.ts(2 hunks)evm-e2e/test/debug_queries.test.ts(1 hunks)evm-e2e/test/erc20.test.ts(1 hunks)evm-e2e/test/eth_queries.test.ts(1 hunks)evm-e2e/test/native_transfer.test.ts(1 hunks)evm-e2e/test/setup.ts(1 hunks)evm-e2e/test/tx_receipt.test.ts(1 hunks)evm-e2e/test/utils.ts(1 hunks)evm-e2e/types/ethers-contracts/InfiniteLoopGasCompiled.ts(0 hunks)evm-e2e/types/ethers-contracts/ReceiveNibiCompiled.ts(0 hunks)evm-e2e/types/ethers-contracts/SendNibiCompiled.ts(0 hunks)evm-e2e/types/ethers-contracts/TestERC20Compiled.ts(0 hunks)evm-e2e/types/ethers-contracts/common.ts(0 hunks)evm-e2e/types/ethers-contracts/factories/InfiniteLoopGasCompiled__factory.ts(0 hunks)evm-e2e/types/ethers-contracts/factories/ReceiveNibiCompiled__factory.ts(0 hunks)evm-e2e/types/ethers-contracts/factories/SendNibiCompiled__factory.ts(0 hunks)evm-e2e/types/ethers-contracts/factories/TestERC20Compiled__factory.ts(0 hunks)evm-e2e/types/ethers-contracts/factories/index.ts(0 hunks)evm-e2e/types/ethers-contracts/index.ts(0 hunks)
💤 Files with no reviewable changes (15)
- evm-e2e/contracts/InfiniteLoopGasCompiled.json
- evm-e2e/contracts/ReceiveNibiCompiled.json
- evm-e2e/contracts/SendNibiCompiled.json
- evm-e2e/contracts/TestERC20Compiled.json
- evm-e2e/types/ethers-contracts/InfiniteLoopGasCompiled.ts
- evm-e2e/types/ethers-contracts/ReceiveNibiCompiled.ts
- evm-e2e/types/ethers-contracts/SendNibiCompiled.ts
- evm-e2e/types/ethers-contracts/TestERC20Compiled.ts
- evm-e2e/types/ethers-contracts/common.ts
- evm-e2e/types/ethers-contracts/factories/InfiniteLoopGasCompiled__factory.ts
- evm-e2e/types/ethers-contracts/factories/ReceiveNibiCompiled__factory.ts
- evm-e2e/types/ethers-contracts/factories/SendNibiCompiled__factory.ts
- evm-e2e/types/ethers-contracts/factories/TestERC20Compiled__factory.ts
- evm-e2e/types/ethers-contracts/factories/index.ts
- evm-e2e/types/ethers-contracts/index.ts
✅ Files skipped from review due to trivial changes (8)
- evm-e2e/.prettierrc
- evm-e2e/test/contract_infinite_loop_gas.test.ts
- evm-e2e/test/contract_send_nibi.test.ts
- evm-e2e/test/debug_queries.test.ts
- evm-e2e/test/erc20.test.ts
- evm-e2e/test/eth_queries.test.ts
- evm-e2e/test/native_transfer.test.ts
- evm-e2e/test/setup.ts
🔇 Additional comments (17)
evm-e2e/contracts/EventsEmitter.sol (1)
7-9: LGTM! Implementation is appropriate for testing purposes.
The function correctly emits the event with the caller's address and provided value. The simple implementation is sufficient for testing transaction receipt marshalling.
evm-e2e/hardhat.config.js (2)
1-4: LGTM: Dependencies and TypeScript setup are well configured.
The required dependencies and TypeScript type annotation are correctly set up. The hardhat-toolbox provides a comprehensive set of development tools, and TypeChain integration enables type-safe contract interactions.
5-13: Verify network configuration for EVM compatibility.
The configuration lacks network settings which might be needed for connecting to the Nibiru EVM network.
Let's verify if there are any network configurations in other files:
Consider adding network configuration if needed:
networks: {
nibiru: {
url: process.env.NIBIRU_RPC_URL || "http://localhost:8545",
chainId: process.env.NIBIRU_CHAIN_ID || 1,
},
}evm-e2e/justfile (1)
21-21: LGTM! Format command updated.
The format command has been correctly updated to use npm, maintaining consistency with the project's transition from bun to npm.
evm-e2e/package.json (2)
25-25: LGTM: Good addition of code formatting tools
The addition of Prettier with specific scripts for formatting TypeScript test files is a good practice for maintaining consistent code style. The configuration targets test files appropriately with "test/**/*.ts".
Also applies to: 34-35
27-27: Verify TypeChain integration with OpenZeppelin contracts
The addition of TypeChain for generating TypeScript bindings is good, but we should ensure it's properly configured to work with OpenZeppelin contracts.
Let's check the TypeChain setup:
#!/bin/bash
# Verify TypeChain configuration and contract imports
# Check for TypeChain artifacts directory in gitignore
if ! rg -q "typechain-types" .gitignore; then
echo "Warning: typechain-types directory should be added to .gitignore"
fi
# Look for OpenZeppelin contract imports
rg -l "@openzeppelin/contracts" .evm-e2e/test/utils.ts (2)
1-9: LGTM! Imports are properly organized.
The new factory imports for EventsEmitter and TransactionReverter align with the introduction of new contracts, maintaining consistency with the codebase changes.
43-55: Verify test coverage for transaction receipt fields.
Since this PR fixes transaction receipt marshalling, ensure that the new EventsEmitter and TransactionReverter contracts include tests that verify the presence of to, from, effectiveGasPrice, and contractAddress fields in their transaction receipts.
✅ Verification successful
Let me check the content of these test files to verify the coverage of transaction receipt fields.
Transaction receipt fields are thoroughly tested
The test coverage for transaction receipt fields is comprehensive and well-structured:
tx_receipt.test.tscontains thorough assertions inassertBaseReceiptFields:- Validates all basic fields including
status,type,hash,index - Verifies block-related fields:
blockNumber,blockHash - Checks address fields:
from,to,contractAddress - Tests gas-related fields:
gasUsed,cumulativeGasUsed - Validates logs:
logsBloom,logs
- Validates all basic fields including
- Tests cover different scenarios:
- Contract creation (null
to, validcontractAddress) - Contract interactions (valid
to, event logs) - ETH transfers (valid
to, no logs)
- Contract creation (null
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for receipt field assertions in test files
rg -l "receipt\.[to|from|effectiveGasPrice|contractAddress]" "evm-e2e/test/"
Length of output: 148
Script:
#!/bin/bash
# Check the content of test files for receipt field assertions
echo "=== contract_send_nibi.test.ts ==="
rg "receipt\." -A 2 -B 2 "evm-e2e/test/contract_send_nibi.test.ts"
echo -e "\n=== tx_receipt.test.ts ==="
rg "receipt\." -A 2 -B 2 "evm-e2e/test/tx_receipt.test.ts"
Length of output: 2674
evm-e2e/test/tx_receipt.test.ts (5)
1-6: LGTM! Well-structured imports.
The imports are properly organized with clear separation between testing framework, external libraries, and local utilities.
7-11: LGTM! Appropriate test suite configuration.
The 15-second timeout is reasonable for e2e tests involving blockchain transactions, and using a random recipient address helps ensure test isolation.
12-23: LGTM! Well-crafted test for basic transaction receipt.
This test case effectively validates the to field, which was one of the missing fields mentioned in the PR objectives. The test provides a good baseline for the simplest transaction type.
39-63: LGTM! Thorough validation of event logs.
The test provides comprehensive validation of event logs, including proper verification of event signatures, topics, and indexed parameters. This helps ensure the integrity of transaction receipts with events.
101-123: LGTM! Comprehensive event log validation.
The helper function provides thorough validation of all event log fields with proper format checking for addresses, hashes, and topics.
eth/rpc/backend/tx_info_test.go (1)
Line range hint 15-192: LGTM! Comprehensive test coverage for transaction retrieval.
The test suite thoroughly covers various transaction retrieval scenarios and includes proper assertions for all relevant fields, including those mentioned in the PR objectives (to, from).
eth/rpc/backend/tx_info.go (2)
138-142: LGTM: TransactionReceipt struct properly extended
The added fields properly complement the embedded gethcore.Receipt struct, ensuring all essential transaction information is captured. The field types are appropriately chosen to handle null cases and hex serialization.
286-286: LGTM: Proper effective gas price calculation
The effective gas price is correctly calculated and converted to hexutil.Big for proper JSON serialization. The error handling for pruned nodes is appropriate.
CHANGELOG.md (1)
Line range hint 1-24: LGTM! Well-structured changelog.
The changelog follows best practices:
- Clear sections for different types of changes
- Chronological ordering with newest changes first
- Links to PRs for traceability
- Follows Keep a Changelog format
| typechain: { | ||
| outDir: "types", | ||
| target: "ethers-v6", | ||
| alwaysGenerateOverloads: false, | ||
| dontOverrideCompile: false, | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing TypeChain configuration for better developer experience.
The current TypeChain configuration could be improved:
- Setting
alwaysGenerateOverloads: truewould provide better TypeScript support for overloaded functions - Consider adding
dontOverrideCompile: trueto prevent unnecessary recompilation during development
typechain: {
outDir: "types",
target: "ethers-v6",
- alwaysGenerateOverloads: false,
- dontOverrideCompile: false,
+ alwaysGenerateOverloads: true,
+ dontOverrideCompile: true,
},📝 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.
| typechain: { | |
| outDir: "types", | |
| target: "ethers-v6", | |
| alwaysGenerateOverloads: false, | |
| dontOverrideCompile: false, | |
| }, | |
| typechain: { | |
| outDir: "types", | |
| target: "ethers-v6", | |
| alwaysGenerateOverloads: true, | |
| dontOverrideCompile: true, | |
| }, |
| install: | ||
| npm install | ||
| npx hardhat typechain |
There was a problem hiding this comment.
💡 Codebase verification
Update required: Add typechain script to package.json
The install recipe in the justfile runs npx hardhat typechain, but there's no corresponding script in package.json. While all required dependencies are present (hardhat, typechain, @typechain/hardhat, @typechain/ethers-v6), you should add a dedicated script for type generation to ensure consistency and maintainability.
Add the following to the "scripts" section in package.json:
"typechain": "hardhat typechain"Then update the justfile to use this script:
install:
npm install
npm run typechain🔗 Analysis chain
LGTM! Verify package.json scripts.
The simplified install recipe looks good, consolidating dependency installation and type generation into a single step.
Let's verify the corresponding npm scripts are properly configured:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that package.json contains the required scripts and dependencies
# Check if package.json exists and contains required dependencies
rg -A 5 '"dependencies"|"devDependencies"' package.json
# Check if hardhat and typechain are configured
rg '"scripts"' package.json -A 10
Length of output: 199
Script:
#!/bin/bash
# Let's check the correct path by finding package.json in the evm-e2e directory
fd package.json evm-e2e
# If found, let's check its contents
fd package.json evm-e2e --exec cat {}
Length of output: 1106
| event ValueUpdateAttempted(address sender, uint256 value); | ||
| function emitAndRevert(uint256 newValue) public { | ||
| emit ValueUpdateAttempted(msg.sender, newValue); | ||
| value = newValue; | ||
| revert("Reverted after event emission"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve event emission timing and error handling.
Consider the following improvements:
- Emit the event after the state change to maintain logical order
- Use a custom error instead of a string message
event ValueUpdateAttempted(address sender, uint256 value);
+ error EventEmissionReverted(address sender, uint256 attemptedValue);
+
function emitAndRevert(uint256 newValue) public {
- emit ValueUpdateAttempted(msg.sender, newValue);
value = newValue;
- revert("Reverted after event emission");
+ emit ValueUpdateAttempted(msg.sender, newValue);
+ revert EventEmissionReverted(msg.sender, newValue);
}📝 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.
| event ValueUpdateAttempted(address sender, uint256 value); | |
| function emitAndRevert(uint256 newValue) public { | |
| emit ValueUpdateAttempted(msg.sender, newValue); | |
| value = newValue; | |
| revert("Reverted after event emission"); | |
| } | |
| event ValueUpdateAttempted(address sender, uint256 value); | |
| error EventEmissionReverted(address sender, uint256 attemptedValue); | |
| function emitAndRevert(uint256 newValue) public { | |
| value = newValue; | |
| emit ValueUpdateAttempted(msg.sender, newValue); | |
| revert EventEmissionReverted(msg.sender, newValue); | |
| } |
| function setAndRevert(uint256 newValue) public { | ||
| value = newValue; | ||
| revert("Transaction reverted after state change"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using custom error and add input validation.
The function could be improved by:
- Using a custom error instead of a string message for gas efficiency
- Adding input validation if there are any value constraints
+ error StateChangeReverted(uint256 attemptedValue);
+
function setAndRevert(uint256 newValue) public {
+ // Add any necessary input validation here
value = newValue;
- revert("Transaction reverted after state change");
+ revert StateChangeReverted(newValue);
}📝 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.
| function setAndRevert(uint256 newValue) public { | |
| value = newValue; | |
| revert("Transaction reverted after state change"); | |
| } | |
| error StateChangeReverted(uint256 attemptedValue); | |
| function setAndRevert(uint256 newValue) public { | |
| // Add any necessary input validation here | |
| value = newValue; | |
| revert StateChangeReverted(newValue); | |
| } |
| // SPDX-License-Identifier: MIT | ||
| pragma solidity ^0.8.20; | ||
|
|
||
| contract TransactionReverter { | ||
| uint256 public value; | ||
|
|
||
| error CustomRevertReason(uint256 providedValue); | ||
|
|
||
| // Will try to set state and revert unconditionally | ||
| function setAndRevert(uint256 newValue) public { | ||
| value = newValue; | ||
| revert("Transaction reverted after state change"); | ||
| } | ||
|
|
||
| // Will revert with custom error | ||
| function revertWithCustomError(uint256 newValue) public { | ||
| value = newValue; | ||
| revert CustomRevertReason(newValue); | ||
| } | ||
|
|
||
| // Will emit event and then revert | ||
| event ValueUpdateAttempted(address sender, uint256 value); | ||
| function emitAndRevert(uint256 newValue) public { | ||
| emit ValueUpdateAttempted(msg.sender, newValue); | ||
| value = newValue; | ||
| revert("Reverted after event emission"); | ||
| } | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Missing test coverage for transaction reversion scenarios
The test suite lacks coverage for the reversion scenarios implemented in TransactionReverter.sol. While there are tests for successful transactions and event emissions, the following test cases are missing:
- State changes before reversion (
setAndRevert) - Custom error data handling (
revertWithCustomError) - Event emission followed by reversion (
emitAndRevert)
These scenarios are critical for verifying proper transaction receipt marshalling in error cases.
Locations needing attention:
evm-e2e/test/tx_receipt.test.ts: Add test cases for reversion scenarios- Consider adding assertions for:
- Receipt status field for failed transactions
- Proper error data capture
- Event persistence in reverted transactions
🔗 Analysis chain
Verify test coverage for transaction receipt marshalling.
The contract provides good test scenarios for transaction receipt marshalling. Ensure that tests cover all reversion cases and verify the correct marshalling of:
- State changes before reversion
- Custom error data
- Emitted events
- Transaction status
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for comprehensive test coverage of this contract
# Look for test files that use this contract
rg -l "TransactionReverter" --type ts
# Check for test cases covering each reversion scenario
rg -A 5 "setAndRevert|revertWithCustomError|emitAndRevert" --type ts
Length of output: 162
Script:
#!/bin/bash
# Let's examine the test files more closely
cat evm-e2e/test/tx_receipt.test.ts
Length of output: 4355
Script:
#!/bin/bash
# Also check the utils file for any helper functions
cat evm-e2e/test/utils.ts
Length of output: 2046
| @@ -11,9 +11,8 @@ | |||
| "devDependencies": { | |||
| "@jest/globals": "^29.7.0", | |||
| "@typechain/ethers-v6": "^0.5.1", | |||
| "@typechain/hardhat": "^9.1.0", | |||
There was a problem hiding this comment.
💡 Codebase verification
Migration to Hardhat tooling needs completion
The verification reveals an incomplete migration from Bun to Hardhat/Jest:
- Hardhat and TypeChain are properly configured in
hardhat.config.js - Test files are present and using Jest (
jest.config.jsexists) - However, Bun references still remain in:
tsconfig.jsonstill includes "bun-types"README.mdstill shows Bun test execution instructions- No TypeChain types directory is set up yet
Please:
- Remove "bun-types" from tsconfig.json
- Update README.md with new Jest test instructions
- Add a script to generate TypeChain types
- Add "types" directory to .gitignore
🔗 Analysis chain
Verify Hardhat tooling compatibility with existing tests
The addition of Hardhat and its toolchain represents a significant change in the testing infrastructure. While this is a good modernization step, we should ensure:
- The transition from the previous testing setup is complete
- All existing tests are compatible with the new Hardhat environment
Let's verify the test setup:
Also applies to: 28-30
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining references to the old testing infrastructure
# and verify the presence of Hardhat configuration
# Look for Hardhat config file
if ! fd -t f "hardhat.config.ts"; then
echo "Warning: Hardhat config file is missing"
fi
# Check for any remaining references to old test setup
rg -l 'bun' .
Length of output: 815
Script:
#!/bin/bash
# Let's verify the test setup more thoroughly
# Check for Hardhat config and test files
echo "=== Checking Hardhat Setup ==="
fd -t f "hardhat.config" .
fd -t f -e test.ts -e spec.ts evm-e2e/
# Check for any remaining bun configurations
echo -e "\n=== Checking Bun References ==="
rg -l "bun" evm-e2e/
rg -l "bunx" evm-e2e/
# Check test scripts in package.json
echo -e "\n=== Package.json Test Scripts ==="
cat evm-e2e/package.json | jq .scripts
# Look for typechain generated files/configs
echo -e "\n=== TypeChain Setup ==="
fd typechain evm-e2e/
Length of output: 1035
Script:
#!/bin/bash
# Let's check the specific test setup and migrations
# Check the content of hardhat config
echo "=== Hardhat Config Content ==="
cat evm-e2e/hardhat.config.js
# Check for test files in correct location
echo -e "\n=== Test Files ==="
fd -e ts . evm-e2e/test/
# Check remaining bun references in specific files
echo -e "\n=== Bun Reference Details ==="
echo "--- tsconfig.json ---"
cat evm-e2e/tsconfig.json | grep -A 5 -B 5 "bun"
echo "--- README.md ---"
cat evm-e2e/README.md | grep -A 5 -B 5 "bun"
# Check Jest configuration
echo -e "\n=== Jest Config ==="
fd -t f "jest.config" evm-e2e/
Length of output: 1693
Fixes the issue of the missing fields:
to,from,effectiveGasPriceandcontractAddressin the transaction receipt.As soon as
TransactionReceiptstruct is inherited fromgethcore.Receipt- its MarshalJSON ignores the extra fields.Summary by CodeRabbit
Release Notes
New Features
EventsEmittercontract for logging events.TransactionRevertercontract demonstrating transaction reversion methods.Bug Fixes
Documentation
CHANGELOG.mdto reflect recent changes and improvements.Tests
Chores
buntonpmfor package management and formatting.