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

fix(fees): set tip on Celo from eth_maxPriorityFeePerGas #4444

Merged
merged 2 commits into from
Nov 7, 2023

Conversation

jeanregisser
Copy link
Member

@jeanregisser jeanregisser commented Nov 7, 2023

Description

We initially got the recommendation to only set maxFeePerGas to the result of eth_gasPrice and that the difference between it and the base fee would be the tip distributed to the validator.

However after testing, we noticed that if we leave maxPriorityFeePerGas to undefined (or if we don't specify it), the final serialized tx has is set to 0.
Resulting to 0 tip being effectively distributed to the validator. Which doesn't look right for validators 😄

So here we bring back the original implementation I was originally proposing where we set it to the result of eth_maxPriorityFeePerGas. Similar to what viem does on Ethereum.

Note: we'll wait for the confirmation from cLabs before merging this though.

Test plan

  • Updated tests

Related issues

  • N/A

Backwards compatibility

Yes

@jeanregisser jeanregisser changed the title fix(fees): add tip via maxPriorityFeePerGas fix(fees): add tip on Celo via maxPriorityFeePerGas Nov 7, 2023
@jeanregisser jeanregisser changed the title fix(fees): add tip on Celo via maxPriorityFeePerGas fix(fees): set tip on Celo from eth_maxPriorityFeePerGas Nov 7, 2023
Copy link

codecov bot commented Nov 7, 2023

Codecov Report

Merging #4444 (ed14032) into main (b06f1e3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #4444   +/-   ##
=======================================
  Coverage   85.09%   85.09%           
=======================================
  Files         711      711           
  Lines       26380    26386    +6     
  Branches     3573     3574    +1     
=======================================
+ Hits        22448    22454    +6     
  Misses       3869     3869           
  Partials       63       63           
Files Coverage Δ
src/viem/estimateFeesPerGas.ts 100.00% <100.00%> (ø)

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b06f1e3...ed14032. Read the comment docs.

Copy link
Contributor

@bakoushin bakoushin left a comment

Choose a reason for hiding this comment

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

💯

@jeanregisser jeanregisser merged commit b31dfd0 into main Nov 7, 2023
18 checks passed
@jeanregisser jeanregisser deleted the jeanregisser/celo-fee-tip branch November 7, 2023 17:20
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