Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Add option to include transaction receipts in eth_getBlockByHash #9075

Closed
medvedev1088 opened this issue Jul 9, 2018 · 16 comments
Closed

Add option to include transaction receipts in eth_getBlockByHash #9075

medvedev1088 opened this issue Jul 9, 2018 · 16 comments
Labels
F8-enhancement 🎊 An additional feature request. M6-rpcapi 📣 RPC API. P9-somedaymaybe 🌞 Issue might be worth doing eventually.
Milestone
2.1 Whatever

Comments

@medvedev1088
Copy link

medvedev1088 commented Jul 9, 2018 •
edited
Loading

In some applications where performance is important it's convenient to query all receipts in a block in a single request.

Since receipts are part of a block, as defined in the yellowpaper, it makes sense to allow including them in the block response for eth_getBlockByHash and eth_getBlockByNumber by adding the boolean parameter to the request:

Parameters

  • QUANTITY|TAG - integer of a block number, or the string "earliest", "latest" or "pending", as in the default block parameter.
  • Boolean - If true it returns the full transaction objects, if false only the hashes of the transactions.
  • Boolean - If true it returns the full receipts objects, if false the receipts objects are not returned.

Here is an analogous issue for go-ethereum ethereum/go-ethereum#17044
@Tbaut Tbaut added F8-enhancement 🎊 An additional feature request. M6-rpcapi 📣 RPC API. labels Jul 9, 2018
@Tbaut Tbaut added this to the 2.0 milestone Jul 9, 2018
@Tbaut Tbaut added P9-somedaymaybe 🌞 Issue might be worth doing eventually. Q3-medium A fair chunk of work, not necessarily very hard but not trivial either labels Jul 9, 2018
@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

This issue now has a funding of 350.0 DAI (350.0 USD @ $1.0/DAI) attached to it.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 4 weeks, 1 day from now. Please review their questions below:

  1. shamardy has started work.
  • Q: Working on this

@gitcoinbot
Copy link

@shamardy Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@shamardy shamardy mentioned this issue Jul 15, 2018
Closed
@5chdn 5chdn modified the milestones: 2.0, 2.1 Jul 17, 2018
@medvedev1088 medvedev1088 mentioned this issue Jul 18, 2018
Closed
@debris
Copy link
Collaborator

debris commented Aug 10, 2018

from: #9126

Do we need proper EIPs for this eth_ namespace change?

This should definitely be an EIP fist. The issue was incorrectly labeled. Please open an EIP here and reopen the issue if accepted. I'm sorry @shamardy

@debris debris closed this as completed Aug 10, 2018
@Tbaut Tbaut removed the Q3-medium A fair chunk of work, not necessarily very hard but not trivial either label Aug 10, 2018
@gitcoinbot
Copy link

@shamardy Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

@shamardy due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@shamardy
Copy link
Contributor

Still working on this. Will open an EIP for this soon.

@gitcoinbot
Copy link

@shamardy Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

  • warning (3 days)
  • escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

@medvedev1088 medvedev1088 mentioned this issue Aug 19, 2018
Closed
@shamardy
Copy link
Contributor

@medvedev1088 Sorry for the late reply. This ethereum/EIPs#1300 is closed. Should we open a new EIP PR with this issues functionality or get behind this ethereum/EIPs#1300 to reopen. What worries me in an EIP for this issue is backwards compatibility since adding a boolean parameter to the request will break anything that relies on existing behaviour. So there is 2 options here either introduce eth_getBlockReceiptsByHash and eth_getBlockReceiptsByNumber new methods as in here ethereum/EIPs#1300, or change the eth_getBlockByHash and eth_getBlockByNumber to accept either a boolean (which it is the old behavior), or a JSON dict with two optional fields { fullTransactions: bool, fullReceipts: bool } as suggested by @sorpaas here #9126 (comment). Which route do you think is better?

@medvedev1088
Copy link
Author

@shamardy I didn't realize it will break backward compatibility. Is it because optional parameters are not allowed?

@shamardy
Copy link
Contributor

@medvedev1088 Optional parameters are not implemented in rust which parity is written by. But I found a way that bypasses that in other RPC functions in parity using the Trailing struct from parity's Rust JSON-RPC implementation. So we can proceed with an EIP using the original solution where there is 2 booleans with the second one as optional without breaking anything that relies on the old implementation.
We can also use the parity_ namespace instead of eth_ to implement parity_getBlockByHash and parity_getBlockByNumber as suggested here #9404 (comment) without the need to wait for an EIP to be accepted. what do you think?

@medvedev1088
Copy link
Author

@shamardy I agree, we can proceed with the parity_ namespace, and once the EIP is accepted move it to the eth_ namespace.

@gitcoinbot
Copy link

⚡️ A tip worth 245.00000 DAI (245.0 USD @ $1.0/DAI) has been granted to @shamardy for this issue from @vs77bb. ⚡️

Nice work @shamardy! Your tip has automatically been deposited in the ETH address we have on file.

@vs77bb
Copy link

vs77bb commented Aug 29, 2018

Hi @shamardy I closed this out on Gitcoin and paid you out 70% of the original bounty, as the work was completed + the EIP wasn't in the original scope. Good luck taking this forward to EIP and happy to pay out for the rest if it makes sense in the future 👍

@gitcoinbot
Copy link

gitcoinbot commented Aug 29, 2018 •
edited
Loading

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

Work has been started.

These users each claimed they can complete the work by 7 months ago.
Please review their action plans below:

1) shamardy has started work.

Working on this

Learn more on the Gitcoin Issue Details page.

@gitcoinbot
Copy link

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

The funding of 350.0 DAI (350.0 USD @ $1.0/DAI) attached to this issue has been approved & issued.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
F8-enhancement 🎊 An additional feature request. M6-rpcapi 📣 RPC API. P9-somedaymaybe 🌞 Issue might be worth doing eventually.
Projects
None yet
Milestone
2.1 Whatever
Development

Successfully merging a pull request may close this issue.

Add option to include transaction receipts in eth_getBlockByHash #9075 shamardy/parity
7 participants
@medvedev1088 @debris @5chdn @vs77bb @gitcoinbot @Tbaut @shamardy