Skip to content

ENSIP-X: Custom Errors for ENSIP-10 #18

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

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

adraffy
Copy link
Member

@adraffy adraffy commented Mar 2, 2025

Proposal

  • took comment from slack and created ENSIP per Nick's suggestion
  • defined an ordering (contrary to the original design)

Original design:

example of (1): error Unreachable(bytes name)
example of (2): error UnknownResolverProfile(bytes4 selector)

when name doesn't exist, reverts Unreachable
when name exists and f() isn't implemented, reverts UnknownResolverProfile
when name exists, and f() is implemented, but f(name) is doesn't exist/is null, then returns null

you could then probe if a name exists by calling resolve(name, 0x00000000) to see if it reverts UnknownResolverProfile or Unreachable

by "exists", i mean "answerable by the resolver", however resolvers with actual availability could certainly be more precise

@adraffy
Copy link
Member Author

adraffy commented Mar 3, 2025

Contrary to the original design, I made it clear in the spec that UnsupportedResolverProfile happens before UnreachableName. After looking at many resolvers, it seems most developers branch on calldata they understand, before interacting with name or making an offchain request.

Explicit ordering is not required but it seems like a nice property to have as it enables the ERC-165-like behavior.

However, this ordering causes my hacky idea above about resolve(name, 0x00000000) to not work as stated.

It seems reasonable to assume that almost all resolvers implement addr(), so receiving UnreachableName when querying addr() is effectively isExists(name). There may be text()-only resolvers, but if addr() reverts UnsupportedResolverProfile, it could try text() as a fallback. Alternatively, this could be multicalled as [addr(), text()] and return false if either call reverts UnreachableName.

Possibly, the proposal should state that UnsupportedResolverProfile can be probed with w/CCIP-Read disabled.

It's unclear if we should have an explicit resolver profile for existence checking. Since zero resolvers implement this today, probably not. But if so, this would be the right time to define one.

@adraffy adraffy changed the title [DRAFT] Custom Errors for ENSIP-10 Custom Errors for ENSIP-10 Mar 16, 2025
@adraffy adraffy changed the title Custom Errors for ENSIP-10 ENSIP-X: Custom Errors for ENSIP-10 Mar 16, 2025
@clowestab
Copy link

A few thoughts:

  • I think "by "exists", i mean "answerable by the resolver" needs to be explicitly clear in the ENSIP body.
  • "It seems reasonable to assume that almost all resolvers implement addr()" - This is not a valid assumption imo. I very rarely implement ENSIP-1 anymore instead choosing to do everything through resolve.

Whilst theres obviously value in more clearly surfacing this information would a separate function definition similar to 165 not be more appropriate rather than potentially reducing implementor flexibility in their resolve definition?

@adraffy
Copy link
Member Author

adraffy commented Mar 24, 2025

You are correct that UnsupportedResolverProfile(selector) could be supportsInterface(selector) but that's a big change.

There is also a desire to expose some other resolver behaviors via supportsInterface(), but I think that should be another ENSIP. Examples: "can resolve(multicall)?" and "recursive-ccip safe?", ...


The main objective of this ENSIP is to clarify the expected behavior so frontends can disambiguate null results (name exists, value doesn't) from results that don't exist, ie. a resolver should not return null when they don't understand a request or when the results are null because name doesn't exist (isn't registered).

As I mentioned above, it might be better to do this explicitly via a resolver profile, but that's also a big change, whereas this proposal just says these cases should be (known) reverts and not null responses. Any revert is better than no revert:

Resolvers that follow this specification but raise a different error do not require redeployment.

It might be too ambitious to say this enables existence checks if addr() isn't 100% (although the multicall test would cover this case, but is not very clean.)


## Abstract

This standard establishes [custom errors](https://docs.soliditylang.org/en/latest/contracts.html#custom-errors) for [ENSIP-10](./10.md) `resolve()` to improve the resolution experience and disambiguate the following situations:
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
This standard establishes [custom errors](https://docs.soliditylang.org/en/latest/contracts.html#custom-errors) for [ENSIP-10](./10.md) `resolve()` to improve the resolution experience and disambiguate the following situations:
This standard establishes [custom errors](https://docs.soliditylang.org/en/latest/contracts.html#custom-errors) for [ENSIP-10](https://docs.ens.domains/ensip/10) `resolve()` to improve the resolution experience and disambiguate the following situations:

Copy link
Contributor

Choose a reason for hiding this comment

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

Relative links break on the ENSIP docs site.

@stevieraykatz
Copy link
Contributor

This proposal seems fine for resolvers and name services that all exist entirely on L1.

However, I'm a bit confused about the implementation described as it applies to a CCIP-read enabled resolver. An L1 Resolver which implements ENSIP-10 and enables CCIP-Read often has no context about what exists off chain. For example:

  1. the existence of a name might be stored in some off-chain database, OR
  2. the array of supported resolver interfaces is implemented on some L2 contract

As far as I can tell, the only reasonable way that the ENSIP-10 resolver contract on L1 can glean the information necessary to throw this ENSIP's errors is to:

  1. rely on the callback by the client from the offchain gateway and then throw the appropriate error given the response data, OR
  2. be updated regularly with some state of the L2/offchain subdomain service.

Number two above is unrealistic and requires too much overhead to expect adoption. One is interesting but, given that the response relies on CCIP-Read in the first place, it makes more sense to me that this ENSIP's errors are instead thrown by the offchain gateway. For example, status codes on the CCIP-read request could instead convey the errors described in this proposal.

How do you expect that this ENSIP plays with CCIP-Read enabled resolvers?

@adraffy
Copy link
Member Author

adraffy commented Apr 29, 2025

The main objective is just to separate errors from nulls.

Correct, the offchain implementation of UnreachableName, when there isn't some immediately obvious issue preventing resolution, occurs in the gateway or callback, for example ETHFallbackResolver. To me, this is the correct response and much better than returning null or a generic revert, as the request was successful it's just there's no Namechain resolver defined for the name.

UnknownResolverProfile can also be handled both on call, gateway, and callback. There are some resolvers that forward every call offchain, regardless of the content, and then either revert (via gateway or callback) or return null (via callback). There are some resolvers that parse the calldata, and revert or return null immediately for requests they don't understand, and then also have similar behavior in the callback.

As stated in Backwards Compatibility, the gateway itself can also revert any error (or ideally, these new errors since no contract needs modified) if the profile isn't supported or the name doesn't exist. I think the only place where I would raise an issue would be an offchain gateway failing for a null record of a name that exists for a profile it supports.

General purpose gateways (like Unruggable) cannot revert at the gateway-level with a specific resolution error as they're general crosschain state fetchers. Even in the situation where the request "failed", the gateway has to prove to the resolver that it has failed, which is ultimately a successful gateway request, that results in a revert.

This ENSIP as-is is kinda stuck as I'm not sure if supportsInterface() or resolve(supportsInterface()) (or some other idea) is a better direction. As I said in a earlier response, those would require massive redeployments, whereas this ENSIP is more a clarification and guidance of best practices.

Co-authored-by: katzman <[email protected]>
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.

3 participants