prePO contest - yongskiws's results

Add constructor initializers

Note that this behaviour is also incorporated the OZ Wizard since the UUPS vulnerability discovery: “Additionally, we modified the code generated by the Wizard 19 to include a constructor that automatically initializes the implementation when deployed.” Furthermore, this thwarts any attempts to frontrun the initialization tx of these contracts:

Collateral.sol

  function initialize(string memory _name, string memory _symbol) public initializer {
    __SafeAccessControlEnumerable_init();
    __ERC20_init(_name, _symbol);
    __ERC20Permit_init(_name);
  }

PrePOMarketFactory.sol

  function initialize() public initializer { OwnableUpgradeable.__Ownable_init(); }

Consider with hardcoded gas amount should be used instead of transfer(), Approve() and transferFrom()

The use of the deprecated transfer() function for an address will inevitably make the transaction fail when:

-The claimer smart contract does not implement a payable function. -The claimer smart contract does implement a payable fallback which uses more than 2300 gas unit. -The claimer smart contract implements a payable fallback function that needs less than 2300 gas units but is called through proxy, -raising the call's gas usage above 2300. -Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Use OpenZeppelin’s safeTransfer(),safeIncreaseAllowance() or safeDecreaseAllowance(),safeTransferFrom() instead

Consider version soldity 0.8.* && Dependencies

Collateral.sol DepositHook.sol DepositRecord.sol DepositTradeHelper.sol LongShortToken.sol ManagerWithdrawHook.sol MintHook.sol PrePOMarket.sol PrePOMarketFactory.sol RedeemHook.sol TokenSender.sol WithdrawHook.sol

@openzeppelin/contracts @openzeppelin/contracts-upgradeable

Consider shortening revert strings to less than 32 bytes and keccak

Consider shortening revert strings to less than 32 bytes Revert strings more than 32 bytes require at least one additional mstore, along with additional operations for computing memory offset, etc.

Even if you need a string to represent an error, it can usually be done in less than 32 bytes / characters.

Note that this will only decrease runtime gas when the revert condition has been met. Regardless, it will decrease deploy time gas.

Warning Compiller

Warning: Unused function parameter. Remove or comment out the variable name to silence this warning. --> contracts/MintHook.sol:16:33: | 16 | function hook(address sender, uint256 amountBeforeFee, uint256 amountAfterFee) external ... | ^^^^^^^^^^^^^^^^^^^^^^^

Warning: Unused function parameter. Remove or comment out the variable name to silence this warning. --> contracts/MintHook.sol:16:58: | 16 | ... s sender, uint256 amountBeforeFee, uint256 amountAfterFee) external virtual override onlyAll ... | ^^^^^^^^^^^^^^^^^^^^^^

Warning: Contract code size exceeds 24576 bytes (a limit introduced in Spurious Dragon). This contract may not be deployable on mainnet. Consider enabling the optimizer (with a low "runs" value!), turning off revert strings, or using libraries. --> contracts/PrePOMarketFactory.sol:12:1: | 12 | contract PrePOMarketFactory is IPrePOMarketFactory, OwnableUpgradeable, ReentrancyGuardUpgradeable { | ^ (Relevant source part starts here and spans across multiple lines).