optimizations: reduce gas usage (#13)

* opt: reduce deployment costs by using constant instead of immutable

* refactor: fix typo in registrable

* refactor: remove comments since we are restricting to EOA and receivable contracts

* opt: avoid unnecessary owner check in renew

* opt: avoid checking for balance != 0 before returning funds

* opt: remove check for recoveryAddress != custodyAdress

* update gas snapshot

Author: varunsrin

.gas-snapshot

@@ -19,53 +19,52 @@ AccountRegistryTest:testRegister() (gas: 89924)
 AccountRegistryTest:testRequestRecovery() (gas: 144852)
 AccountRegistryTest:testSetRecoveryAddress() (gas: 95427)
 AccountRegistryTest:testTransfer() (gas: 75750)
-NameSpaceTest:testBidAfterOneStep() (gas: 217674)
-NameSpaceTest:testBidAndOverpay() (gas: 213205)
-NameSpaceTest:testBidFlatRate() (gas: 216052)
-NameSpaceTest:testBidImmediately() (gas: 229367)
-NameSpaceTest:testBidOnHundredthStep() (gas: 216052)
-NameSpaceTest:testBidOnPenultimateStep() (gas: 216042)
-NameSpaceTest:testBidShouldClearRecovery() (gas: 260030)
-NameSpaceTest:testCancelRecoveryFromCustodyAddress() (gas: 221424)
-NameSpaceTest:testCancelRecoveryFromRecoveryAddress() (gas: 221008)
-NameSpaceTest:testCannotBidIfRegisterable() (gas: 30973)
-NameSpaceTest:testCannotBidUnlessBiddable() (gas: 173326)
-NameSpaceTest:testCannotCancelRecoveryIfNotStarted() (gas: 185626)
-NameSpaceTest:testCannotCancelRecoveryIfUnauthorized() (gas: 232195)
-NameSpaceTest:testCannotCommitWithInvalidName() (gas: 22838)
-NameSpaceTest:testCannotCompleteRecoveryIfExpired() (gas: 244224)
-NameSpaceTest:testCannotCompleteRecoveryIfNotStarted() (gas: 189905)
-NameSpaceTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 217487)
-NameSpaceTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 233829)
+NameSpaceTest:testBidAfterOneStep() (gas: 217603)
+NameSpaceTest:testBidAndOverpay() (gas: 213134)
+NameSpaceTest:testBidFlatRate() (gas: 215981)
+NameSpaceTest:testBidImmediately() (gas: 229318)
+NameSpaceTest:testBidOnHundredthStep() (gas: 215959)
+NameSpaceTest:testBidOnPenultimateStep() (gas: 215971)
+NameSpaceTest:testBidShouldClearRecovery() (gas: 259941)
+NameSpaceTest:testCancelRecoveryFromCustodyAddress() (gas: 221316)
+NameSpaceTest:testCancelRecoveryFromRecoveryAddress() (gas: 220922)
+NameSpaceTest:testCannotBidIfRegistrable() (gas: 30961)
+NameSpaceTest:testCannotBidUnlessBiddable() (gas: 173313)
+NameSpaceTest:testCannotCancelRecoveryIfNotStarted() (gas: 185540)
+NameSpaceTest:testCannotCancelRecoveryIfUnauthorized() (gas: 232109)
+NameSpaceTest:testCannotCommitWithInvalidName() (gas: 22905)
+NameSpaceTest:testCannotCompleteRecoveryIfExpired() (gas: 244138)
+NameSpaceTest:testCannotCompleteRecoveryIfNotStarted() (gas: 189819)
+NameSpaceTest:testCannotCompleteRecoveryIfUnauthorized() (gas: 217405)
+NameSpaceTest:testCannotCompleteRecoveryWhenInEscrow() (gas: 233743)
 NameSpaceTest:testCannotReclaimUnlessMinted() (gas: 17407)
 NameSpaceTest:testCannotRegisterWithoutPayment() (gas: 85851)
-NameSpaceTest:testCannotRenewIfBiddable() (gas: 163382)
-NameSpaceTest:testCannotRenewIfOwnerIncorrect() (gas: 165376)
-NameSpaceTest:testCannotRenewIfRegisterable() (gas: 32191)
-NameSpaceTest:testCannotRenewIfRegistered() (gas: 163466)
-NameSpaceTest:testCannotRenewWithoutPayment() (gas: 156300)
-NameSpaceTest:testCannotRequestRecoveryIfRegisterable() (gas: 26437)
-NameSpaceTest:testCannotRequestRecoveryToZeroAddr() (gas: 185464)
-NameSpaceTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 163876)
-NameSpaceTest:testCannotSetRecoveryIfExpired() (gas: 165042)
-NameSpaceTest:testCannotSetRecoveryIfRegisterable() (gas: 20854)
-NameSpaceTest:testCannotSetRecoveryUnlessOwner() (gas: 158705)
-NameSpaceTest:testCannotSetSelfAsRecovery() (gas: 154747)
-NameSpaceTest:testCurrYear() (gas: 78269)
+NameSpaceTest:testCannotRenewIfBiddable() (gas: 162869)
+NameSpaceTest:testCannotRenewIfRegistered() (gas: 162953)
+NameSpaceTest:testCannotRenewIfRegistrable() (gas: 31908)
+NameSpaceTest:testCannotRenewIfRegistrable2() (gas: 31950)
+NameSpaceTest:testCannotRenewWithoutPayment() (gas: 156012)
+NameSpaceTest:testCannotRequestRecoveryIfRegistrable() (gas: 26459)
+NameSpaceTest:testCannotRequestRecoveryToZeroAddr() (gas: 185400)
+NameSpaceTest:testCannotRequestRecoveryUnlessAuthorized() (gas: 163808)
+NameSpaceTest:testCannotSetRecoveryIfExpired() (gas: 164996)
+NameSpaceTest:testCannotSetRecoveryIfRegistrable() (gas: 20896)
+NameSpaceTest:testCannotSetRecoveryUnlessOwner() (gas: 158592)
+NameSpaceTest:testCurrYear() (gas: 78247)
 NameSpaceTest:testCurrYearPayment() (gas: 45881)
