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.

Sign up for GitHub

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

Jump to bottom

Max long & short failures in fuzz testing #136

Open
dpaiton opened this issue Jun 11, 2024 · 2 comments
Open

Max long & short failures in fuzz testing #136

dpaiton opened this issue Jun 11, 2024 · 2 comments

Comments

@dpaiton
Copy link
Member

dpaiton commented Jun 11, 2024 •
edited
Loading

Fuzz testing is seeing the following errors:

  • calculate_max_long
    • InsufficientLiquidity: Negative Interest
    • Initial guess in calculate_max_long is insolvent
    • target share reserves less than effective share reserves
  • calculate_max_short
    • absolute_max_short is insolvent
    • arithmetic operation overflow

The fuzz pool is initialized to a known-good state (@slundqui is 80% certain it is good based on visual inspection of the parameters from himself, Jonny, & Alex and comparison against rust CI tests).
Random bots make trades and call calc_max_long and calc_max_short at random times throughout.
@dpaiton dpaiton changed the title Max long & short failures Max long & short failures in fuzz testing Jun 11, 2024
@dpaiton
Copy link
Member Author

dpaiton commented Jun 11, 2024 •
edited
Loading

possibly related:
fuzz_calculate_max_short is failing with large differences in budget amount: #121

This is happening rarely, but I believe that is because the test itself is rarely hitting the assert. I rewrote the test to always hit the assert and it fails much more often.

@dpaiton dpaiton mentioned this issue Jun 12, 2024
Merged
dpaiton added a commit that referenced this issue Jun 12, 2024
@dpaiton @jalextowle
max short fixes (#137)
ce169d6 
# Resolved Issues
Partially addresses #121 & #136


# Description
These changes arose in my other pr (#116) while trying to debug issues
and high tolerances in `short::max`. There are a lot of commits in order
to separate out meaningful changes from trivial ones. A majority of the
changes are code reorganization, docstring fixes, and some additional
sanity checks.

Major changes were:
- `fuzz_calculate_max_short`
- was renamed to more accurately reflect what it is testing -- max short
in the budget constrained regime
- was rewritten to ensure the budget is constrained. I believe the
reason the failures were "intermittent" before was because it was not
always hitting the failure case. It hits it a lot now.
- the tolerance was updated so that it consistently passes, to give us
an idea of how much it is off from what we want.
  
- `calculate_max_short`
this code was incorrect:
 ```
          let absolute_max_deposit =
match self.calculate_open_short(max_bond_amount, open_vault_share_price)
{
                Ok(d) => d,
                Err(_) => return Ok(max_bond_amount),
``` 

we do **not** want to return the bond amount if `calculate_open_short` is throwing an error. I updated it.

I ran the tests locally with `FUZZ_RUNS=1_000` and `FAST_FUZZ_RUNS=50_000` without any tests failing.

---------

Co-authored-by: Alex Towle <jalextowle@gmail.com>
@dpaiton
Copy link
Member Author

dpaiton commented Aug 9, 2024

Related to #185

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@dpaiton