Add missing block fields to the type used for eth_getBlockByNumber and eth_getBlockByHash
#185
Conversation
WalkthroughThe update involves refactoring the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
api/models.go
Outdated
| Number hexutil.Uint64 `json:"number"` | ||
| Hash common.Hash `json:"hash"` | ||
| ParentHash common.Hash `json:"parentHash"` | ||
| Nonce hexutil.Uint64 `json:"nonce"` | ||
| Sha3Uncles common.Hash `json:"sha3Uncles"` | ||
| LogsBloom hexutil.Bytes `json:"logsBloom"` | ||
| TransactionsRoot common.Hash `json:"transactionsRoot"` | ||
| StateRoot common.Hash `json:"stateRoot"` | ||
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | ||
| Miner common.Address `json:"miner"` | ||
| Difficulty hexutil.Uint64 `json:"difficulty"` | ||
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | ||
| ExtraData hexutil.Bytes `json:"extraData"` | ||
| Size hexutil.Uint64 `json:"size"` | ||
| GasLimit hexutil.Uint64 `json:"gasLimit"` | ||
| GasUsed hexutil.Uint64 `json:"gasUsed"` | ||
| Timestamp hexutil.Uint64 `json:"timestamp"` | ||
| Transactions interface{} `json:"transactions"` | ||
| Uncles []common.Hash `json:"uncles"` | ||
| MixHash common.Hash `json:"mixHash"` |
There was a problem hiding this comment.
Ensure the Transactions field in the Block struct can correctly handle both transaction hashes and full transaction data based on the context.
- Transactions interface{} `json:"transactions"`
+ Transactions []interface{} `json:"transactions"`This change ensures that the Transactions field can hold a slice of either transaction hashes or full transaction objects, enhancing flexibility and clarity.
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.
| Number hexutil.Uint64 `json:"number"` | |
| Hash common.Hash `json:"hash"` | |
| ParentHash common.Hash `json:"parentHash"` | |
| Nonce hexutil.Uint64 `json:"nonce"` | |
| Sha3Uncles common.Hash `json:"sha3Uncles"` | |
| LogsBloom hexutil.Bytes `json:"logsBloom"` | |
| TransactionsRoot common.Hash `json:"transactionsRoot"` | |
| StateRoot common.Hash `json:"stateRoot"` | |
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | |
| Miner common.Address `json:"miner"` | |
| Difficulty hexutil.Uint64 `json:"difficulty"` | |
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | |
| ExtraData hexutil.Bytes `json:"extraData"` | |
| Size hexutil.Uint64 `json:"size"` | |
| GasLimit hexutil.Uint64 `json:"gasLimit"` | |
| GasUsed hexutil.Uint64 `json:"gasUsed"` | |
| Timestamp hexutil.Uint64 `json:"timestamp"` | |
| Transactions interface{} `json:"transactions"` | |
| Uncles []common.Hash `json:"uncles"` | |
| MixHash common.Hash `json:"mixHash"` | |
| Number hexutil.Uint64 `json:"number"` | |
| Hash common.Hash `json:"hash"` | |
| ParentHash common.Hash `json:"parentHash"` | |
| Nonce hexutil.Uint64 `json:"nonce"` | |
| Sha3Uncles common.Hash `json:"sha3Uncles"` | |
| LogsBloom hexutil.Bytes `json:"logsBloom"` | |
| TransactionsRoot common.Hash `json:"transactionsRoot"` | |
| StateRoot common.Hash `json:"stateRoot"` | |
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | |
| Miner common.Address `json:"miner"` | |
| Difficulty hexutil.Uint64 `json:"difficulty"` | |
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | |
| ExtraData hexutil.Bytes `json:"extraData"` | |
| Size hexutil.Uint64 `json:"size"` | |
| GasLimit hexutil.Uint64 `json:"gasLimit"` | |
| GasUsed hexutil.Uint64 `json:"gasUsed"` | |
| Timestamp hexutil.Uint64 `json:"timestamp"` | |
| Transactions []interface{} `json:"transactions"` | |
| Uncles []common.Hash `json:"uncles"` | |
| MixHash common.Hash `json:"mixHash"` |
api/api.go
Outdated
| func (b *BlockChainAPI) prepareBlockResponse( | ||
| ctx context.Context, | ||
| block *evmTypes.Block, | ||
| fullTx bool, | ||
| ) (*Block, error) { | ||
| h, err := block.Hash() | ||
| if err != nil { | ||
| b.logger.Error().Err(err).Msg("failed to calculate hash for block by number") | ||
| return nil, errs.ErrInternal | ||
| } | ||
|
|
||
| blockResponse := &Block{ | ||
| Hash: h, | ||
| Number: hexutil.Uint64(block.Height), | ||
| ParentHash: block.ParentBlockHash, | ||
| ReceiptsRoot: block.ReceiptRoot, | ||
| Transactions: block.TransactionHashes, | ||
| Uncles: []common.Hash{}, | ||
| GasLimit: hexutil.Uint64(15_000_000), | ||
| } | ||
|
|
||
| transactions, err := b.fetchBlockTransactions(ctx, block) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| for _, tx := range transactions { | ||
| blockResponse.GasUsed += tx.Gas | ||
| } | ||
|
|
||
| if fullTx { | ||
| blockResponse.Transactions = transactions | ||
| } | ||
|
|
||
| return blockResponse, nil | ||
| } |
There was a problem hiding this comment.
Refactor the prepareBlockResponse method to ensure the GasUsed calculation is accurate and efficient.
- for _, tx := range transactions {
- blockResponse.GasUsed += tx.Gas
- }
+ if len(transactions) > 0 {
+ totalGasUsed := hexutil.Uint64(0)
+ for _, tx := range transactions {
+ totalGasUsed += tx.Gas
+ }
+ blockResponse.GasUsed = totalGasUsed
+ }This change ensures that GasUsed is only calculated if there are transactions, and it uses a temporary variable to accumulate the total gas used before assigning it to the Block struct. This approach minimizes potential issues with directly modifying the struct field in a loop and clarifies the intention of the code.
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.
| func (b *BlockChainAPI) prepareBlockResponse( | |
| ctx context.Context, | |
| block *evmTypes.Block, | |
| fullTx bool, | |
| ) (*Block, error) { | |
| h, err := block.Hash() | |
| if err != nil { | |
| b.logger.Error().Err(err).Msg("failed to calculate hash for block by number") | |
| return nil, errs.ErrInternal | |
| } | |
| blockResponse := &Block{ | |
| Hash: h, | |
| Number: hexutil.Uint64(block.Height), | |
| ParentHash: block.ParentBlockHash, | |
| ReceiptsRoot: block.ReceiptRoot, | |
| Transactions: block.TransactionHashes, | |
| Uncles: []common.Hash{}, | |
| GasLimit: hexutil.Uint64(15_000_000), | |
| } | |
| transactions, err := b.fetchBlockTransactions(ctx, block) | |
| if err != nil { | |
| return nil, err | |
| } | |
| for _, tx := range transactions { | |
| blockResponse.GasUsed += tx.Gas | |
| } | |
| if fullTx { | |
| blockResponse.Transactions = transactions | |
| } | |
| return blockResponse, nil | |
| } | |
| func (b *BlockChainAPI) prepareBlockResponse( | |
| ctx context.Context, | |
| block *evmTypes.Block, | |
| fullTx bool, | |
| ) (*Block, error) { | |
| h, err := block.Hash() | |
| if err != nil { | |
| b.logger.Error().Err(err).Msg("failed to calculate hash for block by number") | |
| return nil, errs.ErrInternal | |
| } | |
| blockResponse := &Block{ | |
| Hash: h, | |
| Number: hexutil.Uint64(block.Height), | |
| ParentHash: block.ParentBlockHash, | |
| ReceiptsRoot: block.ReceiptRoot, | |
| Transactions: block.TransactionHashes, | |
| Uncles: []common.Hash{}, | |
| GasLimit: hexutil.Uint64(15_000_000), | |
| } | |
| transactions, err := b.fetchBlockTransactions(ctx, block) | |
| if err != nil { | |
| return nil, err | |
| } | |
| if len(transactions) > 0 { | |
| totalGasUsed := hexutil.Uint64(0) | |
| for _, tx := range transactions { | |
| totalGasUsed += tx.Gas | |
| } | |
| blockResponse.GasUsed = totalGasUsed | |
| } | |
| if fullTx { | |
| blockResponse.Transactions = transactions | |
| } | |
| return blockResponse, nil | |
| } |
…d eth_getBlockByHash
m-Peter
force-pushed
the
add-missing-block-fields
branch
from
3cb3db6 to
22afd7c
Compare
| Number hexutil.Uint64 `json:"number"` | ||
| Hash common.Hash `json:"hash"` | ||
| ParentHash common.Hash `json:"parentHash"` | ||
| Nonce types.BlockNonce `json:"nonce"` | ||
| Sha3Uncles common.Hash `json:"sha3Uncles"` | ||
| LogsBloom hexutil.Bytes `json:"logsBloom"` | ||
| TransactionsRoot common.Hash `json:"transactionsRoot"` | ||
| StateRoot common.Hash `json:"stateRoot"` | ||
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | ||
| Miner common.Address `json:"miner"` | ||
| Difficulty hexutil.Uint64 `json:"difficulty"` | ||
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | ||
| ExtraData hexutil.Bytes `json:"extraData"` | ||
| Size hexutil.Uint64 `json:"size"` | ||
| GasLimit hexutil.Uint64 `json:"gasLimit"` | ||
| GasUsed hexutil.Uint64 `json:"gasUsed"` | ||
| Timestamp hexutil.Uint64 `json:"timestamp"` | ||
| Transactions interface{} `json:"transactions"` | ||
| Uncles []common.Hash `json:"uncles"` | ||
| MixHash common.Hash `json:"mixHash"` |
There was a problem hiding this comment.
Consider changing the Transactions field type from interface{} to []interface{} to explicitly support both transaction hashes and full transaction objects.
- Transactions interface{} `json:"transactions"`
+ Transactions []interface{} `json:"transactions"`This modification ensures that the Transactions field can hold a slice of either transaction hashes or full transaction objects, enhancing flexibility and clarity.
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.
| Number hexutil.Uint64 `json:"number"` | |
| Hash common.Hash `json:"hash"` | |
| ParentHash common.Hash `json:"parentHash"` | |
| Nonce types.BlockNonce `json:"nonce"` | |
| Sha3Uncles common.Hash `json:"sha3Uncles"` | |
| LogsBloom hexutil.Bytes `json:"logsBloom"` | |
| TransactionsRoot common.Hash `json:"transactionsRoot"` | |
| StateRoot common.Hash `json:"stateRoot"` | |
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | |
| Miner common.Address `json:"miner"` | |
| Difficulty hexutil.Uint64 `json:"difficulty"` | |
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | |
| ExtraData hexutil.Bytes `json:"extraData"` | |
| Size hexutil.Uint64 `json:"size"` | |
| GasLimit hexutil.Uint64 `json:"gasLimit"` | |
| GasUsed hexutil.Uint64 `json:"gasUsed"` | |
| Timestamp hexutil.Uint64 `json:"timestamp"` | |
| Transactions interface{} `json:"transactions"` | |
| Uncles []common.Hash `json:"uncles"` | |
| MixHash common.Hash `json:"mixHash"` | |
| Number hexutil.Uint64 `json:"number"` | |
| Hash common.Hash `json:"hash"` | |
| ParentHash common.Hash `json:"parentHash"` | |
| Nonce types.BlockNonce `json:"nonce"` | |
| Sha3Uncles common.Hash `json:"sha3Uncles"` | |
| LogsBloom hexutil.Bytes `json:"logsBloom"` | |
| TransactionsRoot common.Hash `json:"transactionsRoot"` | |
| StateRoot common.Hash `json:"stateRoot"` | |
| ReceiptsRoot common.Hash `json:"receiptsRoot"` | |
| Miner common.Address `json:"miner"` | |
| Difficulty hexutil.Uint64 `json:"difficulty"` | |
| TotalDifficulty hexutil.Uint64 `json:"totalDifficulty"` | |
| ExtraData hexutil.Bytes `json:"extraData"` | |
| Size hexutil.Uint64 `json:"size"` | |
| GasLimit hexutil.Uint64 `json:"gasLimit"` | |
| GasUsed hexutil.Uint64 `json:"gasUsed"` | |
| Timestamp hexutil.Uint64 `json:"timestamp"` | |
| Transactions []interface{} `json:"transactions"` | |
| Uncles []common.Hash `json:"uncles"` | |
| MixHash common.Hash `json:"mixHash"` |
|
cc @jribbink |
There was a problem hiding this comment.
Actionable comments posted: 0
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (2)
- api/api.go (3 hunks)
- config/config.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- api/api.go
Additional comments not posted (1)
config/config.go (1)
84-84: Ensure the updated "5m" notation for thefilter-expiryflag's default value is consistent with time duration representations elsewhere in the project.
Work towards: #145
Description
Sample output:
{ "jsonrpc": "2.0", "id": 7, "result": { "number": "0x3", "hash": "0x631dff9eec346af7f1ec8843e8bc02be4297d0e40d6ce20839ff07cbfd560e7d", "parentHash": "0x4d17f3966a809b9755375ca6229ab5a4caa55f61d54446cad8d0bc6568900b2a", "nonce": "0x0100000000000000", "sha3Uncles": "0x0000000000000000000000000000000000000000000000000000000000000000", "logsBloom": "0x", "transactionsRoot": "0x0000000000000000000000000000000000000000000000000000000000000000", "stateRoot": "0x0000000000000000000000000000000000000000000000000000000000000000", "receiptsRoot": "0x0000000000000000000000000000000000000000000000000000000000000000", "miner": "0x0000000000000000000000000000000000000000", "difficulty": "0x0", "totalDifficulty": "0x0", "extraData": "0x", "size": "0x0", "gasLimit": "0xe4e1c0", "gasUsed": "0x195b0", "timestamp": "0x0", "transactions": [ "0xe414f90fea2aebd75e8c0b3b6a4a0c9928e86c16ea724343d884f40bfe2c4c6b" ], "uncles": [], "mixHash": "0x0000000000000000000000000000000000000000000000000000000000000000" } }Running the following JS script:
we no longer get an error, but the response below:
Block { provider: JsonRpcProvider {}, number: 3, hash: '0x631dff9eec346af7f1ec8843e8bc02be4297d0e40d6ce20839ff07cbfd560e7d', timestamp: 0, parentHash: '0x4d17f3966a809b9755375ca6229ab5a4caa55f61d54446cad8d0bc6568900b2a', parentBeaconBlockRoot: null, nonce: '0x0100000000000000', difficulty: 0n, gasLimit: 15000000n, gasUsed: 103856n, stateRoot: '0x0000000000000000000000000000000000000000000000000000000000000000', receiptsRoot: '0x0000000000000000000000000000000000000000000000000000000000000000', blobGasUsed: null, excessBlobGas: null, miner: '0x0000000000000000000000000000000000000000', extraData: '0x', baseFeePerGas: null }For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
api/models.gowith new fields for a more detailed block representation.filter-expiryflag inconfig/config.gofrom "5min" to "5m" for defining filter expiration time.