AutoCancel Updates

When updating an order with the OrderHandler.updateOrder function, orders which were already present in the AutoCancel list are attempted to be re-added with the updateAutoCancelList function.

The AutoCancel list is a set, therefore no orders are duplicated. However as the maxAutoCancelOrders validation is performed in the addAutoCancelOrderKey function, updateOrder calls are DoS'd in the following circumstance:

• The account holds the maxAutoCancelOrders in their autoCancelOrderList
• Order A is an existing order which is in the autoCancelOrderList
• User attempts to update Order A, but wishes to update parameters other than the autoCancel value

As a result the user cannot update this order without removing it from the auto cancel list or removing other auto cancel orders.

Only call the updateAutoCancelList in the event that the order autoCancel value is changed from the existing value.

The syncAutoCancelOrderList function relies on an account's open positions to collect the autoCancel orders which need to be synced.

However there may exist accounts where the position has been closed through the old contracts, and therefore autoCancel orders exist for that null position.

Additionally some of these autoCancel orders may only be present in the autoCancel list, and not in the orderStore, therefore requiring a sync. They should all be synced and removed from the list.

However these autoCancel orders cannot be synced as they do not belong to a position which is currently open for the account. Users may have their autoCancel orders go unsynced during the period of syncing, open their position again in the future, and have unsynced autoCancel orders which prevent their position from closing.

Consider adding a new function to target a specific positionKey instead of relying solely on the account's open positions with the getAccountPositionKeys function.

The initOracleProviderForToken function allows the keeper to initialize a provider. However, the provider has no validation on it and may be the zero address.

Add a zero address check on address provider.

In the setPositionImpactDistributionRate function the configKeeper may assign any arbitrary positionImpactPoolDistributionRate, therefore if the config keeper is compromised they may drain the position impact pool by setting an excessively high rate and potential arbitrage this by depositing into the GM token market beforehand.

Consider adding a maximum impact pool distribution rate to the setPositionImpactDistributionRate function.

The Keys.HOLDING_ADDRESS key is allowed in the _initAllowedBaseKeys function, therefore the a compromised config keeper may re-assign the holding address to their own account or assign it to the zero address.

If the holding address is assigned to the zero address, all instances where the holding address is used will lead to a DoS, which can re-enable several risk-free trade vectors which the holding address solves.

Consider only allowing the holding address to be updated by the config keeper if it is not assigned. And allow the holding address to be configured by the more trusted timelock address.

In the _initAllowedBaseKeys function the MIN_POSITION_SIZE_USD and MIN_COLLATERAL_FACTOR keys are allowed as a value that the config keeper may assign, as a result a compromised config keeper may assign an exceedingly high minimum position size or minimum collateral factor and cause all GMX positions to be liquidated.

The config keeper could short the GMX token in order to profit from such a catastrophe.

Consider moving these configurations to the more trusted time lock.