feat:attempt to implement L1ETHRegistry for ejected names #20
Conversation
Pull Request Test Coverage Report for Build 13589122509Details
💛 - Coveralls |
|
Feel free to take the ticket! |
| event NameRenewed(uint256 indexed tokenId, uint64 newExpiration, address renewedBy); | ||
| event NameRelinquished(uint256 indexed tokenId, address relinquishedBy); | ||
| event NameMigratedToL2(uint256 indexed tokenId, address sendTo); | ||
| event FallbackResolverSet(address resolver); |
There was a problem hiding this comment.
I don't think we need an event for this, since the registry will emit an event in any case.
| event TokenObserverSet(uint256 indexed tokenId, address observer); | ||
|
|
||
| // Address of the fallback resolver for names not found in this registry | ||
| address public fallbackResolver; |
There was a problem hiding this comment.
This will be resolved via the parent registry's resolver field, no need to store it here.
| _grantRole(DEFAULT_ADMIN_ROLE, msg.sender); | ||
| } | ||
|
|
||
| function uri(uint256 /*tokenId*/ ) public pure override returns (string memory) { |
| * @param expires Expiration timestamp | ||
| * @return tokenId The token ID of the ejected name | ||
| */ | ||
| function ejectFromL2(string calldata label, address owner, IRegistry registry, uint96 flags, uint64 expires) |
There was a problem hiding this comment.
I think we should probably be using the label hash here, not the plaintext label.
| flags = (flags & FLAGS_MASK) | (uint96(expires) << 32); | ||
|
|
||
| // If there is a previous owner, burn the token | ||
| address previousOwner = super.ownerOf(tokenId); |
There was a problem hiding this comment.
We need to check that the previous name is expired first, so that a malicious ejection controller can't override L1 names.
| * @param expires Expiration timestamp | ||
| * @return tokenId The token ID of the ejected name | ||
| */ | ||
| function ejectFromL2(string calldata label, address owner, IRegistry registry, uint96 flags, uint64 expires) |
There was a problem hiding this comment.
Either we should accept only a uint32 of flags, or we should accept a combined flags+expiration as a single field. Accepting a uint96 of which 64 bits have to be zero seems odd.
| * @dev Relinquish an ejected name | ||
| * @param tokenId The token ID of the name to relinquish | ||
| */ | ||
| function relinquish(uint256 tokenId) external onlyTokenOwner(tokenId) { |
There was a problem hiding this comment.
How does this differ from migrateToL2?
| * @param tokenId The token ID of the name to migrate | ||
| * @param sendTo The address to send the name to on L2 | ||
| */ | ||
| function migrateToL2(uint256 tokenId, address sendTo) |
There was a problem hiding this comment.
This needs to notify the ejection controller, so that control can be given back to the user on L2.
Using @hiddentao 's ETHRegistry as a reference I tried to implement L1ETHRegistry.
Note: Just now I see that the ticket is assigned today already. So feel free to skip if there is another effort.
The code implements L1-native renewals, based on the feedback, but open to discussion.
urifunction implementation, at the moment it returns emtpy string just as ETHRegistry, but I thoughtcould be also an approach.