Skip to content

Infinite Chocolate Fix#661

Merged
jrhea merged 14 commits intomainfrom
rounding-fix
Nov 9, 2023
Merged

Infinite Chocolate Fix#661
jrhea merged 14 commits intomainfrom
rounding-fix

Conversation

@jrhea
Copy link
Copy Markdown
Contributor

@jrhea jrhea commented Nov 9, 2023

This PR is a followup to PR #594 and provides a better fix to issue #587 in that it allows for infinite netting within a checkpoint (the previous fix simply disregarded the shortDeposit when calculating exposure for openShort()).

Summary of the Problem and Fix:
Edge cases were found fuzz testing that caused longs to be insolvent in rare cases. What would happen is that after the shorts were closed in _applyCheckpoint() it would underflow when trying to close out the longs. This would happen bc fuzzing found cases were a short would have a larger deposit than a long's fixed rate despite the short being for less bonds than the long (see issue #587). I determined that this was caused by numerical inaccuracies in fixed point math pow(). The fix was to round the short's base deposit part of the exposure calc down by the magnitude of the numerical error. This ensures that when LP's withdraw, the exposure is high enough to ensure there is enough left in the share reserves to allow longs to close at maturity.

Notes:

  • New PoolConfig variable called precisionThreshold was added to make the fix configurable.

  • resolves Exposure Symmetry Tests #662 by adding checkpoint exposure check in IntraCheckpointNettingTest.open_close_long_short

  • In the process of implementing this fix, I audited OpenLong, CloseLong, OpenShort, and CloseShort for consistency.

OpenLong

  • state deltas are calculated from result of deposit()
  • _minimumTransactionAmount is checked against output from deposit

NOTE: This will cause some tests to fail that use minimumTransactionAmount as a lowerBound for fuzzing inputs. Including steth and sdai tests.

  • minOutput is checked against values derived from deposit()
  • bondProceeds is calculated from output of deposit and is checked against minOutput

CloseLong

  • state deltas are calculated from the external function inputs
  • minOutput is checked against output from withdraw()

OpenShort

  • state deltas are calculated from the function input
  • _maxDeposit shouldn't be checked against the output
    of deposit bc slippage could make a larger than maxDeposit
    potentially pass the check

CloseShort

  • minOutput is checked against output from withdraw
  • state deltas are calculated from the external function inputs

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Nov 9, 2023

Coverage Status

coverage: 97.401% (-0.2%) from 97.592%
when pulling 6b2f220 on rounding-fix
into 2f638cb on main.

Comment thread test/integrations/hyperdrive/LpWithdrawalTest.t.sol
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 9, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: 6b2f220 Previous: 2f638cb Deviation Status
addLiquidity: min 755 gas 755 gas 0% 🟰
addLiquidity: avg 51475 gas 52287 gas -1.5530%
addLiquidity: max 97684 gas 97684 gas 0% 🟰
checkpoint: min 558 gas 558 gas 0% 🟰
checkpoint: avg 47612 gas 47626 gas -0.0294%
checkpoint: max 99085 gas 99410 gas -0.3269%
closeLong: min 755 gas 755 gas 0% 🟰
closeLong: avg 24357 gas 24257 gas 0.4123% 🚨
closeLong: max 114894 gas 114894 gas 0% 🟰
closeShort: min 713 gas 713 gas 0% 🟰
closeShort: avg 26958 gas 27198 gas -0.8824%
closeShort: max 113370 gas 113370 gas 0% 🟰
initialize: min 706 gas 706 gas 0% 🟰
initialize: avg 178066 gas 178066 gas 0% 🟰
initialize: max 252327 gas 252327 gas 0% 🟰
openLong: min 757 gas 757 gas 0% 🟰
openLong: avg 57852 gas 57649 gas 0.3521% 🚨
openLong: max 219289 gas 219128 gas 0.0735% 🚨
openShort: min 712 gas 712 gas 0% 🟰
openShort: avg 57104 gas 56978 gas 0.2211% 🚨
openShort: max 218776 gas 218660 gas 0.0531% 🚨
removeLiquidity: min 777 gas 777 gas 0% 🟰
removeLiquidity: avg 78634 gas 78286 gas 0.4445% 🚨
removeLiquidity: max 204374 gas 204374 gas 0% 🟰

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

@jrhea jrhea requested a review from jalextowle November 9, 2023 21:39
Comment thread test/integrations/hyperdrive/IntraCheckpointNettingTest.t.sol
Comment thread contracts/src/HyperdriveBase.sol
Comment thread contracts/src/HyperdriveLP.sol Outdated
Comment thread contracts/src/HyperdriveLong.sol
Comment thread contracts/src/HyperdriveShort.sol Outdated
Co-authored-by: Alex Towle <jalextowle@gmail.com>
Comment thread contracts/src/HyperdriveShort.sol Outdated
Copy link
Copy Markdown
Contributor

@jalextowle jalextowle left a comment

Choose a reason for hiding this comment

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

LGTM

@jrhea jrhea enabled auto-merge (squash) November 9, 2023 22:35
@jrhea jrhea merged commit abda55e into main Nov 9, 2023
@jrhea jrhea deleted the rounding-fix branch November 9, 2023 23:58
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.

Exposure Symmetry Tests

3 participants