Skip to content

Adding a warning when sending a token to its own contract address#10546

Merged
ryanml merged 1 commit intodevelopfrom
fix-9437-2
Mar 3, 2021
Merged

Adding a warning when sending a token to its own contract address#10546
ryanml merged 1 commit intodevelopfrom
fix-9437-2

Conversation

@ryanml
Copy link
Copy Markdown
Contributor

@ryanml ryanml commented Mar 2, 2021

Fixes #9437

Screen Shot 2021-03-02 at 10 13 00 AM

Manual testing steps:

  • Obtain some test ERC-20 tokens, such as LINK
  • Go to send, enter 0x514910771af9ca656af840dff83e8264ecf986ca (LINK contract address)
  • Confirm when ETH or other asset is selected to send that the error does not show
  • Confirm the error shows when LINK is the selected asset

@ryanml ryanml self-assigned this Mar 2, 2021
@ryanml ryanml requested a review from a team as a code owner March 2, 2021 17:37
@ryanml ryanml requested a review from brad-decker March 2, 2021 17:37
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 2, 2021

CLA Signature Action:

Thank you for your submission, we really appreciate it. We ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository.

0 out of 1 committers have signed the CLA.
@ryanml

GitHub can't find an account for ryanml.
You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.

@brad-decker
Copy link
Copy Markdown
Contributor

running yarn lint:fix will organize the messages file alphabetically, but other than lint failure this looks good to me. Obviously, three alerts here is a bit much, but it's a temporary holdover until we implement the new designs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I noticed a bug here with the "Known Contract Address" warning that also presented itself for the new error. It seems these messages are not intended to show when ETH is the selected asset, however, when switching back to ETH from an ERC-20 token, the messages don't clear properly. The validate method of this component does not trigger when selecting a new send asset

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [a294423]
Page Load Metrics (617 ± 21 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint45725774
domContentLoaded5777366164421
load5787376174421
domInteractive5777356154421

Copy link
Copy Markdown
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@ryanml ryanml merged commit 3c6cdef into develop Mar 3, 2021
@ryanml ryanml deleted the fix-9437-2 branch March 3, 2021 00:28
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Warn when sending a token to its own address

4 participants