Alchemix contest - Funen's results

1. Floating pragma

The contracts was used floating pragma ^0.8.11. It can be consider using locking the pragma version whenever possible and avoid using a floating pragma in the final deployment. Since it can be problematic, if there are publicly disclosed bugs and issues that affect the current compiler version used.

Tool Used

Manual Review

Recommendation

Use these compiler versions are to compile your code: 0.8.11, 0.8.12, 0.8.13

2. Title : Initialize UUPS Implementation Contracts

Since AlchemistV2 , TransmuterBuffer & TransmuterV2 was used constructor() initializer {}. Implementation below will tell the upgrade plugins that you know what you're doing, and let you keep the constructor. The latest version generates upgradeable contracts using this pattern and it will not receive the following error later.

Tool Used

Manual Review

##POC https://docs.openzeppelin.com/upgrades-plugins/1.x/writing-upgradeable https://forum.openzeppelin.com/t/constructor-initializer-inclusion/17412

Recommendation

It's necessary to also include the comment above that says

/// @custom:oz-upgrades-unsafe-allow constructor

3. Title : Comment was not same as actual code

@return was not used in actual code, since it just using emit. comment can be changed or can be deleted instead/

##Tool Used Manual Review

1. Title : Value can be set as constant

Since this can be set as constant, cause of values assigned at compile time and will not allow for any modifications at runtime.

Tool Used

Manual Review, VSC

2. using ++i than i++ for saving more gas

Using i++ instead ++i for all the loops, the variable i is incremented using i++. It is known that implementation by using ++i costs less gas per iteration than i++.

Tools Used

Manual Review

Occurances

AlchemistV2.sol#L990 AlchemistV2.sol#L1282 AlchemistV2.sol#L1355 AlchemistV2.sol#L1461 AlchemistV2.sol#L1524 CrossChainCanonicalBase.sol#L57 CrossChainCanonicalBase.sol#L141 TransmuterBuffer.sol#L172 TransmuterBuffer.sol#L186 TransmuterBuffer.sol#L235 TransmuterBuffer.sol#L242 TransmuterBuffer.sol#L272 TransmuterBuffer.sol#L387 ThreePoolAssetManager.sol#L250 ThreePoolAssetManager.sol#L254 ThreePoolAssetManager.sol#L353 ThreePoolAssetManager.sol#L773 ThreePoolAssetManager.sol#L902 Multicall.sol#L14

3. Title : change uint256 i = 0 into uint256 i for saving more gas

using this implementation can saving more gas for each loops.

Tool Used

Manual Review

##Recommended Mitigation Change it

##Occurances

AlchemistV2.sol#L990 AlchemistV2.sol#L1282 AlchemistV2.sol#L1355 AlchemistV2.sol#L1461 AlchemistV2.sol#L1524 CrossChainCanonicalBase.sol#L57 CrossChainCanonicalBase.sol#L141 TransmuterBuffer.sol#L172 TransmuterBuffer.sol#L186 TransmuterBuffer.sol#L235 TransmuterBuffer.sol#L242 TransmuterBuffer.sol#L272 TransmuterBuffer.sol#L387 ThreePoolAssetManager.sol#L250 ThreePoolAssetManager.sol#L254 ThreePoolAssetManager.sol#L353 ThreePoolAssetManager.sol#L773 ThreePoolAssetManager.sol#L902 Multicall.sol#L14

4. Caching array length can saving more gas

This implementation can be saving more gas, since if caching the array length is more gas efficient. just because access to a local variable in solidity is more efficient.

##Tool Used Manual Review

##Occurances

AlchemistV2.sol#L990 AlchemistV2.sol#L1282 AlchemistV2.sol#L1355 AlchemistV2.sol#L1461 AlchemistV2.sol#L1524 CrossChainCanonicalBase.sol#L57 CrossChainCanonicalBase.sol#L141 TransmuterBuffer.sol#L172 TransmuterBuffer.sol#L186 TransmuterBuffer.sol#L235 TransmuterBuffer.sol#L242 TransmuterBuffer.sol#L272 TransmuterBuffer.sol#L387 ThreePoolAssetManager.sol#L250 ThreePoolAssetManager.sol#L254 ThreePoolAssetManager.sol#L353 ThreePoolAssetManager.sol#L773 ThreePoolAssetManager.sol#L902 Multicall.sol#L14

5. Title : Caching unlockedCredit in memory can save gas

unlockedCredit is called multiple times (2 times to be exact) in the _wrap() function. i recommend to cache unlockedCredit

Tool Used

Manual Review

6. Title : Using ++self.position instead of self.position++ for cost less gas

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/libraries/Tick.sol#L38

Using prefix was common for saving more gas, since this implementation can be saving more gas

Tool Used

Manual Review

7. Title : Saving gas by removing = 0

This code can be saving more gas by removing = 0, it because If a variable was not set/initialized, it is assumed to have default value to 0

Tool Used

Manual Review

Mitigation Step

Remove = 0

8. Title : Caching it in memory can be saving more gas

@Param _self can be caching in memory, since function getUpdatedTotalUnclaimed() was used to be internal view and you can saving gas by caching memory cause _self was read-only

Tool Used

Manual Review