delvtech / hyperdrive

An automated market maker for fixed and variable yield with on-demand terms.
Apache License 2.0
33 stars 4 forks source link

How to handle when `_bondReservesDelta < _shareReservesDelta.mulDown(_sharePrice)` #554

Closed jrhea closed 1 year ago

jrhea commented 1 year ago

This issue can be demonstrated by the following test:

    function test_nonstandard_decimals_long() external {
        uint256 basePaid = 822;
        uint256 holdTime = 42926726755174866498705307300395078586856374274;
        int256 variableRate = 36114043398255272984828782955001847686428098562;

        // Normalize the fuzzed variables.
        initialize(alice, 0.02e18, 500_000_000e6);
        basePaid = basePaid.normalizeToRange(
            0.1e6,
            HyperdriveUtils.calculateMaxLong(hyperdrive)
        );
        holdTime = holdTime.normalizeToRange(0, POSITION_DURATION);
        variableRate = variableRate.normalizeToRange(0, 2e18);

        // Bob opens a long and holds for a random time less than the position
        // duration. He should receive the base he paid plus fixed interest.
        {
            // Deploy and initialize the pool.
            IHyperdrive.PoolConfig memory config = testConfig(0.02e18);
            config.minimumShareReserves = 1e6;
            deploy(deployer, config);
            initialize(alice, 0.02e18, 500_000_000e6);

            // Bob opens a long.
            (uint256 maturityTime, uint256 longAmount) = openLong(
                bob,
                basePaid
            );
            uint256 fixedRate = HyperdriveUtils.calculateAPRFromRealizedPrice(
                basePaid,
                longAmount,
                HyperdriveUtils.calculateTimeRemaining(hyperdrive, maturityTime)
            );

            // The term passes with checkpoints minted regularly
            advanceTime(POSITION_DURATION, variableRate);

            // Bob closes the long.
            (uint256 expectedBaseProceeds, ) = HyperdriveUtils
                .calculateInterest(basePaid, int256(fixedRate), holdTime);
            uint256 baseProceeds = closeLong(bob, maturityTime, longAmount);
            uint256 range = baseProceeds > 1e6
                ? baseProceeds.mulDown(0.01e18)
                : 1e3; // TODO: This is a large bound. Investigate this further
            assertApproxEqAbs(baseProceeds, expectedBaseProceeds, range);
        }
    }

If we insert a revert in _applyCloseLong() when

        if (_bondReservesDelta < _shareReservesDelta.mulDown(_sharePrice)) {
            revert IHyperdrive.ShareReservesDeltaExceedsBondReservesDelta();
        }

The the test above will revert with those inputs; however, if you change the holdTime variable to be equal to POSITION_DURATION it will pass bc the position is closed out _applyCheckpoint() where bondReservDelta and shareReserveDelta are set to 0.

How should we handle this scenario? Also, we might want to add that revert case to open/close short as well

jrhea commented 1 year ago

This no longer reverts on main. #577 fixed this particular issue