Alchemix contest - sashik_eth's results

G01 - Use immutable instead of constant expressions.

Constants expressions are recomputed each time when it used in code, it's cost some gas instead of immutable, which would compute only once in the constructor.

Use immutable instead of constant or hardcode expressions values in constants with accompanying comments.

contracts-full/AlchemicTokenV1.sol:22    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
contracts-full/AlchemicTokenV1.sol:25    bytes32 public constant SENTINEL_ROLE = keccak256("SENTINEL");

contracts-full/AlchemicTokenV2.sol:21    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
contracts-full/AlchemicTokenV2.sol:24    bytes32 public constant SENTINEL_ROLE = keccak256("SENTINEL");
contracts-full/AlchemicTokenV2.sol:27    bytes32 public constant CALLBACK_SUCCESS = keccak256("ERC3156FlashBorrower.onFlashLoan");

contracts-full/AlchemicTokenV2Base.sol:22    bytes32 public constant ADMIN_ROLE = keccak256("ADMIN");
contracts-full/AlchemicTokenV2Base.sol:25    bytes32 public constant SENTINEL_ROLE = keccak256("SENTINEL");
contracts-full/AlchemicTokenV2Base.sol:28    bytes32 public constant CALLBACK_SUCCESS = keccak256("ERC3156FlashBorrower.onFlashLoan");

contracts-full/TransmuterBuffer.sol:31    bytes32 public constant ADMIN = keccak256("ADMIN");
contracts-full/TransmuterBuffer.sol:34    bytes32 public constant KEEPER = keccak256("KEEPER");

contracts-full/TransmuterV2.sol:96    address public constant ZERO_ADDRESS = address(0);
contracts-full/TransmuterV2.sol:99    bytes32 public constant ADMIN = keccak256("ADMIN");
contracts-full/TransmuterV2.sol:102    bytes32 public constant SENTINEL = keccak256("SENTINEL");

G02 - Emitting memory variables

Cheaper to emit memory values already existing inside the function instead of additional reading state variables from storage.

contracts-full/AlchemicTokenV2.sol:94    emit SetFlashMintFee(flashMintFee); // Could emit newFee

contracts-full/AlchemicTokenV2Base.sol:100    emit SetFlashMintFee(flashMintFee);  // Could emit newFee

contracts-full/TransmuterV2.sol:203    emit Paused(isPaused); // Could emit pauseState

contracts-full/TransmuterBuffer.sol:247    emit SetAlchemist(alchemist); // Could emit _alchemist

G03 - Don’t use too long error message

Revert messages in require statements that fit in 32 bytes would cost lower gas.

Additionally, using a custom error instead of a revert string could save some gas.

contracts-full/AlchemicTokenV1.sol:51    require(whiteList[msg.sender], "AlTokenV1: Alchemist is not whitelisted");
contracts-full/AlchemicTokenV1.sol:81    require(total <= ceiling[msg.sender], "AlUSD: Alchemist's ceiling was breached.");

contracts-full/StakingPools.sol:106    require(_governance != address(0), "StakingPools: governance address cannot be 0x0");
contracts-full/StakingPools.sol:124    require(_pendingGovernance != address(0), "StakingPools: pending governance address cannot be 0x0");
contracts-full/StakingPools.sol:131    require(msg.sender == pendingGovernance, "StakingPools: only pending governance");
contracts-full/StakingPools.sol:160    require(tokenPoolIds[_token] == 0, "StakingPools: token already has a pool");
contracts-full/StakingPools.sol:183    require(_rewardWeights.length == _pools.length(), "StakingPools: weights length mismatch");

G04 - Cache array length before for loops

Reading array length inside for loop costs more gas than reading its cached value.

