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

Ownable L1 Resolver #6

Merged
merged 14 commits into from
May 23, 2024
Merged

Ownable L1 Resolver #6

merged 14 commits into from
May 23, 2024

Conversation

stevieraykatz
Copy link
Collaborator

This PR modifies examples of existing CCIP-enabled Resolvers:

  • Adds ownable methods for updating the url and signer(s)
  • Adds a concept of a rootResolver which resolves requests for the base name
  • Adds ownable methods for managing the rootResolver
  • Adds fallback functionality to support forwarding calls to the rootResolver

In addition, a lot of new testing infrastructure has been added.

// Handler for arbitrary resolver calls
fallback() external {
address RESOLVER = rootResolver;
assembly {
Copy link
Contributor

Choose a reason for hiding this comment

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

how much do we save here with the assembly?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can characterize; I used this format since it's a well-understood pattern from proxy contracts w/ one small change to call instead of delegatecall since we need the state of the rootResolver to respond.

@@ -40,6 +75,11 @@ contract L1Resolver is IExtendedResolver, ERC165 {
* @return The return data, ABI encoded identically to the underlying function.
*/
function resolve(bytes calldata name, bytes calldata data) external view override returns (bytes memory) {
// Resolution for root name should fallback to existing resolver
if (keccak256(BASE_ETH_NAME) == keccak256(name)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

so base.eth goes to root, and what else? Longer comment might be helpful

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will clarify in an updated comment. But yeah, in short, only "base.eth" resolves to root; all other resolution requests would be for *.base.eth subdomains

Copy link
Contributor

Choose a reason for hiding this comment

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

got it, I am curious how name comes through in those other cases. Don't we also fall back to root for some other subdomains?

Copy link
Collaborator Author

@stevieraykatz stevieraykatz May 20, 2024

Choose a reason for hiding this comment

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

Nominally, all ENS name resolution starts on L1. So there are three cases:

  1. base.eth -> we want to call the rootResolver since that has our base.eth records
  2. New base username, i.e. wilson.base.eth -> Resolution fails on L1 so the flow requires resolution to be called against the next higher domain, in this case "base.eth", triggering the cross-chain resolution flow
  3. An existing base subdomain, i.e. mint.base.eth -> The L1 resolution process finds the existing subdomain resolver which exists on L1 and this contract is never queried

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok cool--do you have diagrams of these? Would help me. In particular trying to understand 3. better

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is probably the best reference (sorry not a diagram). https://docs.ens.domains/ensip/10

@stevieraykatz stevieraykatz merged commit 27b1857 into master May 23, 2024
0 of 2 checks passed
@cb-heimdall
Copy link
Collaborator

This Pull Request was merged without enough reviews
Please go here to review and +1

}

// Handler for arbitrary resolver calls
fallback() external {
Copy link
Contributor

Choose a reason for hiding this comment

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

might be helpful to document which functions we expect to called

@stevieraykatz stevieraykatz deleted the ownable-l1-resolver branch May 24, 2024 22:52
stevieraykatz added a commit that referenced this pull request Jul 23, 2024
* Remove addr.reverse setting from ReverseRegistrar

* Address PR suggestion
stevieraykatz added a commit that referenced this pull request Jul 24, 2024
* [audit2] #2 Typos and Documentation (#72)

* Fix typo in PaymentReceiverUpdated

* Fix typo in L2Resolver commment

* Fix typos in L2Resolver and ReverseRegistrar natspec

* Fix comment in IPriceOracle

* Fix natspec in controllers

* Remove unused event (#71)

* [audit2] #4 Cleanup GRACE_PERIOD handling for launch auction (#73)

* Cleanup GRACE_PERIOD handling for launch auction

* Add GRACE_PERIOD to EARegistrarController

* [audit2] #5 Explicitly declare state visibility (#75)

* Explicitly declare state visibility

* Style guide

* Change resolver for reverse node when changing default resolver (#76)

* Remove reverse claiming from EARegistrarController (#78)

* Add IExtendedResolver supported signal for L2Resolver (#77)

* Add reverse node claim to L2 Resolver (#81)

* [audit2] #6 Remove addr.reverse setting from ReverseRegistrar (#74)

* Remove addr.reverse setting from ReverseRegistrar

* Address PR suggestion
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