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(providers): ens::namehash with Unicode characters #2510

Merged
merged 3 commits into from Jul 15, 2023

Conversation

Sabnock01
Copy link
Contributor

@Sabnock01 Sabnock01 commented Jul 15, 2023

Motivation

ens::namehash doesn't correctly compute nodes for ENS names that contain Unicode characters as documented in foundry#5386

Currently, when providers::resolve_name is called on an ENS name like ret↩️rn.eth, it will produce a flawed node.
ethers-rs treats the nodes as sets of 32 decimal values that when converted into their hex equivalent, concatenated, and prepended with 0x, produce a node which is used to get the resolver contract for the ENS name.

# Currently, `ens::namehash` will return:
[58, 221, 97, 107, 77, 172, 118, 9, 34, 35, 212, 59, 68, 92, 73, 191, 217, 137, 183, 219, 23, 92, 60, 236, 100, 137, 88, 71, 201, 249, 185, 116]

# This will be converted to the invalid node:
0xadb545a232c011f5aa341c04e144e7ab15be672fe94cfea9138fc0b293229ad5
# The correct return value should be: 
[61, 229, 244, 192, 45, 182, 27, 34, 30, 125, 231, 241, 196, 14, 41, 182, 226, 240, 126, 180, 141, 101, 191, 126, 48, 71, 21, 205, 158, 211, 59, 36]

# which produces the valid node:
0x3de5f4c02db61b221e7de7f1c40e29b6e2f07eb48d65bf7e304715cd9ed33b24

Nodes can be verified through the ENS Registry contract's resolver and owner functions here.

Solution

The reason for the discrepancy is that ret↩️rn.eth was not being stripped of its trailing VARIATION SELECTOR 16 after the Unicode character. ENS ignores these but the namehash function did not account for them.

The namehash function has been modified to strip any VS16's thus enabling it to successfully resolve ENS names with emojis.

A test with the appropriate name has been added for verification.

PR Checklist

  • Added Tests
  • Added Documentation
  • Breaking changes

@Sabnock01 Sabnock01 changed the title [WIP]: ens::namehash with Unicode characters fix(providers): ens::namehash with Unicode characters Jul 15, 2023
@Sabnock01 Sabnock01 marked this pull request as ready for review July 15, 2023 07:20
Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

lgtm

wdyt @DaniPopes

@DaniPopes DaniPopes merged commit 4a6984e into gakonst:master Jul 15, 2023
12 of 19 checks passed
@Sabnock01 Sabnock01 deleted the sabnock/fix-ens-namehash branch July 15, 2023 21:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants