Ondo Finance contest - tnevler's results

Report

Low Risk

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

Context:

1. function setFeeRecipient( L452
2. function setAssetRecipient( L465
3. function setAssetSender( L803
4. function _setKYCRegistry(address _kycRegistry) internal { L39

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 totalCashAmountRefunded; L795

Recommendation:

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

[N-2]: Use of immutable instead of constant keccak expression

Context:

1. bytes32 public constant TRANSFER_ROLE = keccak256("TRANSFER_ROLE"); L22
2. keccak256("KYC_CONFIGURER_ROLE"); L27
3. keccak256("KYC_CONFIGURER_ROLE"); L27
4. bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); L44
5. bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); L45
6. bytes32 public constant MINTER_ROLE = keccak256("MINTER_ROLE"); L44
7. bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); L45
8. keccak256( L32
9. bytes32 public constant MANAGER_ADMIN = keccak256("MANAGER_ADMIN"); L122
10. bytes32 public constant PAUSER_ADMIN = keccak256("PAUSER_ADMIN"); L123
11. bytes32 public constant SETTER_ADMIN = keccak256("SETTER_ADMIN"); L124

Description:

According to official solidity documentation for a constant variable, the expression assigned to it is copied to all the places where it is accessed and also re-evaluated each time. It is recommended to use immutable instead.

[N-3]: No same value input check

Context:

1. exchangeRateDeltaLimit = _exchangeRateDeltaLimit; L396
2. mintFee = _mintFee; L417
3. minimumDepositAmount = _minimumDepositAmount; L440
4. feeRecipient = _feeRecipient; L456
5. epochDuration = _epochDuration; L550
6. mintLimit = _mintLimit; L598
7. redeemLimit = _redeemLimit; L613
8. assetSender = newAssetSender; L807
9. minimumRedeemAmount = newRedeemMinimum; L821
10. kycRegistry = IKYCRegistry(_kycRegistry); L44
11. kycRequirementGroup = _kycRequirementGroup; L56

Recommendation:

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

[N-4]: Wrong order of functions

Context:

1. event CashDeployed( L143 (event definition can not go after external function)
2. event CashKYCSenderDeployed( L153 (event definition can not go after external function)
3. function assignRoletoKYCGroup( L144 (external function can not go after external view function)
4. pragma solidity 0.8.16; L19 (pragma directive can not go after import directive)
5. pragma solidity 0.8.16; L20 (pragma directive can not go after import directive)
6. pragma solidity 0.8.16; L20 (pragma directive can not go after import directive)
7. event FTokenToCTokenSet( L41 (event definition can not go after external function)
8. event FTokenToCTokenSet( L41 (event definition can not go after external function)
9. event PriceCapSet( L101 (event definition can not go after external 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-5]: NatSpec is missing

Context:

1. Proxy.sol
2. Cash.sol
3. CashKYCSender.sol
4. CashKYCSenderReceiver.sol

[N-6]: Typos

Context:

1. * `kycRequirementGroup`. In order to sucessfully call this function, L62 (Change sucessfully to successfully)
2. * @param addresses Array of addresses being added as elligible L205 (Change elligible to eligible)
3. * @param addresses Array of addresses being added as elligible L236 (Change elligible to eligible)
4. /// @notice Error for when caller attempts to set the KYC registry refernce L38 (Change refernce to reference)