Is it dangerous for the effective share reserves to be too small?

[jalextowle]

In #537, I considered enforcing the condition that $z - \zeta \geq z_{min}$ because it seemed like a good idea to restrict the range to avoid issues with small integer values. After attempting to make this change, there were a lot of test failures. The primary culprit of the failures is that we were paying out too much idle and/or withdrawal proceeds, which resulted in $z - \zeta < z_{min}$. Everything worked fine when we only enforced that $z - \zeta \geq 0$. To get a better sense of the problem, it's helpful to derive the restriction that should be placed on the idle and withdrawal proceeds.

Let $z_0$ and $\zeta_0$ be the initial share reserves and share adjustment. If $z_1 = z_0 - \Delta z$ for $\Delta z > 0$, then $\zeta_1 = \zeta_0 \cdot \tfrac{z_1}{z_0}$. We can substitute these values into the invariant as follows:

$$ (z_0 - \Delta z) - \zeta_0 \cdot \tfrac{z_0 - \Delta z}{z_0} \geq z_{min} $$

From this, we can simplify to get our constraint that:

$$ \Delta z \leq z_0 - \tfrac{z_{min}}{1 - \tfrac{\zeta_0}{z_0}} $$

If we were to enforce the invariant that $z - \zeta \geq z_{min}$, then we'd need to clamp the idle amount (and withdrawal proceeds prior to #367 being closed) to this maximum share reserves delta. This complicates the LP accounting and the LP UX. Aside from this, restricting our effective share reserves prevents us from utilizing the full bonding curve if we switched to an invariant like Uniswap or Curve V2 that doesn't intersect the y-axis.

If we do the same analysis on the weaker invariant that $z - \zeta \geq 0$, then we get the constraint that:

$$ (z_0 - \Delta z) - \zeta_0 \cdot \tfrac{z_0 - \Delta z}{z_0} \geq 0 \implies \Delta z \leq z $$

Unlike the constraint imposed by the stricter invariant, this invariant imposes a restriction that we already enforce on the share reserves delta.

With these benefits established, let's think through some potential downsides of allowing small values of $z - \zeta$. Keep in mind that #577 established a minimum trade size that prevents the user from attempting to open a tiny trade. The biggest risks are:

1. Small values of $z - \zeta$ result in dangerous instability in the YieldSpace calculations (trades, spot price, and spot rate).
2. Allowing a small $z - \zeta$ amount results in a DOS vector against the system due to a revert in _updateLiquidity.

The ability for a trader or LP to abuse 1. seems very limited. The minimum transaction amount ensures that the spot rate and price always go in the right direction (opening a long increases the price and lowers the rate), and also eliminates the possibility of a trader receiving a better realized rate than the spot price. Some edge cases that could still be possible are manipulating $z - \zeta$ to create a favorable $k$ value. Hypothetically, this could be used to steal from LPs in a similar style to the curve exploit. With this said, only small values are dangerous from this perspective, and small values have a small impact on the k value (the other coordinate dominates in the case that one is very small). This reduces the vulnerabilities scope, and the curve fee should eliminate it entirely. It still warrants a rigorous investigation and seems like a fruitful area for fuzzing and further review in audits.

Initially, it seems like 2. is a more pressing issue because we calculate the updated bond reserves in _updateLiquidity as:

        _marketState.bondReserves = HyperdriveMath
            .calculateEffectiveShareReserves(
                uint256(updatedShareReserves),
                updatedShareAdjustment
            )
            .mulDivDown(
                _marketState.bondReserves,
                HyperdriveMath.calculateEffectiveShareReserves(
                    shareReserves,
                    shareAdjustment
                )
            )
            .toUint128();

If the new effective share reserves is very large, the bond reserves are large, and the old effective share reserves are small, it's possible for this calculation to overflow. In the past, these kinds of overflows created critical vulnerabilities (like in #425) when _updateLiquidity was called in routine operations like _applyCheckpoint. The zeta adjustment logic changes closeLong and closeShort so that they don't utilize _updateLiquidity to apply flat updates to the reserves. This eliminates one potential culprit. All of the uses of _updateLiquidity in the entire codebase (as of the zeta adjustment PR) are listed here:

contracts/src/HyperdriveLP.sol
169:            _updateLiquidity(int256(shares));
504:        _updateLiquidity(-int256(shareProceeds));
529:            _updateLiquidity(int256(overestimatedProceeds));
609:        _updateLiquidity(-int256(withdrawalPoolProceeds));

Of these instances, the reference on line 169 is in addLiquidity, the reference on line 504 is in removeLiquidity, the reference on line 529 is in removeLiquidity, and the reference on line 609 is in _compensateWithdrawalPool. The first three can't be DOS vectors because a user has to choose to call addLiquidity or removeLiquidity. Even with large values of the bond reserves and small values of $z - \zeta$, it should be possible to add a smaller amount of liquidity. It's possible that the bond reserves are extremely large and $z - \zeta$ is small enough that it's impossible to addLiquidity; however, this can be addressed by trading the reserves back towards the middle of the range if the LP really wants to add liquidity. The last reference is the most likely to be a cause for concern because it is used to pay out the withdrawal pool when checkpoints are closed.

Despite being a cause for concern, it does not appear to be problematic because the share reserves delta is negative. With this in mind, the effective share reserves will be decreasing, so even if the bond reserves are large and the effective share reserves are small, the new effective share reserves are smaller. This ensures that the result will safely fit into the uint128 range.

With this analysis in mind, I feel comfortable moving forward with an invariant of $z - \zeta > 0$ for the time being.

[jrhea]

This constraint:

$$ (z_0 - \Delta z) - \zeta_0 \cdot \tfrac{z_0 - \Delta z}{z_0} \geq 0 $$

simplifies algebraically to:

$$ z_0 \geq \zeta_0 $$

not exactly sure why you end up with:

$$ \Delta z \le z $$

it looks to me like you took:

$$ \Delta z \leq z_0 - \tfrac{z_{min}}{1 - \tfrac{\zeta_0}{z_0}} $$

and just substituted $z_{min}$ with 0

I think the implication is the same bc assuming it was already true, then the scaling done by

$$ \zeta_0 \cdot \tfrac{z_0 - \Delta z}{z_0} $$

also means this is true afterwards