Merge pull request #341 from consenlabs/refactor/gas-opt-techniques

optimize gas usage with common techniques and refactor code structure

Author: alex0207s

.github/workflows/gas-diff.yml

@@ -17,7 +17,7 @@ jobs:
         node-version: [16.x]
 
     steps:
-      - uses: actions/checkout@v3
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
 
@@ -47,14 +47,14 @@ jobs:
           cat gasreport-fork.ansi >> $GITHUB_STEP_SUMMARY
 
       - name: Compare gas reports of local tests
-        uses: Rubilmax/foundry-gas-diff@v3.14
+        uses: Rubilmax/foundry-gas-diff@v3
         with:
           report: gasreport-local.ansi
           ignore: test/**/*
         id: gas_diff_local
 
       - name: Compare gas reports of fork tests
-        uses: Rubilmax/foundry-gas-diff@v3.14
+        uses: Rubilmax/foundry-gas-diff@v3
         with:
           report: gasreport-fork.ansi
           ignore: test/**/*

.github/workflows/tokenlon-contracts.yml

@@ -14,7 +14,7 @@ jobs:
         node-version: [16.x]
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
         with:
           submodules: recursive
       - name: Use Node.js ${{ matrix.node-version }}

contracts/GenericSwap.sol

@@ -66,6 +66,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
     ) private returns (uint256 returnAmount) {
         if (_swapData.expiry < block.timestamp) revert ExpiredOrder();
         if (_swapData.recipient == address(0)) revert ZeroAddress();
+        if (_swapData.takerTokenAmount == 0) revert SwapWithZeroAmount();
 
         address _inputToken = _swapData.takerToken;
         address _outputToken = _swapData.makerToken;
@@ -77,7 +78,7 @@ contract GenericSwap is IGenericSwap, TokenCollector, EIP712 {
             _collect(_inputToken, _authorizedUser, _swapData.maker, _swapData.takerTokenAmount, _takerTokenPermit);
         }
 
-        IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_inputToken, _outputToken, _swapData.takerTokenAmount, _swapData.strategyData);
+        IStrategy(_swapData.maker).executeStrategy{ value: msg.value }(_outputToken, _swapData.strategyData);
 
         returnAmount = _outputToken.getBalance(address(this));
         if (returnAmount > 1) {

contracts/LimitOrderSwap.sol

@@ -193,7 +193,7 @@ contract LimitOrderSwap is ILimitOrderSwap, Ownable, TokenCollector, EIP712, Ree
             (address strategy, bytes memory strategyData) = abi.decode(takerParams.extraAction, (address, bytes));
             // the coverage report indicates that the following line causes the if statement to not be fully covered,
             // even if the logic of the executeStrategy function is empty, this if statement is still not covered.
-            IStrategy(strategy).executeStrategy(order.makerToken, order.takerToken, makerSpendingAmount - fee, strategyData);
+            IStrategy(strategy).executeStrategy(order.takerToken, strategyData);
         }
 
         // taker -> maker

contracts/SmartOrderStrategy.sol

@@ -37,32 +37,20 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement {
     }
 
     /// @inheritdoc IStrategy
