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

Adding L2 notes to Naming Contracts page. #354

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

Conversation

serenae-fansubs
Copy link
Contributor

Also adding note to subname registrar page.

Copy link

cloudflare-workers-and-pages bot commented Nov 14, 2024

Deploying ens-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: cc19328
Status: ✅  Deploy successful!
Preview URL: https://4444e719.docs-bao.pages.dev
Branch Preview URL: https://naming-contracts-tweaks.docs-bao.pages.dev

View logs

@gskril
Copy link
Member

gskril commented Nov 21, 2024

Instead of relying on Ownable, is it possible to just call setName() within the constructor directly? I think this is a preferred approach if possible, or at least worth mentioning both options.

@serenae-fansubs
Copy link
Contributor Author

serenae-fansubs commented Nov 21, 2024

Instead of relying on Ownable, is it possible to just call setName() within the constructor directly? I think this is a preferred approach if possible, or at least worth mentioning both options.

I intentionally left that out, because don't think that would be a preferred option, it's something that should be discouraged actually, in my opinion.

First of all, you typically don't know what a contract address is going to be until you deploy it (you can probably simulate and derive that based on nonce, maybe, depending on the chain). So you don't even know what to set your ENS name's ETH record to until after you deploy usually. There is no forward-resolution check on the reverse registrar though, so technically you can set the reverse record first, and then work backwards to set that ETH address on whatever ENS name you set it to. Technically possible, but possibly confusing.

However the bigger issue is that if you just call setName directly in the constructor, then it's a one-time-only thing. You cannot change it from then on. If your ENS name gets compromised, or if you need to migrate to a different ENS name / subname, then you're stuck, because you cannot update the reverse record for that contract anymore. That's why (in my opinion) we don't present that as an option for L1 contracts either.

@gskril
Copy link
Member

gskril commented Nov 26, 2024

Hmmm I'm thinking along the lines of the "default names" concept — seems like something would have to happen in the constructor for that to work ?

@gskril
Copy link
Member

gskril commented Dec 12, 2024

Worth mentioning that you can call setName in constructor, but its immutable and not necessarily recommended.

Also worth mentioning that the name must forward resolve to the relevant address with the respective cointype for it to actually work.

@gskril
Copy link
Member

gskril commented Dec 12, 2024

Also would be nice to reference this list of deployments vs duplicate it.

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.

2 participants