Alchemix contest - joestakey's results

QA Report

Table of Contents

• summary
• Blacklist should be able to be amended
• Event should be emitted in setters
• Events emitted early
• Related data should be grouped in struct
• Unused imports
• Uint256 alias
• Setters should check the input value

summary

Few vulnerabilities were found examining the contracts. The main concerns are with setters not emitting events

Setters should also check the input value before updating a storage variable.

Blacklist should be able to be amended

PROBLEM

in AlchemicTokenV1.sol, the sentinel can add addresses to a blacklist, preventing them from minting. It is an irreversible process as there is no "unblacklist" function.

SEVERITY

Low

PROOF OF CONCEPT

Example: The sentinel calls setBlacklist, passing the address to blacklist, but they make a mistake in typing the address and blacklist someone else instead. There is no way to remove that address from the blacklist

TOOLS USED

Manual Analysis

MITIGATION

You could add a removeBlacklist function that would perform blacklist[minter] = false, but effectively you can also remove setBlacklist altogether: instead, just call setCeiling with maximum = 0, which does the same thing.

Events emitted early

PROBLEM

It is not recommended to emit events before the end of the computations, as the function might revert based on conditions ahead of the event emission

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

gALCX.sol

gALCX.sol:77: emit ExchangeRateChange(exchangeRate); //emitted before restaking happens line 78.

TOOLS USED

Manual Analysis

MITIGATION

Place the event emission in the last position in the function.

Event should be emitted in setters

PROBLEM

Setters should emit an event so that Dapps can detect important changes to storage. In particular, it is important that access control changes can be tracked off-chain.

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

AlchemicTokenV1.sol:92: setWhiteList()
AlchemicTokenV1.sol:101: setSentinel()
AlchemicTokenV1.sol:110: setBlacklist()
AlchemicTokenV1.sol:131: setCeiling()

AlchemicTokenV2.sol

AlchemicTokenV2.sol:119: setWhiteList()
AlchemicTokenV2.sol:128: setSentinel()
AlchemicTokenV2.sol:164: setMaxFlashLoan()

AlchemicTokenV2Base.sol

AlchemicTokenV2Base.sol:132: setWhiteList()
AlchemicTokenV2Base.sol:141: setSentinel()
AlchemicTokenV2Base.sol:151: setCeiling()
AlchemicTokenV2Base.sol:196: setMaxFlashLoan()

gALCX.sol

gALCX.sol:39: transferOwnership()
gALCX.sol:46: migrateSource()

TransmuterBuffer.sol

TransmuterBuffer.sol:178: setWeights()

TransmuterV2.sol

TransmuterV2.sol:158: initialize() //state variable: buffer
TransmuterV2.sol:196: setCollateralSource()

TOOLS USED

Manual Analysis

MITIGATION

Emit an event in all setters.

Related data should be grouped in struct

PROBLEM

When there are mappings that use the same key value, having separate fields is error prone, for instance in case of deletion or with future new fields.

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

AlchemicTokenV1.sol:37: mapping (address => uint256) public ceiling;
AlchemicTokenV1.sol:40: mapping (address => uint256) public hasMinted;

AlchemicTokenV2Base.sol

AlchemicTokenV2Base.sol:40: mapping (address => uint256) public mintCeiling;
AlchemicTokenV2Base.sol:43: mapping (address => uint256) public hasMinted;

AlchemistV2.sol

AlchemistV2.sol:34: mapping (address => uint256) balances;
AlchemistV2.sol:36: mapping (address => uint256) lastAccruedWeights;

TOOLS USED

Manual Analysis

MITIGATION

Group the related data in a struct and use one mapping. For instance, for the AlchemicTokenV1.sol mappings, the mitigation could be:

struct MintInfo {
  uint256 ceiling;
  uint256 hasMinted;
}

And it would be used as a state variable:

mapping(address => MintInfo) mintInfo;

Unused imports

IMPACT

unused imports should be removed

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

AlchemicTokenV2.sol

AlchemicTokenV2.sol:6: import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

StakingPools.sol

StakingPools.sol:4: import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

TOOLS USED

Manual Analysis

MITIGATION

Remove unused imports

Uint256 alias

IMPACT

uint is an alias for uint256.

It is better to use uint256: it brings readability and consistency in the code, and it future proofs it in case of any changes to the alias of uint

SEVERITY

Non-Critical

PROOF OF CONCEPT

Instances include:

