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

Add option to include transaction receipts in eth_getBlockByHash #9075

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

Comments

Projects
None yet
7 participants
@medvedev1088
Copy link

commented Jul 9, 2018

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 this to the 2.0 milestone Jul 9, 2018

@Tbaut Tbaut added this to Needs Assignment in Core via automation Jul 9, 2018

@Tbaut Tbaut moved this from Needs Assignment to Easy Picks (Good for external contribution) in Core Jul 9, 2018

@gitcoinbot

This comment has been minimized.

Copy link

commented Jul 10, 2018

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

This comment has been minimized.

Copy link

commented Jul 10, 2018

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

This comment has been minimized.

Copy link

commented Jul 14, 2018

@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

@debris

This comment has been minimized.

Copy link
Member

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 Aug 10, 2018

Core automation moved this from Easy Picks (Good for external contribution) to Done Aug 10, 2018

@Tbaut Tbaut removed the Q3-bounty 💸 label Aug 10, 2018

@gitcoinbot

This comment has been minimized.

Copy link

commented Aug 10, 2018

@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

This comment has been minimized.

Copy link

commented Aug 13, 2018

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

This comment has been minimized.

Copy link
Contributor

commented Aug 13, 2018

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

@gitcoinbot

This comment has been minimized.

Copy link

commented Aug 19, 2018

@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

This comment has been minimized.

Copy link
Contributor

commented Aug 21, 2018

@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

This comment has been minimized.

Copy link
Author

commented Aug 21, 2018

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

@shamardy

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

@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

This comment has been minimized.

Copy link
Author

commented Aug 26, 2018

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

@gitcoinbot

This comment has been minimized.

Copy link

commented Aug 29, 2018

⚡️ 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

This comment has been minimized.

Copy link

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

This comment has been minimized.

Copy link

commented Aug 29, 2018

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

This comment has been minimized.

Copy link

commented Aug 29, 2018

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 join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.