From aa23c1f98133258ec8d82096fef805ff63ff7feb Mon Sep 17 00:00:00 2001 From: nxt3d Date: Thu, 22 Feb 2024 22:03:45 -0500 Subject: [PATCH] allow for renewal controllers to renew a name --- contracts/l2/FuseController.sol | 76 ++++++++++++----- test/l2/TestL2Registry.js | 141 ++++++++++++++++++++++++++++++++ 2 files changed, 196 insertions(+), 21 deletions(-) diff --git a/contracts/l2/FuseController.sol b/contracts/l2/FuseController.sol index 774f9867..cd1e49a0 100644 --- a/contracts/l2/FuseController.sol +++ b/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,12 +369,19 @@ 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( @@ -382,37 +389,64 @@ contract FuseController is Ownable, IFuseController { "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, diff --git a/test/l2/TestL2Registry.js b/test/l2/TestL2Registry.js index 7507bfc5..798d6064 100644 --- a/test/l2/TestL2Registry.js +++ b/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}")`, + ) + }) }) })