Papr contest - 0xhacksmithh's results

[Low-01] ECRECOVER() NOT CHECKED FOR SIGNER ADDRESS OF ZERO

The ecrecover() function returns an address of zero when the signature does not match. This can cause problems if address zero is ever the owner of assets, and someone uses the permit function on address zero. If that happens, any invalid signature will pass the checks, and the assets will be stealable. In this case, the asset of concern is the vault’s ERC20 token, and fortunately OpenZeppelin’s implementation does a good job of making sure that address zero is never able to have a positive balance. If this contract ever changes to another ERC20 implementation that is laxer in its checks in favor of saving gas, this code may become a problem.

1 Instances of this issue

File:   src/ReservoirOracleUnderwriter.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/ReservoirOracleUnderwriter.sol#L68-L86

[LOW-02] Absence of zero address check for oracleSigner during contract deployment

1 Instances of this issue

File:   src/PaprController.sol

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

[LOW-03] Two different and Floating Pragma Used

Instead of Floating solidity, try to use Stable and Locked Solidity version

Below contracts using ```^0.8.17```

File:   src/PaprController.sol
File:   src/UniswapOracleFundingRateController.sol
File:   src/PaprToken.sol
File:   src/ReservoirOracleUnderwriter.sol

Below contracts using ```>=0.8.0 version```

File:   src/NFTEDA/extensions/NFTEDAStarterIncentive.sol
File:   src/NFTEDA/NFTEDA.sol
File:   src/NFTEDA/libraries/EDAPrice.sol
File:   src/libraries/PoolAddress.sol
File:   src/libraries/OracleLibrary.sol
File:   src/libraries/UniswapHelpers.sol
File:   src/interfaces/IUniswapOracleFundingRateController.sol
File:   src/interfaces/IFundingRateController.sol
File:   src/NFTEDA/interfaces/INFTEDA.sol
File:   src/interfaces/IPaprController.sol

[LOW-04] Instead of use of transfer() for transfering asset try to implement Openzeppelin safeERC20 library

5 Instances of this issue

File:   src/PaprController.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L202
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L203
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L514
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L515
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/PaprController.sol#L546

[NC-01] internal function that called only once can be inlined inside parent function

2 Instances of this issue

File:   src/UniswapOracleFundingRateController.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/UniswapOracleFundingRateController.sol#L111-L118
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/UniswapOracleFundingRateController.sol#L122-L130

[NC-02] Absence of error message in require() condition

1 Instances of this issue

File:   src/libraries/PoolAddress.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/PoolAddress.sol#L32

[NC-03] Should return a named return value rather than an expression

1 Instances of this issue

File:   src/libraries/UniswapHelpers.sol
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/UniswapHelpers.sol#L94

[NC-04] Unused Imports

Library imported but never used inside contract file

1 Instances of this issue

File:   src/PaprController.sol

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

[NC-05] Immutable state variable can make as CONSTANT

1 Instances of this issue

File:   src/PaprController.sol

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

[NC-06] Instead of large number try to use scientific notation

It will increase the readability, that lead to less error pone

4 Instances of this issue

File:   src/PaprController.sol

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

File:   src/libraries/OracleLibrary.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/OracleLibrary.sol#L23
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/OracleLibrary.sol#L25
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/OracleLibrary.sol#L28

[NC-07] Instead using magic number, try to use CONSTANT state variable

1 Instances of this issue

File:   src/PaprController.sol

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

[Gas-01] MULTIPLE ADDRESS MAPPINGS CAN BE COMBINED INTO A SINGLE MAPPING OF AN ADDRESS TO A STRUCT, WHERE APPROPRIATE

Saves a storage slot for the mapping. Depending on the circumstances and sizes of types, can avoid a Gsset (20000 gas) per mapping combined. Reads and subsequent writes can also be cheaper when a function requires both values and they both fit in the same storage slot. Finally, if both fields are accessed in the same function, can save ~42 gas per access due to not having to recalculate the key’s keccak256 hash (Gkeccak256 - 30 gas) and that calculation’s associated stack operations.

2 Instances of this issue

File:   src/PaprController.sol

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

[Gas-02] <x> += <y> CONSUME MORE GAS THAN <x> = <x> + <y>

1 Instances of this issue

File:   src/PaprController.sol

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

[Gas-03] public FUNCTION COULD BE external

6 Instances of this issue

File:   src/UniswapOracleFundingRateController.sol

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

File:   src/NFTEDA/NFTEDA.sol

https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/NFTEDA/NFTEDA.sol#L24
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/NFTEDA/NFTEDA.sol#L35

File:   src/UniswapOracleFundingRateController.sol

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

[Gas-04] ABI.ENCODE() IS LESS EFFICIENT THAN ABI.ENCODEPACKED()

1 Instances of this issue

File:   src/NFTEDA/NFTEDA.sol

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

[Gas-05] >= COSTS LESS GAS THAN >

2 Instances of this issue

File:   src/ReservoirOracleUnderwriter.sol

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

File:   src/UniswapOracleFundingRateController.sol

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

[Gas-06] DIVIDE BY 2 SHOULD BE PERFORM USING BIT SHIFTs

1 Instances of this issue

File:   src/libraries/UniswapHelpers.sol
https://github.com/with-backed/papr/blob/9528f2711ff0c1522076b9f93fba13f88d5bd5e6/src/libraries/UniswapHelpers.sol#L111