Alchemix contest - 0xDjango's results

[L-01] Front-runnable initializers

Several initialize() functions lack access control and can be front-run.

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemistV2.sol#L255

[L-02] No ownership transfer pattern

The gALCX.transferOwnership() function lacks a proper ownership transfer pattern. It is recommended to make this a two-step process to ensure that the new owner is truly the desired address. The first function called will set the pending owner and a second function must be called by the pending owner to accept the ownership transfer.

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/gALCX.sol#L39-L41

[L-03] Floating pragma

Contracts contain a floating pragma. It is recommended to deploy all contracts with a single, specific compiler version to reduce the risk of compiler-specific bugs and contracts deployed with different versions.

E.g. https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemicTokenV2.sol#L2

[L-04] pendingGovernance should be reset to address(0)

Accepting the governance change does not reset pendingGovernance back to address(0).

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/StakingPools.sol#L130-L137

[L-05] Reverse Check-Effects-Interactions patter for deposits

It is recommended to follow a reverse CEI pattern for deposits. In the following example, the balances are updated prior to token transfer.

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/StakingPools.sol#L379-L382

[N-01] Consistency in use of UINT256 vs UINT

All references are UINT256 except this one:

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemicTokenV2.sol#L164

[N-02] Consistency with line break formatting

This function contains white space between every function call which the previous function contains the same calls without line breaks.

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemistV2.sol#L639-L643

[N-03] Consistency with _checkArgument

For all references to _checkArgument expect one, the function checks that value > 0. In this example, it checks that the UINT != 0.

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemistV2.sol#L889

[G-01] For loop optimization

The following for loops can be optimized by incrementing i within an unchecked block. The new pattern would be:

for (uint i = 0; i < length;) {
    doStuff();
    unchecked {
        ++i;
    }
}

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemistV2.sol#L990-L992

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/AlchemistV2.sol#L1282-L1284

https://github.com/code-423n4/2022-05-alchemix/blob/de65c34c7b6e4e94662bf508e214dcbf327984f4/contracts-full/EthAssetManager.sol#L214-L219