Good Entry - nonseodion's results

Natspec Comment wrongly assumes Uniswap V3 addresses are constant across chains.

The Natspec comment assumes that Uniswap V3 addresses are identical across all chains. But Uniswap addresses are no longer constant across chains. This is stated explicitly here in the Uniswap documentation. It says:

"Integrators should no longer assume that they are deployed to the same addresses across chains and be extremely careful to confirm mappings below."

The statement above precedes a table of addresses for each Uniswap contract on different chains.

2 instances were found: https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L35

https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/TokenisableRange.sol#L57

Incorrect Natspec comment for succeeding calculation

The Natspec comment here says: "Overflow if dont scale down the sqrtPrice before div 2*192"

The correct statement should be "Overflow if don't scale down the sqrtPrice before div 2**192" as the sqrtPrice has to be divided by 2**192 and not 2*192.

1 instance found: https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L373

Change oraclePrice variable name to oraclePriceX8

The oraclePrice variable has been scaled by 1e8 and can be renamed oraclePriceX8. This will improve readability. It will also make variable naming uniform since the variable preceding it was named priceX8 because it was also scaled by 1e8 [here].

1 Instance found: https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/GeVault.sol#L376

RangeManager: Unused Imports

The interfaces IUniswapV2Router01, IUniswapV2Factory, and IUniswapV2Pair were imported into the RangeManager contract but were unused in the contract or any other contract inheriting from it.

The IUniswapV2Factory and IUniswapV2Pair interfaces were also imported into the PositionManager contract but were unused in the contract or any other contract inheriting from it.

The imports should be removed if they aren't relevant to the contract to reduce bloat.

5 Instances found: https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L13 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L14 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/RangeManager.sol#L15 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/PositionManager.sol#L12 https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/PositionManager.sol#L11

withdrawOptionAssets: Unused Return Variable

The return variable of withdrawOptionAssets function is unused in the executeBuyOptions function which is the only function that calls it.

The variable can be removed if it is not useful in the current implementation of the contract.

1 instance found: https://github.com/code-423n4/2023-08-goodentry/blob/71c0c0eca8af957202ccdbf5ce2f2a514ffe2e24/contracts/PositionManager/OptionsPositionManager.sol#L74)