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

internal/ethapi,webext: add debug_getRawReceipt RPC #24741

Conversation

ryanschneider
Copy link
Contributor

@ryanschneider ryanschneider commented Apr 22, 2022

This is a small tweak to #24726, which I closed since it didn't actually return the correct data for EIP-2718 non-legacy transactions. Turns out the fix was quite simple, just call receipt.MarshalBinary() instead of rlp.EncodeToBytes(receipt), and now the EIP-2718 receipts are handled correctly.

Otherwise, follows the same pattern as debug_getBlockRlp above it, using the same preamble code as eth_getTransactionReceipt (https://github.com/ethereum/go-ethereum/blob/master/internal/ethapi/api.go#L1547).

I fully admit the RPC name is now crossing into unwieldy territory, happy to change it to something more concise if any suggestions arise.. RPC name was changed to debug_getRawReceipt to match Felix's suggestion in #24738 (comment)

Tested on this Sepolia transaction:

> debug.getRawReceipt("0x30296f5f32972c7c3b39963cfd91073000cb882c294adc2dcf0ac9ca34d67bd2")
"0x02f901850182aebbb9010000800000000000000000000000000000000000000000100000020000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000400000000000000000f87cf87a94830bf80a3839b300291915e7c67b70d90823ffedf842a0e1fffcc4923d04b559f4d29a8bfc6cda04eb5b0d3c460751c2402c5c5cc9109ca00000000000000000000000001b57edab586cbdabd4d914869ae8bb78dbc05571a00000000000000000000000000000000000000000000000000de0b6b3a7640000"

https://sepolia.otterscan.io/tx/0x30296f5f32972c7c3b39963cfd91073000cb882c294adc2dcf0ac9ca34d67bd2

I used the ethereumjs trie implementation at https://github.com/ethereumjs/ethereumjs-monorepo/blob/master/packages/trie/src/baseTrie.ts#L43 to confirm that this data correctly recreates the containing blocks receipt root (the block in question only had this one tx in it) using this code snippet: https://replit.com/@ryanleeschneider/TrieTest#index.ts

@ryanschneider ryanschneider force-pushed the debug_getTransactionReceiptBinary branch from 8ee0b5a to 2984538 Compare April 25, 2022 20:27
@ryanschneider ryanschneider changed the title internal/ethapi,webext: add debug_getTransactionReceiptBinary RPC internal/ethapi,webext: add debug_getRawReceipt RPC Apr 25, 2022
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM, but it would be nice to move this off the debug namespace and place it alongside the func (s *PublicTransactionPoolAPI) GetTransactionReceipt further up in this file. I guess that's for later, though...(?)

@s1na
Copy link
Contributor

s1na commented May 23, 2022

@holiman do you think there are use-cases where debug_getRawReceipts can't be used? do we really need both?

@lightclient
Copy link
Member

My 2 wei is that we should stick with debug_getRawReceipts until there is a strong motivation for adding this level of granularity.

@ryanschneider
Copy link
Contributor Author

Closing since rough consensus seems to be that #24773 covers use cases enough for now, happy to reopen and rebase if we change our minds later.

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

Successfully merging this pull request may close these issues.

4 participants