allow for renewal controllers to renew a name

contracts/l2/FuseController.sol

@@ -198,7 +198,7 @@ contract FuseController is Ownable, IFuseController {
         bytes memory tokenData = registry.getData(uint256(node));
 
         // if the tokenData is not of the correct length, return 0.
-        if (tokenData.length < 96) {
+        if (tokenData.length != 96) {
             return 0;
         }
 
@@ -369,50 +369,84 @@ contract FuseController is Ownable, IFuseController {
     }
 
     // Set the expiry of a subnode, with a node and a label.
-    function setExpiry(bytes32 node, uint256 label, uint64 newExpiry) external {
+    function setExpiry(
+        bytes32 node,
+        bytes32 labelhash,
+        uint64 newExpiry
+    ) external {
+        TokenData memory td;
+        TokenData memory sub;
+
         // get the subnode
-        bytes32 subnode = keccak256(abi.encodePacked(node, label));
+        bytes32 subnode = keccak256(abi.encodePacked(node, labelhash));
 
         // get tokenData
-        bytes memory tokenData = registry.getData(uint256(subnode));
+        bytes memory tokenData = registry.getData(uint256(node));
 
         // Make sure the parent node controller is this contract.
         require(
             address(_getController(tokenData)) == address(this),
             "Controller is not this contract"
         );
 
+        // Make sure the tokenData is 96 bytes long.
+        require(tokenData.length == 96, "Invalid tokenData length");
+
         (
-            address owner,
-            address resolver, // we don't need the old expiry
+            td.owner, // resolver // expiry
             ,
-            /*uint64 expiry*/ uint64 fuses,
-            address renewalController
+            ,
+            td.fuses,
+            td.renewalController
         ) = _unpack(tokenData);
 
-        // Check to make sure partent cannot control is not burned.
-        require((fuses & PARENT_CANNOT_CONTROL) != 0, "Parent cannot control");
-
         // Make sure the caller is authroized in the parent node.
         bool isAuthorized = registry.getAuthorization(
             uint256(node),
-            owner,
+            td.owner,
             msg.sender
         );
 
-        if (owner != msg.sender && !isAuthorized) {
-            revert Unauthorised(node, msg.sender);
+        // get tokenDataSubnode
+        bytes memory tokenDataSubnode = registry.getData(uint256(subnode));
+
+        // Get the data of the subnode, including the fuses and renewal controller, get the data
+        (sub.owner, sub.resolver, , sub.fuses, sub.renewalController) = _unpack(
+            tokenDataSubnode
+        );
+
+        // Check to make sure partent cannot control is not burned.
+        require(
+            (sub.fuses & PARENT_CANNOT_CONTROL) == 0,
+            "Parent cannot control"
+        );
+
+        // Check to make sure the caller is authorized.
+        if (
+            // Allow the owner of the parent node to set the expiry.
+            !(td.owner == msg.sender) &&
+            // Allow an authorized user of the parent node to set the expiry.
+            !(isAuthorized) &&
+            // Allow the renewal controller of the parent node
+            // as long as the there is no renewal controller set on the subnode
+            // to set the expiry.
+            !(td.renewalController == msg.sender &&
+                sub.renewalController == address(0)) &&
+            // Allow the renewal controller of the subnode to set the expiry.
+            !(sub.renewalController == msg.sender)
+        ) {
+            revert Unauthorised(subnode, msg.sender);
         }
 
         registry.setNode(
             uint256(subnode),
             _pack(
                 address(this),
-                owner,
-                resolver,
+                sub.owner,
+                sub.resolver,
                 newExpiry,
-                fuses,
-                renewalController
+                sub.fuses,
+                sub.renewalController
             )
         );
     }
@@ -481,7 +515,7 @@ contract FuseController is Ownable, IFuseController {
 
     function setSubnode(
         bytes32 node,
-        uint256 label,
+        bytes32 labelhash,
         address subnodeOwner,
         address subnodeResolver,
         uint64 subnodeExpiry,
@@ -507,7 +541,7 @@ contract FuseController is Ownable, IFuseController {
         );
 
         // Make the node of the subnode.
-        bytes32 subnode = keccak256(abi.encodePacked(node, label));
+        bytes32 subnode = keccak256(abi.encodePacked(node, labelhash));
 
         // Get the subnode fuses.
         uint64 subnodeFusesOld = fusesOf(subnode);
@@ -546,7 +580,7 @@ contract FuseController is Ownable, IFuseController {
 
         registry.setSubnode(
             uint256(node),
-            label,
+            uint256(labelhash),
             _pack(
                 address(this),
                 subnodeOwner,

test/l2/TestL2Registry.js

@@ -85,6 +85,8 @@ describe.only('L2Registry', () => {
     hackerAddress = await hacker.getAddress()
     dummyAccount = await signers[4]
     dummyAccountAddress = await dummyAccount.getAddress()
+    renewalController = await signers[5]
+    renewalControllerAddress = await renewalController.getAddress()
 
     resolver = await DelegatableResolver.new()
     metaDataservice = await StaticMetadataService.new('https://ens.domains')
@@ -404,5 +406,144 @@ describe.only('L2Registry', () => {
         EMPTY_ADDRESS,
       )
     })
+
+    // Make sure that a test subnode can be renewed by the renewal controller address.
+    it('should renew a subnode using the renewal controller', async () => {
+      const blockTime = (await ethers.provider.getBlock('latest')).timestamp
+
+      await controller.setSubnode(
+        TEST_NODE,
+        labelhash('sub'),
+        subnodeOwnerAddress,
+        resolver.address,
+        // blocktime + 60 DAYs
+        blockTime + 60 * DAY,
+        0, // no fuse
+        renewalControllerAddress, // no controller
+        { from: ownerAddress },
+      )
+
+      // Make sure the subnode is owned by the subnodeOwnerAddress
+      assert.equal(await controller.ownerOf(TEST_SUBNODE), subnodeOwnerAddress)
+
+      // Make sure the renewal controller is set to the renewalControllerAddress
+      assert.equal(
+        await controller.renewalControllerOf(TEST_SUBNODE),
+        renewalControllerAddress,
+      )
+
+      // Extend the expiry of the subnode by 30 days by calling the setExpiry function from the renewal controller address.
+      await controller.setExpiry(
+        TEST_NODE,
+        labelhash('sub'),
+        blockTime + 90 * DAY,
+        {
+          from: renewalControllerAddress,
+        },
+      )
+
+      // Make sure the subnode is still owned by the subnodeOwnerAddress
+      assert.equal(await controller.ownerOf(TEST_SUBNODE), subnodeOwnerAddress)
+
+      // Make sure the expiry of the subnode has been extended by 30 days
+      assert.equal(
+        await controller.expiryOf(TEST_SUBNODE),
+        blockTime + 90 * DAY,
+      )
+    })
+
+    // Make sure that the test subnode can be renewed by the parent renewal controller address.
+    it('should renew a subnode using the parent renewal controller', async () => {
+      const blockTime = (await ethers.provider.getBlock('latest')).timestamp
+
+      await controller.setSubnode(
+        TEST_NODE,
+        labelhash('sub'),
+        subnodeOwnerAddress,
+        resolver.address,
+        // blocktime + 60 DAYs
+        blockTime + 60 * DAY,
+        0, // no fuse
+        renewalControllerAddress, // no controller
+        { from: ownerAddress },
+      )
+
+      // Predict the node hash of the sub-subnode
+      const subSubNode = namehash('sub-sub.sub.test')
+
+      // Make a sub-subnode without a renewal controller
+      await controller.setSubnode(
+        TEST_SUBNODE,
+        labelhash('sub-sub'),
+        dummyAccountAddress,
+        resolver.address,
+        // blocktime + 60 DAYs
+        blockTime + 60 * DAY,
+        0, // no fuse
+        EMPTY_ADDRESS, // no controller
+        { from: subnodeOwnerAddress },
+      )
+
+      // Make sure the sub-subnode is owned by the dummyAccountAddress
+      assert.equal(await controller.ownerOf(subSubNode), dummyAccountAddress)
+
+      // Make sure we can renew the sub-subnode using the parent renewal controller
+      await controller.setExpiry(
+        TEST_SUBNODE,
+        labelhash('sub-sub'),
+        blockTime + 90 * DAY,
+        {
+          from: renewalControllerAddress,
+        },
+      )
+
+      // Make sure the sub-subnode is still owned by the dummyAccountAddress
+      assert.equal(await controller.ownerOf(subSubNode), dummyAccountAddress)
+
+      // Make sure the expiry of the sub-subnode has been extended by 30 days
+      assert.equal(await controller.expiryOf(subSubNode), blockTime + 90 * DAY)
+    })
+
+    // Make sure that a hacker can't renew a subnode using the renewal controller address.
+    it('should revert when a hacker tries to renew a subnode using the renewal controller', async () => {
+      const blockTime = (await ethers.provider.getBlock('latest')).timestamp
+
+      await controller.setSubnode(
+        TEST_NODE,
+        labelhash('sub'),
+        subnodeOwnerAddress,
+        resolver.address,
+        // blocktime + 60 DAYs
+        blockTime + 60 * DAY,
+        0, // no fuse
+        renewalControllerAddress, // no controller
+        { from: ownerAddress },
+      )
+
+      // Make sure the subnode is owned by the subnodeOwnerAddress
+      assert.equal(await controller.ownerOf(TEST_SUBNODE), subnodeOwnerAddress)
+
+      // Make sure the renewal controller is set to the renewalControllerAddress
+      assert.equal(
+        await controller.renewalControllerOf(TEST_SUBNODE),
+        renewalControllerAddress,
+      )
+
+      // Extend the expiry of the subnode by 30 days by calling the setExpiry
+      // function from the renewal controller address, expect it to revert
+      // with custom error, Unauthorised(bytes32 node, address addr);
+      await expect(
+        controller.setExpiry(
+          TEST_NODE,
+          labelhash('sub'),
+          blockTime + 90 * DAY,
+          {
+            from: hackerAddress,
+          },
+        ),
+      ).to.be.revertedWith(
+        `Unauthorised("${TEST_SUBNODE}", "${hackerAddress}")`,
+      )
+    })
   })
 })