prePO contest - Mukund's results

INDEX

1. Unsafe use of transfer()/transferFrom()

Use openzeppelin safeERC20 library instead because some token may not revert in case of failure and may cause loss of funds so its better to use safeTransfer()/safeTransferFrom() https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L49 https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L76 https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/Collateral.sol#L82 https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contracts/core/contracts/TokenSender.sol#L42

2. Missing checks for address 0

In DepositHook.sol#L77 function manager can set treasury to address 0 which will eventually lead to loss of fees because fees is transferred to treasury Collateral.sol#L85-L88 Collateral.sol#L102-L105 Collateral.sol#L107-L110 Collateral.sol#L112-L115

3. Missing check for 0 uint value

for example In function DepositRecord.sol#L40-L43 manager can set globalNetDepositCap to 0 and user will not able to deposit funds because of this check DepositRecord.sol#L29 and also in PrePOMarketFactory.sol#L22-L34 function createMarket can set expiry time to 0 which will make market non expiring some instance of this issue: DepositRecord.sol#L45-L48 WithdrawHook.sol#L91-L114

4. File is missing NatSpec

all the contract are in scope except interface is missing NatSpec consider adding them

5. USE NAMED IMPORTS INSTEAD OF PLAIN `IMPORT ‘FILE.SOL’

for example use it like import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; Collateral.sol#L4-L7 DepositHook.sol#L4-L10 DepositRecord.sol#L4-L5 DepositTradeHelper.sol#L4-L6 WithdrawHook.sol#L4-L7 PrePOMarketFactory.sol#L4-L10

6. INITIALIZE() FUNCTION CAN BE CALLED BY ANYBODY

initialize() function can be called anybody when the contract is not initialized. PrePOMarketFactory.sol#L16 Collateral.sol#L34-L38

7. Contracts are not using their OZ Upgradeable counterparts

The non-upgradeable standard version of OpenZeppelin’s library are inherited / used by the contracts. It would be safer to use the upgradeable versions of the library contracts to avoid unexpected behaviour.

DepositHook.sol#L10 DepositTradeHelper.sol#L6 LongShortToken.sol#L4-L6

8. USE NAMED IMPORTS INSTEAD OF PLAIN `IMPORT ‘FILE.SOL’

for example use it like import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; DepositHook.sol#L4-L10 Collateral.sol#L4-L7 [DepositRecord.sol#L4-L5](https://github.com/prepo-io/prepo-monorepo/blob/feat/2022-12-prepo/apps/smart-contra

INDEX

1. <X> += <Y> COST MORE GAS THAN <X> = <X>+<Y> FOR STATE VARIABLE

Using the addition operator instead of plus-equals saves gas DepositRecord.sol#L31 WithdrawHook.sol#L64 WithdrawHook.sol#L71

2. USAGE OF UINT SMALLER THAN 32 BYTES( 256 BITS) INCURS OVERHEAD

When using elements that are smaller than 32 bytes, your contract’s gas usage may be higher. This is because the EVM operates on 32 bytes at a time. Therefore, if the element is smaller than that, the EVM must use more operations in order to reduce the size of the element from 32 bytes to the desired size. DepositTradeHelper.sol#L12

3. THE RESULT OF FUNCTION CALLS SHOULD BE CACHED RATHER THAN FETCHING INSIDE THE FUNCTION

TokenSender.sol#L37

4. FUNCTION GUARANTEED TO REVERT WHEN CALLED BY NORMAL USERS CAN BE MARKED PAYABLE

If a function modifier such as onlyOwner is used, the function will revert if a normal user tries to pay the function. Marking the function as payable will lower the gas cost for legitimate callers because the compiler will not include checks for whether a payment was provided. Collateral.sol#L80-L115 DepositHook.sol#L54-L79 DepositRecord.sol#L40-L53 ManagerWithdrawHook.sol#L19-L33 PrePOMarket.sol#L109-L130 TokenSender.sol#L45-L62

5. USING FUNCTION INSTEAD OF MODIFIERS CAN SAVE GAS

use function instead of modifiers if possible because it will save gas DepositHook.sol#L27-L30 DepositRecord.sol#L18-L21

6. Use assembly to check for address(0)

using assembly can save lot of gas so it better to use assembly for address(0) check Collateral.sol#L51 Collateral.sol#L71 Collateral.sol#L81 PrePOMarket.sol#L68 PrePOMarket.sol#L93