Popcorn contest - tnevler's results

Report

Non-Critical Issues

[N-1]: Function defines a named return variable but then it uses return statements

Context:

1. return target.call(callData); L24

Recommendation:

Choose named return variable or return statement. It is unnecessary to use both.

[N-2]: Variable is unused

Context:

1. function _registerVault(address vault, VaultMetadata memory metadata) internal { L390

[N-3]: Wrong order of functions

Context:

1. event Deployment(address indexed clone); L29 (event definition can not go after constructor)
2. mapping(address => Permission) public permissions; L26 (state variable declaration can not go after constructor)
3. mapping(address => bool) public cloneExists; L28 (state variable declaration can not go after constructor)
4. mapping(address => VaultMetadata) public metadata; L28 (state variable declaration can not go after constructor)
5. mapping(bytes32 => mapping(bytes32 => Template)) public templates; L31 (state variable can not go after constructor)
6. function convertToUnderlyingShares(uint256, uint256 shares) L129 (public function can not go after internal function)
7. mapping(bytes32 => Escrow) public escrows; L64 (state variable can not go after external function)
8. function deposit(uint256 _amount) external returns (uint256) { L75 (external function can not go after public function)

Description:

According to official solidity documentation functions should be grouped according to their visibility and ordered:

• constructor

• receive function (if exists)

• fallback function (if exists)

• external

• public

• internal

• private

Within a grouping, place the view and pure functions last.

Recommendation:

Put the functions in the correct order according to the documentation.

[N-4]: Typos

Context:

1. * Is used by `VaultController` to check if a target is a registerd clone. L14 (Change registerd to registered)
2. * @param template Contains the implementation address and necessary informations to clone the implementation. L65 (Change informations to information)
3. /// @notice The amount of assets that are free to be withdrawn from the yVault after locked profts. L100 (Change profts to profits)
4. * @dev This function is the one stop solution to create a new vault with all necessary admin functions or auxiliery contracts. L87 (Change auxiliery to auxiliary)
5. // Dont wait more than X seconds L792 (Change dont to don't)
6. * @notice Sets a new `DeploymentController` and saves its auxilary contracts. Caller must be owner. L829 (Change auxilary to auxiliary)
7. /// @Notice Optional - Metadata CID which can be used by the frontend to add informations to a vault/adapter... L13 (Change informations to information)

[N-5]: Line is too long

Context:

1. ```* AdminProxy is controlled by VaultController. VaultController executes management functions on other contracts through `execute()```` L13
2. * @notice Registers a new vault with Metadata which can be used by a frontend. Caller must be owner. (`VaultController`) L41
3. * @notice Clones an implementation and initializes the clone. Caller must be owner. (`VaultController` via `AdminProxy`) L91
4. * @dev The basic templateCategories will be added via `VaultController` they are ("Vault", "Adapter", "Strategy" and "Staking"). L49
5. * @dev there is no check to ensure that all escrows are owned by the same account. Make sure to account for this either by only sending ids for a specific account or by filtering the Escrows by account later on. L49
6. * Only the owner can add new tokens as rewards. Once added they cant be removed or changed. RewardsSpeed can only be adjusted if the rewardsSpeed is not 0. L22
7. * @notice Changes rewards speed for a rewardToken. This works only for rewards that accrue over time. Caller must be owner. L291
8. // If a deposit/withdraw operation gets called for another user we should accrue for both of them to avoid potential issues like in the Convex-Vulnerability L380
9. * @dev Usually the adapter should already be pre configured. Otherwise a new one can only be added after a ragequit time. L55
10. /// @return Maximum amount of underlying `asset` token that may be deposited for a given address. Delegates to adapter. L398
11. /// @return Maximum amount of underlying `asset` token that can be withdrawn by `caller` address. Delegates to adapter. L408
12. * @dev Management fee is annualized per minute, based on 525,600 minutes per year. Total assets are calculated using L425
13. * the average of their current value and the value at the previous fee harvest checkpoint. This method is similar to L426
14. * @dev This function is the one stop solution to create a new vault with all necessary admin functions or auxiliery contracts. L87
15. * @notice Changes rewards speed for a rewardToken. This works only for rewards that accrue over time. Caller must be creator of the Vault. L477
16. /// @notice Pause Deposits and withdraw all funds from the underlying protocol. Caller must be owner or creator of the Vault. L604
17. /// @notice Unpause Deposits and deposit all funds into the underlying protocol. Caller must be owner or creator of the Vault. L630

Description:

Maximum suggested line length is 120 characters.