Merge pull request #21 from decentraland/feat/add-reclaim-by-controller

feat: add reclaim to regenerate subdomains
decentraland · Feb 4, 2020 · be0b4d5 · be0b4d5
2 parents 67f4ecd + 03905f2
commit be0b4d5
Show file tree

Hide file tree

Showing 4 changed files with 127 additions and 1 deletion.
diff --git a/contracts/ens/DCLRegistrar.sol b/contracts/ens/DCLRegistrar.sol
@@ -208,6 +208,22 @@ contract DCLRegistrar is ERC721Full, Ownable {
 	 * @dev Re-claim the ownership of a subdomain (e.g. "nacho").
      * @notice After a subdomain is transferred by this contract, the owner in the ENS registry contract
      * is still the old owner. Therefore, the owner should call `reclaim` to update the owner of the subdomain.
+     * It is also useful to recreate the subdomains in case of an ENS migration.
+	 * @param _tokenId - erc721 token id which represents the node (subdomain).
+     */
+    function reclaim(uint256 _tokenId) public onlyController {
+        address owner = ownerOf(_tokenId);
+
+        registry.setSubnodeOwner(domainNameHash, bytes32(_tokenId), ownerOf(_tokenId));
+
+        emit Reclaimed(msg.sender, owner, _tokenId);
+    }
+
+    /**
+	 * @dev Re-claim the ownership of a subdomain (e.g. "nacho").
+     * @notice After a subdomain is transferred by this contract, the owner in the ENS registry contract
+     * is still the old owner. Therefore, the owner should call `reclaim` to update the owner of the subdomain.
+     * It is also useful to recreate the subdomains in case of an ENS migration.
 	 * @param _tokenId - erc721 token id which represents the node (subdomain).
      * @param _owner - new owner.
      */

diff --git a/contracts/mocks/FakeDCLRegistrar.sol b/contracts/mocks/FakeDCLRegistrar.sol
@@ -20,6 +20,10 @@ contract FakeDCLRegistrar is DCLRegistrar {
         string memory _baseURI
     ) public DCLRegistrar(_registry, _base, _topdomain, _domain, _baseURI) {}
 
+    function reclaimByController(uint256 _tokenId) public {
+        super.reclaim(_tokenId);
+    }
+
     function bytes32ToString(bytes32 _str) public pure returns (string memory) {
         return _bytes32ToString(_str);
     }

diff --git a/full/DCLRegistrar.sol b/full/DCLRegistrar.sol
@@ -1563,6 +1563,22 @@ contract DCLRegistrar is ERC721Full, Ownable {
 	 * @dev Re-claim the ownership of a subdomain (e.g. "nacho").
      * @notice After a subdomain is transferred by this contract, the owner in the ENS registry contract
      * is still the old owner. Therefore, the owner should call `reclaim` to update the owner of the subdomain.
+     * It is also useful to recreate the subdomains in case of an ENS migration.
+	 * @param _tokenId - erc721 token id which represents the node (subdomain).
+     */
+    function reclaim(uint256 _tokenId) public onlyController {
+        address owner = ownerOf(_tokenId);
+
+        registry.setSubnodeOwner(domainNameHash, bytes32(_tokenId), ownerOf(_tokenId));
+
+        emit Reclaimed(msg.sender, owner, _tokenId);
+    }
+
+    /**
+	 * @dev Re-claim the ownership of a subdomain (e.g. "nacho").
+     * @notice After a subdomain is transferred by this contract, the owner in the ENS registry contract
+     * is still the old owner. Therefore, the owner should call `reclaim` to update the owner of the subdomain.
+     * It is also useful to recreate the subdomains in case of an ENS migration.
 	 * @param _tokenId - erc721 token id which represents the node (subdomain).
      * @param _owner - new owner.
      */

diff --git a/test/Subdomain.spec.js b/test/Subdomain.spec.js
@@ -807,7 +807,97 @@ describe('DCL Names V2', function() {
       })
     })
 
-    describe('reclaim', function() {
+    describe('reclaim :: by controller', function() {
+      beforeEach(async () => {
+        await dclRegistrarContract.migrationFinished()
+      })
+
+      it('should reclaim an owned name', async function() {
+        await dclRegistrarContract.addController(userController)
+        await dclRegistrarContract.register(
+          subdomain1,
+          user,
+          fromUserController
+        )
+
+        const { logs } = await dclRegistrarContract.reclaimByController(
+          subdomain1LabelHash,
+          fromUserController
+        )
+
+        expect(logs.length).to.be.equal(2)
+
+        expect(logs[0].event).to.be.equal('NewOwner')
+        expect(logs[0].args.node).to.be.equal(dclDomainHash)
+        expect(logs[0].args.label).to.be.equal(subdomain1LabelHash)
+        expect(logs[0].args.owner).to.be.equal(user)
+
+        expect(logs[1].event).to.be.equal('Reclaimed')
+        expect(logs[1].args._caller).to.be.equal(userController)
+        expect(logs[1].args._owner).to.be.equal(user)
+        expect(logs[1].args._tokenId).to.eq.BN(
+          web3.utils.toBN(subdomain1LabelHash)
+        )
+
+        const subdomainOwner = await ensRegistryContract.owner(subdomain1Hash)
+        expect(subdomainOwner).to.be.equal(user)
+      })
+
+      it('should reclaim a name previously transferred', async function() {
+        await dclRegistrarContract.addController(userController)
+        await dclRegistrarContract.register(
+          subdomain1,
+          user,
+          fromUserController
+        )
+
+        await dclRegistrarContract.transferFrom(
+          user,
+          anotherUser,
+          subdomain1LabelHash,
+          fromUser
+        )
+
+        await dclRegistrarContract.reclaimByController(
+          subdomain1LabelHash,
+          fromUserController
+        )
+
+        const subdomainOwner = await ensRegistryContract.owner(subdomain1Hash)
+        expect(subdomainOwner).to.be.equal(anotherUser)
+      })
+
+      it('reverts when trying to reclaim by an unauthorized user', async function() {
+        await dclRegistrarContract.addController(userController)
+        await dclRegistrarContract.register(
+          subdomain1,
+          user,
+          fromUserController
+        )
+
+        await assertRevert(
+          dclRegistrarContract.reclaimByController(
+            subdomain1LabelHash,
+            fromHacker
+          ),
+          'Only a controller can call this method'
+        )
+      })
+
+      it('reverts when trying to reclaim an non-exist name', async function() {
+        await dclRegistrarContract.addController(userController)
+
+        await assertRevert(
+          dclRegistrarContract.reclaimByController(
+            subdomain1LabelHash,
+            fromUserController
+          ),
+          'ERC721: owner query for nonexistent token'
+        )
+      })
+    })
+
+    describe('reclaim :: by owner', function() {
       beforeEach(async () => {
         await dclRegistrarContract.migrationFinished()
       })