Limit Order Manager

Gamma has added Pausable to the Limit Order Manager with the intention of pausing order creation when necessary. However, the contract does not expose any external functions to actually pause or unpause it.

Pausable only provides internal functions _pause() and _unpause() and expects the implementing contract to wrap them in access-controlled public or external functions.

function _pause() internal virtual whenNotPaused {
    _paused = true;
    emit Paused(_msgSender());
}

function _unpause() internal virtual whenPaused {
    _paused = false;
    emit Unpaused(_msgSender());
}

Add access-controlled external functions to allow authorized parties (e.g., the admin) to pause and unpause the contract.

A malicious actor can mint a small position directly on the Algebra pool with the recipient set to the LimitOrderManager contract. This triggers a feeGrowth update for the position (keyed by top, bottom, recipient) during the mint call.

Later, when LimitOrderManager invokes retrackPositionFee, it uses the updated feeGrowthInside values, effectively skipping over the previously accumulated fees — thereby denying them to the legitimate order owners.

The same vulnerability exists on Gamma’s Uniswap v4 Limit Order Manager as well.

Consider allowing addition of liquidity only if recipient is msg.sender of the call via beforeModifyPosition hook.

The variable limitOrderPluginAddr is declared and is admin-writable, but it is never read in the contract. The same value can be fetched directly using pool.plugin().

Consider removing limitOrderPluginAddr and its corresponding setter to reduce unnecessary storage and simplify the contract.

The onlyKeeper modifier is defined in the contract but is never used. Instead, Gamma uses require(msg.sender == keeper, "...") directly wherever the keeper check is needed.

Either remove the unused onlyKeeper modifier to reduce dead code, or refactor the contract to consistently use the modifier instead of repeating the require check.

The function validateScaleOrderSizes() is invoked even for limit orders. The function checks both the first and last order's amount against minRequired, which results in the same value being checked twice when orders.length == 1 (limit orders).

if (orders[0].amount < minRequired) {
    revert MinimumAmountNotMet(orders[0].amount, minRequired);
}
if (orders[orders.length - 1].amount < minRequired) {
    revert MinimumAmountNotMet(orders[orders.length - 1].amount, minRequired);
}

Consider doing second validation only when orders.length > 1

Gamma currently encodes bytes memory callbackData = abi.encode(msg.sender); within each loop iteration, despite the encoded data being the same each time.

Define callbackData once before the loop and reuse it to avoid redundant encoding operations.

The WrongTickRange, InvalidPrice, and PriceMustBeGreaterThanZero errors in TickLibrary are not used anywhere in the codebase and can be removed.

Remove unused errors.

LimitOrderManager accepts and handles native ETH.

• Mint: Excess ETH is refunded immediately upon receipt.
• Claim: ETH is transferred back to the user.

However, ETH transfers hand over control to the user during the call, which can introduce subtle reentrancy paths or cause inconsistent state views.

Example – Mint:

1. createLimitOrder is called with excess ETH.
2. Ticks are validated such that both bottomTick and topTick are either above or below the current tick.
3. Excess ETH is refunded.
4. User reenters the pool and performs a swap moving the tick from bottomTick → currentTick → topTick.
5. This causes the mint to pull both tokens, instead of just one.

