-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add missing fields to the response of eth_getTransactionByHash
#309
Conversation
WalkthroughThe changes enhance the Changes
Sequence DiagramssequenceDiagram
participant Client
participant BlockChainAPI
participant Config
participant Transaction
Client->>BlockChainAPI: eth_getTransactionByHash(hash)
BlockChainAPI->>Config: Fetch Config
BlockChainAPI->>Transaction: NewTransaction(tx, rcp, config)
Transaction->>BlockChainAPI: Transaction Details
BlockChainAPI-->>Client: Return Transaction details
Assessment against linked issues
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- api/api.go (1 hunks)
- api/models.go (3 hunks)
- api/stream.go (1 hunks)
- models/transaction.go (4 hunks)
- tests/e2e_web3js_test.go (2 hunks)
- tests/web3js/eth_get_transaction_by_hash_test.js (1 hunks)
- tests/web3js/eth_non_interactive_test.js (3 hunks)
Files skipped from review due to trivial changes (2)
- tests/e2e_web3js_test.go
- tests/web3js/eth_get_transaction_by_hash_test.js
Additional comments not posted (6)
models/transaction.go (2)
Line range hint
87-121
: TheDirectCall
struct now implements the newly added methods in theTransaction
interface. Check if the default values returned by these methods (big.NewInt(0)
and[]common.Hash{}
) are appropriate for all use cases. If there are scenarios where these defaults are not suitable, consider parameterizing or providing a configuration option.
25-32
: Ensure the newly added methods in theTransaction
interface are implemented in all concrete types that implement this interface. This is crucial for maintaining the integrity of the type system.Verification successful
The verification script executed successfully, identifying the types implementing the
Transaction
interface. It also verified the presence of the new methods (GasFeeCap
,GasTipCap
,BlobGasFeeCap
,BlobHashes
, andAccessList
) in each type.No missing methods were reported for the types implementing the
Transaction
interface, indicating that all required methods are present.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all concrete types implementing the `Transaction` interface have the new methods. # Test: Search for classes implementing `Transaction` and check for the new methods. ast-grep --lang go --pattern $'type $_ implements Transaction { $$$ GasFeeCap() GasTipCap() BlobGasFeeCap() BlobHashes() AccessList() $$$ }'Length of output: 704
Script:
#!/bin/bash # Description: Verify all concrete types implementing the `Transaction` interface have the new methods. # Test: Search for types implementing `Transaction` and check for the new methods. # Correct the pattern for Go interface implementation and method definitions ast-grep --lang go --pattern $'type $_ struct { $$$ } func ($_ *$_) GasFeeCap() *big.Int { $$$ } func ($_ *$_) GasTipCap() *big.Int { $$$ } func ($_ *$_) BlobGasFeeCap() *big.Int { $$$ } func ($_ *$_) BlobHashes() []common.Hash { $$$ } func ($_ *$_) AccessList() gethTypes.AccessList { $$$ }'Length of output: 1000
Script:
#!/bin/bash # Description: Verify all concrete types implementing the `Transaction` interface have the new methods. # Test: Search for types implementing `Transaction` and check for the new methods. # Simplify the pattern to focus on method signatures only ast-grep --lang go --pattern $'func ($_ *$_) GasFeeCap() *big.Int { $$$ } func ($_ *$_) GasTipCap() *big.Int { $$$ } func ($_ *$_) BlobGasFeeCap() *big.Int { $$$ } func ($_ *$_) BlobHashes() []common.Hash { $$$ } func ($_ *$_) AccessList() gethTypes.AccessList { $$$ }'Length of output: 950
Script:
#!/bin/bash # Description: Verify all concrete types implementing the `Transaction` interface have the new methods. # Step 1: Identify types implementing the `Transaction` interface implementing_types=$(ast-grep --lang go --pattern 'type $_ struct { $$$ }' | grep -oP 'type \K\w+') # Step 2: Check each implementing type for the required methods for type in $implementing_types; do echo "Checking methods for type: $type" ast-grep --lang go --pattern "func ($_ *$type) GasFeeCap() *big.Int { $$$ }" || echo "Missing GasFeeCap in $type" ast-grep --lang go --pattern "func ($_ *$type) GasTipCap() *big.Int { $$$ }" || echo "Missing GasTipCap in $type" ast-grep --lang go --pattern "func ($_ *$type) BlobGasFeeCap() *big.Int { $$$ }" || echo "Missing BlobGasFeeCap in $type" ast-grep --lang go --pattern "func ($_ *$type) BlobHashes() []common.Hash { $$$ }" || echo "Missing BlobHashes in $type" ast-grep --lang go --pattern "func ($_ *$type) AccessList() gethTypes.AccessList { $$$ }" || echo "Missing AccessList in $type" doneLength of output: 149438
tests/web3js/eth_non_interactive_test.js (1)
Line range hint
80-95
: The test assertions foreth_getTransactionByHash
have been updated to reflect the new gas fields. Ensure that the hardcoded values (e.g.,300000n
for gas and21000n
for cumulative gas used) are consistent with expected results based on the transaction context used in these tests.api/models.go (1)
Line range hint
95-174
: TheNewTransaction
function has been significantly modified to include a newconfig
parameter and to handle different transaction types. Ensure all transaction types are correctly handled and that theconfig
parameter is used appropriately throughout the function. Also, verify that the error handling is robust, especially in cases where transaction or receipt retrieval might fail.Verification successful
The
NewTransaction
function inapi/models.go
has been verified to handle different transaction types correctly. The error handling within the function is also robust, with appropriate error returns in case of issues with transaction hashing or retrieving theFrom
address.
- The function handles
LegacyTxType
,AccessListTxType
,DynamicFeeTxType
, andBlobTxType
appropriately.- Error handling is in place for transaction hashing and retrieving the
From
address.This confirms that the changes made to the
NewTransaction
function are correctly implemented.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct handling of different transaction types and robust error handling in `NewTransaction`. # Test: Search for usages of `NewTransaction` and verify that all possible transaction types are handled. rg --type go $'NewTransaction'Length of output: 1667
Script:
#!/bin/bash # Verify the handling of different transaction types and error handling in `NewTransaction`. # Test: Search for the definition and detailed usage of `NewTransaction` to verify transaction type handling. ast-grep --lang go --pattern $'func NewTransaction(tx models.Transaction, receipt types.Receipt, config config.Config) (*Transaction, error) { $$$ }' # Test: Search for error handling within the `NewTransaction` function. rg --type go 'return nil, err' api/models.goLength of output: 4331
api/stream.go (1)
148-148
: TheNewPendingTransactions
function now passes theconfig
parameter to theNewTransaction
function. Ensure that this configuration is being correctly utilized withinNewTransaction
to affect transaction creation as intended.api/api.go (1)
244-244
: The addition of*b.config
in theNewTransaction
call is a positive change, enhancing the transaction creation with configuration data.Verification successful
The
NewTransaction
function inapi/models.go
correctly includes theconfig
parameter. Additionally, the usage inapi/stream.go
also correctly passes the configuration parameter*s.config
. This confirms that the integration of the new config parameter is consistent and correct across the relevant files.
api/models.go
: TheNewTransaction
function signature includes theconfig
parameter.api/stream.go
: The call toNewTransaction
correctly passes*s.config
.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new config parameter with existing configurations. # Test: Search for the usage of the `NewTransaction` function. Expect: Correct passing of config in all usages. rg --type python $'NewTransaction'Length of output: 64
Script:
#!/bin/bash # Description: Verify the integration of the new config parameter with existing configurations. # Test: Search for the usage of the `NewTransaction` function. Expect: Correct passing of config in all usages. rg --type go $'NewTransaction'Length of output: 1667
Script:
#!/bin/bash # Description: Extract and review the context of `NewTransaction` calls in `api/stream.go` and `api/models.go`. # Extract 5 lines before and after the instances of `NewTransaction` in the specified files. rg --type go --context 5 'NewTransaction' api/stream.go api/models.goLength of output: 947
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- models/transaction.go (4 hunks)
- tests/web3js/eth_non_interactive_test.js (2 hunks)
Files skipped from review as they are similar to previous changes (2)
- models/transaction.go
- tests/web3js/eth_non_interactive_test.js
Closes: #306
Description
Adds the following optional fields:
chainId
accessList
maxFeePerGas
maxPriorityFeePerGas
Fixes the value returned for the following fields:
Gas
(this is the gas limit set by the sender, and not the consumed gas)GasPrice
(this is the gas price set by the sender, and not theeffectiveGasPrice
from the transaction receipt)For contributor use:
master
branchFiles changed
in the Github PR explorerSummary by CodeRabbit
New Features
Bug Fixes
Tests
eth_getTransactionByHash
.