ethereum / web3.py

A python interface for interacting with the Ethereum blockchain and ecosystem.
http://web3py.readthedocs.io
MIT License
5.01k stars 1.71k forks source link

Update ENS normalization to new standard #2967

Closed djstrong closed 1 year ago

djstrong commented 1 year ago

The NameHash team is open sourcing a new library that we hope can be useful for the Ethereum Python Community 🚀

https://github.com/namehash/ens-normalize-python

ENS has been developing a new standard for normalizing ENS names such as the ".eth" names you're familiar with. This library linked above is fully compliant with this new ENS Normalize standard. E.g. ens.domains is using the new standard.

web3py needs to be updated for this new ENS Normalize standard. The existing ENS API in web3py is no longer compliant. For example, names that are ENS Normalized will not be recognized as normalized by the current web3py implementation.

How to approach this? Should we create a pull request?

kclowes commented 1 year ago

Thanks for raising an issue! I looked briefly but couldn't immediately tell what had changed. Can you outline what is changing/won't be compliant, and what you hope to add/change in web3.py?

djstrong commented 1 year ago

@kclowes Thanks! New normalization standard is here: https://github.com/ensdomains/docs/blob/master/ens-improvement-proposals/ensip-15-normalization-standard.md ens.domains is using it (JS implementation: https://github.com/adraffy/ens-normalize.js)

You can check JS implementation online: https://adraffy.github.io/ens-normalize.js/test/resolver.html

If you ask for example names that status changes then you can try e.g. $alice.eth. ens.utils.normalize_name('$alice.eth') raises InvalidName but it is a valid name according to the new standard (you can also register it: https://app.ens.domains/%24alice.eth/register)

ens.utils.normalize_name is used in many other functions, e.g. is_valid_name, raw_name_to_hash.

Summing up, ens.utils.normalize_name needs to be changed.

Carbon225 commented 1 year ago

Hi, I'm Jakub from the NameHash team. A quick reminder, this is the web3py normalize_name function:

def normalize_name(name: str) -> str:
    """
    Clean the fully qualified name, as defined in ENS `EIP-137
    <https://github.com/ethereum/EIPs/blob/master/EIPS/eip-137.md#name-syntax>`_

    This does *not* enforce whether ``name`` is a label or fully qualified domain.

    :param str name: the dot-separated ENS name
    :raises InvalidName: if ``name`` has invalid syntax
    """
    if not name:
        return name
    elif isinstance(name, (bytes, bytearray)):
        name = name.decode("utf-8")

    try:
        return idna.uts46_remap(name, std3_rules=True, transitional=False)
    except idna.IDNAError as exc:
        raise InvalidName(f"{name} is an invalid name, because {exc}") from exc

To be compliant with the new normalization this would have to be replaced with a call to our ens_normalize function. We would also have to replace this error in ens.exceptions with our exception class:

class InvalidName(idna.IDNAError, ENSException):
    """
    This exception is raised if the provided name does not meet
    the syntax standards specified in `EIP 137 name syntax
    <https://github.com/ethereum/EIPs/blob/master/EIPS/eip-137.md#name-syntax>`_.

    For example: names may not start with a dot, or include a space.
    """

    pass

We could submit a PR with our library added as a dependancy.

Link to our library: https://github.com/namehash/ens-normalize-python

New normalization standard: https://github.com/ensdomains/governance-docs/pull/38

fselmo commented 1 year ago

@djstrong are there any official tests for normalization or normalization + hashing? That would be very useful for implementers.

@Carbon225 thanks for making us aware that you have an implementation. A PR is not necessary at the moment. We will work on updating our method in house first and proceed from there.

fselmo commented 1 year ago

@djstrong oops I see some validation tests now in ENSIP-15 itself, all good there.

djstrong commented 1 year ago

@fselmo Great! FYI We have spent a lot of man hours to implement the new standard - it is not so simple as previous one. We are validating against all available normalization tests.

web3dev2023 commented 1 year ago

@fselmo @djstrong

Hello, may I ask a question? I was trying to get the DNS name for the address '0x7E8d2190dde46f666Ec7E578611B5728DBeaFc1a':

ns = ENS.from_web3(web3)
ns.name('0x7E8d2190dde46f666Ec7E578611B5728DBeaFc1a')

and then I got the error InvalidName: Invalid character: '➊' | codepoint 10122 (0x278a)

I looked up at Etherscan and got 30303.crypto, which is the first ENS name on Matic chain

Still, I don't understand why there is a ➊ in the error.

Could the code be updated so that it either returns None or just the first name on other chains rather than raising an error?

fselmo commented 1 year ago

@web3dev2023, the owner / manager address points to an invalid name according to the standardized ENSIP-15 validation rules. If you look here at the owner / manager name, that's what is causing this issue. This is the invalid name. According to the ENS web app, this is invalid. Keeping the exception seems reasonable here.

web3dev2023 commented 1 year ago

Thanks @fselmo Sorry for the trouble

fselmo commented 1 year ago

No trouble at all! Hope you're able to manage that on your end. Perhaps a try / except block for catching invalid names. Best of luck.