contracts-full/AlchemistV2.sol:990    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1282    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1355    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1461    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1524    for (uint256 i = 0; i < depositedTokens.values.length; i++) {

contracts-full/CrossChainCanonicalBase.sol:57    for (uint256 i = 0; i < _bridgeTokens.length; i++){
contracts-full/CrossChainCanonicalBase.sol:141    for (uint i = 0; i < bridgeTokensArray.length; i++){

contracts-full/StakingPools.sol:188    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {
contracts-full/StakingPools.sol:363    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {

contracts-full/TransmuterBuffer.sol:172    for (uint256 i = 0; i < _yieldTokens[underlyingToken].length; i++) {
contracts-full/TransmuterBuffer.sol:186    for (uint256 i = 0; i < tokens.length; i++) {
contracts-full/TransmuterBuffer.sol:235    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:242    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:272    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:382    for (uint256 j = 0; j < registeredUnderlyings.length; j++) {
contracts-full/TransmuterBuffer.sol:479    for (uint256 j = 0; j < weighting.tokens.length; j++) {

contracts-full/base/Multicall.sol:14    for (uint256 i = 0; i < data.length; i++) {

G05 - Use prefix counter incrementing ++i since it cost less gas than i++.

contracts-full/AlchemistV2.sol:990    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1282    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1355    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1461    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1524    for (uint256 i = 0; i < depositedTokens.values.length; i++) {

contracts-full/CrossChainCanonicalBase.sol:57    for (uint256 i = 0; i < _bridgeTokens.length; i++){
contracts-full/CrossChainCanonicalBase.sol:141    for (uint i = 0; i < bridgeTokensArray.length; i++){

contracts-full/EthAssetManager.sol:214    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/EthAssetManager.sol:567    for (uint256 i = 0; i < NUM_META_COINS; i++) {

contracts-full/StakingPools.sol:188    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {
contracts-full/StakingPools.sol:363    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {

contracts-full/ThreePoolAssetManager.sol:250    for (uint256 i = 0; i < NUM_STABLE_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:254    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:353    for (uint256 i = 0; i < 256; i++) {
contracts-full/ThreePoolAssetManager.sol:773    for (uint256 i = 0; i < NUM_STABLE_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:902    for (uint256 i = 0; i < NUM_META_COINS; i++) {

contracts-full/TransmuterBuffer.sol:172    for (uint256 i = 0; i < _yieldTokens[underlyingToken].length; i++) {
contracts-full/TransmuterBuffer.sol:186    for (uint256 i = 0; i < tokens.length; i++) {
contracts-full/TransmuterBuffer.sol:235    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:242    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:272    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:382    for (uint256 j = 0; j < registeredUnderlyings.length; j++) {
contracts-full/TransmuterBuffer.sol:387    for (uint256 i = 0; i < numYTokens; i++) {
contracts-full/TransmuterBuffer.sol:479    for (uint256 j = 0; j < weighting.tokens.length; j++) {

contracts-full/base/Multicall.sol:14    for (uint256 i = 0; i < data.length; i++) {

G06 - For computations that can’t underflow/overflow use unchecked

Using unchecked blocks for operations that can’t underflow/overflow could save a lot of gas, especially inside for loops.

Counter i with type uint256 could be safely incremented in unchecked block at the end of loop code, like:

for (uint256 i; i < n;) {
    …
    unchecked { ++i; }
}

Check comments on lines for other cases than for loop, where unchecked could be used.

contracts-full/AlchemistV2.sol:468    _yieldTokens[yieldToken].creditUnlockRate = FIXED_POINT_SCALAR / blocks; // Could be unchecked since blocks > 0, checked on L466
contracts-full/AlchemistV2.sol:933    uint256 distributeAmount = amountUnderlyingTokens - feeAmount; // Could be unchecked since feeAmount can’t be grater than amountUnderlyingTokens due to protocolFee <= BPS  L442
contracts-full/AlchemistV2.sol:990    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1014    uint256 harvestable = _convertUnderlyingTokensToYield(yieldToken, currentValue - expectedValue); // Could be unchecked because checked on L1010
contracts-full/AlchemistV2.sol:1282    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1355    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1461    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1493    _accounts[recipient].balances[yieldToken] += shares; // Could be unchecked because if overflow happens it will be checked on L1494 since totalShares always >= any individual balance
contracts-full/AlchemistV2.sol:1508    _yieldTokens[yieldToken].totalShares -= shares; // Could be unchecked because  if underflow happens it will be checked on L11507 since any individual balance always <= totalShares 
contracts-full/AlchemistV2.sol:1524    for (uint256 i = 0; i < depositedTokens.values.length; i++) {
contracts-full/AlchemistV2.sol:1569    uint256 harvestable = _convertUnderlyingTokensToYield(yieldToken, currentValue - expectedValue); // Could be unchecked because checked on L1569

contracts-full/CrossChainCanonicalBase.sol:57    for (uint256 i = 0; i < _bridgeTokens.length; i++){
contracts-full/CrossChainCanonicalBase.sol:141    for (uint i = 0; i < bridgeTokensArray.length; i++){

contracts-full/EthAssetManager.sol:214    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/EthAssetManager.sol:549    uint256 reduction = totalCliffs - cliff; // // Cliff can’t be grater than totalCloffs, checked on L547
contracts-full/EthAssetManager.sol:567    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/EthAssetManager.sol:589    if (value > address(this).balance) weth.withdraw(value - address(this).balance); // Could be unchecked since if statement checks underflow
contracts-full/EthAssetManager.sol:629    if (value > address(this).balance) weth.withdraw(value - address(this).balance); // Could be unchecked since if statement checks underflow
contracts-full/EthAssetManager.sol:721    return x > y ? x - y : y - x; // Could be unchecked due to return statement logic

contracts-full/StakingPools.sol:188    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {
contracts-full/StakingPools.sol:363    for (uint256 _poolId = 0; _poolId < _pools.length(); _poolId++) {

contracts-full/ThreePoolAssetManager.sol:250    for (uint256 i = 0; i < NUM_STABLE_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:254    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:353    for (uint256 i = 0; i < 256; i++) {
contracts-full/ThreePoolAssetManager.sol:399    return v.minimizedBalance > v.startingBalance // Could be unchecked due to return statement logic     
contracts-full/ThreePoolAssetManager.sol:751    uint256 reduction = totalCliffs - cliff; // Cliff can’t be grater than totalCloffs, checked on L749
contracts-full/ThreePoolAssetManager.sol:773    for (uint256 i = 0; i < NUM_STABLE_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:902    for (uint256 i = 0; i < NUM_META_COINS; i++) {
contracts-full/ThreePoolAssetManager.sol:1037    return x > y ? x - y : y - x; // Could be unchecked due to return statement logic

contracts-full/TransmuterBuffer.sol:172    for (uint256 i = 0; i < _yieldTokens[underlyingToken].length; i++) {
contracts-full/TransmuterBuffer.sol:186    for (uint256 i = 0; i < tokens.length; i++) {
contracts-full/TransmuterBuffer.sol:235    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:242    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:272    for (uint256 i = 0; i < registeredUnderlyings.length; i++) {
contracts-full/TransmuterBuffer.sol:382    for (uint256 j = 0; j < registeredUnderlyings.length; j++) {
contracts-full/TransmuterBuffer.sol:387    for (uint256 i = 0; i < numYTokens; i++) {
contracts-full/TransmuterBuffer.sol:429    if (localBalance - amount < currentExchanged[underlyingToken]) { // Could be unchecked since amount <= localBalance, checked on L421
contracts-full/TransmuterBuffer.sol:479    for (uint256 j = 0; j < weighting.tokens.length; j++) {
contracts-full/TransmuterBuffer.sol:541    want = flowAvailable[underlyingToken] - initialLocalBalance; // Could be unchecked since checked on L539


contracts-full/TransmuterV2.sol:551    return amount / conversionFactor; // Could be unchecked since conversionFactor can’t be 0 

contracts-full/base/Multicall.sol:14    for (uint256 i = 0; i < data.length; i++) {

contracts-full/libraries/Sets.sol:42    index--; // Could be unchecked since index > 0, checked on L37

G07 - Caching storage variables

Since reading from memory is much cheaper than reading from storage, state variables that are called more than 1 SLOAD inside the function should be cached.

contracts-full/EthAssetManager.sol:429    SafeERC20.safeTransfer(address(convexToken), rewardReceiver, convexBalance); // rewardReceiver 2 SLOADs 
contracts-full/EthAssetManager.sol:500    IERC20TokenReceiver(transmuterBuffer).onERC20Received(address(weth), amount); // address(weth) and transmuterBuffer, both 2 SLOADs
contracts-full/EthAssetManager.sol:699    SafeERC20.safeTransfer(address(convexToken), rewardReceiver, convexBalance); // rewardReceiver 2 SLOADs 

contracts-full/ThreePoolAssetManager.sol:633    SafeERC20.safeTransfer(address(convexToken), rewardReceiver, convexBalance); // rewardReceiver 2 SLOADs
contracts-full/ThreePoolAssetManager.sol:721    IERC20TokenReceiver(transmuterBuffer).onERC20Received(address(token), amount);  // transmuterBuffer 2 SLOADs
contracts-full/ThreePoolAssetManager.sol:1015    SafeERC20.safeTransfer(address(convexToken), rewardReceiver, convexBalance); //  rewardReceiver 2 SLOADs

contracts-full/TransmuterBuffer.sol:154    return flowAvailable[underlyingToken]; // flowAvailable[underlyingToken] 2 SLOADs
contracts-full/TransmuterBuffer.sol:245    TokenUtils.safeApprove(debtToken, alchemist, type(uint256).max); // alchemist 2 SLOADs after update and 4 SLOADs before update
contracts-full/TransmuterBuffer.sol:390    IAlchemistV2.YieldTokenParams memory params = IAlchemistV2(alchemist) //  alchemist 3 SLOADs
contracts-full/TransmuterBuffer.sol:406    IAlchemistV2(alchemist).mint(credit, address(this)); // alchemist 2 SLOADs
contracts-full/TransmuterBuffer.sol:446    uint256 tokensPerShare = IAlchemistV2(alchemist) // alchemist 3 SLOADs
contracts-full/TransmuterBuffer.sol:522    IAlchemistV2(alchemist).withdrawUnderlying(token, wantShares, address(this), minimumAmountOut); // alchemist 3 SLOADs
contracts-full/TransmuterBuffer.sol:551    exchangeDelta = flowAvailable[underlyingToken] - currentExchanged[underlyingToken]; // flowAvailable[underlyingToken] 5 SLOADs, 
contracts-full/TransmuterBuffer.sol:568    IERC20TokenReceiver(amos[underlyingToken]).onERC20Received(underlyingToken, amount); // amos[underlyingToken] 2 SLOADs

contracts-full/TransmuterV2.sol:263    totalUnexchanged: totalUnexchanged, // state var totalUnexchanged 2 SLOADs

contracts-full/gALCX.sol:51    pools.withdraw(poolId, poolBalance); // poolId 2 SLOADs

G08 - Set immutable variables that never change

contracts-full/TransmuterConduit.sol:13    address public token;
contracts-full/TransmuterConduit.sol:16    address public sourceTransmuter;
contracts-full/TransmuterConduit.sol:19    address public sinkTransmuter;

contracts-full/WETHGateway.sol:20    address public whitelist;

G09 - Ordering of logical operators

Insteaf of using:

contracts-full/TransmuterV2.sol:176     if (!hasRole(SENTINEL, msg.sender) && !hasRole(ADMIN, msg.sender)) {

Would be more gas efficient variant:

contracts-full/TransmuterV2.sol:176     if (!(hasRole(SENTINEL, msg.sender) || hasRole(ADMIN, msg.sender))) {