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

add market valuation functions #153

Merged
merged 26 commits into from
Jul 22, 2024
Merged

Conversation

MazyGio
Copy link
Contributor

@MazyGio MazyGio commented Jun 25, 2024

Resolved Issues

Adds calculate_value_long and calculate_value_short functions as described in #138 (#138)

Description

formula for value_long:

market_estimate = trading_proceeds - flat_fees_paid - curve_fees_paid

trading_proceeds = flat_value + curve_value
flat_value       = dy * (1 - t)
curve_bonds      = dy * t
curve_value      = curve_bonds * p
                 = dy * t * p
flat_fees_paid   = flat_value * flat_fee
                 = dy * (1 - t) * flat_fee
curve_fees_paid  = (curve_bonds - curve_value) * curve_fee 
                 = dy * t * (1 - p) * curve_fee

dy = bond amount
p  = spotPrice
t  = termRemaining (positionDuration - (currentTimestamp - openTimestamp))

formula for value_short:

market_estimate = yield_accrued + trading_proceeds - curve_fees_paid + flat_fees_returned

yield_accrued      = dy * (c-c0)/c0
trading_proceeds   = dy * (1 - p) * t
curve_fees_paid    = trading_proceeds * curve_fee
flat_fees_returned = dy * t * flat_fee

dy = bond amount
c  = vaultSharePrice (current if non-matured, or checkpoint's if matured)
c0 = openVaultSharePrice
p  = spotPrice
t  = termRemaining (positionDuration - (currentTimestamp - openTimestamp))

Review Checklists

Please check each item before approving the pull request. While going
through the checklist, it is recommended to leave comments on items that are
referenced in the checklist to make sure that they are reviewed.

  • Testing
    • Are there new or updated unit or integration tests?
    • Do the tests cover the happy paths?
    • Do the tests cover the unhappy paths?
    • Are there an adequate number of fuzz tests to ensure that we are
      covering the full input space?
    • If matching Solidity behavior, are there differential fuzz tests that
      ensure that Rust matches Solidity?

@MazyGio
Copy link
Contributor Author

MazyGio commented Jun 25, 2024

I'm not sure about how we'd want to write tests for these functions, as they're basically estimates. Any direction is appreciated!

@MazyGio MazyGio force-pushed the mazygio/add/calculate_value_short_long branch from c5099bd to a0f6fd5 Compare July 5, 2024 00:02
@dpaiton
Copy link
Member

dpaiton commented Jul 5, 2024

I like Mihai's second suggestion, calculate_market_value_* better than calculate_value_*. But that might just be because I'm not native in this field and thus have a preference for more verbose names. Your call in the end.

Copy link
Member

@dpaiton dpaiton left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this, it'll be helpful for several different applications! I made comments on the long/close.rs file; prop those to short & I'll give it another look.

@MazyGio MazyGio enabled auto-merge (squash) July 11, 2024 20:14
@MazyGio MazyGio disabled auto-merge July 11, 2024 20:38
@MazyGio MazyGio marked this pull request as draft July 17, 2024 15:37
@MazyGio
Copy link
Contributor Author

MazyGio commented Jul 17, 2024

test_calculate_market_value_short passes with a tolerance of 1e12. It's worth noting that the test attempts to close a short of size minimum_transaction_amount (range from 0.1 to 1 bonds).

There's a couple things I've been able to drill down on while testing this function:

  1. I've reworked the function to match calculate_close_long as closely as possible. The only difference is now on how both functions calculate the curve part of the trade. This makes up the entire error amount on this test.
  2. Using a relatively large liquidity amount (1e27) by doing .calculate_pool_state_after_add_liquidity(fixed!(1e27), true)?; after the initial State is generated does result in a smaller market_impact, but it somehow creates a larger discrepancy between hyperdrive_valuation and spot_valuation. I can't say for certain why that is, but it makes the hyperdrive_valuation result a bit erratic. I'd expect it to be slightly lower than the spot_valuation, since the hyperdrive_valuation considers market impact, but sometimes it's not.

I suspect small rounding-related inaccuracies when computing calculate_shares_in_given_bonds_out_up_safe, made relatively large due to the large share reserve and adjustment levels compared to the small position size.

Example failure using .calculate_pool_state_after_add_liquidity(fixed!(1e27), true)?;:

==========================================
spot_price:       0.073223040957386746
spot_price_after: 0.073223040957595844
market_impact:        209098         <--[O(1e7). very small!]
valuation_error:      11363446861    <--[O(1e11). pretty large]
spot_valuation:       0.111224043778463171
hyperdrive_valuation: 0.111224055141910032    <--[larger than spot, which is weird]
share_reserves:            676920352.732639701955744293
share_adjustment:          -15427047685553773566980402
effective_share_reserves:  692347400.418193475522724695
failed 603 times.    <--[out of 10k]
==========================================
test short::close::tests::test_calculate_market_value_short ... ok

Example failure without that line:

==========================================
spot_price:       0.353913457944902287
spot_price_after: 0.353913875316959714
market_impact:        417372057427   <--[O(1e12)]
valuation_error:      54535898369    <--[O(1e11). Same order of magnitude as above]
spot_valuation:       0.726982046773411663
hyperdrive_valuation: 0.726981992237513294    <--[when it's higher than spot, the error is smaller than 1e10]
share_reserves:            95894096.977191008442078684
share_adjustment:          95886552995656698674111646
effective_share_reserves:  7543.981534309767967038
failed 17 times.    <--[out of 10k]
==========================================

Both tests used a tolerance of 1e10 (to ensure it failed a bunch of times)

  1. When I don't use the larger liquidity amount, the error is always within one order of magnitude from the price impact. It's worth noting that, in lieu of a calculate_spot_price_after_close_short function, I've approximated the price impact by using calculate_spot_price_after_long, using the amount of non-matured bonds, which should have a fairly similar effect.

Would we be okay with the tests passing with "large" discrepancies between hyperdrive_valuation and spot_valuation, as long as these discrepancies are e.g. within one or two orders of magnitude of the position's market impact?

@MazyGio
Copy link
Contributor Author

MazyGio commented Jul 20, 2024

After reworking these to match their calculate_close_ counterparts, I've tested extensively to figure out a base tolerance, which we can scale depending on the ratio of effective_share_reserves relative to bond_reserves. When this ratio ze / y is too low, it creates a large discrepancy due to the market_impact that affects the hyperdrive valuation, so we bump the base tolerance of 1e12 to 1e13 in such cases.

Thanks @dpaiton for all the help in getting these completed!

@MazyGio MazyGio marked this pull request as ready for review July 20, 2024 05:25
dpaiton added a commit to delvtech/agent0 that referenced this pull request Jul 22, 2024
Chainsync uses `calc_close_*` for estimating closeout value, which could
fail if the pool is in a situation where positions cannot be closed. We
ware working on a general solution for this here:
delvtech/hyperdrive-rs#153

This PR is a temporary solution that approximates the closeout value
using spot price.
@MazyGio MazyGio enabled auto-merge (squash) July 22, 2024 19:43
@MazyGio MazyGio merged commit 88e0281 into main Jul 22, 2024
8 checks passed
@MazyGio MazyGio deleted the mazygio/add/calculate_value_short_long branch July 22, 2024 20:26
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.

2 participants