fix: remove payable from external functions where it was used to save gas (#145)

Author: varunsrin

Diff for: .gas-snapshot

@@ -1,6 +1,6 @@
-BundleRegistryGasUsageTest:testGasRegister() (gas: 1882173)
-BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 1747817)
-IdRegistryGasUsageTest:testGasRegisterAndRecover() (gas: 2078840)
-IdRegistryGasUsageTest:testGasRegisterFromTrustedCaller() (gas: 839504)
-NameRegistryGasUsageTest:testGasRegister() (gas: 2119333)
-NameRegistryGasUsageTest:testGasTrustedRegister() (gas: 1122592)
+BundleRegistryGasUsageTest:testGasRegister() (gas: 1882701)
+BundleRegistryGasUsageTest:testGasTrustedRegister() (gas: 1748537)
+IdRegistryGasUsageTest:testGasRegisterAndRecover() (gas: 2081451)
+IdRegistryGasUsageTest:testGasRegisterFromTrustedCaller() (gas: 840128)
+NameRegistryGasUsageTest:testGasRegister() (gas: 2119544)
+NameRegistryGasUsageTest:testGasTrustedRegister() (gas: 1122616)

Diff for: src/IdRegistry.sol

@@ -194,7 +194,7 @@ contract IdRegistry is ERC2771Context, Ownable {
         address to,
         address recovery,
         string calldata url
-    ) external payable {
+    ) external {
         // Perf: Don't check to == address(0) to save 29 gas since 0x0 can only register 1 fid
 
         if (trustedOnly == 1) revert Invitable();
@@ -217,7 +217,7 @@ contract IdRegistry is ERC2771Context, Ownable {
         address to,
         address recovery,
         string calldata url
-    ) external payable {
+    ) external {
         // Perf: Don't check to == address(0) to save 29 gas since 0x0 can only register 1 fid
 
         if (trustedOnly == 0) revert Registrable();
@@ -238,7 +238,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param url The new home url for the fid
      */
-    function changeHome(string calldata url) external payable {
+    function changeHome(string calldata url) external {
         uint256 id = idOf[_msgSender()];
         if (id == 0) revert HasNoId();
 
@@ -272,7 +272,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param to The address to transfer the fid to.
      */
-    function transfer(address to) external payable {
+    function transfer(address to) external {
         address sender = _msgSender();
         uint256 id = idOf[sender];
 
@@ -342,7 +342,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param recovery The address which can recover the fid (set to 0x0 to disable recovery)
      */
-    function changeRecoveryAddress(address recovery) external payable {
+    function changeRecoveryAddress(address recovery) external {
         uint256 id = idOf[_msgSender()];
         if (id == 0) revert HasNoId();
 
@@ -362,7 +362,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      * @param from The address that owns the fid
      * @param to   The address where the fid should be sent
      */
-    function requestRecovery(address from, address to) external payable {
+    function requestRecovery(address from, address to) external {
         uint256 id = idOf[from];
         if (_msgSender() != recoveryOf[id]) revert Unauthorized();
 
@@ -384,7 +384,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param from The address that owns the id.
      */
-    function completeRecovery(address from) external payable {
+    function completeRecovery(address from) external {
         uint256 id = idOf[from];
 
         if (_msgSender() != recoveryOf[id]) revert Unauthorized();
@@ -415,7 +415,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param from The address that owns the id.
      */
-    function cancelRecovery(address from) external payable {
+    function cancelRecovery(address from) external {
         uint256 id = idOf[from];
         address sender = _msgSender();
 
@@ -442,7 +442,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      *
      * @param _trustedCaller The address of the new trusted caller
      */
-    function changeTrustedCaller(address _trustedCaller) external payable onlyOwner {
+    function changeTrustedCaller(address _trustedCaller) external onlyOwner {
         trustedCaller = _trustedCaller;
         emit ChangeTrustedCaller(_trustedCaller);
     }
@@ -451,7 +451,7 @@ contract IdRegistry is ERC2771Context, Ownable {
      * @notice Disable trustedRegister() and let anyone get an fid by calling register(). This must
      *         be called by the contract's owner.
      */
-    function disableTrustedOnly() external payable onlyOwner {
+    function disableTrustedOnly() external onlyOwner {
         delete trustedOnly;
         emit DisableTrustedOnly();
     }

Diff for: src/NameRegistry.sol

@@ -388,7 +388,7 @@ contract NameRegistry is
      *
      * @param commit The commitment hash to be persisted on-chain
      */
-    function makeCommit(bytes32 commit) external payable {
+    function makeCommit(bytes32 commit) external {
         if (trustedOnly == 1) revert Invitable();
 
         unchecked {
@@ -818,7 +818,7 @@ contract NameRegistry is
      * @param tokenId  The uint256 representation of the fname
      * @param recovery The address which can recover the fname (set to 0x0 to disable recovery)
      */
-    function changeRecoveryAddress(uint256 tokenId, address recovery) external payable whenNotPaused {
+    function changeRecoveryAddress(uint256 tokenId, address recovery) external whenNotPaused {
         if (ownerOf(tokenId) != _msgSender()) revert Unauthorized();
 
         recoveryOf[tokenId] = recovery;
@@ -838,7 +838,7 @@ contract NameRegistry is
      * @param tokenId The uint256 representation of the fname
      * @param to      The address to transfer the fname to, which cannot be address(0)
      */
-    function requestRecovery(uint256 tokenId, address to) external payable whenNotPaused {
+    function requestRecovery(uint256 tokenId, address to) external whenNotPaused {
         if (to == address(0)) revert InvalidRecovery();
 
         // Invariant 3 ensures that a request cannot be made after ownership change without consent
@@ -865,7 +865,7 @@ contract NameRegistry is
      *
      * @param tokenId The uint256 representation of the fname
      */
-    function completeRecovery(uint256 tokenId) external payable {
+    function completeRecovery(uint256 tokenId) external {
         if (block.timestamp >= expiryOf[tokenId]) revert Expired();
 
         // Invariant 3 ensures that a request cannot be completed after ownership change without consent
@@ -890,7 +890,7 @@ contract NameRegistry is
      *
      * @param tokenId The uint256 representation of the fname
      */
-    function cancelRecovery(uint256 tokenId) external payable {
+    function cancelRecovery(uint256 tokenId) external {
         address sender = _msgSender();
 
         // Perf: super.ownerOf is called instead of ownerOf since cancellation has no undesirable
@@ -945,7 +945,7 @@ contract NameRegistry is
      *
      * @param _trustedCaller The address of the new trusted caller
      */
-    function changeTrustedCaller(address _trustedCaller) external payable {
+    function changeTrustedCaller(address _trustedCaller) external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(ADMIN_ROLE, msg.sender)) revert NotAdmin();
         trustedCaller = _trustedCaller;
@@ -955,7 +955,7 @@ contract NameRegistry is
     /**
      * @notice Disables registerTrusted and enables register calls from any address.
      */
-    function disableTrustedOnly() external payable {
+    function disableTrustedOnly() external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(ADMIN_ROLE, msg.sender)) revert NotAdmin();
         delete trustedOnly;
@@ -967,7 +967,7 @@ contract NameRegistry is
      *
      * @param _vault The address of the new vault
      */
-    function changeVault(address _vault) external payable {
+    function changeVault(address _vault) external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(ADMIN_ROLE, msg.sender)) revert NotAdmin();
         vault = _vault;
@@ -979,7 +979,7 @@ contract NameRegistry is
      *
      * @param _pool The address of the new pool
      */
-    function changePool(address _pool) external payable {
+    function changePool(address _pool) external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(ADMIN_ROLE, msg.sender)) revert NotAdmin();
         pool = _pool;
@@ -995,7 +995,7 @@ contract NameRegistry is
      *
      * @param _fee The new yearly fee
      */
-    function changeFee(uint256 _fee) external payable {
+    function changeFee(uint256 _fee) external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(TREASURER_ROLE, msg.sender)) revert NotTreasurer();
 
@@ -1009,7 +1009,7 @@ contract NameRegistry is
      *
      * @param amount The amount of ether to withdraw
      */
-    function withdraw(uint256 amount) external payable {
+    function withdraw(uint256 amount) external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(TREASURER_ROLE, msg.sender)) revert NotTreasurer();
 
@@ -1028,7 +1028,7 @@ contract NameRegistry is
     /**
      * @notice pause the contract and prevent registrations, renewals, recoveries and transfers of names.
      */
-    function pause() external payable {
+    function pause() external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(OPERATOR_ROLE, msg.sender)) revert NotOperator();
         _pause();
@@ -1037,7 +1037,7 @@ contract NameRegistry is
     /**
      * @notice unpause the contract and resume registrations, renewals, recoveries and transfers of names.
      */
-    function unpause() external payable {
+    function unpause() external {
         // avoid _msgSender() since meta-tx are unnecessary here and increase attack surface area
         if (!hasRole(OPERATOR_ROLE, msg.sender)) revert NotOperator();
         _unpause();