-
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
Response returned from eth_getBlockReceipts
endpoint is not properly marshalled
#533
Conversation
WalkthroughThe pull request introduces changes to the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
Recent review detailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
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 (
|
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)
- api/api.go (2 hunks)
- tests/web3js/eth_non_interactive_test.js (2 hunks)
Additional comments not posted (2)
tests/web3js/eth_non_interactive_test.js (1)
77-98
: Comprehensive test case for block receipts retrieval.The test case "should get block receipts" is well-structured and covers the essential validations including:
- Fetching the current block number.
- Retrieving block details.
- Obtaining block receipts and validating their count.
- Ensuring each block receipt's transaction receipt matches the block receipt.
This test enhances the robustness of the testing framework by ensuring the consistency and integrity of block receipts and their associated transaction receipts.
api/api.go (1)
Line range hint
443-487
: Refined implementation ofGetBlockReceipts
.The modifications to the
GetBlockReceipts
method enhance its functionality by:
- Changing the parameter name to
blockNumberOrHash
which is more descriptive.- Shifting the return type to a slice of maps, increasing the flexibility of the data structure.
- Adding a new call to fetch transaction details, enriching the data returned in the receipts.
These changes are aligned with the PR objectives to improve data representation and consistency. The method now supports a more complex relationship between transactions and receipts, which allows for richer data to be returned.
|
||
for (let blockReceipt of blockReceipts) { | ||
let response = await helpers.callRPCMethod( | ||
'eth_getTransactionReceipt', |
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.
Why wouldn't we use web3.js function for this https://docs.web3js.org/guides/web3_eth/methods#gettransactionreceipt
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.
Updated in 54534f3
6edb9d7
to
abb20de
Compare
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: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- api/api.go (2 hunks)
- tests/web3js/eth_non_interactive_test.js (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- tests/web3js/eth_non_interactive_test.js
Additional comments not posted (1)
api/api.go (1)
443-444
: Approve the changes made to theGetBlockReceipts
function.The changes made to the
GetBlockReceipts
function align with the objectives of the pull request:
- The function signature has been updated to accept
rpc.BlockNumberOrHash
, which standardizes the representation of block identifiers across different endpoints.- The return type has been changed to
[]map[string]interface{}
, ensuring consistent representation of receipt data in the API responses.- The function correctly handles the case when a block number or hash is not provided by returning an appropriate error.
- The function retrieves the block, transactions, and receipts using the appropriate storage interfaces, promoting modularity and separation of concerns.
- The
MarshalReceipt
function is used to marshal the receipts, ensuring consistent representation of receipt data.- Error handling is performed using the
handleError
function, ensuring consistent error responses.Also applies to: 447-447, 458-461, 463-463, 470-470, 473-473, 475-478, 480-483, 485-485, 487-487
return handleError[[]map[string]interface{}](err, l, b.collector) | ||
} | ||
|
||
receipts[i], err = MarshalReceipt(receipt, tx) |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Closes: #531
Description
We were not using the
MarshalReceipt
method, which is used foreth_getTransactionReceipt
, and the various fields had inconsistent representations.The proper representation can be found in: https://docs.infura.io/api/networks/ethereum/json-rpc-methods/eth_getblockreceipts
For contributor use:
master
branchFiles changed
in the Github PR explorerSummary by CodeRabbit
Summary by CodeRabbit
New Features
GetBlockReceipts
method to improve data representation and flexibility in API responses.Bug Fixes
GetBlockReceipts
method.