While this does not currently result in an exploit (since the contract doesn't hold tokens and they are pulled directly from users), it creates non-obvious and undesirable edge behavior.

• Ideally, remove native ETH support altogether and rely solely on WETH. This eliminates callback exposure and standardizes handling. An external wrapper can be used for ETH ↔ WETH conversion.
• Alternatively, at minimum, remove the refund logic in the mint path. Since the refund occurs before critical state changes, it introduces unnecessary risk.

Instead, enforce that msg.value must exactly match the required amount.

Some public/external functions in LimitOrderManager lack the nonReentrant modifier, despite similar functions being protected.

For example:

1. cancelPositionKeys is guarded with nonReentrant, whereas cancelBatchOrder is not.
2. Inside cancelOrder, during _handlePositionRemoval, before the user is removed from positionContributors, and positionState is changed, if there is an ETH callback in claim, one can reenter and use the unaltered state.

There are multiple such cases where one can reenter LimitOrderManager and use the stale or to be updated state.

While we haven’t been able to exploit these cases profitably, we feel that allowing a set of possibilities where there is no benefit to the user is not ideal, since it leads to unnecessary exposure.

Consider applying nonReentrant to all user-facing actions.

The _claimOrder function updates the user's position. However the position is deleted right after. This is redundant and wastes a lot of gas.

Consider not updating the state here, rely on the variables already in memory and be aware to not read from the not updated state variable.

A large swap can mark many tick ranges as waitingForKeeper. While these ranges are queued for keeper execution, new orders that overlap these ticks cannot be placed. This behavior—though intentional—can block order creation across a wide range of ticks, creating a temporary denial-of-service condition for users and integrators.

This mechanism can be unintuitive to third-party integrators or bots that expect open access to place limit orders at any time.

No changes to core logic are required. However, consider documenting this behavior explicitly**.**

The minAmount0 and minAmount1 params are zero right after deploying the LimitOrderManager contract and the admin needs to set them manually with the setMinAmount later on.

This might allow malicious actors to back run the deployment and create smaller positions than the admin would like to allow, which could lead to bad consequences.

Consider setting the parameters in the constructor or define hardcoded default value for them.

In the handlePositionRemoval case of a cancel—where positionContributors's length is zero—the feePerLiquidity for the position is not reset, and the nonce is not updated.

As a result, if a user creates an order, cancels it, and then creates it again with the same range, the feePerLiquidity from the previous position persists and is reused in the next createOrder for that range.

While we haven’t been able to exploit this (since the logic is additive), this behavior is inconsistent—especially considering the cancel flow mimics execute (isActive = false, isWaitingForKeeper = false, removePositionFromTick).

This inconsistency could lead to issues down the line.

Consider updating the nonce whenever positionContributors's length is zero in cancel.

On line 401 of the LimitOrderManager contract, the pool variable is cast to the IAlgebraPool interface.

IAlgebraPool(pool).burn(bottomTick, topTick, uint128(userLiquidity), "");

However, pool is already an IAlgebraPool, and the cast is unnecessary.

Consider removing unnecessary casting

Algebra allows pool owners to modify tickSpacing via the setTickSpacing function. However, Gamma’s LimitOrderManager treats tick spacing as immutable and relies on it for all range-based calculations. If tickSpacing is changed on an Algebra pool after deployment, it will result in inconsistent and potentially incorrect behavior across Gamma's logic, including fee calculation, position placement, and execution.

Although Algebra permits tickSpacing updates, such changes must not be performed on any pools associated with Gamma. This constraint should be explicitly documented for both internal developers and external integrators.

Gamma reads Algebra pool state directly from global storage slots (e.g., globalState()), which exposes it to read-only reentrancy attacks. Since Algebra uses locking to guard state consistency during critical operations, bypassing these locks by reading global state directly may lead to edge case inconsistencies — especially if exploited by another contract in the same transaction context.

Replace direct calls to globalState() with safelyGetStateOfAMM, which respects the pool's internal locking mechanism. If additional state is needed beyond what safelyGetStateOfAMM provides, first verify pool state via isUnlocked() before proceeding with reads.

In the _claimOrder a full share price calculation is performed twice to figure out the amount of fees that belong to the user and to the treasury.

As both round down this could leave dust tokens stuck inside the contract.

Consider working with subtraction instead to be more precise.

The following imports are not used in the codebase and can be removed:

• console.sol and Constants.sol in LimitOrderManager
• console.sol in PositionManagement
• {console} from Test.sol and IAlgebraVirtualPool.sol in LimitOrderPlugin

Consider removing unused imports.

Users can provide any sizeSkew param when calling the createScaleOrders function except from 0. A very big sizeSkew value leads to a buffer overflow Dos in the denominator1 calculation, in the _calculateOrderSize function.

Consider capping the sizeSkew param.