Papr contest - gz627's results

QA report

Non Critical Issues

[NC-1] Do not use magic numbers or values, define a constant or enum

Instances (6):

Using constants/immutables/enums helps code maintenance and reduces errors when upgrading and refactoring.

File: https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/UniswapOracleFundingRateController.sol

41:    _setFundingPeriod(4 weeks);

123:    if (_fundingPeriod < 7 days) revert FundingPeriodTooShort();

124:    if (_fundingPeriod > 90 days) revert FundingPeriodTooLong();

Link to code

File: https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol

87:    initSqrtRatio = UniswapHelpers.oneToOneSqrtRatio(underlyingONE, 10 ** 18);

89:    initSqrtRatio = UniswapHelpers.oneToOneSqrtRatio(10 ** 18, underlyingONE);

92:    address _pool = UniswapHelpers.deployAndInitPool(address(underlying), address(papr), 10000, initSqrtRatio);

Link to code

---

Low Risk Issues

[L-1] Missing checks when initializing immutables

Instances (1):

Affected code:

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/UniswapOracleFundingRateController.sol#L38-L39

In Contract UniswapOracleFundingRateController, missing checks when initializing immutables targetMarkRatioMax and targetMarkRatioMin, which results in the possibility of the setting result: targetMarkRatioMax < targetMarkRatioMin due to typos or some mistakes. While this may not lead to serious issues, it will confuse users since everybody can see the values as both immutables are defined as puclic.

It is suggested to add a check in contract construtor to avoid this issue: e.g.

    error TargetMarkRatioMaxLtTargetMarkRatioMin();  //@audit custom error
    
    constructor(ERC20 _underlying, ERC20 _papr, uint256 _targetMarkRatioMax, uint256 _targetMarkRatioMin) {
        underlying = _underlying;
        papr = _papr;
        
        if(_targetMarkRatioMax < _targetMarkRatioMin) revert TargetMarkRatioMaxLtTargetMarkRatioMin();  //@audit add this check
        targetMarkRatioMax = _targetMarkRatioMax;
        targetMarkRatioMin = _targetMarkRatioMin;

        _setFundingPeriod(4 weeks);
    }

Ofcourse, a simple require() will do the trick as this is an one-off check: require(_targetMarkRatioMax >= _targetMarkRatioMin, "targetMarkRatioMax < targetMarkRatioMin");

[L-2] Function parameters shadow state variables or other function names

Total instances (3):

Function parameters shadow other function names: instances (1)

Link to code:

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/UniswapOracleFundingRateController.sol#L95

In Contract UniswapOracleFundingRateController, the parameter target of function _init(uint256 target, address _pool) shadows a function name target: function target() external view override returns (uint256).

It is recommended to rename the function parameter to other name, e.g. _target.

Function parameters shadow state variables: instances (2)

Link to code:

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L73-L74

File: https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol

67  constructor(
        string memory name,
        string memory symbol,
        uint256 _maxLTV,
        uint256 indexMarkRatioMax,
        uint256 indexMarkRatioMin,
73:     ERC20 underlying,
74:     address oracleSigner
    )

In the above contract PaprController's constructor, some parameters (L73-L74) shadow inherited state variables: underlying and oracleSigner. It is recomended to rename these parameters to other names, e.g. _underlying and _oracleSigner