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

fix: Correct conditions for triggering name lookup during send #27880

Merged
merged 5 commits into from
Oct 16, 2024

Conversation

FrederikBolding
Copy link
Member

@FrederikBolding FrederikBolding commented Oct 15, 2024

Take changes from 5e08c06

Description

Hotfixes a problem preventing certain name resolution Snaps from being triggered due to faulty IS_FLASK conditions in 12.4.1. These conditions were removed in #26242. This PR picks these changes from the previously mentioned PR, without touching the ENS integration.

The problem in question occurs when trying to trigger name resolution for a given input. lookupDomainName is never called on stable unless the input looks similar to an ENS name. This prevents resolution of inputs that don't use TLDs for instance.

Open in GitHub Codespaces

Manual testing steps

  1. Use a build of stable
  2. Install https://snaps.metamask.io/snap/npm/social-names-snap/
  3. See that you can resolve a Farcaster name using that Snap, e.g. fc:frederik
  4. Check that you can also type a valid ENS name
  5. Check that you can also type a valid address

@FrederikBolding FrederikBolding added the team-snaps-platform Snaps Platform team label Oct 15, 2024
@FrederikBolding FrederikBolding requested review from a team as code owners October 15, 2024 20:27
@metamaskbot metamaskbot added the INVALID-PR-TEMPLATE PR's body doesn't match template label Oct 15, 2024
Comment on lines +70 to +73
* Determines if a string is a possible ethereum address
*
* @param candidate - the input to check
* @returns true if the input is a 40 char hex string with optional 0x prefix, false otherwise
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Determines if a string is a possible ethereum address
*
* @param candidate - the input to check
* @returns true if the input is a 40 char hex string with optional 0x prefix, false otherwise
* Determine if a string is a possible Ethereum address.
*
* @param candidate - The input to check.
* @returns `true` if the input is a 40-character-long hex string with optional 0x-prefix, `false` otherwise.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is picked directly from develop. I don't think it makes sense to improve it much in master 😄

Mrtenz
Mrtenz previously approved these changes Oct 15, 2024
Mrtenz
Mrtenz previously approved these changes Oct 15, 2024
Copy link
Contributor

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.

@metamaskbot
Copy link
Collaborator

Builds ready [0c9f4e8]
Page Load Metrics (1838 ± 89 ms)
PlatformPageMetricMin (ms)Max (ms)Average (ms)StandardDeviation (ms)MarginOfError (ms)
ChromeHomefirstPaint36122681760370178
domContentLoaded15882199181817986
load15972265183818689
domInteractive146537147

@danjm danjm merged commit e4672a8 into Version-v12.4.2 Oct 16, 2024
72 of 74 checks passed
@danjm danjm deleted the fb/name-lookup-hotfix branch October 16, 2024 10:35
@github-actions github-actions bot locked and limited conversation to collaborators Oct 16, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
INVALID-PR-TEMPLATE PR's body doesn't match template team-snaps-platform Snaps Platform team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants