style: more comments and finishing touches (#68)

* style: more comments and finishing touches

* Apply suggestions from code review

Co-authored-by: Edison <6057323+edison0x@users.noreply.github.com>
Co-authored-by: zlace0x <81418809+zlace0x@users.noreply.github.com>

.forge-snapshots/approveRenewable.snap

@@ -1 +1 @@
-80508
+78546

.forge-snapshots/approveRenewableAndCall.snap

@@ -1 +1 @@
-88948
+85020

.forge-snapshots/deployFunnelForToken.snap

@@ -1 +1 @@
-155768
+152535

.forge-snapshots/executeMetaTransactionApproveRenewable.snap

@@ -1 +1 @@
-113961
+108767

.forge-snapshots/executeMetaTransactionTransfer.snap

@@ -1 +1 @@
-82023
+75914

.forge-snapshots/permit.snap

@@ -1 +1 @@
-105246
+101400

.forge-snapshots/transferFrom.snap

@@ -1 +1 @@
-46427
+43046

.forge-snapshots/transferFromAndCall.snap

@@ -1 +1 @@
-56479
+50821

src/Funnel.sol

@@ -88,36 +88,6 @@ contract Funnel is IFunnel, NativeMetaTransaction, MetaTxContext, Initializable,
         _fallback(address(_baseToken));
     }
 
-    /// @notice Fallback implementation
-    /// @dev Delegates execution to an implementation contract (i.e. base token)
-    /// This is a low level function that doesn't return to its internal call site.
-    /// It will return to the external caller whatever the implementation returns.
-    /// @param implementation Address to delegate.
-    function _fallback(address implementation) internal virtual {
-        assembly {
-            // Copy msg.data. We take full control of memory in this inline assembly
-            // block because it will not return to Solidity code. We overwrite the
-            // Solidity scratch pad at memory position 0.
-            calldatacopy(0, 0, calldatasize())
-
-            // Call the implementation.
-            // out and outsize are 0 because we don't know the size yet.
-            let result := staticcall(gas(), implementation, 0, calldatasize(), 0, 0)
-
-            // Copy the returned data.
-            returndatacopy(0, 0, returndatasize())
-
-            switch result
-            // delegatecall returns 0 on error.
-            case 0 {
-                revert(0, returndatasize())
-            }
-            default {
-                return(0, returndatasize())
-            }
-        }
-    }
-
     /// @notice Sets fixed allowance with signed approval.
     /// @dev The address cannot be zero
     /// @param owner The address of the token owner
@@ -285,17 +255,6 @@ contract Funnel is IFunnel, NativeMetaTransaction, MetaTxContext, Initializable,
         return address(_baseToken);
     }
 
-    /// @notice Query if a contract implements an interface
-    /// @param interfaceId The interface identifier, as specified in ERC-165
-    /// @dev Interface identification is specified in ERC-165. See https://eips.ethereum.org/EIPS/eip-165
-    /// @return `true` if the contract implements `interfaceID`
-    function supportsInterface(bytes4 interfaceId) external pure virtual returns (bool) {
-        return
-            interfaceId == type(IERC5827).interfaceId ||
-            interfaceId == type(IERC5827Payable).interfaceId ||
-            interfaceId == type(IERC5827Proxy).interfaceId;
-    }
-
     /// @inheritdoc IERC20
     function balanceOf(address account) external view returns (uint256 balance) {
         return _baseToken.balanceOf(account);
@@ -329,10 +288,58 @@ contract Funnel is IFunnel, NativeMetaTransaction, MetaTxContext, Initializable,
         return block.chainid == INITIAL_CHAIN_ID ? INITIAL_DOMAIN_SEPARATOR : _computeDomainSeparator();
     }
 
+    /// @notice Query if a contract implements an interface
+    /// @param interfaceId The interface identifier, as specified in ERC-165
+    /// @dev Interface identification is specified in ERC-165. See https://eips.ethereum.org/EIPS/eip-165
+    /// @return `true` if the contract implements `interfaceID`
+    function supportsInterface(bytes4 interfaceId) external pure virtual returns (bool) {
+        return
+            interfaceId == type(IERC5827).interfaceId ||
+            interfaceId == type(IERC5827Payable).interfaceId ||
+            interfaceId == type(IERC5827Proxy).interfaceId;
+    }
+
     /// =================================================================
     ///                 Internal Functions
     /// =================================================================
 
+    /// @notice Fallback implementation
+    /// @dev Delegates execution to an implementation contract (i.e. base token)
+    /// This is a low level function that doesn't return to its internal call site.
+    /// It will return to the external caller whatever the implementation returns.
+    /// @param implementation Address to delegate.
+    function _fallback(address implementation) internal virtual {
+        assembly {
+            // Copy msg.data. We take full control of memory in this inline assembly
+            // block because it will not return to Solidity code. We overwrite the
+            // Solidity scratch pad at memory position 0.
+            calldatacopy(0, 0, calldatasize())
+
+            // Call the implementation.
+            // out and outsize are 0 because we don't know the size yet.
+            let result := staticcall(gas(), implementation, 0, calldatasize(), 0, 0)
+
+            // Copy the returned data.
+            returndatacopy(0, 0, returndatasize())
+
+            switch result
+            // delegatecall returns 0 on error.
+            case 0 {
+                revert(0, returndatasize())
+            }
+            default {
+                return(0, returndatasize())
+            }
+        }
+    }
+
+    /// @notice Internal function to process the approve
+    /// Updates the mapping of `RenewableAllowance`
+    /// @dev recoveryRate must be lesser than the value
+    /// @param _owner The address of the owner
+    /// @param _spender The address of the spender
+    /// @param _value The amount of tokens to be approved
+    /// @param _recoveryRate The amount of tokens to be recovered per second
     function _approve(
         address _owner,
         address _spender,
@@ -353,7 +360,7 @@ contract Funnel is IFunnel, NativeMetaTransaction, MetaTxContext, Initializable,
         emit RenewableApproval(_owner, _spender, _value, _recoveryRate);
     }
 
-    /// @dev Internal function to invoke {IERC1363Receiver-onTransferReceived} on a target address
+    /// @notice Internal function to invoke {IERC1363Receiver-onTransferReceived} on a target address
     /// The call is not executed if the target address is not a contract
     /// @param from address Representing the previous owner of the given token amount
     /// @param recipient address Target address that will receive the tokens
@@ -393,6 +400,12 @@ contract Funnel is IFunnel, NativeMetaTransaction, MetaTxContext, Initializable,
         }
     }
 
