Skip to content

Conversation

@jrhea
Copy link
Contributor

@jrhea jrhea commented Feb 16, 2023

This PR fixes several rounding issues that were uncovered in tests and adds mulUp and divUp to the FixedPointMathLib. The following issues have been solved:

  1. test_open_long_with_small_amount() with a trade amount of 0.01e18 had an issue where the average maturity time was unable to pass the following check:
        assertApproxEqAbs(
            poolInfoAfter.longAverageMaturityTime,
            maturityTime,
            1
        );

this issue was fixed in _calculateAverageMaturityTime() by using divUp() instead of divDown()

  1. test_close_long_redeem() had an issue where bob ends up with more base than bonds after redemption. This issue was fixed in HyperDriveMathLib's calculateInGivenOut() by ensuring the YieldSpace math isn't called on redemption.

  2. test_open_long() had an issue where a small long position would make the APR go up instead of down. The root cause was a rounding bug in YieldSpaceMath when we would calculate 1/(1-tau). This was fixed by using divUp() instead of divDown(). I mapped out the difference in using divUp() and divDown() with various trade sizes. Here is a summary of what I found:

        trade size: 10e18
        with divDown: 49999999999886067
        with divUp: 49999996900228853
        delta: 49999999999886067 - 49999996900228853 = 3099657200
        trade size: 1e18
        with divDown: 50000006551536041
        with divUp: 49999975554963707
        delta: 50000006551536041 - 49999975554963707 = 30996572304
        trade size: 0.1e18
        with divDown: 50000057980327600
        with divUp: 49999762774876600
        delta: 50000057980327600 - 49999762774876600 = 295205451000
        trade size: 0.01e18
        with divDown: 50000583700909500
        with divUp: 49997631646399300
        delta: 50000583700909500 - 49997631646399300 = 2952054510208

@coveralls
Copy link
Collaborator

coveralls commented Feb 16, 2023

Pull Request Test Coverage Report for Build 4198542259

  • 0 of 6 (0.0%) changed or added relevant lines in 2 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 26.863%

Changes Missing Coverage Covered Lines Changed/Added Lines %
contracts/libraries/FixedPointMath.sol 0 3 0.0%
contracts/libraries/HyperdriveMath.sol 0 3 0.0%
Files with Coverage Reduction New Missed Lines %
contracts/libraries/HyperdriveMath.sol 1 0%
Totals Coverage Status
Change from base Build 4196898498: -0.2%
Covered Lines: 137
Relevant Lines: 510

💛 - Coveralls

@jrhea jrhea changed the title Fixed point math mods numerical precision fixes for long calcs Feb 16, 2023
@jrhea jrhea marked this pull request as ready for review February 16, 2023 21:25
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.

Left a few nits, but good work sir 🖖

@jrhea jrhea mentioned this pull request Feb 16, 2023
@jrhea jrhea merged commit 73419c6 into main Feb 16, 2023
@jrhea jrhea deleted the fixed-point-math-mods branch February 16, 2023 22:00
jalextowle added a commit that referenced this pull request Mar 6, 2024
* Update the rounding behavior of `calculateUpdateLiquidity`

* Round the fee calculations up

* Fixed the fee tests
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.

4 participants