Refactor (Registar) Managed Names #1433
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
lightwalker-eth
left a comment
lightwalker-eth
left a comment
There was a problem hiding this comment.
@shrugs Hey I like making this code less diffuse 👍 Reviewed and shared a few suggestions.
| * @see https://github.com/base/basenames/blob/1b5c1ad/src/L2/RegistrarController.sol#L488 | ||
| * @see https://github.com/Consensys/linea-ens/blob/3a4f02f/packages/linea-ens-contracts/contracts/ethregistrar/ETHRegistrarController.sol#L447 | ||
| */ | ||
| export const tokenIdToLabelHash = (tokenId: bigint): LabelHash => uint256ToHex32(tokenId); |
There was a problem hiding this comment.
Let's find a strategy where this isn't at the global scope and/or is not given a name that sounds like it should work across all contexts the way that tokenIdToLabelHash does. The current framing of this function suggests it should be a universal utility however it's an implementation detail that each subregistry can implement a custom strategy for.
For example, all registries that manage multiple / (almost) all names, including the NameWrapper or 3DNS registries use namehash, rather than labelhash based tokenIds.
A strategy I've had in mind for this would be to define some class / interface for each of the registry's we implement special logic for, and then each of those class / interface implementations could define custom logic as relevant. Wdyt?
The constructor for such a class might read from a config to decide what the managed name is depending on the namespace, etc.
Please check related ideas in apps/ensindexer/src/lib/tokenscope/nft-issuers.ts including the getDomainId: (tokenId: TokenId): Node functions.
There was a problem hiding this comment.
the act of calling tokenIdToLabelHash encodes the information that this contract (whatever it is) is encoding labelhashes as tokenIds. would a name like this assuage your concerns?
decodeTokenIdAsLabelHashdecodeTokenIdAsNameHash
closes #1403