Good Entry - 0x70C9's results

GeVault.modifyTicks can break ticks order

Github -> https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L171 The issue here is that this function allows the owner to add a TR on an arbitrary place in the ticks list. This breaks the assumption that the ticks list is always ordered.

GeVault.checkSetApprove may revert

Github -> https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/GeVault.sol#L386 If amount > 0 and allowance < amount, the function safeIncreaseAllowance will revert, because we are passing an increase of type(uint).max (will overflow). The usage of safeIncreaseAllowance with uint256 max is discouraged, and safeApprove should be used (approval race condition is irrelevant if we want max allowance). Though this function reverting seems pretty critical, it seems that in the current state of the code we will not have any case where allowance is neither 0 nor max. Still, we encourage this change.

RoeRouter allows the addition of the same pool more than once

Github -> https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/RoeRouter.sol#L76 The owner can call addPool to add an already existing pool. This leads to a state where a single pool address will have more than one poolId. What's more, one of those poolIds might be deprecated and the other not.

DeprecatePool event gets emitted even if the pool was already deprecated (QA)

Github -> https://github.com/code-423n4/2023-08-goodentry/blob/main/contracts/RoeRouter.sol#L50 Function deprecatePool always emits a DeprecatePool event, regardless of whether or not the pool in questioning had already been deprecated before.