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

[Bug]: Gas - Duplicated fee section for Legacy transactions in L2 #23291

Closed
seaona opened this issue Mar 4, 2024 · 9 comments · Fixed by #24264
Closed

[Bug]: Gas - Duplicated fee section for Legacy transactions in L2 #23291

seaona opened this issue Mar 4, 2024 · 9 comments · Fixed by #24264
Assignees
Labels
release-11.17.0 Issue or pull request that will be included in release 11.17.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug

Comments

@seaona
Copy link
Contributor

seaona commented Mar 4, 2024

Describe the bug

Problem: whenever we are performing a legacy transaction on Optimism, we see the gas fees section duplicated

Screenshot from 2024-03-04 08-37-25

Expected behavior

Not sure how the design/organization of the fees should be, but it feels strange to see the same info twice. Once in the main gas fee and the other under gas fee details.
cc @bschorchit

Screenshots/Recordings

No response

Steps to reproduce

  1. Select Optimism
  2. Trigger a legacy tx from the test dapp
  3. See gas fees

Error messages or log output

No response

Version

11.10.0 but possibly earlier

Build type

None

Browser

Chrome

Operating system

Linux

Hardware wallet

No response

Additional context

No response

Severity

No response

@seaona seaona added type-bug Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead labels Mar 4, 2024
@bschorchit
Copy link

We should not duplicate information or fees and we should display fees in the same format that we currently do for non legacy transactions.
Screenshot 2024-03-21 at 16 48 18
Screenshot 2024-03-21 at 16 48 25

@pedronfigueiredo
Copy link
Contributor

Hey team! Please add your planning poker estimate with Zenhub @blackdevelopa @digiwand @jpuri @seaona @segun

@seaona
Copy link
Contributor Author

seaona commented Mar 27, 2024

Context: when the new Gas Display was introduced it was decided to leave out of scope the Legacy transactions, and fix it in another PR. This has not been done yet. However, I think that the scope of this ticket is not to fix the design, but just to fix the duplicated fees

#19960 (comment)

@jpuri jpuri self-assigned this Apr 23, 2024
@jpuri
Copy link
Contributor

jpuri commented Apr 23, 2024

This looks already fixed in latest code, can you plz confirm @seaona

Screenshot 2024-04-23 at 5.34.31 PM.png

@seaona
Copy link
Contributor Author

seaona commented Apr 23, 2024

hey Jyoti I think that the issue is we don't want to show the same L1 and L2 sections on both the gas section and the fee details sections, since then there is no actual point of having the Fee details section.
So the solution should be to follow the same approach we do for EIP1559 tx, where the L1 and L2 gas split is only shown on the Fee details.

Does it make sensE?

@cryptotavares cryptotavares added the team-confirmations Push issues to confirmations team label Apr 24, 2024
@jpuri
Copy link
Contributor

jpuri commented Apr 25, 2024

Hey @seaona , @bschorchit

will something like this screenshot look good ?

Screenshot 2024-04-25 at 5.47.58 PM.png

@bschorchit
Copy link

bschorchit commented Apr 25, 2024

Hey @jpuri , the part below Fee details look good. The part above it needs some small tweaks. We should call Estimated fee instead of Total there, hide Amount+fees row and reduce the spacing between the Edit button and the rest of the info.

@SayaGT
Copy link

SayaGT commented Apr 25, 2024

Yup agree with @bschorchit. Here is a screenshot for reference.
Screenshot 2024-04-25 at 13.45.19.png

@SayaGT
Copy link

SayaGT commented Apr 25, 2024

You can find the design here @jpuri

@metamaskbot metamaskbot added the release-11.17.0 Issue or pull request that will be included in release 11.17.0 label May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-11.17.0 Issue or pull request that will be included in release 11.17.0 Sev2-normal Normal severity; minor loss of service or inconvenience. team-confirmations Push issues to confirmations team team-confirmations-secure-ux DEPRECATED: please use "team-confirmations" label instead type-bug
Projects
Archived in project
Status: Fixed
Development

Successfully merging a pull request may close this issue.

7 participants