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.

Sign up for GitHub

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

Internal review feedback #34

Merged
merged 6 commits into from
Jun 17, 2024
Merged

Internal review feedback #34

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
2fe480d
Fix name of test contract
stevieraykatz Jun 14, 2024
2940a95
L1 resolver feedback
stevieraykatz Jun 14, 2024
8d09940
BaseRegistrar feedback from internal review
stevieraykatz Jun 14, 2024
c4efeb1
RegistrarController feedback
stevieraykatz Jun 14, 2024
6f0ebe8
Singature Verifier feedback
stevieraykatz Jun 14, 2024
efb20d3
Reverse Registrar feedback
stevieraykatz Jun 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion script/configure/AddDiscountValidator.s.sol
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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