Add tests for renew

Author: jefflau

contracts/subdomainregistrar/SubdomainRegistrar.sol

@@ -26,7 +26,8 @@ contract SubdomainRegistrar is ERC1155Holder {
     mapping(bytes32 => Name) public names;
     mapping(bytes32 => uint256) public expiries;
 
-    event NameRenewed(bytes32 node, uint256 duration);
+    event NameRegistered(bytes32 node, uint256 expiry);
+    event NameRenewed(bytes32 node, uint256 expiry);
 
     constructor(INameWrapper _wrapper) {
         wrapper = _wrapper;
@@ -109,6 +110,8 @@ contract SubdomainRegistrar is ERC1155Holder {
             fuses | PARENT_CANNOT_CONTROL, // burn the ability for the parent to control
             uint64(block.timestamp + duration)
         );
+
+        emit NameRegistered(node, uint64(block.timestamp + duration));
     }
 
     function renew(
@@ -124,16 +127,18 @@ contract SubdomainRegistrar is ERC1155Holder {
 
         uint256 fee = duration * names[parentNode].registrationFee;
 
+        if (fee > 0) {
+            IERC20(names[parentNode].token).transferFrom(
+                msg.sender,
+                address(names[parentNode].beneficiary),
+                fee
+            );
+        }
+
         newExpiry = expiry += duration;
 
         wrapper.setChildFuses(parentNode, labelhash, 0, newExpiry);
 
-        IERC20(names[parentNode].token).transferFrom(
-            msg.sender,
-            address(names[parentNode].beneficiary),
-            fee
-        );
-
         emit NameRenewed(node, newExpiry);
     }
 

test/subdomainregistrar/TestSubdomainRegistrar.js

@@ -212,6 +212,85 @@ describe('Subdomain registrar', () => {
     })
   })
 
+  describe('renew', () => {
+    it('should allow subdomains to be renewed', async () => {
+      await BaseRegistrar.register(labelhash('test'), account, 200000)
+      await BaseRegistrar.setApprovalForAll(NameWrapper.address, true)
+      await NameWrapper.wrapETH2LD(
+        'test',
+        account,
+        0,
+        MAX_EXPIRY,
+        EMPTY_ADDRESS
+      )
+      expect(await NameWrapper.ownerOf(node)).to.equal(account)
+      await SubdomainRegistrar.setupDomain(node, Erc20.address, 1, account)
+      await NameWrapper.setApprovalForAll(SubdomainRegistrar.address, true)
+      await Erc20WithAccount2.approve(
+        SubdomainRegistrar.address,
+        ethers.constants.MaxUint256
+      )
+      await SubdomainRegistrar2.register(
+        node,
+        'subname',
+        account2,
+        EMPTY_ADDRESS,
+        0,
+        86400,
+        []
+      )
+
+      const [, expiry] = await NameWrapper.getFuses(
+        namehash('subname.test.eth')
+      )
+
+      expect(await NameWrapper.ownerOf(subNode)).to.equal(account2)
+
+      await SubdomainRegistrar2.renew(node, labelhash('subname'), 86400)
+      const [, expiry2] = await NameWrapper.getFuses(
+        namehash('subname.test.eth')
+      )
+      expect(expiry2.toNumber()).to.be.greaterThan(expiry.toNumber())
+    })
+
+    it('should allow subdomains to be renewed even with 0 registration fee', async () => {
+      await BaseRegistrar.register(labelhash('test'), account, 200000)
+      await BaseRegistrar.setApprovalForAll(NameWrapper.address, true)
+      await NameWrapper.wrapETH2LD(
+        'test',
+        account,
+        0,
+        MAX_EXPIRY,
+        EMPTY_ADDRESS
+      )
+      expect(await NameWrapper.ownerOf(node)).to.equal(account)
+      await SubdomainRegistrar.setupDomain(node, Erc20.address, 0, account)
+      await NameWrapper.setApprovalForAll(SubdomainRegistrar.address, true)
+      await SubdomainRegistrar2.register(
+        node,
+        'subname',
+        account2,
+        EMPTY_ADDRESS,
+        0,
+        86400,
+        []
+      )
+
+      const [, expiry] = await NameWrapper.getFuses(
+        namehash('subname.test.eth')
+      )
+
+      expect(await NameWrapper.ownerOf(subNode)).to.equal(account2)
+
+      await SubdomainRegistrar2.renew(node, labelhash('subname'), 86400)
+      const [, expiry2] = await NameWrapper.getFuses(
+        namehash('subname.test.eth')
+      )
+
+      expect(expiry2.toNumber()).to.be.greaterThan(expiry.toNumber())
+    })
+  })
+
   describe('register Subnames with records', () => {
     it('should allow a subname to be registered with records', async () => {
       const node = namehash('test.eth')