Canto contest - Tricko's results

Remove registered flag.

Besides isRegistered function, registered is never used. The flag is only 1bit, but it allocates an entire extra storage slot for every call to register or assign. SSTORE opcode cost up to 20000 gas for uninitialized slots like these. Removing this flag reduces by 50% the total storage needed for each allocation on feeRecipient mapping, resulting in gas reductions for register and assign functions.

Modifications

If we start TokenId count from 1, we can check if the NFT is registered by checking if feeRecipient[_smartContract] != 0. This allows us to remove the registered flag and simplify the code as shown below.

diff --git a/Turnstile.sol.orig b/Turnstile.sol.mod
index b8c216a..c02c467 100644
--- a/Turnstile.sol.orig
+++ b/Turnstile.sol.mod
@@ -12,15 +12,10 @@ import "openzeppelin/utils/Counters.sol";
 contract Turnstile is Ownable, ERC721Enumerable {
     using Counters for Counters.Counter;
 
-    struct NftData {
-        uint256 tokenId;
-        bool registered;
-    }
-
     Counters.Counter private _tokenIdTracker;
 
     /// @notice maps smart contract address to tokenId
-    mapping(address => NftData) public feeRecipient;
+    mapping(address => uint256) public feeRecipient;
 
     /// @notice maps tokenId to fees earned
     mapping(uint256 => uint256) public balances;
@@ -54,7 +49,9 @@ contract Turnstile is Ownable, ERC721Enumerable {
         _;
     }
 
-    constructor() ERC721("Turnstile", "Turnstile") {}
+    constructor() ERC721("Turnstile", "Turnstile") {
+        _tokenIdTracker.increment();
+    }
 
     /// @notice Returns current value of counter used to tokenId of new minted NFTs
     /// @return current counter value
@@ -68,14 +65,14 @@ contract Turnstile is Ownable, ERC721Enumerable {
     function getTokenId(address _smartContract) external view returns (uint256) {
         if (!isRegistered(_smartContract)) revert Unregistered();
 
-        return feeRecipient[_smartContract].tokenId;
+        return feeRecipient[_smartContract];
     }
 
     /// @notice Returns true if smart contract is registered to collect fees
     /// @param _smartContract address of the smart contract
     /// @return true if smart contract is registered to collect fees, false otherwise
     function isRegistered(address _smartContract) public view returns (bool) {
-        return feeRecipient[_smartContract].registered;
+        return feeRecipient[_smartContract] != 0; 
     }
 
     /// @notice Mints ownership NFT that allows the owner to collect fees earned by the smart contract.
@@ -94,10 +91,7 @@ contract Turnstile is Ownable, ERC721Enumerable {
 
         emit Register(smartContract, _recipient, tokenId);
 
-        feeRecipient[smartContract] = NftData({
-            tokenId: tokenId,
-            registered: true
-        });
+        feeRecipient[smartContract] = tokenId;
     }
 
     /// @notice Assigns smart contract to existing NFT. That NFT will collect fees generated by the smart contract.
@@ -111,10 +105,7 @@ contract Turnstile is Ownable, ERC721Enumerable {
 
         emit Assign(smartContract, _tokenId);
 
-        feeRecipient[smartContract] = NftData({
-            tokenId: _tokenId,
-            registered: true
-        });
+        feeRecipient[smartContract] = _tokenId;
 
         return _tokenId;
     }

Use ERC721 instead of ERC721Enumerable

Using ERC721Enumerable is known to incur large gas overheads, as the Turnstile contract already manage the TokenId by itself (using Counters.Counter) and does not use any of the extra functionality present in ERC721Enumerable (e.g totalSupply(), tokenByIndex(index), tokenOfOwnerByIndex(owner, index)). We can safely change to vanilla ERC721.

Modifications

diff --git a/Turnstile.sol.orig b/Turnstile.sol.mod
index b8c216a..7f580b0 100644
--- a/Turnstile.sol.orig
+++ b/Turnstile.sol.mod
@@ -2,14 +2,14 @@
 pragma solidity 0.8.17;
 
 import "openzeppelin/access/Ownable.sol";
-import "openzeppelin/token/ERC721/extensions/ERC721Enumerable.sol";
+import "openzeppelin/token/ERC721/ERC721.sol";
 import "openzeppelin/utils/Counters.sol";
 
 /// @notice Implementation of CIP-001 https://github.com/Canto-Improvement-Proposals/CIPs/blob/main/CIP-001.md
 /// @dev Every contract is responsible to register itself in the constructor by calling `register(address)`.
 ///      If contract is using proxy pattern, it's possible to register retroactively, however past fees will be lost.
 ///      Recipient withdraws fees by calling `withdraw(uint256,address,uint256)`.
-contract Turnstile is Ownable, ERC721Enumerable {
+contract Turnstile is Ownable, ERC721 {
     using Counters for Counters.Counter;
 

Leave 1 wei in balance after withdraw

Everytime that an user withdraws all their balance, their storage slot on the balance mapping is freed, refunding gas to the caller. However the next call to distributeFees on this same user balance would require the storage slot to be re-initialized, which incur high gas cost.

This effect is dependent on users mainly withdrawing their full balances, which seems the most probable behaviour, as there is no incentive to keep any balance left on the Turnstile contract. Considering this user behaviour, modifying the withdraw function to leave 1 wei in storage can result in a net reduction of gas.

* These values are from a simulation of 10 sucessive (full withdraw followed by distributeFees) sequences.

Note that while there is a net reduction of gas, it is heavily skewed to the contract owner (who is the sole caller of distributeFees). While users would be affected negatively as the withdraw gas cost would be increased.

Modification

diff --git a/Turnstile.sol.orig b/Turnstile.sol.mod
index b8c216a..72c6e7c 100644
--- a/Turnstile.sol.orig
+++ b/Turnstile.sol.mod
@@ -132,7 +132,7 @@ contract Turnstile is Ownable, ERC721Enumerable {
         uint256 earnedFees = balances[_tokenId];
 
         if (earnedFees == 0 || _amount == 0) revert NothingToWithdraw();
-        if (_amount > earnedFees) _amount = earnedFees;
+        if (_amount >= earnedFees) _amount = earnedFees - 1;
 
         balances[_tokenId] = earnedFees - _amount;
 

Conclusion

Combining all the modifications described above, we obtain the following total gas diffs.