Platform: Code4rena
Start Date: 05/01/2023
Pot Size: $90,500 USDC
Total HM: 55
Participants: 103
Period: 14 days
Judge: Picodes
Total Solo HM: 18
Id: 202
League: ETH
Rank: 74/103
Findings: 1
Award: $51.32
🌟 Selected for report: 0
🚀 Solo Findings: 0
🌟 Selected for report: ladboy233
Also found by: 0x1f8b, 0xAgro, 0xSmartContract, 0xbepresent, 0xkato, Aymen0909, CodingNameKiki, Cryptor, Deekshith99, Deivitto, HE1M, IllIllI, Kaysoft, Koolex, PaludoX0, Qeew, RaymondFam, Rolezn, Sathish9098, Tointer, a12jmx, arialblack14, ast3ros, ayeslick, bin2chen, btk, caventa, ch0bu, chaduke, chrisdior4, delfin454000, descharre, evan, fatherOfBlocks, georgits, gz627, jasonxiale, joestakey, kaden, lukris02, nicobevi, nogo, oberon, oyc_109, pfapostol, rbserver, sakshamguruji, seeu, shark, simon135, slvDev, synackrst, tnevler, whilom, zaskoh
51.3151 USDC - $51.32
During the audit, 1 low and 12 non-critical issues were found.
| â„– | Title | Risk Rating | Instance Count |
|---|---|---|---|
| L-1 | Use the two-step-transfer of Guardian | Low | 1 |
| NC-1 | Order of Functions | Non-Critical | 22 |
| NC-2 | Order of Layout | Non-Critical | 9 |
| NC-3 | Unused variable | Non-Critical | 22 |
| NC-4 | Typos | Non-Critical | 5 |
| NC-5 | Function is not implemented | Non-Critical | 1 |
| NC-6 | Missing leading underscores | Non-Critical | 15 |
| NC-7 | Empty function bodies | Non-Critical | 2 |
| NC-8 | Missing NatSpec | Non-Critical | 10 |
| NC-9 | Visibility is not marked | Non-Critical | 4 |
| NC-10 | Unused named return variables | Non-Critical | 21 |
| NC-11 | Maximum line length exceeded | Non-Critical | 22 |
| NC-12 | Inconsistency when using the number 1000 | Non-Critical | 1 |
If the guardian accidentally transfers ownership to an incorrect address, protected functions may become permanently inaccessible.
Consider using a two-step-transfer of ownership: the current owner would nominate a new owner, and to become the new owner, the nominated account would have to approve the change, so that the address is proven to be valid.
According to Style Guide, ordering helps readers identify which functions they can call and to find the constructor and fallback definitions easier.
Functions should be grouped according to their visibility and ordered:
fallback() should be placed right after constructor and receive() function:
receive() should be placed right after constructor:
external functions should not go after internal functions:
public functions should not go after internal functions:
external functions should not go after public functions:
internal functions should not go between public functions:
public function should not go between external functions:
Reorder functions where possible.
According to Order of Layout, inside each contract, library or interface, use the following order:
state variable should not go after events:
state variable should not go between functions:
event should be placed after all structs:
modifiers should be placed between state variables/events and functions:
Some variables are not used in the functions code.
Delete unused variables or use them.
//invalid action private vautls can only be the owner or strategist => vaults// reset liquidationWithdrawRatio to prepare for re calcualtion => calculation* @dev Allows an on-chain or off-chain user to simulate the effects of their redeemption at the current block, => redemption/// @return amount0 The amount of token0 to acheive resulting liquidity => achieve/// @return amount1 The amount of token1 to acheive resulting liquidity => achieveFunction balanceOf always returns incorrect result.
It is a good practice when private constants have a leading underscore.
uint256 private constant CLEARING_HOUSE_STORAGE_SLOT =uint256 private constant WITHDRAW_PROXY_SLOT =uint256 private constant COLLATERAL_TOKEN_SLOT =uint256 private constant PUBLIC_VAULT_SLOT =uint256 private constant ROUTER_SLOT =uint256 private constant OUTOFBOUND_ERROR_SELECTOR =uint256 private constant ONE_WORD = 0x20;uint256 private constant LIEN_SLOT =bytes32 constant ACTIVE_AUCTION = bytes32("ACTIVE_AUCTION");uint256 constant ERC20_SLOT =bytes32 private constant PERMIT_TYPEHASH =uint256 private constant ERC721_SLOT =bytes32 constant EIP_DOMAIN =bytes32 constant VERSION = keccak256("0");uint256 private constant VI_SLOT =Consider adding leading underscores.
Some functions do not have implementation.
Consider marking these functions as not implemented or removing them.
NatSpec is missing in 10 contracts.
Add NatSpec for all functions.
It is a good practice to explicitly mark visibility.
bytes32 constant ACTIVE_AUCTION = bytes32("ACTIVE_AUCTION");uint256 constant ERC20_SLOT =bytes32 constant EIP_DOMAIN =bytes32 constant VERSION = keccak256("0");Both named return variable(s) and return statement are used.
To improve clarity use only named return variables.
For example, change:
function functionName() returns (uint id) { return x;
to
function functionName() returns (uint id) { id = x;
According to Style Guide, maximum suggested line length is 120 characters. Longer lines make the code harder to read.
Make the lines shorter.
In some cases, 1000 is used, and in some - 1_000.
1_000:
1000/10000:
uint256 fee = x.mulDivDown(VAULT_FEE(), 10000);s.liquidationFeeDenominator = uint32(1000);s.buyoutFeeDenominator = uint32(1000);auctionData.endAmount = uint88(1000 wei);Stick to one style.
#0 - c4-judge
2023-01-26T14:07:27Z
Picodes marked the issue as grade-b