AlchemicTokenV2.sol

AlchemicTokenV2.sol:164: uint _maxFlashLoanAmount

AlchemicTokenV2Base.sol

AlchemicTokenV2Base.sol:196: uint _maxFlashLoanAmount

CrossChainCanonicalBase.sol

CrossChainCanonicalBase.sol:141: uint i
CrossChainCanonicalBase.sol:189: uint bridgeToCanonical
CrossChainCanonicalBase.sol:189: uint canonicalToOld

gALCX.sol

gALCX.sol:14: uint public poolId = 1
gALCX.sol:15: uint public constant exchangeRatePrecision = 1e18
gALCX.sol:16: uint public exchangeRate = exchangeRatePrecision
gALCX.sol:19: uint _exchangeRate
gALCX.sol:20: uint _gAmount
gALCX.sol:20: uint _amount
gALCX.sol:21: uint _gAmount
gALCX.sol:21: uint _amount
gALCX.sol:46: uint _poolId
gALCX.sol:50: uint poolBalance
gALCX.sol:56: uint balance
gALCX.sol:73: uint balance
gALCX.sol:85: uint amount
gALCX.sol:93: uint gAmount
gALCX.sol:100: uint gAmount
gALCX.sol:102: uint amount

TOOLS USED

Manual Analysis

MITIGATION

replace uint with uint256

Setters should check the input value

PROBLEM

Setters should check the input value - ie make revert if it is the zero address or zero

SEVERITY

Low

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

AlchemicTokenV1.sol:92:setWhitelist(address minter)
AlchemicTokenV1.sol:101:setSentinel(address sentinel)
AlchemicTokenV1.sol:110:setBlacklist(address minter)
AlchemicTokenV1.sol:131:setCeiling(address minter)

AlchemicTokenV2.sol

AlchemicTokenV2.sol:119:setWhitelist(address minter)
AlchemicTokenV2.sol:128:setSentinel(address sentinel)

AlchemicTokenV2Base.sol

AlchemicTokenV2Base.sol:132:setWhitelist(address minter)
AlchemicTokenV2Base.sol:141:setSentinel(address sentinel)
AlchemicTokenV2Base.sol:151:setCeiling(address minter)

AlchemistV2.sol

AlchemistV2.sol:281:setPendingAdmin(address value)
AlchemistV2.sol:303:setSentinel(address sentinel)
AlchemistV2.sol:310:setKeeper(address keeper)

EthAssetManager.sol

EthAssetManager.sol:293:setPendingAdmin(address value)
EthAssetManager.sol:324:setOperator(address value)
EthAssetManager.sol:332:setRewardReceiver(address value)
EthAssetManager.sol:340:setTransmuterBuffer(address value)

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:445:setPendingAdmin(address value)
ThreePoolAssetManager.sol:476:setOperator(address value)
ThreePoolAssetManager.sol:484:setRewardReceiver(address value)
ThreePoolAssetManager.sol:492:setTransmuterBuffer(address value)

TransmuterBuffer.sol

TransmuterBuffer.sol:221:setTransmuter(address newTransmuter)
TransmuterBuffer.sol:251:setAmo(address amo)

TransmuterConduit.sol

TransmuterConduit.sol:21:constructor(address _token, address _source, address _sink)

TransmuterV2.sol

TransmuterV2.sol:21:initialize(
    address _syntheticToken,
    address _underlyingToken,
    address _buffer,
    address _whitelist
  )
TransmuterV2.sol:196:setCollateralSource

TOOLS USED

Manual Analysis

MITIGATION

Add zero address checks

Gas Report

Table of Contents

• Caching storage variables in memory to save gas
• Calldata instead of memory for RO function parameters
• Comparison operators
• Constructor parameters should be avoided when possible
• Custom errors
• Default value initialization
• Event emitting of local variable
• Immutable variables should be used when possible
• Prefix increments
• Shifting cheaper than division
• unchecked arithmetic

Caching storage variables in memory to save gas

IMPACT

Anytime you are reading from storage more than once, it is cheaper in gas cost to cache the variable in memory: a SLOAD cost 100gas, while MLOAD and MSTORE cost 3 gas.

In particular, in for loops, when using the length of a storage array as the condition being checked after each loop, caching the array length in memory can yield significant gas savings if the array length is high

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

scope: mint()

• hasMinted[msg.sender] is read twice:

