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.
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.
0xfoobar
commented
2 years ago
[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