Skip to content
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

RPC API: get receipts by block number #19721

Closed
wants to merge 3 commits into from

Conversation

ramilexe
Copy link

For issue #19634

Added new RPC API: eth_getBlockReceipts. It returns receipts by block number.

Also, implemented tests for:

  • eth_getTransactionByBlockHashAndIndex
  • eth_getBlockReceipts


// Start Ethereum service.
var ethservice *eth.Ethereum
n, err := node.New(&node.Config{})
n.Register(func(ctx *node.ServiceContext) (node.Service, error) {
config := &eth.Config{Genesis: genesis}
config := &eth.Config{Genesis: genesis, NoPrefetch: true}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

without this parameter it produces nonce too high error. For details see #19723

@hadv
Copy link
Contributor

hadv commented Jun 17, 2019

any process to update RPC document? @adamschmideg :)

@adamschmideg
Copy link
Contributor

@hadv No update to the RPC document yet 🤷‍♂

@ramilexe
Copy link
Author

What does status triage mean?

@adamschmideg
Copy link
Contributor

@ramilexe the status:triage label means it's marked for our next weekly discussion of issues. It doesn't necessarily mean we'll have time to discuss it then. This is the step before it's getting assigned to a team member or put into the backlog or closed.

@adamschmideg
Copy link
Contributor

I left a comment to #19634 describing how to get gasUsed with GraphQL. I wonder if that approach works for you. Because changing the RPC comprises of two tasks, a) to change the code which you already did b) write and EIP and change the standard interface. Do you think adding eth_getBlockReceipts to the API is important?

// BlockReceipts returns the receipts of a transaction by block number.
func (ec *Client) BlockReceipts(ctx context.Context, number *big.Int) (types.Receipts, error) {
var result types.Receipts
err := ec.c.CallContext(ctx, &result, "eth_getBlockReceipts", toBlockNumArg(number))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems that both web3ext.go and web3.js need to be modified to bind "eth_getBlockReceipts" and "GetBlockReceipts", and expose "eth_getBlockReceipts".

result := make([]map[string]interface{}, 0, len(receipts))

for index, receipt := range receipts {
tx := txs[index]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Directly using index of receipts to look for corresponding tx is not so rigorous. Suggest to modify it to:

tx := txs[receipt.TransactionIndex]


result := make([]map[string]interface{}, 0, len(receipts))

for index, receipt := range receipts {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

receipt.TransactionIndex could stand for index, so index here makes no sense. Suggest to replace it with blank identifier "_"

"blockHash": block.Hash(),
"blockNumber": hexutil.Uint64(blockNr),
"transactionHash": receipt.TxHash,
"transactionIndex": hexutil.Uint64(index),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest to replace index with receipt.TransactionIndex

@fjl
Copy link
Contributor

fjl commented Jul 1, 2020

We had a bit of an internal discussion around this. The new method is very useful and provides a much faster way of getting block receipts than the current way, which would be to retrieve them individually by transaction hash.

Since this change adds new API surface to the eth RPC namespace, the process here is more complicated than just adding the code to geth. We do not want to add new, undocumented API which is specific to geth. Instead, the right way to push this through is creating an interface EIP. We can then link to the EIP and ask other client implementers to also add this new method.

The PR can remain open, but will not be merged until the EIP has been created.

@fjl fjl removed the status:triage label Jul 1, 2020
@ramilexe
Copy link
Author

ramilexe commented Jul 1, 2020

Hi @fjl
Thank you for update.
I will create new EIP

@adamschmideg adamschmideg removed their assignment Jul 14, 2020
@qyvlik
Copy link

qyvlik commented Feb 24, 2021

@ramilexe Hello, have you created EIP for eth_getBlockReceipts ?

@holiman
Copy link
Contributor

holiman commented Oct 29, 2021

This PR is now < 2 years old, will likely not get merged (afaik there was never any EIP created for this). I'll close it, we can always reimplement this if we want the method to exist.

@holiman holiman closed this Oct 29, 2021
@ryandotsmith
Copy link

This PR is now < 2 years old, will likely not get merged (afaik there was never any EIP created for this). I'll close it, we can always reimplement this if we want the method to exist.

@holiman I would be happy to do the work to get this feature added. Assuming the correct steps are taken, do you think it could be added?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants