SDK calc_close_* not discounting zombie interest

[slundqui]

There's a bug where calling rust's calc_close_long or calc_close_short when calculating the unrealized value of matured, but not closed, positions. Since the rust functions under the hood returns shares, agent0 converts it back to base using the current vault share price. This is correct when the position isn't matured, but since hyperdrive auto closes matured positions via checkpointing, we should be using the vault share price of when the position matured to convert from shares to base.

This issue is exacerbated when a user asks for shares back when closing (in the case of e.g., steth, where agent0 makes trades using yield), as there is a difference between the shares gotten from rust and the actual shares returned when closed due to the same issue. Here, the solution is:

user_return_in_shares = rust_shares_returned * maturity_share_price / current_share_price

[dpaiton]

I wrote this test for rust

    #[tokio::test]
    async fn fuzz_calculate_close_long_after_maturity() -> Result<()> {
        let mut rng = thread_rng();
        for _ in 0..*FAST_FUZZ_RUNS {
            let state = rng.gen::<State>();
            let in_ = rng.gen_range(fixed!(0)..=state.effective_share_reserves()?);
            let maturity_time = state.position_duration();
            let early_time = rng.gen_range(fixed!(0)..=maturity_time);
            let base_earned_before_maturity =
                state.calculate_close_long(in_, maturity_time.into(), early_time.into())?
                    * state.vault_share_price();
            let base_earned_just_after_maturity = state.calculate_close_long(
                in_,
                maturity_time.into(),
                (maturity_time + fixed!(1e2)).into(),
            )? * state.vault_share_price();
            let base_earned_well_after_maturity = state.calculate_close_long(
                in_,
                maturity_time.into(),
                (maturity_time + fixed!(1e9)).into(),
            )? * state.vault_share_price();
            assert!(
                base_earned_before_maturity <= base_earned_just_after_maturity,
                "User lost money holding the long: earnings_before={:?} > earnings_after={:?}",
                base_earned_before_maturity,
                base_earned_just_after_maturity
            );
            assert!(
                base_earned_well_after_maturity == base_earned_just_after_maturity,
                "User should not have earned any more after maturity. {:?} != {:?}",
                base_earned_well_after_maturity,
                base_earned_just_after_maturity
            );
        }

        Ok(())
    }

@MazyGio can you confirm that this matches your intuition?

[dpaiton]

PR w/ this test is here: https://github.com/delvtech/hyperdrive-rs/pull/196

[dpaiton]

The test is failing and should pass. Although vault_share_price is unchanging, simulating 0% yield in the underlying source, the trader should still not make any additional money between "just after maturity" and "well after maturity"

Per @MazyGio :

base_earned_just_after_maturity and base_earned_well_after_maturity should just be doing the same flat trade, so they should still be equal even if there's no interest being accrued, so this test should still pass if there weren't any underlying issues

[MazyGio]

I got curious, ran the test and printed a few of things to see what was going on. It turns out that the just_after time is not quite at maturity yet, which is why the position seems to accrue a bit more value afterwards. I added an after_well_after step where the time is maturity_time + fixed!(1e10) that confirms the code behaves as expected after actual maturity.

It seems the position isn't matured at thejust_after time yet because we calculate the time remaining based on checkpoints. We compare the maturity_time with the latest_checkpoint and it turns out that, for this test's setup, a checkpoint isn't meant to be minted until a little bit later than the time at position_duration.

running 1 test
normalized_time_remaining_early:            0.202883671928705179
normalized_time_remaining_just_after:       0.000403319517020852
normalized_time_remaining_well_after:       0.000000000000000000
normalized_time_remaining_after_well_after: 0.000000000000000000
base_earned_before_maturity:           2597065.352581268471177009
base_earned_just_after_maturity:       2605131.829373042960698870
base_earned_well_after_maturity:       2605147.070122618659783645
base_earned_after_well_after_maturity: 2605147.070122618659783645
maturity_time: 0.000000000016768343

One option could be to use maturity_time + state.checkpoint_duration() for the just_after step to ensure that the position is actually matured.

[dpaiton]

Updated test in the PR. I changed just_after_maturity = state.position_duration() + state.checkpoint_duration(), which should fully cover any combination of those two values. The earlier test is now failing:

User lost money holding the long: earnings_before=FixedPoint(16384052.613607627475940513) > earnings_after=FixedPoint(16158949.286573306770097460)

[dpaiton]

I confirmed, @MazyGio -- the fees account for the discrepancy. I simplified the test to only look at proceeds after maturity & added a short one.

The tests in #196 demonstrate that the problem is not an issue. Closing when that PR merges.