Skip to content

Enforce z - zeta >= z_min (except for LPs)#726

Merged
jalextowle merged 6 commits intomainfrom
jalextowle/chore/effective-share-reserves-greater-than-minimum-share-reserves
Jan 19, 2024
Merged

Enforce z - zeta >= z_min (except for LPs)#726
jalextowle merged 6 commits intomainfrom
jalextowle/chore/effective-share-reserves-greater-than-minimum-share-reserves

Conversation

@jalextowle
Copy link
Copy Markdown
Contributor

@jalextowle jalextowle commented Jan 17, 2024

Fixes: #583, #677.

This PR enforces $z - \zeta \geq z_{min}$ for trading operations but continues to allow LP operations like removeLiquidity and the system level _distributeExcessIdle to bring the effective share reserves below the minimum share reserves.

As discussed theoretically in #583 and practically in #677, it is quite dangerous for the system's liveness to allow traders to manipulate the effective share reserves to be very small. A sane cutoff for this manipulation is the minimumShareReserves since this was selected to be a sane minimum for the share reserves which is subject to similar constraints. Enforcing this check is straightforward in closeLong and openShort. Since the LP operations can violate the invariant, we only enforce $z - \zeta \geq z_{min}$ in closeLong when the effective share reserves are decreasing to allow checkpoints to be minted regardless of the current effective share reserves.

The idea of enforcing $z - \zeta \geq z_{min}$ seems reasonable on its face, but it prevents LPs from being able to remove all of the shares from system even if there are no outstanding positions. Attempting to enforce this check would complicate the logic in LPMath.calculateDistributeExcessIdle and would also create situations in which LPs wouldn't be able to remove non-trivial amounts of liquidity. For example, assume that $\zeta > 0$ and $\tfrac{z}{z - \zeta} = 50$. If we enforced $z - \zeta \geq z_{min}$, then $z - \zeta = z_{min}$ after removing the liquidity but $z = 50 \cdot z_{min}$. This is untenable as the ratio increases. With all of this in mind, this PR doesn't enforce the constraint for LPs and instead adds a test demonstrating that liveness is not compromised by LPs removing all of their liquidity even in extreme trading conditions.

In order for the calculateMaxShort machinery to respect the $z - \zeta \geq z_{min}$ constraint, I needed to update the rounding behavior. I also took the opportunity to simplify $\max(z_{min}, z_{min} + \zeta)$ to $z_{min} + \max(\zeta, 0)$.

@jalextowle jalextowle changed the title Enforce $z - \zeta \geq z_{min}$ (except for LPs) Enforce z - zeta >= z_min (except for LPs) Jan 17, 2024
@jalextowle jalextowle requested a review from jrhea January 17, 2024 22:56
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jan 17, 2024

Hyperdrive Gas Benchmark

Benchmark suite Current: 0371ec0 Previous: 2a8c81f Deviation Status
addLiquidity: min 1633 gas 1600 gas 2.0625% 🚨
addLiquidity: avg 66533 gas 64292 gas 3.4857% 🚨
addLiquidity: max 291577 gas 275113 gas 5.9845% 🚨
checkpoint: min 1150 gas 1150 gas 0% 🟰
checkpoint: avg 47770 gas 47670 gas 0.2098% 🚨
checkpoint: max 190769 gas 190992 gas -0.1168%
closeLong: min 1558 gas 1558 gas 0% 🟰
closeLong: avg 28050 gas 28087 gas -0.1317%
closeLong: max 150962 gas 148328 gas 1.7758% 🚨
closeShort: min 1549 gas 1549 gas 0% 🟰
closeShort: avg 29758 gas 29855 gas -0.3249%
closeShort: max 125358 gas 147421 gas -14.9660%
initialize: min 1582 gas 1538 gas 2.8609% 🚨
initialize: avg 213188 gas 213159 gas 0.0136% 🚨
initialize: max 255056 gas 254817 gas 0.0938% 🚨
openLong: min 1487 gas 1487 gas 0% 🟰
openLong: avg 50377 gas 50366 gas 0.0218% 🚨
openLong: max 184813 gas 185024 gas -0.1140%
openShort: min 1608 gas 1608 gas 0% 🟰
openShort: avg 49989 gas 49663 gas 0.6564% 🚨
openShort: max 180216 gas 179974 gas 0.1345% 🚨
redeemWithdrawalShares: min 1575 gas 1575 gas 0% 🟰
redeemWithdrawalShares: avg 19360 gas 20332 gas -4.7806%
redeemWithdrawalShares: max 57733 gas 106163 gas -45.6185%
removeLiquidity: min 1639 gas 1639 gas 0% 🟰
removeLiquidity: avg 149448 gas 148128 gas 0.8911% 🚨
removeLiquidity: max 323845 gas 323757 gas 0.0272% 🚨

This comment was automatically generated by workflow using github-action-benchmark.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Jan 18, 2024

Coverage Status

coverage: 95.216% (+0.007%) from 95.209%
when pulling 0371ec0 on jalextowle/chore/effective-share-reserves-greater-than-minimum-share-reserves
into 6b3cb74 on main.

Comment thread contracts/src/internal/HyperdriveLong.sol Outdated
Comment thread contracts/src/internal/HyperdriveLong.sol
Comment thread contracts/src/internal/HyperdriveShort.sol Outdated
Comment thread contracts/src/internal/HyperdriveLong.sol Outdated
Comment thread test/integrations/hyperdrive/LPWithdrawalTest.t.sol Outdated
Comment thread test/integrations/hyperdrive/LPWithdrawalTest.t.sol Outdated
jalextowle and others added 2 commits January 18, 2024 15:22
Co-authored-by: Jonny Rhea <5555162+jrhea@users.noreply.github.com>
Co-authored-by: Jonny Rhea <5555162+jrhea@users.noreply.github.com>
Comment thread test/integrations/hyperdrive/LPWithdrawalTest.t.sol
Copy link
Copy Markdown
Contributor

@jrhea jrhea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@jalextowle jalextowle enabled auto-merge (squash) January 18, 2024 22:14
@jalextowle jalextowle merged commit 846b944 into main Jan 19, 2024
@jalextowle jalextowle deleted the jalextowle/chore/effective-share-reserves-greater-than-minimum-share-reserves branch January 19, 2024 00:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

3 participants