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

Avoid redundant calculations for calculate_close_short_flat_plus_curve #43

Closed
slundqui opened this issue Apr 4, 2024 · 0 comments · Fixed by #95
Closed

Avoid redundant calculations for calculate_close_short_flat_plus_curve #43

slundqui opened this issue Apr 4, 2024 · 0 comments · Fixed by #95
Labels
enhancement New feature or request

Comments

@slundqui
Copy link
Contributor

slundqui commented Apr 4, 2024

delvtech/hyperdrive#916 adds a couple of negative interest checks, which requires a few internal variables used by calculate_close_short_flat_plus_curve. Additionally, a new close_short_governance_fee also includes a potentially redundant close_short_curve_fee. Further work is needed to remove redundant calculations of these internal variables.

@ryangoree ryangoree transferred this issue from delvtech/hyperdrive May 1, 2024
@dpaiton dpaiton added the enhancement New feature or request label May 13, 2024
@dpaiton dpaiton linked a pull request May 13, 2024 that will close this issue
6 tasks
dpaiton added a commit that referenced this issue May 14, 2024
# Description
This PR is the result of an audit of the fee functions. The following
changes were made (one commit per bullet):

1. revert the rounding adjustment from #97. We realized that the bug
actually lies in the solidity _test utils_. This has been fixed so I
reverted the rounding behavior to match.
2. update docstrings to use a more consistent format. I introduce a
convention of my own to use upper-case $\Phi_{c,ol}(...)$ for the fee
function and lower-case $\phi_{c}$ for the fee multiplier constant. The
subscript on the fee function is the fee type $\in c, g, f$ for curve,
gov, and flat; as well as the trade type $\in ol, cl, os, cs$ for
opening or closing a long or short. This replaces the inconsistent
method of using e.g. `c(x)` at some times and `curve_fee` at other
times, or using repeat symbols for different equations.
3. add `maybe_curve_fee` arg to gov fee calculations to fix #43 and
reduce computation.
4. change the rounding behavior to match solidity (note it's matching
the eqs in `internal/HyperdriveBase.sol`
5. the above changes revealed a unit issue in `calculate_targeted_long`
where the gov fee (and derivative) was not converted to shares. I
updated my [overleaf
doc](https://www.overleaf.com/read/jkngspgyqzkf#1de6ce), the docstrings,
and the code itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants