Medium Risk
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXChecks.sol#L268-L293
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXChecks.sol#L312-L332
Before a rebalance can occur, checks are implemented to ensure that delta
and debtRatio
remain within their specified limits. However, it's important to note that the check in GMXChecks::beforeRebalanceChecks
ignores the scenario where these values are equal to any of their limits.
In the current implementation of the GMXRebalance::rebalanceAdd
function, it first calculates the current values of debtRatio
and delta
before making any changes. Subsequently, the beforeRebalanceChecks
function, checks if these values meet the requirements for a rebalance to occur. These requirements now dictate that both debtRatio
and delta
must be either ≥ to the UpperLimit
, or ≤ to the LowerLimit
for a rebalance to take place.
function beforeRebalanceChecks(
GMXTypes.Store storage self,
GMXTypes.RebalanceType rebalanceType
) external view {
if (
self.status != GMXTypes.Status.Open &&
self.status != GMXTypes.Status.Rebalance_Open
) revert Errors.NotAllowedInCurrentVaultStatus();
// Check that rebalance type is Delta or Debt
// And then check that rebalance conditions are met
// Note that Delta rebalancing requires vault's delta strategy to be Neutral as well
if (rebalanceType == GMXTypes.RebalanceType.Delta && self.delta == GMXTypes.Delta.Neutral) {
if (
self.rebalanceCache.healthParams.deltaBefore < self.deltaUpperLimit &&
self.rebalanceCache.healthParams.deltaBefore > self.deltaLowerLimit
) revert Errors.InvalidRebalancePreConditions();
} else if (rebalanceType == GMXTypes.RebalanceType.Debt) {
if (
self.rebalanceCache.healthParams.debtRatioBefore < self.debtRatioUpperLimit &&
self.rebalanceCache.healthParams.debtRatioBefore > self.debtRatioLowerLimit
) revert Errors.InvalidRebalancePreConditions();
} else {
revert Errors.InvalidRebalanceParameters();
}
}
Suppose a rebalance is successful. In the afterRebalanceChecks
section, the code verifies whether both delta
and debtRatio
are greater than the UpperLimit
or less than the LowerLimit
. This confirmation implies that these limits are indeed inclusive, meaning that the correct interpretation of these limits should be that LowerLimit ≤ actualValue ≤ UpperLimit
. On the other hand, this also indicates that for a rebalancing to occur, the values of deltaBefore
and debtRatioBefore
need to be outside their limits, i.e., delta
should be greater than Upper
or less than Lower
. However, in the current implementation, if these values are equal to the limit, a rebalance may still occur, which violates the consistency of the afterRebalanceChecks
function, thus indicating that these limits are inclusive. Consequently, a value equal to the limit needs to be treated as valid and not be able to trigger a rebalance.
function afterRebalanceChecks(
GMXTypes.Store storage self
) external view {
// Guards: check that delta is within limits for Neutral strategy
if (self.delta == GMXTypes.Delta.Neutral) {
int256 _delta = GMXReader.delta(self);
if (
_delta > self.deltaUpperLimit ||
_delta < self.deltaLowerLimit
) revert Errors.InvalidDelta();
}
// Guards: check that debt is within limits for Long/Neutral strategy
uint256 _debtRatio = GMXReader.debtRatio(self);
if (
_debtRatio > self.debtRatioUpperLimit ||
_debtRatio < self.debtRatioLowerLimit
) revert Errors.InvalidDebtRatio();
}
Imagine the case when delta
or debtRatio
is equal to any of its limits; a rebalance will occur. However, on the other hand, these values are valid because they are inclusively within the limits.
In such a scenario, the system might incorrectly trigger a rebalance of the vault, even when delta
or debtRatio
is precisely within the established limits, thus potentially causing unintended rebalancing actions.
Manual
Consider a strict check to determine if delta
or debtRatio
is strictly within its limits, including scenarios where they are equal to any of its limits. In such cases, the code should ensure that a rebalance does not occur when these values are precisely at the limit.
function beforeRebalanceChecks(
GMXTypes.Store storage self,
GMXTypes.RebalanceType rebalanceType
) external view {
if (
self.status != GMXTypes.Status.Open &&
self.status != GMXTypes.Status.Rebalance_Open
) revert Errors.NotAllowedInCurrentVaultStatus();
// Check that rebalance type is Delta or Debt
// And then check that rebalance conditions are met
// Note that Delta rebalancing requires vault's delta strategy to be Neutral as well
if (rebalanceType == GMXTypes.RebalanceType.Delta && self.delta == GMXTypes.Delta.Neutral) {
if (
- self.rebalanceCache.healthParams.deltaBefore < self.deltaUpperLimit &&
- self.rebalanceCache.healthParams.deltaBefore > self.deltaLowerLimit
+ self.rebalanceCache.healthParams.deltaBefore <= self.deltaUpperLimit &&
+ self.rebalanceCache.healthParams.deltaBefore >= self.deltaLowerLimit
) revert Errors.InvalidRebalancePreConditions();
} else if (rebalanceType == GMXTypes.RebalanceType.Debt) {
if (
- self.rebalanceCache.healthParams.debtRatioBefore < self.debtRatioUpperLimit &&
- self.rebalanceCache.healthParams.debtRatioBefore > self.debtRatioLowerLimit
+ self.rebalanceCache.healthParams.debtRatioBefore <= self.debtRatioUpperLimit &&
+ self.rebalanceCache.healthParams.debtRatioBefore >= self.debtRatioLowerLimit
) revert Errors.InvalidRebalancePreConditions();
} else {
revert Errors.InvalidRebalanceParameters();
}
}