Merge branch 'master' into pete/n08

CoinbaseStablecoin · Apr 21, 2022 · 26beed0 · 26beed0
2 parents 0742c37 + 9722a2a
commit 26beed0
Show file tree

Hide file tree

Showing 7 changed files with 167 additions and 86 deletions.
diff --git a/README.md b/README.md
@@ -25,9 +25,6 @@ Client                             Offchain Resolver Contract          Gateway
 Please refer to the tests for `.resolve` and `.resolveWithProof` in
 `CoinbaseResolver.test.ts` to learn more about how this works.
 
-CoinbaseResolver conforms to [EIP-1967](https://eips.ethereum.org/EIPS/eip-1967)
-and [EIP-1822](https://eips.ethereum.org/EIPS/eip-1822) for upgradeability.
-
 ## Requirements
 
 - Node.js v16

diff --git a/contracts/CoinbaseResolver.sol b/contracts/CoinbaseResolver.sol
@@ -10,18 +10,32 @@ import { SignatureVerifier } from "./ens-offchain-resolver/SignatureVerifier.sol
 import { IResolverService } from "./ens-offchain-resolver/IResolverService.sol";
 
 /**
- * @notice Coinbase Offchain ENS Resolver
+ * @notice Coinbase Offchain ENS Resolver.
  * @dev Adapted from: https://github.com/ensdomains/offchain-resolver/blob/2bc616f19a94370828c35f29f71d5d4cab3a9a4f/packages/contracts/contracts/OffchainResolver.sol
  */
 contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
     using EnumerableSet for EnumerableSet.AddressSet;
 
+    /// @dev Gateway URL to use to perform offchain lookup.
     string private _url;
+    /// @dev Addresses for the set of signers.
     EnumerableSet.AddressSet private _signers;
 
-    event UrlSet(string indexed newUrl);
-    event SignersAdded(address[] indexed addedSigners);
-    event SignersRemoved(address[] indexed removedSigners);
+    /// @notice Event raised when a new gateway URL is set.
+    event UrlSet(string indexed previousUrl, string indexed newUrl);
+    /// @notice Event raised when a new signer is added.
+    event SignerAdded(address indexed addedSigner);
+    /// @notice Event raised when a signer is removed.
+    event SignerRemoved(address indexed removedSigner);
+
+    /**
+     * @dev Error to raise when an offchain lookup is required.
+     * @param sender Sender address (address of this contract).
+     * @param urls URLs to request to perform the offchain lookup.
+     * @param callData Call data contains all the data to perform the offchain lookup.
+     * @param callbackFunction Callback function that should be called after lookup.
+     * @param extraData Optional extra data to send.
+     */
     error OffchainLookup(
         address sender,
         string[] urls,
@@ -32,11 +46,11 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
 
     /**
      * @notice Initializes the contract with the initial parameters.
-     * @param newOwner Owner address
-     * @param newSignerManager Signer manager address
-     * @param newGatewayManager Gateway manager address
-     * @param newUrl Gateway URL
-     * @param newSigners Signer addresses
+     * @param newOwner Owner address.
+     * @param newSignerManager Signer manager address.
+     * @param newGatewayManager Gateway manager address.
+     * @param newUrl Gateway URL.
+     * @param newSigners Signer addresses.
      */
     constructor(
         address newOwner,
@@ -54,15 +68,15 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
 
     /**
      * @notice Returns the gateway URL.
-     * @return Gateway URL
+     * @return Gateway URL.
      */
     function url() external view returns (string memory) {
         return _url;
     }
 
     /**
      * @notice Returns a list of signers.
-     * @return List of signers
+     * @return List of signers.
      */
     function signers() external view returns (address[] memory) {
         return _signers.values();
@@ -79,19 +93,18 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
     /**
      * @notice Set the gateway URL.
      * @dev Can only be called by the gateway manager.
-     * @param newUrl New gateway URL
+     * @param newUrl New gateway URL.
      */
-
-    function setUrl(string memory newUrl) external onlyGatewayManager {
+    function setUrl(string calldata newUrl) external onlyGatewayManager {
         _setUrl(newUrl);
     }
 
     /**
      * @notice Add a set of new signers.
      * @dev Can only be called by the signer manager.
-     * @param signersToAdd Signer addresses
+     * @param signersToAdd Signer addresses.
      */
-    function addSigners(address[] memory signersToAdd)
+    function addSigners(address[] calldata signersToAdd)
         external
         onlySignerManager
     {
@@ -100,22 +113,25 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
 
     /**
      * @notice Remove a set of existing signers.
-     * @dev Can only be called by the owner.
-     * @param signersToRemove Signer addresses
+     * @dev Can only be called by the signer manager.
+     * @param signersToRemove Signer addresses.
      */
-    function removeSigners(address[] memory signersToRemove)
+    function removeSigners(address[] calldata signersToRemove)
         external
         onlySignerManager
     {
-        for (uint256 i = 0; i < signersToRemove.length; i++) {
-            _signers.remove(signersToRemove[i]);
+        uint256 length = signersToRemove.length;
+        for (uint256 i = 0; i < length; i++) {
+            address signer = signersToRemove[i];
+            if (_signers.remove(signer)) {
+                emit SignerRemoved(signer);
+            }
         }
-        emit SignersRemoved(signersToRemove);
     }
 
     /**
-     * @notice Support ERC-165 introspection
-     * @param interfaceID Interface ID
+     * @notice Support ERC-165 introspection.
+     * @param interfaceID Interface ID.
      * @return True if a given interface ID is supported.
      */
     function supportsInterface(bytes4 interfaceID)
@@ -130,11 +146,10 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
     }
 
     /**
-     * @notice Initiate a resolution conforming to the ENSIP-10. Reverts with an
-     * an OffchainLookup error.
-     * @param name DNS-encoded name to resolve
-     * @param data ABI-encoded data for the underlying resolution function (e.g. addr(bytes32), text(bytes32,string))
-     * @return Always reverts with an OffchainLookup error
+     * @notice Initiate a resolution conforming to the ENSIP-10. Reverts with an OffchainLookup error.
+     * @param name DNS-encoded name to resolve.
+     * @param data ABI-encoded data for the underlying resolution function (e.g. addr(bytes32), text(bytes32,string)).
+     * @return Always reverts with an OffchainLookup error.
      */
     function resolve(bytes calldata name, bytes calldata data)
         external
@@ -161,9 +176,9 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
     /**
      * @notice Callback used by CCIP-read compatible clients to verify and parse the response.
      * @dev Reverts if the signature is invalid.
-     * @param response ABI-encoded response data in the form of (bytes result, uint64 expires, bytes signature)
-     * @param extraData Original request data
-     * @return ABI-encoded result data for the underlying resolution function
+     * @param response ABI-encoded response data in the form of (bytes result, uint64 expires, bytes signature).
+     * @param extraData Original request data.
+     * @return ABI-encoded result data for the underlying resolution function.
      */
     function resolveWithProof(bytes calldata response, bytes calldata extraData)
         external
@@ -174,21 +189,24 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
             extraData,
             response
         );
-        require(_signers.contains(signer), "invalid signature");
+        require(
+            _signers.contains(signer),
+            "CoinbaseResolver::resolveWithProof: invalid signature"
+        );
         return result;
     }
 
     /**
-     * @notice Generates a hash for signing and verifying the offchain response
-     * @param expires Time at which the signature expires
-     * @param request Request data
-     * @param result Result data
-     * @return Hashed data for signing and verifying
+     * @notice Generates a hash for signing and verifying the offchain response.
+     * @param expires Time at which the signature expires.
+     * @param request Request data.
+     * @param result Result data.
+     * @return Hashed data for signing and verifying.
      */
     function makeSignatureHash(
         uint64 expires,
-        bytes memory request,
-        bytes memory result
+        bytes calldata request,
+        bytes calldata result
     ) external view returns (bytes32) {
         return
             SignatureVerifier.makeSignatureHash(
@@ -199,15 +217,27 @@ contract CoinbaseResolver is ERC165, Manageable, IExtendedResolver {
             );
     }
 
+    /**
+     * @notice Sets the new gateway URL and emits a UrlSet event.
+     * @param newUrl New URL to be set.
+     */
     function _setUrl(string memory newUrl) private {
+        string memory previousUrl = _url;
         _url = newUrl;
-        emit UrlSet(newUrl);
+        emit UrlSet(previousUrl, newUrl);
     }
 
+    /**
+     * @notice Adds new signers and emits a SignersAdded event.
+     * @param signersToAdd List of addresses to add as signers.
+     */
     function _addSigners(address[] memory signersToAdd) private {
-        for (uint256 i = 0; i < signersToAdd.length; i++) {
-            _signers.add(signersToAdd[i]);
+        uint256 length = signersToAdd.length;
+        for (uint256 i = 0; i < length; i++) {
+            address signer = signersToAdd[i];
+            if (_signers.add(signer)) {
+                emit SignerAdded(signer);
+            }
         }
-        emit SignersAdded(signersToAdd);
     }
 }
diff --git a/contracts/Manageable.sol b/contracts/Manageable.sol
@@ -15,14 +15,18 @@ import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";
  * manager respectively.
  */
 abstract contract Manageable is Ownable {
+    /// @dev Address of the signer manager.
     address private _signerManager;
+    /// @dev Address of the gateway manager.
     address private _gatewayManager;
 
+    /// @notice Event raised when a signer manager is updated.
     event SignerManagerChanged(
         address indexed previousSignerManager,
         address indexed newSignerManager
     );
 
+    /// @notice Event raised when a gateway manager is updated.
     event GatewayManagerChanged(
         address indexed previousGatewayManager,
         address indexed newGatewayManager
@@ -49,8 +53,8 @@ abstract contract Manageable is Ownable {
      */
     modifier onlySignerManager() {
         require(
-            _signerManager == msg.sender,
-            "Manageable: caller is not the signer manager"
+            _signerManager == _msgSender(),
+            "Manageable::onlySignerManager: caller is not signer manager"
         );
         _;
     }
@@ -60,15 +64,15 @@ abstract contract Manageable is Ownable {
      */
     modifier onlyGatewayManager() {
         require(
-            _gatewayManager == msg.sender,
-            "Manageable: caller is not the gateway manager"
+            _gatewayManager == _msgSender(),
+            "Manageable::onlyGatewayManager: caller is not gateway manager"
         );
         _;
     }
 
     /**
      * @notice Change signer manager of the contract to a new account (`newSignerManager`).
-     * Can only be called by the current owner.
+     * @dev Can only be called by the current owner.
      * @param newSignerManager the new signer manager address.
      */
     function changeSignerManager(address newSignerManager)
@@ -78,14 +82,14 @@ abstract contract Manageable is Ownable {
     {
         require(
             newSignerManager != address(0),
-            "Manageable: new signer manager is the zero address"
+            "Manageable::changeSignerManager: manager is the zero address"
         );
         _changeSignerManager(newSignerManager);
     }
 
     /**
      * @notice Change gateway manager of the contract to a new account (`newGatewayManager`).
-     * Can only be called by the current owner.
+     * @dev Can only be called by the current owner.
      * @param newGatewayManager the new gateway manager address.
      */
     function changeGatewayManager(address newGatewayManager)
@@ -95,14 +99,15 @@ abstract contract Manageable is Ownable {
     {
         require(
             newGatewayManager != address(0),
-            "Manageable: new gateway manager is the zero address"
+            "Manageable::changeGatewayManager: manager is the zero address"
         );
         _changeGatewayManager(newGatewayManager);
     }
 
     /**
-     * @dev Change signer manager of the contract to a new account (`newSignerManager`).
-     * Internal function without access restriction.
+     * @notice Change signer manager of the contract to a new account (`newSignerManager`).
+     * @dev Internal function without access restriction.
+     * @param newSignerManager the new signer manager address.
      */
     function _changeSignerManager(address newSignerManager) internal virtual {
         address oldSignerManager = _signerManager;
@@ -111,8 +116,9 @@ abstract contract Manageable is Ownable {
     }
 
     /**
-     * @dev Change gateway manager of the contract to a new account (`newGatewayManager`).
-     * Internal function without access restriction.
+     * @notice Change gateway manager of the contract to a new account (`newGatewayManager`).
+     * @dev Internal function without access restriction.
+     * @param newGatewayManager the new gateway manager address.
      */
     function _changeGatewayManager(address newGatewayManager) internal virtual {
         address oldGatewayManager = _gatewayManager;

diff --git a/contracts/ens-offchain-resolver/IExtendedResolver.sol b/contracts/ens-offchain-resolver/IExtendedResolver.sol
@@ -3,6 +3,11 @@
 pragma solidity 0.8.13;
 
 interface IExtendedResolver {
+    /**
+     * @notice Function interface for the ENSIP-10 wildcard resolution function.
+     * @param name DNS-encoded name to resolve.
+     * @param data ABI-encoded data for the underlying resolution function (e.g. addr(bytes32), text(bytes32,string)).
+     */
     function resolve(bytes memory name, bytes memory data)
         external
         view

diff --git a/contracts/ens-offchain-resolver/IResolverService.sol b/contracts/ens-offchain-resolver/IResolverService.sol
@@ -3,6 +3,15 @@
 pragma solidity 0.8.13;
 
 interface IResolverService {
+    /**
+     * @notice Function interface for the lookup function supported by the off-chain gateway.
+     * @dev This function is executed off-chain by the off-chain gateway.
+     * @param name DNS-encoded name to resolve.
+     * @param data ABI-encoded data for the underlying resolution function (e.g. addr(bytes32), text(bytes32,string)).
+     * @return result ABI-encode result of the lookup.
+     * @return expires Time at which the signature expires.
+     * @return sig A signer's signature authenticating the result.
+     */
     function resolve(bytes calldata name, bytes calldata data)
         external
         view