AlchemicTokenV1.sol:80
AlchemicTokenV1.sol:82

AlchemistV2.sol

scope: acceptAdmin()

• pendingAdmin is read 3 times:

AlchemistV2.sol:289
AlchemistV2.sol:291
AlchemistV2.sol:295

EthAssetManager.sol

scope: constructor()

• weth is read NUM_META_COINS times:

EthAssetManager.sol:217

scope: acceptAdmin()

• pendingAdmin is read 3 times:

EthAssetManager.sol:304
EthAssetManager.sol:308
EthAssetManager.sol:312

scope: claimRewards()

• rewardReceiver is read twice:

EthAssetManager.sol:428
EthAssetManager.sol:429

scope: reclaimEth()

• weth is read 4 times:

EthAssetManager.sol:496
EthAssetManager.sol:496
EthAssetManager.sol:498
EthAssetManager.sol:500

• transmuterBuffer is read twice:

EthAssetManager.sol:498
EthAssetManager.sol:500

scope: _claimRewards()

• rewardReceiver is read twice:

EthAssetManager.sol:698
EthAssetManager.sol:699

gALCX.sol

scope: migrateSource()

• pools is read twice:

gALCX.sol:50
gALCX.sol:51

• poolId is read 3 times:

gALCX.sol:50
gALCX.sol:51
gALCX.sol:58 //this last variable can be replaced with the _poolId argument.

scope: bumpExchangeRate()

• pools is read twice:

gALCX.sol:71
gALCX.sol:79

• poolId is read twice:

gALCX.sol:71
gALCX.sol:79

StakingPools.sol

scope: acceptGovernance()

• pendingGovernance is read twice:

StakingPools.sol:131
StakingPools.sol:133

ThreePoolAssetManager.sol

scope: acceptAdmin()

• pendingAdmin is read 3 times:

ThreePoolAssetManager.sol:456
ThreePoolAssetManager.sol:460
ThreePoolAssetManager.sol:464

scope: claimRewards()

• rewardReceiver is read twice:

ThreePoolAssetManager.sol:632
ThreePoolAssetManager.sol:633

scope: reclaimThreePoolAsset()

• transmuterBuffer is read twice:

ThreePoolAssetManager.sol:719
ThreePoolAssetManager.sol:721

scope: _claimRewards()

• rewardReceiver is read twice:

ThreePoolAssetManager.sol:1014
ThreePoolAssetManager.sol:1015

TransmuterBuffer.sol

scope: getAvailableFlow()

• flowAvailable[underlyingToken] is read twice:

TransmuterBuffer.sol:151
TransmuterBuffer.sol:154

scope: getTotalUnderlyingBuffered()

• _yieldTokens[underlyingToken] is read (_yieldTokens[underlyingToken].length) times: number of reads depending on _yieldTokens[underlyingToken] as it is in a for loop

TransmuterBuffer.sol:172

scope: setAlchemist()

• alchemist is read (6 + registeredUnderlyings.length) times: number of reads depending on registeredUnderlyings as it is in a for loop

TransmuterBuffer.sol:231
TransmuterBuffer.sol:234
TransmuterBuffer.sol:236
TransmuterBuffer.sol:238
TransmuterBuffer.sol:243 //from this point, alchemist can be replaced by the function argument _alchemist
TransmuterBuffer.sol:245
TransmuterBuffer.sol:247

• registeredUnderlyings.length is read (registeredUnderlyings.length) times: number of reads depending on registeredUnderlyings as it is in a for loop

TransmuterBuffer.sol:242

• debtToken is read twice:

TransmuterBuffer.sol:238
TransmuterBuffer.sol:245

scope: refreshStrategies()

• alchemist is read (2 + supportedYieldTokens.length) times: number of reads depending on supportedYieldTokens.length as it is in a for loop

TransmuterBuffer.sol:372
TransmuterBuffer.sol:374
TransmuterBuffer.sol:390

• registeredUnderlyings.length is read (1 + registeredUnderlyings.length) times: number of reads depending on registeredUnderlyings as it is in a for loop

TransmuterBuffer.sol:377
TransmuterBuffer.sol:382

scope: burnCredit()

• alchemist is read twice:

TransmuterBuffer.sol:401
TransmuterBuffer.sol:406

scope: _getTotalBuffered()

• alchemist is read 3 times:

TransmuterBuffer.sol:443
TransmuterBuffer.sol:444
TransmuterBuffer.sol:446

