Alchemix contest - fatherOfBlocks's results

AlchemicTokenV2.sol

• L201 - It doesn't make sense to ask for the token input parameter, if it can only have one value. This results in a cleaner implementation and removes the need for validation if token!= address(this).

gALCX.sol

• L46 - in the migrateSource signature you can directly pass the IALCXSource interface instead of an address and then wrap it. This would make it so that if it doesn't match the interface, it will revert without executing any line inside the function.

• L62 - There is no benefit to creating a reApprove function, it should be in the constructor. In the event that more allowance is needed, it could be executed directly from the contract.

CrossChainCanonicalBase.sol

• L141 - instead of checking with a for if the bridgeTokensArray has the new address to add. you could directly check if bridgeTokens[bridgeTokenAddress] == true.

AlchemistV2.sol

• L259 - The admin can be set to zero on initialize and this would generate a DoS on setPendingAdmin, acceptAdmin, setSentinel, setKeeper, addUnderlyingToken, addYieldToken, configureRepayLimit, configureLiquidationLimit, setTransmuter, setMinimumCollateralization, setProtocolFee, setProtocolFeeReceiver, configureMintingLimit, configureCreditUnlockRate, setTokenAdapter, setMaximumValue, setMaximumExpected snap and sweepTokens. Since they are functions that have the _onlyAdmin() validation.

• L425 - In the setTransmuter() function it is validated that the new address is not zero, but in the initialize it is not validated, therefore it could have an invalid state at each moment of the redeploy.

• L448 - In the setProtocolFeeReceiver() function it is validated that the new address is not zero, but in the initialize it is not validated, therefore it could have an invalid state at each moment of the redeploy.

TransmuterBuffer.sol

• L341 - in the withdeaw function "amount" is passed as the second parameter and because of how it is used, it would make more sense to pass second recipient and then amount.

• L230 - the variable in storage alchemist should be saved in storage as IStorage, this way it would not be necessary to be in each function wrapping the address alchemist.

YearnTokenAdapter.sol

• L16/17/19 - Since the storage token and underlyingToken variables are immutable in the constructor, several things should be validated. In principle, token is always used with the IYearnVaultV2 interface, in the constructor it is never validated if it meets the interface, therefore it could be generated a DoS in the functions: wrap() unwrap() and price(). On the other hand, it should be validated that underlyingToken != 0, since a transferFrom is performed inside wrap(), if it were the address(0) it could never be executed.

FuseTokenAdapterV1.sol

• L46 - Since they are immutable variables, it should be validated that no address is zero.

WstETHAdapterV1.sol

• L40 - Since they are immutable variables, it should be validated that no address is zero.

RETHAdapterV1.sol

• L29 - Since they are immutable variables, it should be validated that no address is zero.

EthAssetManager.sol

• L200 - As they are immutable variables, it should be validated that no address is zero if it does not match.
• L303 - It is not necessary to validate if pendingAdmin == address(0), since address zero will never be able to calling a function, so validating pendingAdmin == msg.sender is enough.

ThreePoolAssetManager.sol

• L235 - As they are immutable variables, it should be validated that no address is zero if it does not match. Also, if you set the admin to zero, a DoS would be generated in several functions with the modifier onlyAdmin.

• L455 - It is not necessary to validate if pendingAdmin == address(0), since address zero will never be able to calling a function, so validating pendingAdmin == msg.sender is enough.

• L744 - In the _getEarnedConvex() function it is not possible to know what values ​​convexToken.reductionPerCliff() and convexToken.totalCliff() will return therefore, it should be validated if it is different from zero, since otherwise it would revert.

AlchemicTokenV2.sol

• L64/72/80 - modifier functions can save gas if a view function is used instead of a modifier.

gALCX.sol

• L32 - modifier functions can save gas if a view function is used instead of a modifier.

CrossChainCanonicalBase.sol

• L33 - modifier functions can save gas if a view function is used instead of a modifier.

• L57/141 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.

AlchemistV2.sol

• L990/1282/1355/1461/1525 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.

TransmuterBuffer.sol

• L96/106/114/122 - modifier functions can save gas if a view function is used instead of a modifier.

• L151 - create a local variable of flowAvailable[underlyingToken], since it will be executed once in the if and if it does not pass, it will be executed a second time in the else.

• L172/186/235/242/272/382/387/479 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.

TransmuterV2.sol

• L108 - the variable syntheticToken only defines its value in the constructor, so it could be immutable, that way you'd save gas.

• L111 - the variable underlyingToken only defines its value in the constructor, so it could be immutable, that way you'd save gas.

• 167/175/183 - modifier functions can save gas if a view function is used instead of a modifier.

FuseTokenAdapterV1.sol

• L36 - Gas is saved by not initializing the base value.

• L53 - modifier functions can save gas if a view function is used instead of a modifier.

• L81/103 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.

WstETHAdapterV1.sol

• L51 - first it would perform the validations, then it would set the parameters, if it reverts it would be spent less gas.
• L68 - modifier functions can save gas if a view function is used instead of a modifier.

RETHAdapterV1.sol

• L45 - modifier functions can save gas if a view function is used instead of a modifier.

EthAssetManager.sol

• L185/193 - modifier functions can save gas if a view function is used instead of a modifier.

• L214/567 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.

ThreePoolAssetManager.sol

• L220/228 - modifier functions can save gas if a view function is used instead of a modifier.

• L250/254/353 - In the for loop, gas could be saved if instead of i++ an unchecked{++i;} is used in the last line and when uint256 i=0; more is saved if it is not set to its default value.