-NameSpaceTest:testGenerateCommit() (gas: 40372)
-NameSpaceTest:testOwnerOfRevertsIfExpired() (gas: 152817)
-NameSpaceTest:testOwnerOfRevertsIfRegisterable() (gas: 12847)
-NameSpaceTest:testReclaimBiddableNames() (gas: 181574)
-NameSpaceTest:testReclaimRegisteredNames() (gas: 190244)
-NameSpaceTest:testReclaimRenewableNames() (gas: 181517)
-NameSpaceTest:testReclaimResetsRecoveryState() (gas: 226138)
-NameSpaceTest:testRecoveryCompletion() (gas: 245203)
-NameSpaceTest:testRegister() (gas: 255657)
-NameSpaceTest:testRenewOther() (gas: 175546)
-NameSpaceTest:testRenewSelf() (gas: 172971)
-NameSpaceTest:testRenewWithOverpayment() (gas: 178190)
-NameSpaceTest:testRequestRecovery() (gas: 241657)
-NameSpaceTest:testSetRecoveryAddress() (gas: 189815)
-NameSpaceTest:testTransferFromCannotTransferExpiredName() (gas: 171509)
-NameSpaceTest:testTransferFromResetsRecovery() (gas: 225498)
+NameSpaceTest:testGenerateCommit() (gas: 40350)
+NameSpaceTest:testOwnerOfRevertsIfExpired() (gas: 152771)
+NameSpaceTest:testOwnerOfRevertsIfRegistrable() (gas: 12872)
+NameSpaceTest:testReclaimBiddableNames() (gas: 181528)
+NameSpaceTest:testReclaimRegisteredNames() (gas: 190207)
+NameSpaceTest:testReclaimRenewableNames() (gas: 181560)
+NameSpaceTest:testReclaimResetsRecoveryState() (gas: 226069)
+NameSpaceTest:testRecoveryCompletion() (gas: 245134)
+NameSpaceTest:testRegister() (gas: 255587)
+NameSpaceTest:testRenewOther() (gas: 174360)
+NameSpaceTest:testRenewSelf() (gas: 171777)
+NameSpaceTest:testRenewWithOverpayment() (gas: 177327)
+NameSpaceTest:testRequestRecovery() (gas: 241593)
+NameSpaceTest:testSetRecoveryAddress() (gas: 189689)
+NameSpaceTest:testTransferFromCannotTransferExpiredName() (gas: 171463)
+NameSpaceTest:testTransferFromResetsRecovery() (gas: 225429)

src/Namespace.sol

@@ -16,8 +16,8 @@ error InvalidTime(); // Time is too far in the future or past
 error IncorrectOwner(); // The username is not owned by the expected address
 
 error Registered(); // The username is currently registered.
-error NotRegisterable(); // The username has been registered and cannot be registered again.
-error Registerable(); // The username has never been registered.
+error NotRegistrable(); // The username has been registered and cannot be registered again.
+error Registrable(); // The username has never been registered.
 
 error Expired(); // The username is expired (renewable or biddable)
 error Biddable(); // The username is biddable