-    function executeStrategy(address inputToken, address outputToken, uint256 inputAmount, bytes calldata data) external payable onlyGenericSwap {
-        if (inputAmount == 0) revert ZeroInput();
-
-        Operation[] memory ops = abi.decode(data, (Operation[]));
+    function executeStrategy(address targetToken, bytes calldata strategyData) external payable onlyGenericSwap {
+        Operation[] memory ops = abi.decode(strategyData, (Operation[]));
         if (ops.length == 0) revert EmptyOps();
 
-        // wrap ETH to WETH if inputToken is ETH
-        if (Asset.isETH(inputToken)) {
-            if (msg.value != inputAmount) revert InvalidMsgValue();
-            // the coverage report indicates that the following line causes this branch to not be covered by our tests
-            // even though we tried all possible success and revert scenarios
-            IWETH(weth).deposit{ value: inputAmount }();
-        } else {
-            if (msg.value != 0) revert InvalidMsgValue();
-        }
-
         uint256 opsCount = ops.length;
         for (uint256 i; i < opsCount; ++i) {
             Operation memory op = ops[i];
             _call(op.dest, op.inputToken, op.ratioNumerator, op.ratioDenominator, op.dataOffset, op.value, op.data);
         }
 
-        // unwrap WETH to ETH if outputToken is ETH
-        if (Asset.isETH(outputToken)) {
+        // unwrap WETH to ETH if targetToken is ETH
+        if (Asset.isETH(targetToken)) {
             // the if statement is not fully covered by the tests even replacing `makerToken.isETH()` with `makerToken == 0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE`
-            // and crafting some cases where outputToken is ETH and non-ETH
+            // and crafting some cases where targetToken is ETH and non-ETH
             uint256 wethBalance = IWETH(weth).balanceOf(address(this));
 
             if (wethBalance > 0) {
@@ -71,15 +59,15 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement {
             }
         }
 
-        uint256 selfBalance = Asset.getBalance(outputToken, address(this));
+        uint256 selfBalance = Asset.getBalance(targetToken, address(this));
         if (selfBalance > 1) {
             unchecked {
                 --selfBalance;
             }
         }
 
         // transfer output tokens back to the generic swap contract
-        Asset.transferTo(outputToken, payable(genericSwap), selfBalance);
+        Asset.transferTo(targetToken, payable(genericSwap), selfBalance);
     }
 
     /// @dev This function adjusts the input amount based on a ratio if specified, then calls the destination contract with data.
@@ -102,12 +90,6 @@ contract SmartOrderStrategy is ISmartOrderStrategy, AdminManagement {
         // adjust amount if ratio != 0
         if (_ratioNumerator != 0) {
             uint256 inputTokenBalance = IERC20(_inputToken).balanceOf(address(this));
-            // leaving one wei for gas optimization
-            if (inputTokenBalance > 1) {
-                unchecked {
-                    --inputTokenBalance;
-                }
-            }
 
             // calculate input amount if ratio should be applied
             if (_ratioNumerator != _ratioDenominator) {

contracts/abstracts/EIP712.sol

@@ -6,9 +6,6 @@ pragma solidity ^0.8.0;
 /// @notice This contract implements the EIP-712 standard for structured data hashing and signing.
 /// @dev This contract provides functions to handle EIP-712 domain separator and hash calculation.
 abstract contract EIP712 {
-    // EIP-191 Header
-    string public constant EIP191_HEADER = "\x19\x01";
-
     // EIP-712 Domain
     string public constant EIP712_NAME = "Tokenlon";
     string public constant EIP712_VERSION = "v6";
@@ -43,9 +40,24 @@ abstract contract EIP712 {
 
     /// @notice Calculate the EIP712 hash of a structured data hash.
     /// @param structHash The hash of the structured data.
-    /// @return The EIP712 hash of the structured data.
-    function getEIP712Hash(bytes32 structHash) internal view returns (bytes32) {
-        return keccak256(abi.encodePacked(EIP191_HEADER, _getDomainSeparator(), structHash));
+    /// @return digest The EIP712 hash of the structured data.
+    function getEIP712Hash(bytes32 structHash) internal view returns (bytes32 digest) {
+        // return keccak256(abi.encodePacked("\x19\x01", _getDomainSeparator(), structHash));
+
+        digest = _getDomainSeparator();
+
+        // reference:
+        // 1. solady: https://github.com/Vectorized/solady/blob/main/src/utils/EIP712.sol#L138-L147
+        // 2. 1inch: https://etherscan.io/address/0x111111125421cA6dc452d289314280a0f8842A65#code (line 1204~1209)
+        // solhint-disable no-inline-assembly
+        assembly {
+            // Compute the digest.
+            mstore(0x00, 0x1901000000000000000000000000000000000000000000000000000000000000) // Store "\x19\x01".
+            mstore(0x02, digest) // Store the domain separator.
+            mstore(0x22, structHash) // Store the struct hash.
+            digest := keccak256(0x0, 0x42)
+            mstore(0x22, 0) // Restore the part of the free memory slot that was overwritten.
+        }
     }
 
     /// @notice Get the current EIP712 domain separator.

contracts/abstracts/Ownable.sol

@@ -9,6 +9,15 @@ abstract contract Ownable {
     address public owner;
     address public nominatedOwner;
 
+    /// @notice Event emitted when a new owner is nominated.
+    /// @param newOwner The address of the new nominated owner.
+    event OwnerNominated(address indexed newOwner);
+
+    /// @notice Event emitted when ownership is transferred.
+    /// @param oldOwner The address of the previous owner.
+    /// @param newOwner The address of the new owner.
+    event OwnerChanged(address indexed oldOwner, address indexed newOwner);
+
     /// @notice Error to be thrown when the caller is not the owner.
     /// @dev This error is used to ensure that only the owner can call certain functions.
     error NotOwner();
@@ -25,14 +34,10 @@ abstract contract Ownable {
     /// @dev This error is used to prevent nominating a new owner when one is already nominated.
     error NominationExists();
 
-    /// @notice Event emitted when a new owner is nominated.
-    /// @param newOwner The address of the new nominated owner.
-    event OwnerNominated(address indexed newOwner);
-
-    /// @notice Event emitted when ownership is transferred.
-    /// @param oldOwner The address of the previous owner.
-    /// @param newOwner The address of the new owner.
-    event OwnerChanged(address indexed oldOwner, address indexed newOwner);
+    modifier onlyOwner() {
+        if (msg.sender != owner) revert NotOwner();
+        _;
+    }
 
     /// @notice Constructor to set the initial owner of the contract.
     /// @param _owner The address of the initial owner.
@@ -41,11 +46,6 @@ abstract contract Ownable {
         owner = _owner;
     }
 
-    modifier onlyOwner() {
-        if (msg.sender != owner) revert NotOwner();
-        _;
-    }
-
     /// @notice Accept the ownership transfer.
     /// @dev Only the nominated owner can call this function to accept the ownership.
     function acceptOwnership() external {

contracts/abstracts/TokenCollector.sol

@@ -15,10 +15,6 @@ import { IUniswapPermit2 } from "../interfaces/IUniswapPermit2.sol";
 abstract contract TokenCollector {
     using SafeERC20 for IERC20;
 
-    /// @notice Error to be thrown when Permit2 data is empty.
-    /// @dev This error is used to ensure Permit2 data is provided when required.
-    error Permit2DataEmpty();
-
     /// @title Token Collection Sources
     /// @notice Enumeration of possible token collection sources.
     /// @dev Represents the various methods for collecting tokens.
@@ -33,6 +29,10 @@ abstract contract TokenCollector {
     address public immutable permit2;
     address public immutable allowanceTarget;
 
+    /// @notice Error to be thrown when Permit2 data is empty.
+    /// @dev This error is used to ensure Permit2 data is provided when required.
+    error Permit2DataEmpty();
+
     /// @notice Constructor to set the Permit2 and allowance target addresses.
     /// @param _permit2 The address of the Uniswap Permit2 contract.
     /// @param _allowanceTarget The address of the allowance target contract.

contracts/interfaces/IGenericSwap.sol

@@ -8,6 +8,28 @@ import { GenericSwapData } from "../libraries/GenericSwapData.sol";
 /// @notice Interface for a generic swap contract.
 /// @dev This interface defines functions and events related to executing swaps and handling swap errors.
 interface IGenericSwap {
+    /// @notice Event emitted when a swap is executed.
+    /// @param swapHash The hash of the swap data.
+    /// @param maker The address of the maker initiating the swap.
+    /// @param taker The address of the taker executing the swap.
+    /// @param recipient The address receiving the output tokens.
+    /// @param inputToken The address of the input token.
+    /// @param inputAmount The amount of input tokens.
+    /// @param outputToken The address of the output token.
+    /// @param outputAmount The amount of output tokens received.
+    /// @param salt The salt value used in the swap.
+    event Swap(
+        bytes32 indexed swapHash,
+        address indexed maker,
+        address indexed taker,
+        address recipient,
+        address inputToken,
+        uint256 inputAmount,
+        address outputToken,
+        uint256 outputAmount,
+        uint256 salt
+    );
+
     /// @notice Error to be thrown when a swap is already filled.
     /// @dev This error is used when attempting to fill a swap that has already been completed.
     error AlreadyFilled();
@@ -32,27 +54,9 @@ interface IGenericSwap {
     /// @dev This error is used to ensure that a valid address is provided.
     error ZeroAddress();
 
-    /// @notice Event emitted when a swap is executed.
-    /// @param swapHash The hash of the swap data.
-    /// @param maker The address of the maker initiating the swap.
-    /// @param taker The address of the taker executing the swap.
-    /// @param recipient The address receiving the output tokens.
-    /// @param inputToken The address of the input token.
-    /// @param inputAmount The amount of input tokens.
-    /// @param outputToken The address of the output token.
-    /// @param outputAmount The amount of output tokens received.
-    /// @param salt The salt value used in the swap.
-    event Swap(
-        bytes32 indexed swapHash,
-        address indexed maker,
-        address indexed taker,
-        address recipient,
-        address inputToken,
-        uint256 inputAmount,
-        address outputToken,
-        uint256 outputAmount,
-        uint256 salt
-    );
+    /// @notice Error to be thrown when a swap amount is zero.
+    /// @dev This error is used to ensure that a valid, nonzero swap amount is provided.
+    error SwapWithZeroAmount();
 
     /// @notice Executes a swap using provided swap data and taker token permit.
     /// @param swapData The swap data containing details of the swap.

contracts/interfaces/ILimitOrderSwap.sol

@@ -8,6 +8,47 @@ import { LimitOrder } from "../libraries/LimitOrder.sol";
 /// @notice Interface for a limit order swap contract.
 /// @dev This interface defines functions and events related to executing and managing limit orders.
 interface ILimitOrderSwap {
+    /// @notice Struct containing parameters for the taker.
+    /// @dev This struct encapsulates the parameters necessary for a taker to fill a limit order.
+    struct TakerParams {
+        uint256 takerTokenAmount; // Amount of tokens taken by the taker.
+        uint256 makerTokenAmount; // Amount of tokens provided by the maker.
+        address recipient; // Address to receive the tokens.
+        bytes extraAction; // Additional action to be performed.
+        bytes takerTokenPermit; // Permit for spending taker's tokens.
+    }
+
+    /// @notice Emitted when the fee collector address is updated.
+    /// @param newFeeCollector The address of the new fee collector.
+    event SetFeeCollector(address newFeeCollector);
+
+    /// @notice Emitted when a limit order is successfully filled.
+    /// @param orderHash The hash of the limit order.
+    /// @param taker The address of the taker filling the order.
+    /// @param maker The address of the maker who created the order.
+    /// @param takerToken The address of the token taken by the taker.
+    /// @param takerTokenFilledAmount The amount of taker tokens filled.
+    /// @param makerToken The address of the token received by the maker.
+    /// @param makerTokenSettleAmount The amount of maker tokens settled.
+    /// @param fee The fee amount paid for the order.
+    /// @param recipient The address receiving the tokens.
+    event LimitOrderFilled(
+        bytes32 indexed orderHash,
+        address indexed taker,
+        address indexed maker,
+        address takerToken,
+        uint256 takerTokenFilledAmount,
+        address makerToken,
+        uint256 makerTokenSettleAmount,
+        uint256 fee,
+        address recipient
+    );
+
+    /// @notice Emitted when an order is canceled.
+    /// @param orderHash The hash of the canceled order.
+    /// @param maker The address of the maker who canceled the order.
+    event OrderCanceled(bytes32 orderHash, address maker);
+
     /// @notice Error to be thrown when an order has expired.
     /// @dev Thrown when attempting to fill an order that has already expired.
     error ExpiredOrder();
@@ -68,47 +109,6 @@ interface ILimitOrderSwap {
     /// @dev Thrown when an operation is attempted by an unauthorized caller who is not the maker of the order.
     error NotOrderMaker();
 
-    /// @notice Emitted when the fee collector address is updated.
-    /// @param newFeeCollector The address of the new fee collector.
-    event SetFeeCollector(address newFeeCollector);
-
-    /// @notice Emitted when a limit order is successfully filled.
-    /// @param orderHash The hash of the limit order.
-    /// @param taker The address of the taker filling the order.
-    /// @param maker The address of the maker who created the order.
-    /// @param takerToken The address of the token taken by the taker.
-    /// @param takerTokenFilledAmount The amount of taker tokens filled.
-    /// @param makerToken The address of the token received by the maker.
-    /// @param makerTokenSettleAmount The amount of maker tokens settled.
-    /// @param fee The fee amount paid for the order.
-    /// @param recipient The address receiving the tokens.
-    event LimitOrderFilled(
-        bytes32 indexed orderHash,
-        address indexed taker,
-        address indexed maker,
-        address takerToken,
-        uint256 takerTokenFilledAmount,
-        address makerToken,
-        uint256 makerTokenSettleAmount,
-        uint256 fee,
-        address recipient
-    );
-
-    /// @notice Emitted when an order is canceled.
-    /// @param orderHash The hash of the canceled order.
-    /// @param maker The address of the maker who canceled the order.
-    event OrderCanceled(bytes32 orderHash, address maker);
-
-    /// @notice Struct containing parameters for the taker.
-    /// @dev This struct encapsulates the parameters necessary for a taker to fill a limit order.
-    struct TakerParams {
-        uint256 takerTokenAmount; // Amount of tokens taken by the taker.
-        uint256 makerTokenAmount; // Amount of tokens provided by the maker.
-        address recipient; // Address to receive the tokens.
-        bytes extraAction; // Additional action to be performed.
-        bytes takerTokenPermit; // Permit for spending taker's tokens.
-    }
-
     /// @notice Fills a limit order.
     /// @param order The limit order to be filled.
     /// @param makerSignature The signature of the maker authorizing the fill.