Skip to content

Commit

Permalink
Internal review feedback (#34)
Browse files Browse the repository at this point in the history 
* Fix name of test contract

* L1 resolver feedback

* BaseRegistrar feedback from internal review

* RegistrarController feedback

* Singature Verifier feedback

* Reverse Registrar feedback
  • Loading branch information
@stevieraykatz
stevieraykatz authored Jun 17, 2024
1 parent 6b02e66 commit 0891ec1
Show file tree
Hide file tree
Showing 14 changed files with 137 additions and 166 deletions.
2 changes: 1 addition & 1 deletion script/configure/AddDiscountValidator.s.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ contract AddDiscountValidator is Script {
address controllerAddr = vm.envAddress("REGISTRAR_CONTROLLER_ADDR");
RegistrarController controller = RegistrarController(controllerAddr);

controller.setDiscountDetails(key, details);
controller.setDiscountDetails(details);

vm.stopBroadcast();
}
Expand Down
37 changes: 20 additions & 17 deletions src/L1/L1Resolver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,13 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {
/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* STORAGE */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice The url endpoint for the CCIP gateway service.
string public url;

/// @notice Storage of approved signers.
mapping(address => bool) public signers;
mapping(address signer => bool isApproved) public signers;

/// @notice address of the rootResolver for `base.eth`.
address public rootResolver;

Expand All @@ -46,19 +49,19 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice Emitted when new signers were added to the approved `signers` mapping.
event NewSigners(address[] signers);
event AddedSigners(address[] signers);

/// @notice Emitted when a new gateway url was stored for `url`.
///
/// @param newUrl the new url being stored.
event NewUrl(string newUrl);
event UrlChanged(string newUrl);

/// @notice Emitted when a new root resolver is set as the `rootResolver`.
///
/// @param resolver The address of the new root resolver.
event NewRootResolver(address resolver);
event RootResolverChanged(address resolver);

///@notice Emitted when a signer has been removed from the approved `signers` mapping.
/// @notice Emitted when a signer has been removed from the approved `signers` mapping.
///
/// @param signer The signer that was removed from the mapping.
event RemovedSigner(address signer);
Expand All @@ -69,7 +72,7 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {

/// @notice Resolver constructor
///
/// @dev Emits `NewSigners(signers_)` after setting the mapping for each signer in the `signers_` arg.
/// @dev Emits `AddedSigners(signers_)` after setting the mapping for each signer in the `signers_` arg.
///
/// @param url_ The gateway url stored as `url`.
/// @param signers_ The approved signers array, each stored as approved in the `signers` mapping.
Expand All @@ -83,29 +86,29 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {
for (uint256 i = 0; i < signers_.length; i++) {
signers[signers_[i]] = true;
}
emit NewSigners(signers_);
emit AddedSigners(signers_);
}

/// @notice Permissioned method letting the owner set the gateway url.
///
/// @dev Emits `NewUrl(url_)` after storing the new url as `url`.
/// @dev Emits `UrlChanged(url_)` after storing the new url as `url`.
///
/// @param url_ The gateway url stored as `url`.
function setUrl(string calldata url_) external onlyOwner {
url = url_;
emit NewUrl(url_);
emit UrlChanged(url_);
}

/// @notice Permissioned method letting the owner add approved signers.
///
/// @dev Emits `NewSigners(signers_)` after setting the mapping for each signer in the `signers_` arg.
///
/// @param _signers Array of signers to set as approved signers in the `signers` mapping.
function addSigners(address[] calldata _signers) external onlyOwner {
for (uint256 i; i < _signers.length; i++) {
signers[_signers[i]] = true;
/// @param signers_ Array of signers to set as approved signers in the `signers` mapping.
function addSigners(address[] calldata signers_) external onlyOwner {
for (uint256 i; i < signers_.length; i++) {
signers[signers_[i]] = true;
}
emit NewSigners(_signers);
emit AddedSigners(signers_);
}

/// @notice Permissioned method letting the owner remove a signer from the approved `signers` mapping.
Expand All @@ -122,12 +125,12 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {

/// @notice Permissioned method letting the owner set the address of the root resolver.
///
/// @dev Emits `NewRootResolver(rootResolver_)` after setting the `rootResolver` address.
/// @dev Emits `RootResolverChanged(rootResolver_)` after setting the `rootResolver` address.
///
/// @param rootResolver_ Address of the new `rootResolver`
function setRootResolver(address rootResolver_) external onlyOwner {
rootResolver = rootResolver_;
emit NewRootResolver(rootResolver_);
emit RootResolverChanged(rootResolver_);
}

/// @notice Hook into the SignatureVerifier lib `makeSignatureHash` method
Expand Down Expand Up @@ -174,7 +177,7 @@ contract L1Resolver is IExtendedResolver, ERC165, Ownable {
///
/// @dev The response data must be encoded per the following format:
/// response = abi.encode(bytes memory result, uint64 expires, bytes memory sig), where:
/// `result` is the resolver repsonse to the resolution request.
/// `result` is the resolver response to the resolution request.
/// `expires` is the signature expiry.
/// `sig` is the signature data used for validating that the gateway signed the response.
/// Per ENSIP-10, the `extraData` arg must match exectly the `extraData` field from the `OffchainLookup` which initiated
Expand Down
126 changes: 57 additions & 69 deletions src/L2/BaseRegistrar.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ pragma solidity ^0.8.23;

import {ENS} from "ens-contracts/registry/ENS.sol";
import {ERC721} from "lib/solady/src/tokens/ERC721.sol";
import {IERC721} from "openzeppelin-contracts/contracts/token/ERC721/IERC721.sol";
import {IERC165} from "openzeppelin-contracts/contracts/utils/introspection/ERC165.sol";
import {Ownable} from "solady/auth/Ownable.sol";

import {GRACE_PERIOD} from "src/util/Constants.sol";
Expand All @@ -24,13 +26,13 @@ contract BaseRegistrar is ERC721, Ownable {
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice A map of expiry times to name ids.
mapping(uint256 id => uint256 expiry) expiries;
mapping(uint256 id => uint256 expiry) public nameExpires;

/// @notice The ENS registry.
ENS public ens;
/// @notice The Registry contract.
ENS public registry;

/// @notice The namehash of the TLD this registrar owns (eg, base.eth).
bytes32 public baseNode;
bytes32 public immutable baseNode;

/// @notice A map of addresses that are authorised to register and renew names.
mapping(address controller => bool isApproved) public controllers;
Expand All @@ -39,18 +41,6 @@ contract BaseRegistrar is ERC721, Ownable {
/* CONSTANTS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice InterfaceId for the meta `supportsInterface` method
bytes4 private constant INTERFACE_META_ID = bytes4(keccak256("supportsInterface(bytes4)"));

/// @notice InterfaceId for IERC721
bytes4 private constant ERC721_ID = bytes4(
keccak256("balanceOf(address)") ^ keccak256("ownerOf(uint256)") ^ keccak256("approve(address,uint256)")
^ keccak256("getApproved(uint256)") ^ keccak256("setApprovalForAll(address,bool)")
^ keccak256("isApprovedForAll(address,address)") ^ keccak256("transferFrom(address,address,uint256)")
^ keccak256("safeTransferFrom(address,address,uint256)")
^ keccak256("safeTransferFrom(address,address,uint256,bytes)")
);

/// @notice InterfaceId for the Reclaim interface
bytes4 private constant RECLAIM_ID = bytes4(keccak256("reclaim(uint256,address)"));

Expand Down Expand Up @@ -129,7 +119,7 @@ contract BaseRegistrar is ERC721, Ownable {

/// @notice Decorator for determining if the contract is actively managing registrations for its `baseNode`.
modifier live() {
if (ens.owner(baseNode) != address(this)) revert RegistrarNotLive();
if (registry.owner(baseNode) != address(this)) revert RegistrarNotLive();
_;
}

Expand All @@ -153,12 +143,12 @@ contract BaseRegistrar is ERC721, Ownable {

/// @notice BaseRegistrar constructor used to initialize the configuration of the implementation.
///
/// @param ens_ The Registry contract.
/// @param registry_ The Registry contract.
/// @param owner_ The permissioned address initialized as the `owner` in the `Ownable` context.
/// @param baseNode_ The node that this contract manages registrations for.
constructor(ENS ens_, address owner_, bytes32 baseNode_) {
constructor(ENS registry_, address owner_, bytes32 baseNode_) {
_initializeOwner(owner_);
ens = ens_;
registry = registry_;
baseNode = baseNode_;
}

Expand Down Expand Up @@ -186,14 +176,7 @@ contract BaseRegistrar is ERC721, Ownable {
///
/// @param resolver The address of the new resolver contract.
function setResolver(address resolver) external onlyOwner {
ens.setResolver(baseNode, resolver);
}

/// @notice Returns the expiration timestamp of the specified id.
///
/// @param id The id of the name being checked.
function nameExpires(uint256 id) external view returns (uint256) {
return expiries[id];
registry.setResolver(baseNode, resolver);
}

/// @notice Register a name.
Expand Down Expand Up @@ -239,7 +222,7 @@ contract BaseRegistrar is ERC721, Ownable {
returns (uint256)
{
uint256 expiry = _localRegister(id, owner, duration);
ens.setSubnodeRecord(baseNode, bytes32(id), owner, resolver, ttl);
registry.setSubnodeRecord(baseNode, bytes32(id), owner, resolver, ttl);
emit NameRegisteredWithRecord(id, owner, expiry, resolver, ttl);
return expiry;
}
Expand All @@ -252,7 +235,7 @@ contract BaseRegistrar is ERC721, Ownable {
///
/// @return address The address currently marked as the owner of the given token ID.
function ownerOf(uint256 tokenId) public view override returns (address) {
if (expiries[tokenId] <= block.timestamp) revert Expired(tokenId);
if (nameExpires[tokenId] <= block.timestamp) revert Expired(tokenId);
return super.ownerOf(tokenId);
}

Expand All @@ -263,7 +246,7 @@ contract BaseRegistrar is ERC721, Ownable {
/// @return `true` if the name is available, else `false`.
function isAvailable(uint256 id) public view returns (bool) {
// Not available if it's registered here or in its grace period.
return expiries[id] + GRACE_PERIOD < block.timestamp;
return nameExpires[id] + GRACE_PERIOD < block.timestamp;
}

/// @notice Allows holders of names to renew their ownerhsip and extend their expiry.
Expand All @@ -277,11 +260,11 @@ contract BaseRegistrar is ERC721, Ownable {
///
/// @return The new expiry date.
function renew(uint256 id, uint256 duration) external live onlyController returns (uint256) {
if (expiries[id] + GRACE_PERIOD < block.timestamp) revert NotRegisteredOrInGrace(id);
if (nameExpires[id] + GRACE_PERIOD < block.timestamp) revert NotRegisteredOrInGrace(id);

expiries[id] += duration;
emit NameRenewed(id, expiries[id]);
return expiries[id];
nameExpires[id] += duration;
emit NameRenewed(id, nameExpires[id]);
return nameExpires[id];
}

/// @notice Reclaim ownership of a name in ENS, if you own it in the registrar.
Expand All @@ -294,9 +277,45 @@ contract BaseRegistrar is ERC721, Ownable {
/// @param owner The address of the owner that will be set in the Registry.
function reclaim(uint256 id, address owner) external live {
if (!_isApprovedOrOwner(msg.sender, id)) revert NotApprovedOwner(id, owner);
ens.setSubnodeOwner(baseNode, bytes32(id), owner);
registry.setSubnodeOwner(baseNode, bytes32(id), owner);
}

/// @notice ERC165 compliant signal for interface support.
///
/// @dev Checks interface support for reclaim OR IERC721 OR ERC165.
/// https://eips.ethereum.org/EIPS/eip-165#how-interfaces-are-identified[EIP section]
///
/// @param interfaceID the ERC165 iface id being checked for compliance
///
/// @return bool Whether this contract supports the provided interfaceID
function supportsInterface(bytes4 interfaceID) public pure override(ERC721) returns (bool) {
return interfaceID == type(IERC165).interfaceId || interfaceID == type(IERC721).interfaceId
|| interfaceID == RECLAIM_ID;
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* ERC721 METADATA */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Returns the token collection name.
function name() public pure override returns (string memory) {
return "";
}

/// @dev Returns the token collection symbol.
function symbol() public pure override returns (string memory) {
return "";
}

/// @dev Returns the Uniform Resource Identifier (URI) for token `id`.
function tokenURI(uint256) public pure override returns (string memory) {
return "";
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* INTERNAL METHODS */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice Register a name and possibly update the Registry.
///
/// @dev This method can only be called if:
Expand All @@ -320,7 +339,7 @@ contract BaseRegistrar is ERC721, Ownable {
{
uint256 expiry = _localRegister(id, owner, duration);
if (updateRegistry) {
ens.setSubnodeOwner(baseNode, bytes32(id), owner);
registry.setSubnodeOwner(baseNode, bytes32(id), owner);
}
emit NameRegistered(id, owner, expiry);
return expiry;
Expand All @@ -337,7 +356,7 @@ contract BaseRegistrar is ERC721, Ownable {
/// @return expiry The expiry date of the registered name.
function _localRegister(uint256 id, address owner, uint256 duration) internal returns (uint256 expiry) {
expiry = block.timestamp + duration;
expiries[id] = expiry;
nameExpires[id] = expiry;
if (_exists(id)) {
// Name was previously owned, and expired
_burn(id);
Expand All @@ -359,35 +378,4 @@ contract BaseRegistrar is ERC721, Ownable {
address owner = ownerOf(tokenId);
return (spender == owner || getApproved(tokenId) == spender || isApprovedForAll(owner, spender));
}

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* ERC721 METADATA */
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @dev Returns the token collection name.
function name() public pure override returns (string memory) {
return "";
}

/// @dev Returns the token collection symbol.
function symbol() public pure override returns (string memory) {
return "";
}

/// @dev Returns the Uniform Resource Identifier (URI) for token `id`.
function tokenURI(uint256) public pure override returns (string memory) {
return "";
}

/// @notice ERC165 compliant signal for interface support.
///
/// @dev Checks interface support for reclaim OR IERC721 OR ERC165.
/// https://eips.ethereum.org/EIPS/eip-165#how-interfaces-are-identified[EIP section]
///
/// @param interfaceID the ERC165 iface id being checked for compliance
///
/// @return bool Whether this contract supports the provided interfaceID
function supportsInterface(bytes4 interfaceID) public pure override(ERC721) returns (bool) {
return interfaceID == INTERFACE_META_ID || interfaceID == ERC721_ID || interfaceID == RECLAIM_ID;
}
}
Loading

0 comments on commit 0891ec1

Please sign in to comment.