Conversation
Deploying ensips with
|
| Latest commit: |
ff6e440
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://87a5d48d.ensips.pages.dev |
| Branch Preview URL: | https://ensip-universal-resolution.ensips.pages.dev |
Arachnid
left a comment
There was a problem hiding this comment.
This needs to explicitly state that callers must handle CCIP-Read reverts.
|
Should the ENSIP mention upgrade paths or are they more of the implementation detail? I personally thinks it should be mentioned as A: if it's user initiated upgrade, there must be a public API that initiates the upgrade, B: If it's DAO initiated upgrade, there still needs a API DAO has to call and also the user should aware that the resolution logic is not immutable |
Co-authored-by: Nick Johnson <arachnid@notdot.net>
|
@Arachnid by "callers", do you mean clients? or UniversalResolver implementations? |
|
@makoto i don't think adding implementation detail on upgrade paths is worth it, upgradability is down to each individual implementation and it isn't particularly relevant to universal resolution itself imo |
Arachnid
left a comment
There was a problem hiding this comment.
Can you add documentation for the errors defined in https://github.com/ensdomains/ens-contracts/pull/421/files#diff-895978e0de194cab98cef8af1c7de86d9a16ac5c37a1b33a5a28faf671f3bf14 too?
|
I think we should include findResolver() in the interface too. |
|
I moved a bunch of stuff around and tried to simplify it. I wanted a pseudocode example for forward resolution, but couldn't come up with anything simple w/o including too many cases, so I just left the Solidity and TypeScript usage examples. A lot of specific details are covered in other ENSIPs so instead of repeating myself, I link out as much as possible, and focus on the mechanics. Under "Smart Execution", I mention ENSIP-X as a placeholder for "ENSIP: Resolver Features", which I'm working on right now. Possibly, this explanation could be moved to that ENSIP, as this is (my take on) the best protocol for efficiently calling a resolver. |
| `findResolver()` does not perform any validity checks on the resolver and simply returns the value in the registry. The resolver may not be a contract or a resolver. | ||
|
|
||
| #### <a name="resolve-example">Pseudocode Example</a> | ||
|
|
There was a problem hiding this comment.
It took a while to figure out what this Pseudocode Example does. Can you add a sentense like below explaining what the example does?
findResolver() walks up the domain hierarchy from "sub.nick.eth" to find the first parent domain with a resolver set (finds one at "nick.eth"), returning that resolver's address, the namehash of the original full name "sub.nick.eth", and the byte offset (4) indicating where in the DNS-encoded name the resolver was found.
There was a problem hiding this comment.
It's defined in ENSIP-1. Additionally, this section will need revisited with ENSv2 as the traversal method changes.
|
|
||
| ### resolve | ||
|
|
||
| This function performs ENS forward resolution using the `resolver` found by [`findResolver()`](#findresolver). It provides a standard interface for interacting [ENSIP-1](./1) and [ENSIP-10](./10) resolvers for onchain and offchain resolution. Provided a DNS-encoded `name` and ABI-encoded `data`, it returns the ABI-encoded resolver `result` and the valid `resolver` address. |
There was a problem hiding this comment.
Like you did at EIP3668(CCIP-Read), worth adding ENSIP-1(ENS Protocol) and ENSIP-10(Wildcard) so that people don't have to click ENSIP to figure out what they actually are
There was a problem hiding this comment.
I've been using the policy where I hyperlink the first time I mention something, and then rarely again unless its far away or I link to a specific anchor.
We could change the policy to all ENSIP references are linked, as that's an easy ENSIP-repo wide change, but might be noisy.
There was a problem hiding this comment.
Changed in new PR, but only mentioned once.
|
|
||
| This function performs ENS forward resolution using the `resolver` found by [`findResolver()`](#findresolver). It provides a standard interface for interacting [ENSIP-1](./1) and [ENSIP-10](./10) resolvers for onchain and offchain resolution. Provided a DNS-encoded `name` and ABI-encoded `data`, it returns the ABI-encoded resolver `result` and the valid `resolver` address. | ||
|
|
||
| UR automatically handles wrapping calldata and unwrapping responses when interacting with an [`IExtendedResolver`](./10#pseudocode) and safely interacts with contracts deployed before [EIP-140](https://eips.ethereum.org/EIPS/eip-140). |
|
|
||
| ## Abstract | ||
|
|
||
| This ENSIP standardizes [IUniversalResolver](#specification) (UR), an universal entrypoint for resolving ENS names. UR incorporates onchain algorithms for [ENSIP-10](./10#pseudocode), [ENSIP-19](./19#algorithm), [ENSIP-21](./21), and [ENSIP-22](./22) to reduce ENS integration complexities. |
There was a problem hiding this comment.
Please put the title of each ENSIP (eg: ENSIP-10(Wildcard Resolution))
| * If the called function reverted, reverts `ResolverError`. | ||
|
|
||
| #### Smart Multicall | ||
|
|
There was a problem hiding this comment.
Is it worth noting that Smart Multicall allow you to combine multiple calls of one node, not being able to combine calls of multiple nodes? For example you can call addr and text of vitalik.eth in one call, but not combining addr of vitalik.eth and nick.eth even though you have to encode node in each call (for the backward compatibility purpose)
There was a problem hiding this comment.
Yeah, agreed. I was relying on the existing resolve() interpretation but that should probably be explicitly stated so it's clear.
We also could add this feature to the UR, as its extremely easy to write with the current design.
There also is the following functions not included in the standard interface:
requireResolver()resolveWithGateways()reverseWithGateways()resolveWithResolver()
Also, somewhere, we could document the multicall-like reverse primary name lookup that each reverse resolver supports. That could be an appendix of ENSIP-19 or a separate ENSIP. It was mentioned in the comments of this ENSIP above.
Preview