scope: _alchemistWithdraw()

• alchemist is read 3 times:

TransmuterBuffer.sol:513
TransmuterBuffer.sol:515
TransmuterBuffer.sol:522

scope: _exchange()

• flowAvailable[underlyingToken] is read 3 times if totalUnderlyingBuffered >= flowAvailable[underlyingToken], 4 times if initialLocalBalance < flowAvailable[underlyingToken], 5 times if localBalance > flowAvailable[underlyingToken]:

TransmuterBuffer.sol:536
TransmuterBuffer.sol:539
TransmuterBuffer.sol:541
TransmuterBuffer.sol:550
TransmuterBuffer.sol:551

• currentExchanged[underlyingToken] is read twice (3 instances but only read once in the first two depending on localBalance > flowAvailable[underlyingToken]):

TransmuterBuffer.sol:551
TransmuterBuffer.sol:553
TransmuterBuffer.sol:557

scope: _flushToAmo()

• amos[underlyingToken] is read twice:

TransmuterBuffer.sol:567
TransmuterBuffer.sol:568

TransmuterConduit.sol

scope: distribute()

• token is read twice:

TransmuterConduit.sol:35
TransmuterConduit.sol:36

• sinkTransmuter is read twice:

TransmuterConduit.sol:35
TransmuterConduit.sol:36

TransmuterV2.sol

scope: exchange()

• totalUnexchanged is read twice:

TransmuterV2.sol:253
TransmuterV2.sol:263

• examineTickData.totalBalance is read twice:

TransmuterV2.sol:315
TransmuterV2.sol:318

TOOLS USED

Manual Analysis

MITIGATION

cache these storage variables in memory

Calldata instead of memory for RO function parameters

PROBLEM

If a reference type function parameter is read-only, it is cheaper in gas to use calldata instead of memory. Calldata is a non-modifiable, non-persistent area where function arguments are stored, and behaves mostly like memory.

Try to use calldata as a data location because it will avoid copies and also makes sure that the data cannot be modified.

PROOF OF CONCEPT

Instances include:

AlchemistV2.sol

scope: initialize()

AlchemistV2.sol:255: InitializationParams memory params

CrossChainCanonicalAlchemicTokenV2.sol

scope: initialize()

CrossChainCanonicalAlchemicTokenV2.sol:9: string memory name
CrossChainCanonicalAlchemicTokenV2.sol:10: string memory symbol
CrossChainCanonicalAlchemicTokenV2.sol:11: address[] memory _bridgeTokens

CrossChainCanonicalBase.sol

scope: __CrossChainCanonicalBase_init()

CrossChainCanonicalBase.sol:43: string memory _name
CrossChainCanonicalBase.sol:44: string memory _symbol
CrossChainCanonicalBase.sol:45: address[] memory _bridgeTokens

CrossChainCanonicalGALCX.sol

scope: initialize()

CrossChainCanonicalGALCX.sol:8: string memory name
CrossChainCanonicalGALCX.sol:9: string memory symbol
CrossChainCanonicalGALCX.sol:10: address[] memory _bridgeTokens

TransmuterBuffer.sol

scope: setWeights()

TransmuterBuffer.sol:180: address[] memory tokens
TransmuterBuffer.sol:181: uint256[] memory weights

TransmuterV2.sol

scope: _updateAccount()

TransmuterV2.sol:381: UpdateAccountParams memory params

TOOLS USED

Manual Analysis

MITIGATION

Replace memory with calldata

Comparison Operators

IMPACT

In the EVM, there is no opcode for >= or <=. When using greater than or equal, two operations are performed: > and =.

Using strict comparison operators hence saves gas

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

AlchemicTokenV1.sol:81

AlchemistV2.sol

AlchemistV2.sol:256
AlchemistV2.sol:324
AlchemistV2.sol:352
AlchemistV2.sol:442
AlchemistV2.sol:494
AlchemistV2.sol:1010
AlchemistV2.sol:1441
AlchemistV2.sol:1565

EthAssetManager.sol

EthAssetManager.sol:281
EthAssetManager.sol:547

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:420
ThreePoolAssetManager.sol:433
ThreePoolAssetManager.sol:749

TransmuterBuffer.sol

TransmuterBuffer.sol:161

TransmuterV2.sol

TransmuterV2.sol:353
TransmuterV2.sol:409
TransmuterV2.sol:557

TOOLS USED

Manual Analysis

