Skip to content

Commit

Permalink
[audit2] Spearbit second audit (#80)
Browse files Browse the repository at this point in the history 
* [audit2] #2 Typos and Documentation (#72)

* Fix typo in PaymentReceiverUpdated

* Fix typo in L2Resolver commment

* Fix typos in L2Resolver and ReverseRegistrar natspec

* Fix comment in IPriceOracle

* Fix natspec in controllers

* Remove unused event (#71)

* [audit2] #4 Cleanup GRACE_PERIOD handling for launch auction (#73)

* Cleanup GRACE_PERIOD handling for launch auction

* Add GRACE_PERIOD to EARegistrarController

* [audit2] #5 Explicitly declare state visibility (#75)

* Explicitly declare state visibility

* Style guide

* Change resolver for reverse node when changing default resolver (#76)

* Remove reverse claiming from EARegistrarController (#78)

* Add IExtendedResolver supported signal for L2Resolver (#77)

* Add reverse node claim to L2 Resolver (#81)

* [audit2] #6 Remove addr.reverse setting from ReverseRegistrar (#74)

* Remove addr.reverse setting from ReverseRegistrar

* Address PR suggestion
  • Loading branch information
@stevieraykatz
stevieraykatz authored Jul 24, 2024
1 parent 02b5997 commit 38c44ee
Show file tree
Hide file tree
Showing 24 changed files with 89 additions and 261 deletions.
20 changes: 5 additions & 15 deletions src/L2/EARegistrarController.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {Ownable} from "solady/auth/Ownable.sol";
import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import {StringUtils} from "ens-contracts/ethregistrar/StringUtils.sol";

import {BASE_ETH_NODE} from "src/util/Constants.sol";
import {BASE_ETH_NODE, GRACE_PERIOD} from "src/util/Constants.sol";
import {BaseRegistrar} from "./BaseRegistrar.sol";
import {IDiscountValidator} from "./interface/IDiscountValidator.sol";
import {IPriceOracle} from "./interface/IPriceOracle.sol";
Expand Down Expand Up @@ -176,17 +176,10 @@ contract EARegistrarController is Ownable {
/// @param expires The date that the registration expires.
event NameRegistered(string name, bytes32 indexed label, address indexed owner, uint256 expires);

/// @notice Emitted when a name is renewed.
///
/// @param name The name that was renewed.
/// @param label The hashed label of the name.
/// @param expires The date that the renewed name expires.
event NameRenewed(string name, bytes32 indexed label, uint256 expires);

/// @notice Emitted when the payment receiver is updated.
///
/// @param newPaymentReceiver The address of the new payment receiver.
event PayemntReceiverUpdated(address newPaymentReceiver);
event PaymentReceiverUpdated(address newPaymentReceiver);

/// @notice Emitted when the price oracle is updated.
///
Expand Down Expand Up @@ -257,8 +250,6 @@ contract EARegistrarController is Ownable {

/// @notice Registrar Controller construction sets all of the requisite external contracts.
///
/// @dev Assigns ownership of this contract's reverse record to the `owner_`.
///
/// @param base_ The base registrar contract.
/// @param prices_ The pricing oracle contract.
/// @param reverseRegistrar_ The reverse registrar contract.
Expand All @@ -281,7 +272,6 @@ contract EARegistrarController is Ownable {
rootName = rootName_;
paymentReceiver = paymentReceiver_;
_initializeOwner(owner_);
reverseRegistrar.claim(owner_);
}

/// @notice Allows the `owner` to set discount details for a specified `key`.
Expand Down Expand Up @@ -323,13 +313,13 @@ contract EARegistrarController is Ownable {

/// @notice Allows the `owner` to set the reverse registrar contract.
///
/// @dev Emits `ReverseRegistrarUpdated` after setting the `paymentReceiver` address.
/// @dev Emits `PaymentReceiverUpdated` after setting the `paymentReceiver` address.
///
/// @param paymentReceiver_ The new payment receiver address.
function setPaymentReceiver(address paymentReceiver_) external onlyOwner {
if (paymentReceiver_ == address(0)) revert InvalidPaymentReceiver();
paymentReceiver = paymentReceiver_;
emit PayemntReceiverUpdated(paymentReceiver_);
emit PaymentReceiverUpdated(paymentReceiver_);
}

/// @notice Checks whether any of the provided addresses have registered with a discount.
Expand Down Expand Up @@ -373,7 +363,7 @@ contract EARegistrarController is Ownable {
/// @return price The `Price` tuple containing the base and premium prices respectively, denominated in wei.
function rentPrice(string memory name, uint256 duration) public view returns (IPriceOracle.Price memory price) {
bytes32 label = keccak256(bytes(name));
price = prices.price(name, base.nameExpires(uint256(label)), duration);
price = prices.price(name, base.nameExpires(uint256(label)) + GRACE_PERIOD, duration);
}

/// @notice Checks the register price for a provided `name` and `duration`.
Expand Down
2 changes: 0 additions & 2 deletions src/L2/ExponentialPremiumPriceOracle.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pragma solidity ~0.8.17;
import {FixedPointMathLib} from "solady/utils/FixedPointMathLib.sol";

import {EDAPrice} from "src/lib/EDAPrice.sol";
import {GRACE_PERIOD} from "src/util/Constants.sol";
import {StablePriceOracle} from "src/L2/StablePriceOracle.sol";

contract ExponentialPremiumPriceOracle is StablePriceOracle {
Expand All @@ -20,7 +19,6 @@ contract ExponentialPremiumPriceOracle is StablePriceOracle {
*/

function _premium(string memory, uint256 expires, uint256) internal view override returns (uint256) {
expires = expires + GRACE_PERIOD;
if (expires > block.timestamp) {
return 0;
}
Expand Down
10 changes: 7 additions & 3 deletions src/L2/L2Resolver.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,16 @@ import {ContentHashResolver} from "ens-contracts/resolvers/profiles/ContentHashR
import {DNSResolver} from "ens-contracts/resolvers/profiles/DNSResolver.sol";
import {ENS} from "ens-contracts/registry/ENS.sol";
import {ExtendedResolver} from "ens-contracts/resolvers/profiles/ExtendedResolver.sol";
import {IExtendedResolver} from "ens-contracts/resolvers/profiles/IExtendedResolver.sol";
import {InterfaceResolver} from "ens-contracts/resolvers/profiles/InterfaceResolver.sol";
import {Multicallable} from "ens-contracts/resolvers/Multicallable.sol";
import {Ownable} from "solady/auth/Ownable.sol";
import {NameResolver} from "ens-contracts/resolvers/profiles/NameResolver.sol";
import {PubkeyResolver} from "ens-contracts/resolvers/profiles/PubkeyResolver.sol";
import {TextResolver} from "ens-contracts/resolvers/profiles/TextResolver.sol";

import {IReverseRegistrar} from "src/L2/interface/IReverseRegistrar.sol";

/// @title L2 Resolver
///
/// @notice The default resolver for the Base Usernames project. This contract implements the functionality of the ENS
Expand Down Expand Up @@ -49,7 +52,7 @@ contract L2Resolver is
/// @notice The reverse registrar contract.
address public reverseRegistrar;

/// @notice A mapping of operators per owner address. An operator is authroized to make changes to
/// @notice A mapping of operators per owner address. An operator is authorized to make changes to
/// all names owned by the `owner`.
mapping(address owner => mapping(address operator => bool isApproved)) private _operatorApprovals;

Expand Down Expand Up @@ -112,6 +115,7 @@ contract L2Resolver is
registrarController = registrarController_;
reverseRegistrar = reverseRegistrar_;
_initializeOwner(owner_);
IReverseRegistrar(reverseRegistrar_).claim(owner_);
}

/// @notice Allows the `owner` to set the registrar controller contract address.
Expand Down Expand Up @@ -173,7 +177,7 @@ contract L2Resolver is
return _tokenApprovals[owner][node][delegate];
}

/// @notice Check to see whether `msg.sender` is authroized to modify records for the specified `node`.
/// @notice Check to see whether `msg.sender` is authorized to modify records for the specified `node`.
///
/// @dev Override for `ResolverBase:isAuthorised()`. Used in the context of each inherited resolver "profile".
/// Validates that `msg.sender` is one of:
Expand Down Expand Up @@ -218,6 +222,6 @@ contract L2Resolver is
)
returns (bool)
{
return super.supportsInterface(interfaceID);
return (interfaceID == type(IExtendedResolver).interfaceId || super.supportsInterface(interfaceID));
}
}
13 changes: 8 additions & 5 deletions src/L2/RegistrarController.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {Ownable} from "solady/auth/Ownable.sol";
import {SafeERC20} from "openzeppelin-contracts/contracts/token/ERC20/utils/SafeERC20.sol";
import {StringUtils} from "ens-contracts/ethregistrar/StringUtils.sol";

import {BASE_ETH_NODE} from "src/util/Constants.sol";
import {BASE_ETH_NODE, GRACE_PERIOD} from "src/util/Constants.sol";
import {BaseRegistrar} from "./BaseRegistrar.sol";
import {IDiscountValidator} from "./interface/IDiscountValidator.sol";
import {IPriceOracle} from "./interface/IPriceOracle.sol";
Expand Down Expand Up @@ -189,7 +189,7 @@ contract RegistrarController is Ownable {
/// @notice Emitted when the payment receiver is updated.
///
/// @param newPaymentReceiver The address of the new payment receiver.
event PayemntReceiverUpdated(address newPaymentReceiver);
event PaymentReceiverUpdated(address newPaymentReceiver);

/// @notice Emitted when the price oracle is updated.
///
Expand Down Expand Up @@ -333,13 +333,13 @@ contract RegistrarController is Ownable {

/// @notice Allows the `owner` to set the reverse registrar contract.
///
/// @dev Emits `ReverseRegistrarUpdated` after setting the `paymentReceiver` address.
/// @dev Emits `PaymentReceiverUpdated` after setting the `paymentReceiver` address.
///
/// @param paymentReceiver_ The new payment receiver address.
function setPaymentReceiver(address paymentReceiver_) external onlyOwner {
if (paymentReceiver_ == address(0)) revert InvalidPaymentReceiver();
paymentReceiver = paymentReceiver_;
emit PayemntReceiverUpdated(paymentReceiver_);
emit PaymentReceiverUpdated(paymentReceiver_);
}

/// @notice Checks whether any of the provided addresses have registered with a discount.
Expand Down Expand Up @@ -515,11 +515,14 @@ contract RegistrarController is Ownable {
/// names. Use the `launchTime` to establish a premium price around the actual launch time.
///
/// @param tokenId The ID of the token to check for expiry.
///
/// @return expires Returns the expiry + GRACE_PERIOD for previously registered names, else `launchTime`.
function _getExpiry(uint256 tokenId) internal view returns (uint256 expires) {
expires = base.nameExpires(tokenId);
if (expires == 0) {
expires = launchTime;
return launchTime;
}
return expires + GRACE_PERIOD;
}

/// @notice Shared registartion logic for both `register()` and `discountedRegister()`.
Expand Down
36 changes: 18 additions & 18 deletions src/L2/Registry.sol
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ contract Registry is ENS {
/*.•°:°.´+˚.*°.˚:*.´•*.+°.•°:´*.´•*.•°.•°:°.´:•˚°.*°.˚:*.´+°.•*/

/// @notice The storage of `Record` structs per `node`.
mapping(bytes32 node => Record record) records;
mapping(bytes32 node => Record record) internal _records;

/// @notice Storage for approved operators on a per-holder basis.
mapping(address nameHolder => mapping(address operator => bool isApproved)) operators;
mapping(address nameHolder => mapping(address operator => bool isApproved)) internal _operators;

/*´:°•.°+.*•´.*:˚.°*.˚•´.°:°•.°•.*•´.*:˚.°*.˚•´.°:°•.°+.*•´.*:*/
/* ERRORS */
Expand All @@ -47,8 +47,8 @@ contract Registry is ENS {
///
/// @param node The node to check authorization approval for.
modifier authorized(bytes32 node) {
address owner_ = records[node].owner;
if (owner_ != msg.sender && !operators[owner_][msg.sender]) revert Unauthorized();
address owner_ = _records[node].owner;
if (owner_ != msg.sender && !_operators[owner_][msg.sender]) revert Unauthorized();
_;
}

Expand All @@ -61,7 +61,7 @@ contract Registry is ENS {
///
/// @param rootOwner The address that can establish new TLDs.
constructor(address rootOwner) {
records[0x0].owner = rootOwner;
_records[0x0].owner = rootOwner;
}

/// @notice Sets the record for a node.
Expand Down Expand Up @@ -130,7 +130,7 @@ contract Registry is ENS {
/// @param node The node to update.
/// @param resolver_ The address of the resolver.
function setResolver(bytes32 node, address resolver_) public virtual override authorized(node) {
records[node].resolver = resolver_;
_records[node].resolver = resolver_;
emit NewResolver(node, resolver_);
}

Expand All @@ -141,7 +141,7 @@ contract Registry is ENS {
/// @param node The node to update.
/// @param ttl_ The TTL in seconds.
function setTTL(bytes32 node, uint64 ttl_) public virtual override authorized(node) {
records[node].ttl = ttl_;
_records[node].ttl = ttl_;
emit NewTTL(node, ttl_);
}

Expand All @@ -153,7 +153,7 @@ contract Registry is ENS {
/// @param operator Address to add to the set of authorized operators.
/// @param approved True if the operator is approved, false to revoke approval.
function setApprovalForAll(address operator, bool approved) external virtual override {
operators[msg.sender][operator] = approved;
_operators[msg.sender][operator] = approved;
emit ApprovalForAll(msg.sender, operator, approved);
}

Expand All @@ -163,7 +163,7 @@ contract Registry is ENS {
///
/// @return The address for the specified node if one is set, returns address(0) if this contract is owner.
function owner(bytes32 node) public view virtual override returns (address) {
address addr = records[node].owner;
address addr = _records[node].owner;
if (addr == address(this)) {
return address(0);
}
Expand All @@ -176,7 +176,7 @@ contract Registry is ENS {
///
/// @return The address of the resolver.
function resolver(bytes32 node) public view virtual override returns (address) {
return records[node].resolver;
return _records[node].resolver;
}

/// @notice Returns the TTL of a node.
Expand All @@ -185,7 +185,7 @@ contract Registry is ENS {
///
/// @return The ttl of the node.
function ttl(bytes32 node) public view virtual override returns (uint64) {
return records[node].ttl;
return _records[node].ttl;
}

/// @notice Returns whether a record exists in this registry.
Expand All @@ -194,7 +194,7 @@ contract Registry is ENS {
///
/// @return `true` if a record exists, else `false`.
function recordExists(bytes32 node) public view virtual override returns (bool) {
return records[node].owner != address(0x0);
return _records[node].owner != address(0x0);
}

/// @notice Query if an address is an authorized operator for another address.
Expand All @@ -204,15 +204,15 @@ contract Registry is ENS {
///
/// @return `true` if `operator` is an approved operator for `owner`, else `fase`.
function isApprovedForAll(address owner_, address operator) external view virtual override returns (bool) {
return operators[owner_][operator];
return _operators[owner_][operator];
}

/// @notice Set the owner in storage.
///
/// @param node The specified node.
/// @param owner_ The owner to store for that node.
function _setOwner(bytes32 node, address owner_) internal virtual {
records[node].owner = owner_;
_records[node].owner = owner_;
}

/// @notice Set the resolver and ttl in storage.
Expand All @@ -221,13 +221,13 @@ contract Registry is ENS {
/// @param resolver_ The address of the resolver.
/// @param ttl_ The TTL in seconds.
function _setResolverAndTTL(bytes32 node, address resolver_, uint64 ttl_) internal {
if (resolver_ != records[node].resolver) {
records[node].resolver = resolver_;
if (resolver_ != _records[node].resolver) {
_records[node].resolver = resolver_;
emit NewResolver(node, resolver_);
}

if (ttl_ != records[node].ttl) {
records[node].ttl = ttl_;
if (ttl_ != _records[node].ttl) {
_records[node].ttl = ttl_;
emit NewTTL(node, ttl_);
}
}
Expand Down
Loading

0 comments on commit 38c44ee

Please sign in to comment.