-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
No Warning appears when a Dapp sets a really high Fees for a tx, potentially loosing all user funds #6358
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. |
db3a873
to
ceb25d1
Compare
app/components/UI/TransactionReview/TransactionReviewEIP1559/index.test.tsx
Show resolved
Hide resolved
PR looks good. @segun please add label |
3e1a1ef
to
44ae3db
Compare
hi @segun I'm seeing the following error when I build this branch, could you take a look? 🙏 |
44ae3db
to
ce99c98
Compare
Empty popoverWhenever I click on the red Warning symbol, I see an empty pop-over. Should we add any text here? cc @bschorchit This warning symbol is displayed in 2 cases:
In both cases, when I click it the popover is empty Warning not displayed on Legacy networksWhenever I am on a Legacy network (i.e. Optimism) I can see how no warning is displayed there, |
This should be the popover content: MetaMask/metamask-extension#17313 (comment) |
I believe the warning tooltip should also be next to Max fee instead of next to the time estimation similar to how it's on extension cc: @holantonela |
Agreed. |
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.
LGTM. Just small changes
… estimate times to orange and show an icon Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
…Use Number instead of +Use Number instead of +Use Number instead of +Use Number instead of +Use Number instead of +Use Number instead of + Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
When icon is clicked, show message Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
Signed-off-by: Olusegun Akintayo <akintayo.segun@gmail.com>
409891f
to
df65bf4
Compare
Kudos, SonarCloud Quality Gate passed! |
Description
When a Dapp triggers a transaction/contract interaction, it sets the proposed fees maxFeePerGas maxPriorityFeePerGas and gasLimit. MetaMask takes the values from the Dapp as they are and displayes Site suggested meaning that it's the Dapp who suggested the fees. Despite of that, if the Dapp sets a high value, user can loose all its money and there is no warning on MM side.
Screenshots/Recordings
Before
After
Issue
Fixes #6361
Checklist