MITIGATION

Replace <= with <, and >= with >. Do not forget to increment/decrement the compared variable

example:

-total <= ceiling[msg.sender];
+total <= ceiling[msg.sender] + 1;

However, if 1 is negligible compared to the value of the variable, we can omit the increment.

Constructor parameters should be avoided when possible

IMPACT

Constructor parameters are expensive. The contract deployment will be cheaper in gas if they are hard coded instead of using constructor parameters.

PROOF OF CONCEPT

Instances include:

AlchemicTokenV2.sol

AlchemicTokenV2.sol:60 flashMintFee = _flashFee

EthAssetManager.sol

EthAssetManager.sol:201: admin            = params.admin;
EthAssetManager.sol:202: operator         = params.operator;
EthAssetManager.sol:203: rewardReceiver   = params.rewardReceiver;
EthAssetManager.sol:204: transmuterBuffer = params.transmuterBuffer;
EthAssetManager.sol:205: weth             = params.weth;
EthAssetManager.sol:208: metaPoolSlippage = params.metaPoolSlippage

StakingPools.sol

StakingPools.sol:108 reward = _reward
StakingPools.sol:109 governance = _governance

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:236: admin            = params.admin;
ThreePoolAssetManager.sol:237: operator         = params.operator;
ThreePoolAssetManager.sol:238: rewardReceiver   = params.rewardReceiver;
ThreePoolAssetManager.sol:239: transmuterBuffer = params.transmuterBuffer;
ThreePoolAssetManager.sol:243: threePoolSlippage = params.threePoolSlippage;
ThreePoolAssetManager.sol:244: metaPoolSlippage = params.metaPoolSlippage

TransmuterConduit.sol

TransmuterConduit.sol:22 token = _token
TransmuterConduit.sol:23 sourceTransmuter = _source
TransmuterConduit.sol:24 sinkTransmuter = _sink

WETHGateway.sol

WETHGateway.sol:24 whitelist = _whitelist

TOOLS USED

Manual Analysis

MITIGATION

Hardcode the state variable with their initial values instead of writing them during contract deployment with constructor parameters.

Custom Errors

IMPACT

Custom errors from Solidity 0.8.4 are cheaper than revert strings (cheaper deployment cost and runtime cost when the revert condition is met) while providing the same amount of information, as explained here

Custom errors are defined using the error statement

PROOF OF CONCEPT

Instances include:

AlchemicTokenV1.sol

AlchemicTokenV1.sol:77: require(!blacklist[msg.sender], "AlUSD: Alchemist is blacklisted.");
AlchemicTokenV1.sol:78: require(!paused[msg.sender], "AlUSD: Currently paused.");
AlchemicTokenV1.sol:81: require(total <= ceiling[msg.sender], "AlUSD: Alchemist's ceiling was breached.");

gALCX.sol

gALCX.sol:90: require(success, "Transfer failed");
gALCX.sol:107: require(success, "Transfer failed");

StakingPools.sol

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

TOOLS USED

Manual Analysis

MITIGATION

Replace require and revert statements with custom errors.

For instance, in StakingPools.sol:

Replace

require(msg.sender == pendingGovernance, "StakingPools: only pending governance");

with

if (msg.sender != pendingGovernance) {
		revert IsNotPendingGovernance(msg.sender);
}

and define the custom error in the contract

error IsNotPendingGovernance(address _address);

Default value initialization

IMPACT

If a variable is not set/initialized, it is assumed to have the default value (0, false, 0x0 etc depending on the data type). Explicitly initializing it with its default value is an anti-pattern and wastes gas.

PROOF OF CONCEPT

Instances include:

AlchemistV2.sol

AlchemistV2.sol:990: uint256 i = 0;
AlchemistV2.sol:1282: uint256 i = 0;
AlchemistV2.sol:1355: uint256 i = 0;
AlchemistV2.sol:1458: uint256 totalValue = 0;
AlchemistV2.sol:1461: uint256 i = 0;
AlchemistV2.sol:1524: uint256 i = 0;

CrossChainCanonicalBase.sol

CrossChainCanonicalBase.sol:57: uint256 i = 0;
CrossChainCanonicalBase.sol:141: uint256 i = 0;

EthAssetManager.sol

EthAssetManager.sol:214: uint256 i = 0;
EthAssetManager.sol:566: uint256 total = 0;
EthAssetManager.sol:567: uint256 i = 0;

