Add support for ENSIP-10 #2411

fselmo · 2022-04-01T17:10:25Z

What was wrong?

ENSIP-10 was introduced and we should implement support for this.

Related to issue #2406

How was it fixed?

Added extended resolver ABI and support for wildcard resolution
Added related tests including creating some simple resolver contracts for the purpose of testing some of the basic functionality. Hopefully we can test against more robust test vectors soon. Also manually tested against hatch.eth ens domain on Ropsten as recommended in issue Support for new ENS features #2406.
ens.utils.ens_encode_name() and ENS.parent() methods added to provide support for this implementation

Todo:

Add tests for resolving contracts that support the extendedResolver interface and functionality
Add entry to the release notes
Add cute animal picture

Cute Animal Picture

fselmo · 2022-04-18T11:19:39Z

This is ready for review / discussion.

~~All that's missing from the To Do list was to add testing around the decodeABI() method and I'll try to get that in here in the next few days or so.~~ <--[no need to expose this api as of yet so this is made internal and is integration tested via the wildcard resolution tests]

Of importance here is, because this is such a new functionality, there aren't official test vectors for wildcard resolution. I did manually test on Ropsten as was recommended but for the purposes of our test suite I just added a SimpleExtendedResolver.sol that makes sure to test the parent ens domain for extended-resolver.eth and its subdomains with different logic appropriate for each case and "resolves" different addresses in each case. It's not an ideal test but it's something to have in our test suite for now until hopefully we have some official test vectors to test against.

makoto · 2022-04-25T17:39:54Z

FYI,

Tested makoto.hatch.eth and seems working

>>> w3.ens.address('makoto.hatch.eth')
'0x0Bc0035cE036984749Fd22978394734B89690b5e'

ens/utils.py

fselmo · 2022-04-28T08:00:56Z

FYI,

Tested makoto.hatch.eth and seems working

@makoto Thanks for testing. Yeah... it was easy to test manually with the examples that were given but I wanted to add some in-house tests that were a bit more difficult for us without official test vectors. I ended up just writing a simple resolver for now. Not ideal but it ends up testing that the functionality works at least.

I appreciated the test examples though. That definitely helped manually test everything 👍

kclowes

LGTM! Thanks for taking this on! I added some comments but nothing major. Some things we should check before we merge if you haven't already:

did you check to make sure the doc generation works? Do you think it's worth adding docs to the ENS overview too?
I can't remember what we decided about backporting this to v5? If we do want to backport, the fn_name needs to be changed back to get for the v5 PR, and the error classes should remain the same.

kclowes · 2022-04-28T21:16:15Z

ens/main.py

+    def resolver(self, normal_name: str) -> Optional['Contract']:
+        return self._get_resolver(normal_name)[0]
+
+    def resolve(self, name: str, fn_name: str = 'addr') -> Optional[Union[ChecksumAddress, str]]:


🤔 I can't remember where we landed on the change from get -> fn_name. Did we decide this was going to be a v6 only feature?

Yeah... changing the signature should def be v6 only. I think it makes more sense for anyone reading the code coming in. v5 would have to remain the same so as to not be a breaking change.

edit for future reference: This will be a v6 only addition since, technically, the new parent resolution functionality is a breaking change.

ens/main.py

tests/ens/test_get_text.py

tests/ens/test_setup_address.py

fselmo · 2022-04-29T00:20:16Z

did you check to make sure the doc generation works? Do you think it's worth adding docs to the ENS overview too?

I think the ENS module could use a big improvement in the docs in general... was thinking about updating the docs in a separate PR but I could tack it on here. Thoughts? I did not check the doc generation... I'll check that out too.

edit: Added some doc improvements here

- Adds support for extended resolvers for wildcard resolution as defined by ENSIP-10. - Adds the method ``parent()`` to the ENS class whose function is to extract the proper parent from an ENS name. - Adds the ``ens_encode_name()`` method that uses DNS name-encoding standards with a few tweaks, such as skipping the fully-encoded length validation for the domain (limit of 255 for DNS) and encoding the empty names as defined by ENSIP-10 as a single zero byte ``b'\x00'``. Unrelated: - Minor cleanup for some existing ens tests

- Test wildcard resolver functionality using a basic resolver with support for the `resolve()` method and validate the parent ens domain and its subdomains within the resolve method of the contract. - Test the new ``ens_encode_name()`` from the ``ens.utils`` module. - Test the new ``ENS.parent()`` method to extract a parent from an ENS name.

- Make ``ENS.resolve()`` an internal method to provide a better user experience / less confusion. ``ENS.address()`` and ``ENS.name()`` should better abstract forward and backward resolution without the need to expose the ``resolve()`` method as part of the API. - Normalize the name being passed into ``ENS.resolver()`` since this is an exposed method for user input. This is done on other methods for the class but was not yet done for ``ENS.resolver()``.

kclowes

This looks good to me! Nice work!

fselmo force-pushed the ensip10-issue-2406 branch 6 times, most recently from 1306e2a to 9b4ef00 Compare April 13, 2022 12:17

