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

eth_feeHistory Implementation #204

Merged
merged 28 commits into from
Mar 10, 2023
Merged

eth_feeHistory Implementation #204

merged 28 commits into from
Mar 10, 2023

Conversation

sfyll
Copy link
Contributor

@sfyll sfyll commented Mar 9, 2023

Implement eth_feeHistory as requested in #200 .

Veracity check on public RPC response done by comparing the response's base fee per gas with the local view provided by the ExecutionPayload object. This approach has two main corollaries:

  • We take an optimistic view by assuming that if the response's base fee per block are correct, so are the rewards and gas ratios. Obviously, that's a subject for conversation;
  • The get_fee_history in execution/src/execution.rsfunction checks the parameters and discards blocks that are not stored (and as such "validated") in ExecutionPayload. That makes returned values slightly less deterministic than one might have hoped, but seem in line with Alchemy's.

Given that the most insightful test were done by impersonating the user, i.e. putting in place integration tests, these were implemented in a new directory tests/.

Please note that this is my first PR into a public project + first time coding in Rust. So feel free to go heavy on the changes !

@ncitron
Copy link
Collaborator

ncitron commented Mar 9, 2023

This is a good start. I do however think that we shouldn't trust the rest of response given that just part of it is provably correct. You can imagine attacks where an RPC sends correct base fees, but wildly high priority fees to trick Helios users into sending transactions with a high priority fee. They could then auction that artificially valuable transaction off to a miner using something like flashbots.

We should however be able to implement this method without needing any trust at all. Since Helios has pastExecutionPayloads, we can already get the base fees that we need. We also have the transaction list as a member of each ExecutionPayload, which we can use to decode the priority fees paid out in that block. With that list of priority fees, and a bit of math to figure out percentiles, we should be able to return a response with zero trust in the rpc.

Does this strategy make sense?

@sfyll
Copy link
Contributor Author

sfyll commented Mar 10, 2023

Hey guys,

Tried my best but unfortunately computing the reward parameters ourselves has quite some overhead (can push on another branch if needed, just let me know). As can be seen with reth, some client actually don't give an answer for this parameter.

I tried though, and unfortunately amount of request quickly starts going out of hand. Indeed, you need the gas_used parameter per transaction to accumulate it until you reach the gas_used percentile, at which point you can return the gas paid for that specific transaction. Ultimately, what that means is that you need to get the receipt for every transaction in each block queried as there is no other way to get the gas used (practical way at least, not planning to track opcode raised for that endpoint 😬 ) ...

If there was a way to prove that the content was correct without querying it ourselves (wink wink zero-knowledge proof), it would make getting that list possible. Maybe we can follow reth for now and re-assess later on?

Copy link
Collaborator

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Some minor changes requested. Overall, very nice work!

execution/src/errors.rs Outdated Show resolved Hide resolved
execution/src/execution.rs Outdated Show resolved Hide resolved
execution/src/execution.rs Outdated Show resolved Hide resolved
execution/src/execution.rs Outdated Show resolved Hide resolved
sfyll and others added 3 commits March 10, 2023 17:47
Co-authored-by: refcell.eth <abigger87@gmail.com>
Co-authored-by: refcell.eth <abigger87@gmail.com>
Copy link
Collaborator

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Very nice!

@refcell refcell changed the base branch from master to fee_history March 10, 2023 18:18
@refcell
Copy link
Collaborator

refcell commented Mar 10, 2023

Hey @sfyll, merging into a fee_history branch in the a16z/helios repo so we can resolve these formatting and api key issues locally :)

@refcell refcell merged commit 552f7bc into a16z:fee_history Mar 10, 2023
@refcell refcell mentioned this pull request Mar 10, 2023
@sfyll sfyll deleted the fee_history branch March 11, 2023 11:34
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.

3 participants