StakingPools.sol

StakingPools.sol:188: uint256 _poolId = 0;
StakingPools.sol:363: uint256 _poolId = 0;

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:250: uint256 i = 0;
ThreePoolAssetManager.sol:254: uint256 i = 0;
ThreePoolAssetManager.sol:353: uint256 i = 0;
ThreePoolAssetManager.sol:771: uint256 normalizedTotal = 0;
ThreePoolAssetManager.sol:773: uint256 i = 0;
ThreePoolAssetManager.sol:901: uint256 total = 0;
ThreePoolAssetManager.sol:902: uint256 i = 0;

TransmuterBuffer.sol

TransmuterBuffer.sol:171: uint256 i= 0;
TransmuterBuffer.sol:186: uint256 i= 0;
TransmuterBuffer.sol:235: uint256 i= 0;
TransmuterBuffer.sol:242: uint256 i= 0;
TransmuterBuffer.sol:272: uint256 i= 0;
TransmuterBuffer.sol:382: uint256 j= 0;
TransmuterBuffer.sol:387: uint256 i= 0;
TransmuterBuffer.sol:479: uint256 j= 0;
TransmuterBuffer.sol:534: uint256 want = 0;
TransmuterBuffer.sol:549: uint256 exchangeDelta = 0;

TransmuterV2.sol

TransmuterV2.sol:162: isPaused = false; 

TOOLS USED

Manual Analysis

MITIGATION

Remove explicit initialization for default values.

Event emitting of local variable

PROBLEM

When emitting an event, using a local variable instead of a storage variable saves gas.

PROOF OF CONCEPT

Instances include:

AlchemicTokenV2.sol

AlchemicTokenV2.sol:94: emit SetFlashMintFee(flashMintFee);

AlchemicTokenV2Base.sol

AlchemicTokenV2Base.sol:100: emit SetFlashMintFee(flashMintFee);

AlchemistV2.sol

AlchemistV2.sol:272: emit AdminUpdated(admin);
AlchemistV2.sol:273: emit TransmuterUpdated(transmuter);
AlchemistV2.sol:274: emit MinimumCollateralizationUpdated(minimumCollateralization);
AlchemistV2.sol:275: emit ProtocolFeeUpdated(protocolFee);
AlchemistV2.sol:276: emit ProtocolFeeReceiverUpdated(protocolFeeReceiver);
AlchemistV2.sol:298: emit AdminUpdated(admin);

EthAssetManager.sol

EthAssetManager.sol:221: emit AdminUpdated(admin);
EthAssetManager.sol:222: emit OperatorUpdated(operator);
EthAssetManager.sol:223: emit RewardReceiverUpdated(rewardReceiver);
EthAssetManager.sol:224: emit TransmuterBufferUpdated(transmuterBuffer);
EthAssetManager.sol:225: emit MetaPoolSlippageUpdated(metaPoolSlippage);
EthAssetManager.sol:315: emit AdminUpdated(admin);

gALCX.sol

gALCX.sol:77: emit ExchangeRateChange(exchangeRate);

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:258: emit AdminUpdated(admin);
ThreePoolAssetManager.sol:259: emit OperatorUpdated(operator);
ThreePoolAssetManager.sol:260: emit RewardReceiverUpdated(rewardReceiver);
ThreePoolAssetManager.sol:261: emit TransmuterBufferUpdated(transmuterBuffer);
ThreePoolAssetManager.sol:262: emit ThreePoolSlippageUpdated(threePoolSlippage);
ThreePoolAssetManager.sol:263: emit MetaPoolSlippageUpdated(metaPoolSlippage);
ThreePoolAssetManager.sol:467: emit AdminUpdated(admin);

TransmuterBuffer.sol

TransmuterBuffer.sol:247: emit SetAlchemist(alchemist);

TransmuterV2.sol

TransmuterV2.sol:203: emit Paused(isPaused);

TOOLS USED

Manual Analysis

MITIGATION

Emit the function argument instead of the state variable where possible. In some instances, the storage variable is read multiple times in the function and it is recommended to cache them into memory, then passing these cached variables in the emit statement, as explained in the cache paragraph

Immutable variables should be used when possible

IMPACT

If a storage variable is never changed, it's possible to avoid storage access and save gas by making it immutable.

PROOF OF CONCEPT

Instances include:

EthAssetManager.sol

