-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Fixing ENS input entry in send flow #10923
Conversation
Builds ready [25a028f]
Page Load Metrics (600 ± 37 ms)
|
@@ -314,7 +314,7 @@ export default class EnsInput extends Component { | |||
} | |||
|
|||
if (ensFailure) { | |||
return <i className="fa fa-warning fa-lg warning'" />; | |||
return <i className="fa fa-warning fa-lg warning" />; |
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.
this just seemed to be a typo
Builds ready [60ae1b9]
Page Load Metrics (556 ± 43 ms)
|
Builds ready [d6d1f59]
Page Load Metrics (568 ± 38 ms)
|
@@ -22,7 +22,7 @@ export function getToErrorObject(to, sendTokenAddress, chainId) { | |||
let toError = null; | |||
if (!to) { | |||
toError = REQUIRED_ERROR; | |||
} else if (!isValidAddress(to)) { | |||
} else if (!isValidAddress(to) && !isValidDomainName(to)) { |
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.
I see we use isValidDomainName
throughout the codebase to support ENS checks: https://github.com/MetaMask/metamask-extension/blob/develop/ui/app/pages/settings/contact-list-tab/add-contact/add-contact.component.js#L57
In this case, it has no improper fall through as part of add-recipient.js
. It allows ENS names to move forward, and in the case of a valid domain name, not an ENS name, (ex: brantly.com
) it will just default to the address book lookup, as domain names are fine there.
Great work, @ryanml |
Fixes #10691
Explanation:
With recent updates to
isValidAddress
as provided byethereumjs-util
, it is not accepting of invalid hex strings.This adds an
isHex
validation check to our shared utility forisValidAddress
to address the regression. With regards to #10691, we additionally check domain validity so as to not throw an invalid recipient error.Manual testing steps:
Steps for #10691 are defined in the issue. A general regression test should be performed with sending to ENS and hex addresses.
With fix: