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

1559: Removes unnecessary max #3640

Merged
merged 1 commit into from
Jul 28, 2021
Merged

1559: Removes unnecessary max #3640

merged 1 commit into from
Jul 28, 2021

Conversation

MicahZoltu
Copy link
Contributor

We can prove that the parent_base_fee_per_gas - base_fee_per_gas_delta > 0, so flooring to 0 is unnecessary.

We can prove that the `parent_base_fee_per_gas - base_fee_per_gas_delta > 0`, so flooring to 0 is unnecessary.
expected_base_fee_per_gas = max(parent_base_fee_per_gas - base_fee_per_gas_delta, 0)
expected_base_fee_per_gas = parent_base_fee_per_gas - base_fee_per_gas_delta
Copy link
Contributor Author

@MicahZoltu MicahZoltu Jul 5, 2021

Choose a reason for hiding this comment

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

  • (parent_gas_target - parent_gas_used) / parent_gas_target is always between 0 and 1 since parent_gas_used < parent_gas_target in this code path and both numbers are always positive.
  • This means that base_fee_per_gas_delta will be between 0/8 and parent_base_fee_per_gas/8 in this code path.
  • Which means base_fee_per_gas_delta will always be greater than or equal to 0.

@eth-bot eth-bot enabled auto-merge (squash) July 5, 2021 18:02
@eth-bot
Copy link
Collaborator

eth-bot commented Jul 5, 2021

Hi! I'm a bot, and I wanted to automerge your PR, but couldn't because of the following issue(s):

 - eip-1559.md requires approval from one of (vbuterin, econoar, afdudley, mslipper, i-norden, abdelhamidbakhta)

@vbuterin @econoar @AFDudley @mslipper @i-norden @abdelhamidbakhta

Copy link
Member

@lightclient lightclient left a comment

Choose a reason for hiding this comment

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

Looks correct to me.

@alita-moore
Copy link
Contributor

@MicahZoltu @lightclient looks like the bot is also expecting author review, does editor review override lack of author in this case?

@lightclient
Copy link
Member

@alita-moore no, I was just approving to share my opinion. It's up to the authors to merge.

@axic
Copy link
Member

axic commented Jul 27, 2021

Copy link
Contributor

@AbdelStark AbdelStark left a comment

Choose a reason for hiding this comment

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

Looks good to me

@axic
Copy link
Member

axic commented Jul 27, 2021

Ping @MicahZoltu.

@MicahZoltu MicahZoltu closed this Jul 28, 2021
auto-merge was automatically disabled July 28, 2021 06:34

Pull request was closed

@MicahZoltu MicahZoltu reopened this Jul 28, 2021
@eth-bot eth-bot enabled auto-merge (squash) July 28, 2021 06:34
@eth-bot eth-bot merged commit 289fbaa into master Jul 28, 2021
@eth-bot eth-bot deleted the MicahZoltu-patch-3 branch July 28, 2021 06:38
PhABC pushed a commit to PhABC/EIPs that referenced this pull request Jan 25, 2022
We can prove that the `parent_base_fee_per_gas - base_fee_per_gas_delta > 0`, so flooring to 0 is unnecessary.
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.

6 participants