EthAssetManager.sol:157: IWETH9 public weth
EthAssetManager.sol:182: IERC20[NUM_META_COINS] private _metaPoolAssetCache

gALCX.sol

gALCX.sol:188: IERC20 public alcx
gALCX.sol:188: uint public exchangeRate
gALCX.sol:363: address public owner

StakingPools.sol

StakingPools.sol:188: IERC20Mintable public reward;

TransmuterConduit.sol

TransmuterConduit.sol:13: address public token;
TransmuterConduit.sol:16: address public sourceTransmuter;
TransmuterConduit.sol:19: address public sinkTransmuter;

TransmuterV2.sol

TransmuterV2.sol:108: address public syntheticToken;

TOOLS USED

Manual Analysis

MITIGATION

add the immutable keyword to these variables.

Prefix increments

IMPACT

Prefix increments are cheaper than postfix increments.

PROOF OF CONCEPT

Instances include:

AlchemistV2.sol

AlchemistV2.sol:990: i++
AlchemistV2.sol:1282: i++
AlchemistV2.sol:1355: i++
AlchemistV2.sol:1461: i++
AlchemistV2.sol:1524: i++

CrossChainCanonicalBase.sol

CrossChainCanonicalBase.sol:57: i++
CrossChainCanonicalBase.sol:141: i++

EthAssetManager.sol

EthAssetManager.sol:214: i++
EthAssetManager.sol:567: i++

StakingPools.sol

StakingPools.sol:188: _poolId + 1;
StakingPools.sol:188: _poolId++
StakingPools.sol:363: _poolId++

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:250: i++
ThreePoolAssetManager.sol:254: i++
ThreePoolAssetManager.sol:353: i++
ThreePoolAssetManager.sol:773: i++
ThreePoolAssetManager.sol:902: i++

TransmuterBuffer.sol

TransmuterBuffer.sol:172: i++
TransmuterBuffer.sol:186: i++
TransmuterBuffer.sol:235: i++
TransmuterBuffer.sol:242: i++
TransmuterBuffer.sol:272: i++
TransmuterBuffer.sol:382: j++
TransmuterBuffer.sol:387: i++
TransmuterBuffer.sol:479: j++

TOOLS USED

Manual Analysis

MITIGATION

change variable + 1 and variable++ to ++variable.

Shifting cheaper than division

IMPACT

A division by 2 can be calculated by shifting one to the right. While the DIV opcode uses 5 gas, the SHR opcode only uses 3 gas. Furthermore, Solidity's division operation also includes a division-by-0 prevention which is bypassed using shifting.

PROOF OF CONCEPT

Instances include:

ThreePoolAssetManager.sol

ThreePoolAssetManager:355: (v.maximum + v.minimum) / 2)

TOOLS USED

Manual Analysis

MITIGATION

-(v.maximum + v.minimum) / 2);
+(v.maximum + v.minimum) >> 1);

Unchecked arithmetic

IMPACT

The default "checked" behavior costs more gas when adding/diving/multiplying, because under-the-hood those checks are implemented as a series of opcodes that, prior to performing the actual arithmetic, check for under/overflow and revert if it is detected.

if it can statically be determined there is no possible way for your arithmetic to under/overflow (such as a condition in an if statement), surrounding the arithmetic in an unchecked block will save gas

PROOF OF CONCEPT

Instances include:

AlchemistV2.sol

AlchemistV2.sol:933: feeAmount bounded by amountUnderlyingTokens (see line 256, protocolFee < BPS), underflow check unnecessary
AlchemistV2.sol:1014: expectedValue bounded by currentValue (see line 1010), underflow check unnecessary
AlchemistV2.sol:1569: expectedValue bounded by currentValue (see line 1565), underflow check unnecessary

EthAssetManager.sol

EthAssetManager.sol:496: balance bounded by amount, underflow check unnecessary
EthAssetManager.sol:552: cliff bounded by totalCliffs (see line 547), underflow check unnecessary
EthAssetManager.sol:589: address(this).balance bounded by value, underflow check unnecessary
EthAssetManager.sol:629: address(this).balance bounded by value, underflow check unnecessary

ThreePoolAssetManager.sol

ThreePoolAssetManager.sol:399: because of the condition check, underflow checks are unnecessary
ThreePoolAssetManager.sol:751: cliff bounded by totalCliffs (see line 749), underflow check unnecessary

TOOLS USED

Manual Analysis

MITIGATION

Place the arithmetic operations in an unchecked block