Steadefi

Summary

The Execution Fee GMX requires in order to process transactions is calculated dynamically; depending on Gas Prices at processing time.
Steadefi's use of the minExecutionFee state variable and the updateMinExecutionFee setter function are not enough to manage volatile Execution Fees and avoid transaction cancellations.

References

GMX fee volatility can be visualised here: https://dune.com/gmx-io/gmx-analytics
GMX Docs - how Execution Fee is calculated: https://docs.gmx.io/docs/api/contracts-v2#execution-fee

Vulnerability Details

GMX uses this function to validate the provided Execution Fee against their calculated Execution Fee requirement.

If their minExecutionFee is greater than the provided executionFee, such as in times of high market congestion, transactions will revert.

Given current design Steadefi will be unable to adjust to changing market conditions, e.g. congestion, and set GMXVault.sol::_store.minExecutionFee accordingly via GMXVault.sol::updateMinExecutionFee due to it's protection by timelocks and muti-sig.

Impact

Transactions on the SteadeFi side would go through without any issue as long as user's Execution Fee is higher than the vault's minExecutionFee.
However, in periods of network congestion which drives fees up, the return transactions from GMX would be cancellations, potentially DOSing the additon or removal of liquidity. This will waste a lot of gas in both the outgoing and the return functions; requiring the reversal of state changes, rebalances, etc; a lot of avoidable computation. Not to mention a bad usr experience.

Tools Used

Manual Review

Recommendations

1. Set a high minExecutionFee
   Recommend setting a higher minExecutionFee than is currently used in the, for example, the deploy script (1e15).
   It is difficult to pinpoint exactly where minExecutionFee should be given it's volatility but, given the two transaction nature of interacting with GMX and the wasted gas any reversion costs, preventing as many failed transactions on the Steadefi side before sending them to GMX should be the priority.
   Setting too high an Execution Fee would be better than too low given that any unused fee will be refunded; e.g. fee is set to 1e18 in GMX sample implementation

2. Replicate GMX's gas-inclusive fee estimation on the Steadefi side before sending the transaction to GMX in GMXManager.sol or GMXWorker.sol to minimise likelihood of reversion.