-
Notifications
You must be signed in to change notification settings - Fork 5.4k
fix: misleading gas fee label in UI #22434
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
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
I have read the CLA Document and I hereby sign the CLA |
179ff24 to
8311e85
Compare
77ad231 to
bb40278
Compare
|
I'm confused about why this PR fails the step |
|
In the first screenshot,
I did simple math. Wait... Yes, technically, max base fee (per gas) is the same as max fee (per gas) because it is a upper limit. If it is intended, please ignore this pull request. |
|
Hmm, I think it IS wrong, but in a different way. I think the "Max fee" in ETH is not counting the "Priority fee." I'm going to post this internally and get opinions. |
|
Follow up on slack discussion around this issue https://consensys.slack.com/archives/C4V2HTG0Y/p1708710402351709 |
brad-decker
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Erroneously approved. waiting for resolution of conversation internally.
|
This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions. |
digiwand
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The update looks good to me. Base fee ≠ MaxFeePerGas and Max Fee Per Gas may be shortened to Max Fee.
If we wanted to also display the base fee, we could add this in a follow-up PR.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #22434 +/- ##
===========================================
- Coverage 65.77% 65.77% -0.00%
===========================================
Files 1366 1366
Lines 54254 54254
Branches 14101 14101
===========================================
- Hits 35685 35684 -1
- Misses 18569 18570 +1 ☔ View full report in Codecov by Sentry. |
Description
The UI label for max
basefee per gas is "Max base fee" (keymaxBaseFee). I correct this label to "Max fee" (keymaxFee). Thank Hee Jung Sonny Son for reporting!Related issues
Manual testing steps
Screenshots/Recordings
Before
screenshot in English https://gist.github.com/assets/3060954/a1850425-f25a-46a3-960c-5ab36c390632
screenshot in Korean https://gist.github.com/assets/3060954/6737b0c5-778b-43d4-bd0c-393e4b664aad
After
Pre-merge author checklist
Pre-merge reviewer checklist