fselmo changed the title ~~[WIP] Add support for ENSIP-10~~ Add support for ENSIP-10 Apr 13, 2022

fselmo force-pushed the ensip10-issue-2406 branch 3 times, most recently from 2e8cb54 to 79c490b Compare April 18, 2022 11:05

fselmo marked this pull request as ready for review April 18, 2022 11:10

fselmo requested review from kclowes, wolovim and pacrob April 18, 2022 11:10

fselmo force-pushed the ensip10-issue-2406 branch 5 times, most recently from 96d9c4e to 457f510 Compare April 20, 2022 10:00

Arachnid approved these changes Apr 25, 2022

View reviewed changes

ens/utils.py Outdated Show resolved Hide resolved

ens/utils.py Show resolved Hide resolved

Arachnid approved these changes Apr 27, 2022

View reviewed changes

ens/utils.py Show resolved Hide resolved

ens/utils.py Outdated Show resolved Hide resolved

fselmo force-pushed the ensip10-issue-2406 branch from 457f510 to 5f95ffe Compare April 28, 2022 07:51

kclowes approved these changes Apr 28, 2022

View reviewed changes

fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 29, 2022

Improvements from comments in PR ethereum#2411

d4dbe17

fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 29, 2022

Improvements from comments in PR ethereum#2411

344bf44

fselmo force-pushed the ensip10-issue-2406 branch from d4dbe17 to 4d452b9 Compare April 29, 2022 21:00

fselmo force-pushed the ensip10-issue-2406 branch from 4d452b9 to fbe084e Compare April 29, 2022 21:04

fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 29, 2022

Add v6 warnings to ENS changes from pr ethereum#2411

41202ec

fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 29, 2022

Add v6 warnings to ENS changes from pr ethereum#2411

b9e3115

fselmo mentioned this pull request Apr 29, 2022

Add v6 warnings to ENS changes from pr #2411 #2452

Merged

1 task

fselmo added a commit to fselmo/web3.py that referenced this pull request Apr 29, 2022

Add v6 warnings to ENS changes from pr ethereum#2411

05bf3ac

fselmo force-pushed the ensip10-issue-2406 branch from fbe084e to 355dbac Compare April 30, 2022 17:00

fselmo added a commit to fselmo/web3.py that referenced this pull request May 2, 2022

Improvements from comments in PR ethereum#2411

60174c0

fselmo added 5 commits May 2, 2022 16:50

Improvements from comments in PR ethereum#2411

3c4d88c

Updates to ENS documentation

41e004b

fselmo force-pushed the ensip10-issue-2406 branch from 355dbac to 41e004b Compare May 2, 2022 22:50

kclowes approved these changes May 3, 2022

View reviewed changes

fselmo merged commit fe36d83 into ethereum:master May 3, 2022

fselmo added a commit that referenced this pull request May 3, 2022

Improvements from comments in PR #2411

ec7be3c

fselmo added a commit to fselmo/web3.py that referenced this pull request May 3, 2022

Add v6 warnings to ENS changes from pr ethereum#2411

c4a11a2

pacrob pushed a commit that referenced this pull request May 23, 2022

Improvements from comments in PR #2411

5e8f849

fselmo mentioned this pull request Jun 1, 2022

Support for new ENS features #2406

Closed

fselmo deleted the ensip10-issue-2406 branch June 8, 2022 21:21

fselmo added a commit that referenced this pull request Jun 11, 2022

Add v6 warnings to ENS changes from pr #2411

b6a7e04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for ENSIP-10 #2411

Add support for ENSIP-10 #2411

fselmo commented Apr 1, 2022 •

edited

Loading

fselmo commented Apr 18, 2022 •

edited

Loading

makoto commented Apr 25, 2022 •

edited

Loading

fselmo commented Apr 28, 2022

kclowes left a comment

kclowes Apr 28, 2022

fselmo Apr 28, 2022 •

edited

Loading

fselmo commented Apr 29, 2022 •

edited

Loading

kclowes left a comment

Add support for ENSIP-10 #2411

Add support for ENSIP-10 #2411

Conversation

fselmo commented Apr 1, 2022 • edited Loading

What was wrong?

How was it fixed?

Todo:

Cute Animal Picture

fselmo commented Apr 18, 2022 • edited Loading

makoto commented Apr 25, 2022 • edited Loading

fselmo commented Apr 28, 2022

kclowes left a comment

Choose a reason for hiding this comment

kclowes Apr 28, 2022

Choose a reason for hiding this comment

fselmo Apr 28, 2022 • edited Loading

Choose a reason for hiding this comment

fselmo commented Apr 29, 2022 • edited Loading

kclowes left a comment

Choose a reason for hiding this comment

fselmo commented Apr 1, 2022 •

edited

Loading

fselmo commented Apr 18, 2022 •

edited

Loading

makoto commented Apr 25, 2022 •

edited

Loading

fselmo Apr 28, 2022 •

edited

Loading

fselmo commented Apr 29, 2022 •

edited

Loading