Merge Review of #40 (#41)

* README update * Purge masterhash() * README.md update * README.md update: 2 * Reinstate signedRedirect() & ownerhash[] * some more NatSpec * Goerli-v4: 0x4b9A4521442485ad7e1b25295125240b5f3F5aaC * Remove redundant parts; more natSpec * update ownerhash[addr] to accept keccak(addr) * natSpec for GatewayManager * Mainnet: 0x57532d78FfBcC6ac5534A9b39899C7eC89082CdA * Fixes for bugs in v1-beta * Fixes for bugs in v1-beta (#34) * Some natSpec and fix for avatar/contenthash * Updated Fixes in v1.0.0-beta (#36) * Fixes for bugs in v1-beta * Some natSpec and fix for avatar/contenthash * Fix error in test.yml * Fix errors in test.yml (#38) * Fixes for bugs in v1-beta * Some natSpec and fix for avatar/contenthash * Fix error in test.yml * Review of #40
namesys-eth · Aug 14, 2023 · 418c415 · 418c415
1 parent cc44814
commit 418c415
Show file tree

Hide file tree

Showing 7 changed files with 336 additions and 231 deletions.
diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
@@ -83,4 +83,4 @@ jobs:
         with:
           branch: main
           force: true
-          github_token: ${{ secrets.TOKEN }}
+          github_token: ${{ secrets.TOKEN }}
diff --git a/script/Deploy.s.sol b/script/Deploy.s.sol
@@ -10,7 +10,9 @@ contract CCIP2ETHDeploy is Script {
     function run() external {
         vm.startBroadcast();
         GatewayManager manager = new GatewayManager();
-        new CCIP2ETH(address(manager));
+        //uint256 ChainID = uint256(5);
+        //require(ChainID == uint256(1), "WARNING: Deploying with Goerli ChainID. Please double check [!]"); // Comment this for Goerli deploy
+        new CCIP2ETH(address(manager), "5");
         vm.stopBroadcast();
     }
 }
diff --git a/src/CCIP2ETH.sol b/src/CCIP2ETH.sol
diff --git a/src/GatewayManager.sol b/src/GatewayManager.sol
@@ -18,8 +18,8 @@ contract GatewayManager is iERC173, iGatewayManager {
 
     /// @dev - Errors
     error ContenthashNotImplemented(bytes1 _type);
-    error InvalidRequest(string _msg);
-    error NotImplemented(bytes4 _func);
+    error InvalidRequest(string _message);
+    error UnimplementedFeature(bytes4 func);
 
     /// @dev - Contract owner/multisig address
     address public owner;
@@ -68,11 +68,12 @@ contract GatewayManager is iERC173, iGatewayManager {
         view
         returns (string[] memory gateways)
     {
+        /// @dev Filter recordhash vs. web2 gateway
         if (_recordhash.length == 32) {
-            // ipns short
+            // Short IPNS hash
             _recordhash = abi.encodePacked(hex"e5010172002408011220", _recordhash);
         } else if (iGatewayManager(this).isWeb2(_recordhash)) {
-            //https://
+            // Web2 fallback
             gateways = new string[](1);
             gateways[0] = string.concat(string(_recordhash), _path, ".json?t={data}");
             return gateways;
@@ -111,7 +112,7 @@ contract GatewayManager is iERC173, iGatewayManager {
             } else if (_prefix == bytes1("b")) {
                 _fullPath = string.concat("/ipfs/", string(_recordhash), _path, ".json?t={data}");
             } else {
-                revert InvalidRequest("UNSUPPORTED_RECORDHASH");
+                revert InvalidRequest("BAD_RECORDHASH");
             }
             while (i < len) {
                 seed = uint256(keccak256(abi.encodePacked(block.number * i, seed)));
@@ -127,10 +128,14 @@ contract GatewayManager is iERC173, iGatewayManager {
         this;
         response;
         extradata;
-        revert NotImplemented(iGatewayManager.__fallback.selector);
+        revert UnimplementedFeature(iGatewayManager.__fallback.selector);
     }
 
-    // support managed web2 gateway
+    /**
+     * @dev Checks if recordhash is a web2 gateway
+     * @param _recordhash - Recordhash to check
+     * @return - Bool
+     */
     function isWeb2(bytes calldata _recordhash) external pure returns (bool) {
         return (bytes8(_recordhash[:8]) == bytes8("https://"));
     }
@@ -155,15 +160,17 @@ contract GatewayManager is iERC173, iGatewayManager {
         } else if (func == iResolver.ABI.selector) {
             _jsonPath = string.concat("abi/", uintToString(abi.decode(data[36:], (uint256))));
         } else if (func == iResolver.dnsRecord.selector || func == iOverloadResolver.dnsRecord.selector) {
+            // e.g. .well-known/eth/domain/dns/<record>.json
             uint256 resource;
             if (data.length == 100) {
-                (resource) = abi.decode(data[68:], (uint256));
+                // 4 + 32 + 32 + 32 = 100
+                (resource) = abi.decode(data[68:], (uint256)); // Expect uint16
             } else {
                 (,, resource) = abi.decode(data[4:], (bytes32, bytes, uint256));
             }
             _jsonPath = string.concat("dns/", uintToString(resource));
         } else {
-            revert NotImplemented(func);
+            revert UnimplementedFeature(func);
         }
     }
 

diff --git a/src/Interface.sol b/src/Interface.sol
@@ -42,7 +42,7 @@ interface iCCIP2ETH is iENSIP10 {
     function redirectService(bytes calldata _encoded, bytes calldata _requested)
         external
         view
-        returns (bytes4 _selector, bytes32 _namehash, bytes memory _redirectRequest, string memory domain);
+        returns (bytes4 _selector, bytes32 _namehash, bytes memory _redirectRequest, string memory _domain);
     function setDeepSubRecordhash(bytes32 _node, string[] memory _subdomains, bytes calldata _recordhash)
         external
         payable;
@@ -98,18 +98,18 @@ interface iToken {
 }
 
 // Note - Owner = Owner of domain.eth
-// Note - Manager = On-/Off-chain address approved by Owner
+// Note - Manager = On-/Off-Chain address approved by Owner
 // Note - Signer = Record signer
 interface iCallbackType {
     function signedRecord(
-        address recordSigner, // Owner OR On-chain Manager OR Off-chain Manager
+        address recordSigner, // Owner OR On-Chain Manager OR Off-Chain Manager
         bytes memory recordSignature, // Signature from signer for result value
         bytes memory approvedSignature, // bytes length >0 & <64 IF signer is owner or on-chain approved manager
         bytes memory result // ABI-encoded result
     ) external view returns (bytes memory);
 
-    function signedDAppService(
-        address recordSigner, // Owner OR On-chain Manager OR Off-chain Manager
+    function signedRedirect(
+        address recordSigner, // Owner OR On-Chain Manager OR Off-Chain Manager
         bytes memory recordSignature, // Signature from signer for redirect value
         bytes memory approvedSignature, // bytes length >0 & <64 IF signer is owner or on-chain approved manager
         bytes memory redirect // DNS-encoded sub/domain.eth to redirect