Skip to content

Fix token validation in Send flow#10045

Merged
Gudahtt merged 1 commit intodevelopfrom
fix-send-token-validation
Dec 10, 2020
Merged

Fix token validation in Send flow#10045
Gudahtt merged 1 commit intodevelopfrom
fix-send-token-validation

Conversation

@Gudahtt
Copy link
Copy Markdown
Member

@Gudahtt Gudahtt commented Dec 10, 2020

Additional validation was added in #9907 to ensure that the "Known contract address" warning was shown when sending tokens to another token address after switching assets on the Send screen. Unfortunately this change had the unintended side-effect of preventing all token sends after switching assets, so long as the recipient was not an internal address.

The problem is that the validate function expects to be passed the address of the token send recipient in the case where a token is selected. Instead the token address was being passed to the validate function.

The query state is now used, which should always contain the recipient address. This is the same state used in the only other place the validate function is called.

Fixes: #10044

Manual testing steps:

To test that the validation doesn't incorrectly flag a non-contract-address as a contract address:

  1. Click 'Send' on the Home screen
  2. Enter some external address (one that is not a contract address)
  3. Select a token in the "Asset" dropdown
  4. You should not see the "Known contract address" warning

To test that the validation still works when the recipient really is a known token address:

  1. On the home screen, Click Send button.
  2. Add a token contract to recipient address. i.e. 0x0F5D2fB29fb7d3CFeE444a200298f468908cC942 (MANA)
  3. On the Screen, the Asset should be ETH. Change to any token.
  4. The warning Known contract address should show.

Additional validation was added in #9907 to ensure that the "Known
contract address" warning was shown when sending tokens to another
token address after switching assets on the Send screen. Unfortunately
this change had the unintended side-effect of preventing _all_ token
sends after switching assets, so long as the recipient was not an
internal address.

The problem is that the `validate` function expects to be passed the
address of the token send recipient in the case where a token is
selected. Instead the token address was being passed to the validate
function.

The `query` state is now used, which should always contain the
recipient address. This is the same state used in the only other place
the `validate` function is called.
@Gudahtt Gudahtt marked this pull request as ready for review December 10, 2020 17:34
@Gudahtt Gudahtt requested a review from a team as a code owner December 10, 2020 17:34
@Gudahtt Gudahtt requested a review from darkwing December 10, 2020 17:34
Copy link
Copy Markdown
Contributor

@darkwing darkwing left a comment

Choose a reason for hiding this comment

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

I executed both sets of testing steps manually, both worked as described. Well done, so quick!

@metamaskbot
Copy link
Copy Markdown
Collaborator

Builds ready [1f9de59]
Page Load Metrics (582 ± 24 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint326749115
domContentLoaded5127425815124
load5137435825124
domInteractive5127415815124

@Gudahtt Gudahtt merged commit 4350a14 into develop Dec 10, 2020
@Gudahtt Gudahtt deleted the fix-send-token-validation branch December 10, 2020 17:58
@github-actions github-actions bot locked and limited conversation to collaborators Dec 10, 2020
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.

Invalid "Known contract address" warning shown in token send flow

3 participants