Ondo Finance contest - 0xAgro's results

QA Report

Finding Summary

[NC-01 Long Lines (> 120 Characters)

Lines with greater length than 120 characters are used. The Solidity Style Guide suggests that all lines should be 120 characters or less in width.

The following lines are longer than 120 characters, it is suggested to shorten these lines:

contracts/lending/JumpRateModelV2.sol

• 57.

contracts/lending/tokens/cCash/CCash.sol

• 147, 191, 230, 234.

contracts/lending/tokens/cToken/CErc20.sol

• 147, 191, 230, 234.

contracts/lending/OndoPriceOracleV2.sol

• 298.

contracts/lending/tokens/cErc20ModifiedDelegator.sol

• 997, 1095.

contracts/lending/tokens/cCash/CTokenCash.sol

• 287, 573, 574, 1056, 1196, 1414, 1425.

contracts/lending/tokens/cToken/CTokenModified.sol

• 287, 573, 574, 1059, 1199, 1417, 1428.

[NC-02] Spelling Mistakes

/contracts/cash/kyc/KYCRegistry.sol

• successfully is misspelled as sucessfully
• eligible is misspelled as elligible

/contracts/lending/tokens/cCash/CCash.sol

• of is misspelled as fo

/contracts/lending/OndoPriceOracleV2.sol

• common is misspelled as comnmon

/contracts/lending/tokens/cErc20ModifiedDelegator.sol

• amount is misspelled as amnount (1)
• amount is misspelled as amnount (2)
• setter is misspelled as settor

/contracts/lending/IOndoPriceOracleV2.sol

• asset is misspelled as assset

[NC-03] assert Used Over require

assert should only be used in tests. Consider changing all occurrences of assert to require. Prior to Solidity 0.8 require will refund all remaining gas whereas assert will not. Even after Solidity 0.8 assert will result in a panic which should not occur in production code. As stated in the Solidity Documentation: "[p]roperly functioning code should never create a Panic".

/contracts/cash/factory/CashFactory.sol Links: 97.

97:	assert(cashProxyAdmin.owner() == guardian);

/contracts/cash/factory/CashKYCSenderFactory.sol Links: 106.

106:	assert(cashKYCSenderProxyAdmin.owner() == guardian);

[NC-04] Underscore Notation Not Used / Not Used Consistently

Consider using underscore notation to help with contract readability (Ex. 23453 -> 23_453).

/contracts/lending/JumpRateModelV2.sol Links: 29.

29:	uint public constant blocksPerYear = 2628000;

/contracts/lending/OndoPriceOracleV2.sol Links: 77.

77:	uint256 public maxChainlinkOracleTimeDelay = 90000;

[NC-05] Named Imports Not Used

All files in scope use file explicit imports in the format: import './file.sol'. Consider using name explicit imports for better code clarity: import {name} from './file.sol'.

[NC-06] Contract Layout Voids Solidity Docs

The Solidity Style Guide suggests the following contract layout order: type declarations, state variables, events, modifiers, functions.

The following contracts are not compliant (examples are only to prove the layout are out of order NOT a full description):

• CashFactory.sol: events are positioned after functions.
• CashKYCSenderFactory.sol: events are positioned after functions.
• CashKYCSenderReceiverFactory.sol: events are positioned after functions.
• JumpRateModelV2.sol: State is positioned after Events.
• KYCRegistry.sol: events are positioned after functions.
• OndoPriceOracleV2.sol: type declarations are positioned after state.
• CashManager.sol: modifiers positioned after functions.
• CTokenCash.sol: modifiers are positioned after functions.
• CTokenModified.sol: modifiers are positioned after functions.
• IKYCRegistryClient.sol: events are positioned after functions.
• IOndoPriceOracle.sol: events are positioned after functions.
• IOndoPriceOracleV2.sol: events are positioned after functions.
• ICashManager.sol: events are positioned after functions.

[NC-07] Explicit Data Types Not Used Consistently

uint / int default to the explicit data type uint256 / int256. Data types should be explicit like in CashManager.sol.

/contracts/lending/JumpRateModelV2.sol

• All uint / int's are not explicit

/contracts/lending/tokens/cCash/CCash.sol

• All uint / int's are not explicit except for L37, and L159.

/contracts/lending/tokens/cToken/CErc20.sol

• All uint / int's are not explicit except for L37, and L159.

/contracts/lending/OndoPriceOracleV2.sol

• L287, and L289 uint / int's are not explicit.

/contracts/lending/tokens/cCash/CTokenInterfacesModifiedCash.sol

• All uint / int's are not explicit except for L32, L137, L159, L160, L177.

/contracts/lending/tokens/cToken/CTokenInterfacesModified.sol

• All uint / int's are not explicit except for L30, L135, L157, L158, L175.

/contracts/lending/tokens/cCash/CTokenCash.sol

• All uint / int's are not explicit except for L40, L42, L154, L169, L184, L201, L210, L1356, L1367.

/contracts/lending/tokens/cToken/CTokenModified.sol

• All uint / int's are not explicit except for L40, L42, L154, L169, L184, L1359, L1370, L1371.

/contracts/lending/IOndoPriceOracle.sol

• uint not explicit: L24.

/contracts/lending/IOndoPriceOracleV2.sol#L24

• uint not explicit: L24.

[NC-08] Contracts Missing @title NatSpec Tag

8 out of 30 of the contracts in scope are missing a @title tag. Given that 22 contracts all have a @title tag, consider adding one per the 8 remaining contracts.

Proxy.sol, Cash.sol, CashKYCSender.sol, CashKYCSenderReceiver.sol, CashManager.sol, IOndoPriceOracle.sol, IOndoPriceOracleV2.sol, and ICashManager.sol are missing a @title tag.

[NC-09] Order of Functions Not Compliant With Solidity Docs

The Solidity Style Guide suggests the following function order: constructor, receive function (if exists), fallback function (if exists), external, public, internal, private.

The following contracts are not compliant (examples are only to prove the functions are out of order NOT a full description):

• CCash.sol: external functions are positioned after public functions.
• CErc20.sol: external functions are positioned after public functions.
• OndoPriceOracleV2.sol: external functions are positioned after internal functions.
• CashManager.sol: external functions are positioned after private functions.
• cErc20ModifiedDelegator.sol: external functions are positioned after public functions.
• CTokenCash.sol: external functions are positioned after internal functions.
• CTokenModified.sol: external functions are positioned after internal functions.

[NC-10] No License Indication

Some contracts are missing a license indication. If no license is used SPDX-License-Identifier: UNLICENSED should be at the top of a contract.

JumpRateModelV2.sol, and cErc20ModifiedDelegator.sol are missing a license.