Badger Citadel contest - rfa's results

###BADGER-CITADEL GAS-OPT FINDINGS ##TokenSaleUpgradeable.sol 1-- -set variable as immutable https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol#L19-L21 the tokenOut & tokenIn can set as immutable because the value is just set once in the initialize() function 2-- -caching saleStart in memory can save gas https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol#L149-L151 saleStart is called multiple times (2 times to be exact) in the buy() function. i recommend to cache saleStart:

	uint64 _saleStart = saleStart;
	require(_saleStart <= block.timestamp, "TokenSale: not started");
        require(
            block.timestamp < _saleStart + saleDuration,
            "TokenSale: already ended"
        );

3-- -unnecesary variable declaration https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol#L164 unncecessary boughtAmountTillNow declarartion. It was called 2 times in this function. But for line:

boughtAmounts[msg.sender] = boughtAmountTillNow + tokenOutAmount_;

can changed by:

boughtAmounts[msg.sender] += tokenOutAmount_;

since the boughtAmounts[msg.sender] == boughtAmountTillNow

Recommended Mitigation Steps Remove line 164 and replace all boughtAmountTillNow with boughtAmounts[msg.sender] 4-- -better way to use SafeERC20Upgradable for gas optimization https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol#L16 by removing line 16 and call SafeERC20Upgradable directly (SafeERC20Upgradeable.function(arguments)) can save 15 gas per call example:

tokenIn.safeTransferFrom(msg.sender, saleRecipient, _tokenInAmount);

change to:

SafeERC20Upgradable.safeTransferFrom(tokenIn, msg.sender, saleRecipient, _tokenInAmount);

5-- https://github.com/code-423n4/2022-02-badger-citadel/blob/main/contracts/TokenSaleUpgradeable.sol the returns value of some function is unused (there are no function which is using the value of the return). i recommend just run the function without returns() example: buy(), claim() 6-- -Using saleEnded() to validate buy() instead of using:

	require(
            block.timestamp < saleStart + saleDuration,
            "TokenSale: already ended"
        );
        require(
            totalTokenIn + _tokenInAmount <= tokenInLimit,
            "total amount exceeded"
        );

using:

require(saleEnded(), 'newError');

can save gas.