prePO contest - gz627's results

Non Critical Issues

[NC-1] Missing address(0) check

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol

86:     manager = _newManager;

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositHook.sol

77:   function setTreasury(address _treasury) public override onlyRole(SET_TREASURY_ROLE) { super.setTreasury(_treasury); }

Link to code

---

Low Issues

[L-1] Redundant code

In the below code, else { _actualFee = 0; } on Line 99 is redundant since the default value of _actualFee is 0, which is defined on Line 89.

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/PrePOMarket.sol

89  uint256 _actualFee;

91  if (address(_redeemHook) != address(0)) {
      collateral.approve(address(_redeemHook), _expectedFee);

99: } else { _actualFee = 0; }  //@audit `else { _actualFee = 0; }` is redundant

Link to code

[L-2] Local variables shadowed state variables

Instances (2):

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/WithdrawHook.sol

116:    function setTreasury(address _treasury) public override onlyRole(SET_TREASURY_ROLE) { //@audit `_treasury` shadows inherited state variable `TokenSenderCaller._treasury`

120:    function setTokenSender(ITokenSender _tokenSender) public override onlyRole(SET_TOKEN_SENDER_ROLE) {  //@audit `_tokenSender` shadows inherited state variable `TokenSenderCaller._tokenSender`

Link to code

Gas Optimizations

[GAS-1] Using named variable returns saves gas

Instances (3):

By using named variable returns, the return statement can be omitted.

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol

// ------ start of the 1st instance ------
      //@audit `returns (uint256)`  => `returns (uint256 _collateralMintAmount)`
45    function deposit(address _recipient, uint256 _amount) external override nonReentrant returns (uint256) {

        //@audit remove `uint256` since the variable type has been defined in `returns (...)`
57      uint256 _collateralMintAmount = (_amountAfterFee * 1e18) / baseTokenDenominator;

        //@audit remove the below line (Line 60) since it's redundant now.
60      return _collateralMintAmount;
// ------ end of the 1st instance ------

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/packages/prepo-shared-contracts/contracts/NFTScoreRequirement.sol

//@audit `returns (uint256)`  =>  `returns (uint256 _score)`
50    function getCollectionScore(IERC721 collection) external view virtual override returns (uint256) {

51      if (_collectionToScore.contains(address(collection))) return _collectionToScore.get(address(collection));

        //@audit remove the below line (Line 52), since `_score` has default value `0`
52      return 0;

53    }
//-------------------------------------------------------------------------------------------------

55    function getAccountScore(address account) public view virtual override returns (uint256) {

Link to code

---

[GAS-2] Caching state variables and expressions being referenced multiple times

Caching state variables and expressions will save gas and improve maintenance.

Please refer to the below code for details:

• Statements annotated with //@audit caching are where cache added;
• Statements annotated with //@audit using cache are where cache used;
• The line number, if any, is the original line number;
• If a line starts with +, this line is newly added;
• If a line starts with -, this line is removed;
• If a line starts with M, this line is modified;

Instances (6).

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol

45  function deposit(address _recipient, uint256 _amount) external override nonReentrant returns (uint256) {
+     uint256 _depositFee = depositFee; //@audit caching
+     address depositHookAddress = address(depositHook); //@audit caching
M     uint256 _fee = (_amount * _depositFee) / FEE_DENOMINATOR;  //@audit using cache
M     if (_depositFee > 0) { require(_fee > 0, "fee = 0"); }  //@audit using cache
      else { require(_amount > 0, "amount = 0"); }
      baseToken.transferFrom(msg.sender, address(this), _amount);
      uint256 _amountAfterFee = _amount - _fee;
M     if (depositHookAddress != address(0)) {  //@audit using cache
M       baseToken.approve(depositHookAddress, _fee);  //@audit using cache
        depositHook.hook(_recipient, _amount, _amountAfterFee);
M       baseToken.approve(depositHookAddress, 0);  //@audit using cache
      }
      /// Converts amount after fee from base token units to collateral token units.
      uint256 _collateralMintAmount = (_amountAfterFee * 1e18) / baseTokenDenominator;
      _mint(_recipient, _collateralMintAmount);
      emit Deposit(_recipient, _amountAfterFee, _fee);
      return _collateralMintAmount;
61  }


64    function withdraw(uint256 _amount) external override nonReentrant {
+       uint256 _withdrawFee = withdrawFee; //@audit caching
+       address withdrawtHookAddress = address(withdrawHook); //@audit caching
65      uint256 _baseTokenAmount = (_amount * baseTokenDenominator) / 1e18;
M       uint256 _fee = (_baseTokenAmount * _withdrawFee) / FEE_DENOMINATOR;  //@audit using cache
M       if (_withdrawFee > 0) { require(_fee > 0, "fee = 0"); }  //@audit using cache
        else { require(_baseTokenAmount > 0, "amount = 0"); }
        _burn(msg.sender, _amount);
        uint256 _baseTokenAmountAfterFee = _baseTokenAmount - _fee;
M       if (withdrawtHookAddress != address(0)) {  //@audit using cache
M         baseToken.approve(withdrawtHookAddress, _fee);  //@audit using cache
          withdrawHook.hook(msg.sender, _baseTokenAmount, _baseTokenAmountAfterFee);
M         baseToken.approve(withdrawtHookAddress, 0);  //@audit using cache
        }
        baseToken.transfer(msg.sender, _baseTokenAmountAfterFee);
        emit Withdraw(msg.sender, _baseTokenAmountAfterFee, _fee);
78    }

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/PrePOMarket.sol

+   address _redeemHookAddress = address(_redeemHook);  //@audit caching
M93 if (_redeemHookAddress != address(0)) {  //@audit using cache
M     collateral.approve(_redeemHookAddress, _expectedFee);  //@audit using cache
M     uint256 _collateralAllowanceBefore = collateral.allowance(address(this), _redeemHookAddress);  //@audit using cache
      _redeemHook.hook(msg.sender, _collateralAmount, _collateralAmount - _expectedFee);
M     _actualFee = _collateralAllowanceBefore - collateral.allowance(address(this), _redeemHookAddress);  //@audit using cache
M     collateral.approve(_redeemHookAddress, 0);  //@audit using cache
    } else { _actualFee = 0; }

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/packages/prepo-shared-contracts/contracts/WithdrawERC1155.sol

17  ) external override onlyOwner nonReentrant {
+     uint256 _arrayLength = _erc1155Tokens.length;  //@audit caching
M  	  require(_arrayLength == _recipients.length && _arrayLength == _ids.length && _arrayLength == _amounts.length, "Array length mismatch");  //@audit using cache
-     // uint256 _arrayLength = _erc1155Tokens.length;  //@audit remove this line
20    for (uint256 i; i < _arrayLength; ) {

---

[GAS-3] Applying unchecked operations where no overflow/underflow

Instances: (6):

Please refer to code lines annotated with //@audit apply unchecked.

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol

46    uint256 _fee = (_amount * depositFee) / FEE_DENOMINATOR;  //@audit: `_fee < _amount`  since `depositFee < FEE_DENOMINATOR`

50:   uint256 _amountAfterFee = _amount - _fee;  //@audit apply `unchecked`, Line 46 guarantees `_fee < _amount`

65    uint256 _baseTokenAmount = (_amount * baseTokenDenominator) / 1e18;
66    uint256 _fee = (_baseTokenAmount * withdrawFee) / FEE_DENOMINATOR;

70:   uint256 _baseTokenAmountAfterFee = _baseTokenAmount - _fee;  //@audit apply `unchecked`, please refer to L65-66

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositHook.sol

47:  uint256 _fee = _amountBeforeFee - _amountAfterFee; //@audit apply `unchecked`, guaranteed by calling contract

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositRecord.sol

29     require(_amount + globalNetDepositAmount <= globalNetDepositCap, "Global deposit cap exceeded");
30     require(_amount + userToDeposits[_sender] <= userDepositCap, "User deposit cap exceeded");

31:    globalNetDepositAmount += _amount;  //@audit apply `unchecked`, see L29

32:    userToDeposits[_sender] += _amount;  //@audit apply `unchecked`, see L30

36:    if (globalNetDepositAmount > _amount) { globalNetDepositAmount -= _amount; }  //@audit apply `unchecked`:=> if (globalNetDepositAmount > _amount) { unchecked{globalNetDepositAmount -= _amount;} }

[GAS-4] Use calldata instead of memory for function's immutable array arguments

Instances (10):

Since Solidity v0.6.9, memory and calldata are allowed in all functions regardless of their visibility type (ie external, public, etc).

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/DepositHook.sol

73:  function setCollectionScores(IERC721[] memory _collections, uint256[] memory _scores) public override onlyRole(SET_COLLECTION_SCORES_ROLE) { super.setCollectionScores(_collections, _scores); }

75:  function removeCollections(IERC721[] memory _collections) public override onlyRole(REMOVE_COLLECTIONS_ROLE) { super.removeCollections(_collections); }

Link to code

File: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/PrePOMarketFactory.sol

22:  function createMarket(string memory _tokenNameSuffix, string memory _tokenSymbolSuffix, bytes32 longTokenSalt, bytes32 shortTokenSalt, address _governance, address _collateral, uint256 _floorLongPrice, uint256 _ceilingLongPrice, uint256 _floorValuation, uint256 _ceilingValuation, uint256 _expiryTime) external override onlyOwner nonReentrant {

41:  function _createPairTokens(string memory _tokenNameSuffix, string memory _tokenSymbolSuffix, bytes32 _longTokenSalt, bytes32 _shortTokenSalt) internal returns (LongShortToken _newLongToken, LongShortToken _newShortToken) {

FIle: https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/packages/prepo-shared-contracts/contracts/NFTScoreRequirement.sol

22:    function setCollectionScores(IERC721[] memory collections, uint256[] memory scores) public virtual override {

35:    function removeCollections(IERC721[] memory collections) public virtual override {