Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

idle fix #581

Merged
merged 17 commits into from
Sep 26, 2023
Merged

idle fix #581

merged 17 commits into from
Sep 26, 2023

Conversation

jrhea
Copy link
Contributor

@jrhea jrhea commented Sep 19, 2023

idle fix

Fixes #367, #397.

A detailed explanation of the issue and fix is given in the issue, but the TL;DR is that the previous implementation of the conservation of present value dealt with excess idle funds only during removeLiquidity, which created situations in which the active LPs had control over idle funds that would ultimately go to the withdrawal pool. This PR fixes this issue by updating the logic used to pay out the withdrawal pool in closeLong and closeShort.

netting fix

In order for the idle fix to work properly, a modification was made to how we update the checkpoint exposure. the tl;dr is that we only zero out checkpoint exposure when:

  • it is a long and a checkpoint boundary bc we know all the positions have been closed out. Remember, applyCloseLong() closes out shorts, then longs.
  • it is a short and a checkpoint boundary and there are no longs to close out
  • it is not a checkpoint boundary, but there are no more open positions

NOTE: This PR replaces #370

@jrhea jrhea changed the title Netting bug more netting tests Sep 19, 2023
@coveralls
Copy link
Collaborator

coveralls commented Sep 19, 2023

Coverage Status

coverage: 91.845% (-0.7%) from 92.508% when pulling d635a52 on netting-bug into bdac1aa on main.

@jrhea jrhea changed the title more netting tests idle fix Sep 19, 2023
@jrhea jrhea marked this pull request as ready for review September 21, 2023 20:27
@github-actions
Copy link

github-actions bot commented Sep 21, 2023

Hyperdrive Gas Benchmark

Benchmark suite Current: d635a52 Previous: 0cb80cc Deviation Status
addLiquidity: min 785 gas 785 gas 0% 🟰
addLiquidity: avg 54981 gas 46879 gas 17.2828% 🚨
addLiquidity: max 99007 gas 80516 gas 22.9656% 🚨
checkpoint: min 514 gas 514 gas 0% 🟰
checkpoint: avg 29512 gas 29372 gas 0.4766% 🚨
checkpoint: max 68792 gas 82072 gas -16.1809%
closeLong: min 852 gas 852 gas 0% 🟰
closeLong: avg 22958 gas 21498 gas 6.7913% 🚨
closeLong: max 126406 gas 103566 gas 22.0536% 🚨
closeShort: min 765 gas 765 gas 0% 🟰
closeShort: avg 24042 gas 22248 gas 8.0636% 🚨
closeShort: max 104808 gas 87175 gas 20.2271% 🚨
initialize: min 803 gas 803 gas 0% 🟰
initialize: avg 159750 gas 159627 gas 0.0771% 🚨
initialize: max 233287 gas 233222 gas 0.0279% 🚨
openLong: min 740 gas 740 gas 0% 🟰
openLong: avg 58932 gas 42003 gas 40.3043% 🚨
openLong: max 229308 gas 224368 gas 2.2017% 🚨
openShort: min 782 gas 782 gas 0% 🟰
openShort: avg 61259 gas 49097 gas 24.7714% 🚨
openShort: max 244147 gas 218455 gas 11.7608% 🚨
removeLiquidity: min 762 gas 762 gas 0% 🟰
removeLiquidity: avg 75147 gas 63388 gas 18.5508% 🚨
removeLiquidity: max 171108 gas 123036 gas 39.0715% 🚨

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

@jrhea jrhea mentioned this pull request Sep 22, 2023
Copy link
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.

This is looking great! Nice job broski. Just a few nits, and then let's :shipit:.

jrhea and others added 5 commits September 25, 2023 14:04
* make exposure updates more specific to close functions vs applyCheckpoint

* Update test/integrations/hyperdrive/NonstandardDecimals.sol

* address review comments
@jrhea jrhea enabled auto-merge (squash) September 26, 2023 18:41
@jrhea jrhea merged commit 9e02805 into main Sep 26, 2023
6 checks passed
@jrhea jrhea deleted the netting-bug branch September 26, 2023 18: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.

bug: Withdrawal pool doesn't receive idle when the trade amount is zero
3 participants