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

Remove ENS backward compatibility code in abi_ens_resolver (v7 TODO) #3580

Open
Tracked by #3539
abhinay opened this issue Jan 16, 2025 · 4 comments
Open
Tracked by #3539

Remove ENS backward compatibility code in abi_ens_resolver (v7 TODO) #3580

abhinay opened this issue Jan 16, 2025 · 4 comments

Comments

@abhinay
Copy link
Contributor

abhinay commented Jan 16, 2025

What is the current behavior?

There is a try/except NameNotFound block in abi_ens_resolver that was marked with a TODO comment to remove it in Web3.py v7. Specifically:

try:
    return type_str, validate_name_has_address(_ens, val)
except NameNotFound as e:
    # TODO: This try/except is to keep backwards compatibility when we
    #  removed the mainnet requirement. Remove this in web3.py v7 and allow
    #  NameNotFound to raise.
    if not isinstance(_ens, StaticENS):
        raise InvalidAddress(f"{e}")
    raise e

This backward compatibility handling catches NameNotFound and converts it to InvalidAddress (unless _ens is a StaticENS). According to the comment, we intended to remove this logic once we no longer required backward compatibility for mainnet-only ENS.


What should happen?

  • The try/except block should be removed so that NameNotFound is raised directly as intended in Web3.py v7.
  • Code paths that depend on this legacy behavior should be updated or removed, and associated tests should be adjusted accordingly.

Additional context

  • I’ve already prepared a PR that removes this code. This Issue is just to track the change and generate an Issue number for cross-referencing.
  • If maintainers have feedback on timing (e.g., if it should wait for the next major release), we can discuss in this thread.
@abhinay
Copy link
Contributor Author

abhinay commented Jan 16, 2025

#3581

@kclowes
Copy link
Collaborator

kclowes commented Jan 16, 2025

Thanks for this PR! Since v7 is stable, we'll have to hold onto this until v8. Added to v8 issue (#3539).

@abhinay
Copy link
Contributor Author

abhinay commented Jan 16, 2025

Thanks for this PR! Since v7 is stable, we'll have to hold onto this until v8. Added to v8 issue (#3539).

Should I keep the PR open as-is or close it for now and reopen when you’re ready to start merging for v8?

@kclowes
Copy link
Collaborator

kclowes commented Jan 16, 2025

You can keep it open. We'll rebase and merge once we start work on v8. Thanks!

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

No branches or pull requests

2 participants