+    /// @notice Internal functionn that is called after `approve` function.
+    /// `onRenewableApprovalReceived` may revert. Function also checks if the address called is a IERC5827Spender
+    /// @param _spender The address which will spend the funds
+    /// @param _value The amount of tokens to be spent
+    /// @param _recoveryRate The amount of tokens to be recovered per second
+    /// @param data bytes Additional data with no specified format
     function _checkOnApprovalReceived(
         address _spender,
         uint256 _value,

src/FunnelFactory.sol

@@ -9,7 +9,6 @@ import { Clones } from "openzeppelin-contracts/proxy/Clones.sol";
 
 /// @title Factory for all the funnel contracts
 /// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
-
 contract FunnelFactory is IFunnelFactory, IFunnelErrors {
     using Clones for address;
 

src/interfaces/IERC5827.sol

@@ -1,9 +1,12 @@
-// SPDX-License-Identifier: CC0-1.0
+// SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
 import "openzeppelin-contracts/interfaces/IERC20.sol";
 import "openzeppelin-contracts/interfaces/IERC165.sol";
 
+/// @title Interface for IERC5827 contracts
+/// @notice Please see https://eips.ethereum.org/EIPS/eip-5827 for more details on the goals of this interface
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 interface IERC5827 is IERC20, IERC165 {
     /// Note: the ERC-165 identifier for this interface is 0x93cd7af6.
     /// 0x93cd7af6 ===

src/interfaces/IERC5827Payable.sol

@@ -3,6 +3,8 @@ pragma solidity 0.8.17;
 
 import "openzeppelin-contracts/interfaces/IERC165.sol";
 
+/// @title IERC5827Payable interface
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 interface IERC5827Payable is IERC165 {
     /// Note: the ERC-165 identifier for this interface is 0x3717806a
     /// 0x3717806a ===

src/interfaces/IERC5827Proxy.sol

@@ -1,6 +1,8 @@
-// SPDX-License-Identifier: CC0-1.0
+// SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
+/// @title IERC5827Proxy interface
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 interface IERC5827Proxy {
     /// Note: the ERC-165 identifier for this interface is 0xc55dae63.
     /// 0xc55dae63 ===

src/interfaces/IERC5827Spender.sol

@@ -3,6 +3,7 @@ pragma solidity 0.8.17;
 
 /// @title IERC5827Spender defines a callback function that is called when renewable allowance is approved
 /// @author Zlace
+/// @notice This interface must be implemented if the spender contract wants to react to an renewable approval
 /// @dev Allow transfer/approval call chaining inspired by https://eips.ethereum.org/EIPS/eip-1363
 interface IERC5827Spender {
     /// Note: the ERC-165 identifier for this interface is 0xb868618d.

src/lib/EIP712.sol

@@ -3,6 +3,9 @@ pragma solidity 0.8.17;
 
 import { IERC1271 } from "openzeppelin-contracts/interfaces/IERC1271.sol";
 
+/// @title EIP712
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
+/// @notice https://eips.ethereum.org/EIPS/eip-712
 abstract contract EIP712 {
     /// @dev Invalid signature
     error InvalidSignature();
@@ -19,6 +22,9 @@ abstract contract EIP712 {
     /// @notice Checks if signer's signature matches the data
     /// @param signer address of the signer
     /// @param hashStruct hash of the typehash & abi encoded data, see https://eips.ethereum.org/EIPS/eip-712#definition-of-hashstruct[hashed struct]
+    /// @param v recovery identifier
+    /// @param r signature parameter
+    /// @param s signature parameter
     /// @return bool true if the signature is valid, false otherwise
     function _verifySig(
         address signer,

src/lib/MathUtil.sol

@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
+/// @notice Common library for math utils
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 library MathUtil {
     /// @dev returns the sum of two uint256 values, saturating at 2**256 - 1
     function saturatingAdd(uint256 a, uint256 b) internal pure returns (uint256) {

src/lib/MetaTxContext.sol

@@ -3,12 +3,13 @@ pragma solidity 0.8.17;
 
 import { Context } from "openzeppelin-contracts/utils/Context.sol";
 
-/// @dev Provides information about the current execution context, including the
+/// @notice Provides information about the current execution context, including the
 /// sender of the transaction and its data. While these are generally available
 /// via msg.sender and msg.data, they should not be accessed in such a direct
 /// manner, since when dealing with meta-transactions the account sending and
 /// paying for execution may not be the actual sender (as far as an application
 /// is concerned).
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 abstract contract MetaTxContext is Context {
     /// @notice Allows the recipient contract to retrieve the original sender
     /// in the case of a meta-transaction sent by the relayer

src/lib/NativeMetaTransaction.sol

@@ -4,6 +4,9 @@ pragma solidity 0.8.17;
 import { EIP712 } from "./EIP712.sol";
 import { Nonces } from "./Nonces.sol";
 
+/// @title NativeMetaTransaction
+/// @notice Functions that enables meta transactions
+/// @author Zac (zlace0x), zhongfu (zhongfu), Edison (edison0xyz)
 abstract contract NativeMetaTransaction is EIP712, Nonces {
     /// Precomputed typeHash as defined in EIP712
     /// keccak256("MetaTransaction(uint256 nonce,address from,bytes functionSignature)")
@@ -36,6 +39,9 @@ abstract contract NativeMetaTransaction is EIP712, Nonces {
     /// Allows a relayer to send another user's transaction and pay the gas
     /// @param userAddress Address of the user the sender is performing on behalf of
     /// @param functionSignature The signature of the user
+    /// @param sigR Output of the ECDSA signature
+    /// @param sigS Output of the ECDSA signature
+    /// @param sigV recovery identifier
     /// @return data encoded return data of the underlying function call
     function executeMetaTransaction(
         address userAddress,

src/lib/Nonces.sol

@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: MIT
 pragma solidity 0.8.17;
 
+/// @notice Handles nonces mapping. Required for EIP712-based signatures
 abstract contract Nonces {
     /// mapping between the user and the nonce of the account
     mapping(address => uint256) internal _nonces;