@@ -38,7 +38,7 @@ contract Namespace is ERC721, Owned {
                         REGISTRATION EVENTS
     //////////////////////////////////////////////////////////////*/
 
-    event Renew(uint256 indexed tokenId, address indexed to, uint256 expiry);
+    event Renew(uint256 indexed tokenId, uint256 expiry);
 
     /*//////////////////////////////////////////////////////////////
                         RECOVERY EVENTS
@@ -81,11 +81,11 @@ contract Namespace is ERC721, Owned {
     /*//////////////////////////////////////////////////////////////
                                 CONSTANTS
     //////////////////////////////////////////////////////////////*/
-    string public baseURI = "http://www.farcaster.xyz/";
+    string public constant baseURI = "http://www.farcaster.xyz/";
 
-    uint256 public immutable gracePeriod = 30 days;
+    uint256 public constant gracePeriod = 30 days;
 
-    uint256 public immutable fee = 0.01 ether;
+    uint256 public constant fee = 0.01 ether;
 
     // The epoch timestamp of Jan 1 for each year starting from 2022
     uint256[] internal _yearTimestamps = [
@@ -110,7 +110,7 @@ contract Namespace is ERC721, Owned {
 
     address public immutable vault;
 
-    uint256 immutable escrowPeriod = 3 days;
+    uint256 constant escrowPeriod = 3 days;
 
     constructor(
         string memory _name,
@@ -186,7 +186,7 @@ contract Namespace is ERC721, Owned {
         delete stateOf[commit];
 
         uint256 tokenId = uint256(bytes32(username));
-        if (expiryOf[tokenId] != 0) revert NotRegisterable();
+        if (expiryOf[tokenId] != 0) revert NotRegistrable();
 
         _mint(owner, tokenId);
 
@@ -195,26 +195,21 @@ contract Namespace is ERC721, Owned {
             expiryOf[tokenId] = timestampOfYear(currYear() + 1);
         }
 
-        // TODO: this may fail if called by a smart contract
-        if (msg.value > _currYearFee) {
-            payable(msg.sender).transfer(msg.value - _currYearFee);
-        }
+        payable(msg.sender).transfer(msg.value - _currYearFee);
     }
 
     /**
      * @notice Renew a name for another year while it is in the renewable period
      *
      * @param tokenId the tokenId of the name to renew
-     * @param owner the current owner of the name
      */
-    function renew(uint256 tokenId, address owner) external payable {
+    function renew(uint256 tokenId) external payable {
         if (msg.value < fee) revert InsufficientFunds();
 
         uint256 expiryTs = expiryOf[tokenId];
-        if (expiryTs == 0) revert Registerable();
+        if (expiryTs == 0) revert Registrable();
 
         // Invariant 1B + 2 guarantee that the name is not owned by address(0) at this point.
-        if (_ownerOf[tokenId] != owner) revert IncorrectOwner();
 
         unchecked {
             // renewTs and gracePeriod are pre-determined values and cannot overflow
@@ -226,12 +221,9 @@ contract Namespace is ERC721, Owned {
             expiryOf[tokenId] = timestampOfYear(currYear() + 1);
         }
 
-        emit Renew(tokenId, owner, expiryOf[tokenId]);
+        emit Renew(tokenId, expiryOf[tokenId]);
 
-        // TODO: this may fail if called by a smart contract
-        if (msg.value > fee) {
-            payable(msg.sender).transfer(msg.value - fee);
-        }
+        payable(msg.sender).transfer(msg.value - fee);
     }
 
     /**
@@ -246,7 +238,7 @@ contract Namespace is ERC721, Owned {
      */
     function bid(uint256 tokenId) external payable {
         uint256 expiryTs = expiryOf[tokenId];
-        if (expiryTs == 0) revert Registerable();
+        if (expiryTs == 0) revert Registrable();
 
         uint256 auctionStartTimestamp;
 
@@ -278,10 +270,7 @@ contract Namespace is ERC721, Owned {
             expiryOf[tokenId] = timestampOfYear(currYear() + 1);
         }
 
-        // TODO: this may revert if called by a smart contract
-        if (msg.value > price) {
-            payable(msg.sender).transfer(msg.value - price);
-        }
+        payable(msg.sender).transfer(msg.value - price);
     }
 
     /*//////////////////////////////////////////////////////////////
@@ -295,7 +284,7 @@ contract Namespace is ERC721, Owned {
         uint256 expiryTs = expiryOf[tokenId];
 
         // Invariant 1A will ensure a throw if a name was not minted, as per the ERC-721 spec.
-        if (expiryTs == 0) revert Registerable();
+        if (expiryTs == 0) revert Registrable();
 
         if (block.timestamp >= expiryTs) revert Expired();
 
@@ -321,7 +310,7 @@ contract Namespace is ERC721, Owned {
         _clearRecovery(id);
     }
 
-    function tokenURI(uint256 tokenId) public view override returns (string memory) {
+    function tokenURI(uint256 tokenId) public pure override returns (string memory) {
         return string(abi.encodePacked(baseURI, tokenId, ".json"));
     }
 
@@ -350,8 +339,6 @@ contract Namespace is ERC721, Owned {
     function setRecoveryAddress(uint256 tokenId, address recoveryAddress) external payable {
         if (ownerOf(tokenId) != msg.sender) revert Unauthorized();
 
-        if (recoveryAddress == msg.sender) revert InvalidRecovery();
-
         recoveryOf[tokenId] = recoveryAddress;
         emit SetRecoveryAddress(recoveryAddress, tokenId);
     }
@@ -436,7 +423,7 @@ contract Namespace is ERC721, Owned {
      * @param tokenId the uint256 representation of the username.
      */
     function reclaim(uint256 tokenId) external payable onlyOwner {
-        if (expiryOf[tokenId] == 0) revert Registerable();
+        if (expiryOf[tokenId] == 0) revert Registrable();
 
         unchecked {
             // this value is deterministic and cannot overflow for any known year