Astaria contest - tnevler's results

Report

Low Risk

[L-1]: Critical changes should use two-step procedure

Context:

1. function setNewGuardian(address _guardian) external { L339

Recommendation:

The best practice is to use two-step procedure for critical changes to make them less error-prone.

Non-Critical Issues

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

Context:

1. return shares; L140
2. return super.withdraw(assets, receiver, owner); L174
3. return super.redeem(shares, receiver, owner); L195
4. return L164
5. return L179
6. return L144
7. return timeToEpochEnd() + EPOCH_LENGTH(); L719
8. return super.mint(vault, to, shares, maxAmountIn); L136
9. return super.deposit(vault, to, amount, minSharesOut); L152
10. return super.withdraw(vault, to, amount, maxSharesOut); L168
11. return super.redeem(vault, to, shares, minAmountOut); L184
12. return vault.redeemFutureEpoch(shares, receiver, msg.sender, epoch); L193
13. _validateCommitment(_loadRouterSlot(), commitment, timeToSecondEpochEnd); L431
14. return vaultAddr; L758
15. return _buyoutLien(s, params); L119
16. return _getBuyout(_loadLienStorageSlot(), stack); L582
17. return _makePayment(_loadLienStorageSlot(), stack, amount); L605
18. return _getMaxPotentialDebtForCollateralUpToNPositions(stack, stack.length); L712
19. return deposit(vault, to, amount, minSharesOut); L31
20. return deposit(vault, to, amount, minSharesOut); L45
21. return redeem(vault, to, amountShares, minAmountOut); L58

Recommendation:

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

[N-2]: Variable is unused

Context:

1. function balanceOf(address account, uint256 id) L82 (account)
2. function balanceOf(address account, uint256 id) L82 (id)
3. function balanceOfBatch(address[] calldata accounts, uint256[] calldata ids) L90 (ids)
4. function isApprovedForAll(address account, address operator) L106 (account and operator)
5. address tokenContract, // collateral token sending the fake nft L115
6. address to, // buyer L116
7. ILienToken.AuctionStack[] storage stack = s.auctionStack.stack; L139
8. bytes calldata data //empty from seaport L174
9. address operator_, L189
10. address from_, L190
11. uint256 tokenId_, L191
12. bytes calldata data_ L192
13. function deposit(uint256 assets, address receiver) L143 (assets and receiver)
14. address tokenContract = underlying.tokenContract; L138
15. uint256 tokenId = underlying.tokenId; L139
16. address caller, L159
17. address offerer, L160
18. address caller, L173
19. bytes32[] calldata priorOrderHashes, L175
20. CriteriaResolver[] calldata criteriaResolvers L176
21. address tokenContract = underlying.tokenContract; L342
22. uint256 assets = totalAssets(); L262
23. function _beforeCommitToLien(IAstariaRouter.Commitment calldata params) L413 (params)
24. function afterDeposit(uint256 assets, uint256 shares) L575 (shares)
25. uint256 end = stack[position].end; L632
26. function _getRemainingInterest(LienStorage storage s, Stack memory stack) L775 (s)

[N-3]: No same value input check

Context:

1. s.newGuardian = _guardian; L342
2. s.delegate = delegate_; L213

Recommendation:

Example how to fix require(_newOwner != owner, " Same address");

[N-4]: Wrong order of functions

Context:

1. function _delegate(address implementation) internal virtual { L29 (internal function can not go after internal pure function)
2. fallback() external payable virtual { L69 (fallback function can not go after internal function)
3. receive() external payable virtual { L77 (receive function can not go after fallback function)
4. function deposit(uint256 amount, address receiver) L59 (public function can not go after public pure function)
5. function setAuctionData(ILienToken.AuctionData calldata auctionData) L66 (external function can not go after internal function)
6. uint256(keccak256("xyz.astaria.WithdrawProxy.storage.location")) - 1; L50 (state variable declaration can not go after event definition)
7. function liquidatorNFTClaim(OrderParameters memory params) external { L109 (external function can not go after public function)
8. modifier releaseCheck(uint256 collateralId) { L253 (modifier definition can not go after internal function)
9. function name() L76 (public view function can not go after public pure function)
10. modifier validVault(address targetVault) { L196 (modifier definition can not go after public function)
11. function file(File calldata incoming) external requiresAuth { L82 (external function can not go after internal function)
12. modifier validateStack(uint256 collateralId, Stack[] memory stack) { L265 (modifier definition can not go after internal functions)
13. function ROUTER() external pure returns (IAstariaRouter) { L26 (external function can not go after public function)
14. function COLLATERAL_TOKEN() public view returns (ICollateralToken) { L62 (public view function can not go after public pure function)
15. function depositToVault( L24 (external function can not go after public function)
16. function deposit(uint256 assets, address receiver) L19 (public function can not go after public view function)
17. uint256(keccak256("xyz.astaria.VaultImplementation.storage.location")) - 1; L58 (state variable declaration can not go after external function)
18. event FileUpdated(FileType what, bytes data); L34 (Struct definition can not go after event definition)

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-5]: NatSpec is missing

Context:

1. TransferProxy.sol
2. Vault.sol
3. ClearingHouse.sol
4. WithdrawProxy.sol (16 functions are missing NatSpec)
5. CollateralToken.sol (21 functions are missing NatSpec)
6. PublicVault.sol (45 functions are missing NatSpec)
7. AstariaRouter.sol (37 functions are missing NatSpec)
8. LienToken.sol (41 functions are missing NatSpec)
9. WithdrawVaultBase.sol
10. AstariaVaultBase.sol
11. VaultImplementation.sol (15 functions are missing NatSpec)
12. ISecurityHook.sol
13. IRouterBase.sol
14. ITransferProxy.sol
15. IStrategyValidator.sol
16. IAstariaVaultBase.sol
17. IERC721.sol
18. IVaultImplementation.sol

[N-6]: Typos

Context:

1. //invalid action private vautls can only be the owner or strategist L90 (Change vautls to vaults)
2. //revert auction params dont match L126 (Change dont to don't)
3. // reset liquidationWithdrawRatio to prepare for re calcualtion L307 (Change calcualtion to calculation)
4. * @dev Allows an on-chain or off-chain user to simulate the effects of their redeemption at the current block, L230 (Change redeemption to redemption)
5. /// @return amount0 The amount of token0 to acheive resulting liquidity L122 (Change acheive to achieve)
6. /// @return amount1 The amount of token1 to acheive resulting liquidity L123 (Change acheive to achieve)

[N-7]: Line is too long

Context:

1. uint88 expected; // The sum of the remaining debt (amountOwed) accrued against the NFT at the timestamp when it is liquidated. yIntercept (virtual assets) of a PublicVault are not modified on liquidation, only once an auction is completed. L54
2. uint256 withdrawReserveReceived; // amount received from PublicVault. The WETH balance of this contract - withdrawReserveReceived = amount received from liquidations. L56
3. s.withdrawReserveReceived; // will never underflow because withdrawReserveReceived is always increased by the transfer amount from the PublicVault L256
4. * @notice This contract handles the creation, payments, buyouts, and liquidations of tokenized NFT-collateralized debt (liens). Vaults which originate loans against supported collateral are issued a LienToken representing the right to loan repayments and auctioned funds on liquidation. L42
5. // Blocking off payments for a lien that has exceeded the lien.end to prevent repayment unless the msg.sender() is the AuctionHouse L804
6. * @param params The Commitment information containing the loan parameters and the merkle proof for the strategy supporting the requested loan. L226
7. * Origination consists of a few phases: pre-commitment validation, lien token issuance, strategist reward, and after commitment actions L281
8. * Starts by depositing collateral and take optimized-out a lien against it. Next, verifies the merkle proof for a loan commitment. Vault owners are then rewarded fees for successful loan origination. L282
9. * @notice Retrieves the recipient of loan repayments. For PublicVaults (VAULT_TYPE 2), this is always the vault address. For PrivateVaults, retrieves the owner() of the vault. L363
10. * @param c The Commitment information containing the loan parameters and the merkle proof for the strategy supporting the requested loan. L376
11. * @notice Called at epoch boundary, computes the ratio between the funds of withdrawing liquidity providers and the balance of the underlying PublicVault so that claim() proportionally pays optimized-out to all parties. L27
12. * @notice Adds an auction scheduled to end in a new epoch to this WithdrawProxy, to ensure that withdrawing LPs get a proportional share of auction returns. L33
13. * @param finalAuctionDelta The timestamp by which the auction being added is guaranteed to end. As new auctions are added to the WithdrawProxy, this value will strictly increase as all auctions have the same maximum duration. L35
14. * Returns the end timestamp of the last auction tracked by this WithdrawProxy. After this timestamp has passed, claim() can be called. L74
15. The base router is a multicall style router inspired by Uniswap v3 with built-in features for permit, WETH9 wrap/unwrap, and ERC20 token pulling/sweeping/approving. L10
16. NOTE the router is capable of pulling any approved token from your wallet. This is only possible when your address is msg.sender, but regardless be careful when interacting with the router or ERC4626 Vaults. L15
17. /// Returns secondsPerLiquidityCumulativeX128 the seconds per in range liquidity for the life of the pool as of the observation timestamp, L105
18. * @return withdrawProxyIfNearBoundary The address of the WithdrawProxy to set the payee to if the liquidation is triggered near an epoch boundary. L146
19. * @notice Executes a FlashAction using locked collateral. A valid FlashAction performs a specified action with the collateral within a single transaction and must end with the collateral being returned to the Vault it was locked in. L100
20. * @param params LienActionEncumber data containing CollateralToken information and lien parameters (rate, duration, and amount, rate, and debt caps). L181
21. * @param params The LienActionBuyout data specifying the lien position, receiver address, and underlying CollateralToken information of the lien. L193
22. * Calculates the debt accrued by all liens against a CollateralToken, assuming no payments are made until the end timestamp in the stack. L263

Description:

Maximum suggested line length is 120 characters.