`IUniswapModule.sol` use an immutable variable `router` can save gas and simplify implementation

[code423n4]

Handle

WatchPug

Vulnerability details

UniswapModule.sol can be changed into a regular (non-abstract) contract. And add an immutable variable router and set the value for it in the constructor function.

https://github.com/code-423n4/2021-10-slingshot/blob/9c0432cca2e43731d5a0ae9c151dacf7835b8719/contracts/module/IUniswapModule.sol#L34-L34

address router = getRouter();

Then, L34 and getRouter() can be removed and save 1 internal call and reduce the code size.

SushiSwapModule.sol and UniswapModule.sol will no longer be needed as the router can be configurated at deployment.

Recommendation

Change to:

contract UniV2Module is ISlingshotModule {
    using LibERC20Token for IERC20;

    address public immutable router;

    constructor (address _router) {
        router = _router;
    }

    /// @param amount Amount of the token being exchanged
    /// @param path Array of token addresses to swap
    /// @param tradeAll If true, it overrides totalAmountIn with current token balance
    function swap(
        uint256 amount,
        address[] memory path,
        bool tradeAll
    ) external payable {
        require(path.length > 0, "UniswapModule: path length must be >0");

        if (tradeAll) {
            amount = IERC20(path[0]).balanceOf(address(this));
        }

        IERC20(path[0]).approveIfBelow(router, amount);

        // for now, we only supporting .swapExactTokensForTokens()
        // amountOutMin is 1, because all we care is final check or output in Slingshot contract
        IUniswapV2Router02(router).swapExactTokensForTokensSupportingFeeOnTransferTokens(
            amount,
            1, // amountOutMin
            path,
            address(this),
            block.timestamp
        );
    }
}