Skip to content

fix: eth_feeHistory#7792

Merged
mattsse merged 3 commits intomasterfrom
klkvr/fix-feehistory
Apr 27, 2024
Merged

fix: eth_feeHistory#7792
mattsse merged 3 commits intomasterfrom
klkvr/fix-feehistory

Conversation

@klkvr
Copy link
Member

@klkvr klkvr commented Apr 26, 2024

Motivation

Closes #7769

Logic for determining pending block base fee is currently different in eth_feeHistory handler and miner itself. Thus, using value for pending block base fee returned by feeHistory will result in errors as it will be lower than actual base fee

This PR contains 2 changes:

  1. Correctly process empty genesis block, thus base fee for 1st block is 87500000000 by default as genesis block is always empty.
  2. Use alloy calculation fn in eth_feeHistory.

@klkvr klkvr marked this pull request as ready for review April 26, 2024 19:11
@klkvr klkvr changed the title Klkvr/fix feehistory fix: eth_feeHistory Apr 26, 2024
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

I see, nice find!

Comment on lines 1323 to 1328
Copy link
Member

Choose a reason for hiding this comment

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

nice

@klkvr klkvr force-pushed the klkvr/fix-feehistory branch from a651007 to b13d7e9 Compare April 26, 2024 19:15
@klkvr klkvr force-pushed the klkvr/fix-feehistory branch from b13d7e9 to 012337b Compare April 26, 2024 23:08
@klkvr
Copy link
Member Author

klkvr commented Apr 26, 2024

@mattsse decided to go with a simpler fix and just push the backend.fees().base_fee() value to the response. don't think we should account for empty genesis as I believe users would expect block #1 base fee to be equal to the --base-fee flag value if provided, which is something we are breaking by accounting for genesis

Comment on lines -1058 to +1063
// notify all listeners
self.notify_on_new_block(header, block_hash);

// update next base fee
self.fees.set_base_fee(next_block_base_fee);

// notify all listeners
self.notify_on_new_block(header, block_hash);

Copy link
Member Author

Choose a reason for hiding this comment

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

this was a subject to race condition anyway, but this just ensures that value in fees is always up to date

Copy link
Member

Choose a reason for hiding this comment

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

right, nice catch

Comment on lines -1058 to +1063
// notify all listeners
self.notify_on_new_block(header, block_hash);

// update next base fee
self.fees.set_base_fee(next_block_base_fee);

// notify all listeners
self.notify_on_new_block(header, block_hash);

Copy link
Member

Choose a reason for hiding this comment

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

right, nice catch

@mattsse mattsse merged commit 12e53e5 into master Apr 27, 2024
@mattsse mattsse deleted the klkvr/fix-feehistory branch April 27, 2024 03:42
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.

anvil: problems with estimate gas - max fee per gas less than block base fee

2 participants