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

[CRASH REPORT] fuzz invariant -- continuous random bot checks #702

Closed dpaiton closed 9 months ago

dpaiton commented 11 months ago

Crash Report

https://app.rollbar.com/a/delv/fix/item/FirstProject/40

Description

Bots are running in competition mode (one block is mined every 12 seconds; every mined block accelerates time by 52*12 seconds).

Test: fuzz_bot_invariant_checks.py

Expected Behavior

The expected & actual vault shares should be the same.

# Total shares is correctly calculated
expected_vault_shares = (
    pool_state.pool_info.share_reserves
    + pool_state.pool_info.shorts_outstanding / pool_state.pool_info.share_price
    + pool_state.gov_fees_accrued
    + pool_state.pool_info.withdrawal_shares_proceeds
)
actual_vault_shares = pool_state.vault_shares

if not fp_isclose(expected_vault_shares, actual_vault_shares, abs_tol=epsilon):
  # fail

epsilon = 1e-4, so 1e14 WEI

Actual Behavior

The expected and actual returned amounts do not match. The difference in WEI is always off by the same amount (25 counts, all identically 4.225176e+19, or approximately 40 ETH).

count    2.500000e+01
mean     4.225176e+19
std      0.000000e+00
min      4.225176e+19
25%      4.225176e+19
50%      4.225176e+19
75%      4.225176e+19
max      4.225176e+19
Name: additional_info.invariance_check:vault_shares_difference_in_wei, dtype: float64

Steps to Reproduce

Run the fuzz test

Logs or Stack Traces

fuzz_bots_invariant_checks2023_12_12_18_10_10_Z.json

jalextowle commented 11 months ago

There are a few things that come to mind here. First, we need to incorporate the minimum share reserves into the expected vault shares. If you just add the minimum shares to the expected calculation, that should be fixed. Second, this calculation doesn't account for matured positions. Now that @jrhea has landed #685, we'll be able to update this invariant in the next release.

dpaiton commented 11 months ago

Ok, we will wait for the next release to update this check

slundqui commented 10 months ago

Revising this issue, we're able to reproduce by:

  1. Initialize a pool with default parameters.
  2. Open a short.
  3. Check vault shares

The expected vault shares is calculated below:

expected_vault_shares = (
    pool_state.pool_info.share_reserves
    # The term below replaces this calculation:
    + pool_state.pool_info.shorts_outstanding / pool_state.pool_info.share_price
    + pool_state.gov_fees_accrued
    + pool_state.pool_info.withdrawal_shares_proceeds
    + pool_state.pool_info.zombie_share_reserves
)
actual_vault_shares = yield_contract.functions.balanceOf(hyperdrive_contract.address).call()

Opening a short for 11111 base results in a difference of 5.556133872366779485. Turning off the flat fee reduces the discrepancy here to 0.000634189461510241

slundqui commented 10 months ago

Turning off variable rate and replacing the expected_vault_shares to below fixes this issue:

expected_vault_shares = (
    pool_state.pool_info.share_reserves
    # + pool_state.pool_info.shorts_outstanding / pool_state.pool_info.share_price
    + (
        pool_state.pool_info.shorts_outstanding
        + (pool_state.pool_info.shorts_outstanding * pool_state.pool_config.fees.flat)
    )
    / pool_state.pool_info.share_price
    + pool_state.gov_fees_accrued
    + pool_state.pool_info.withdrawal_shares_proceeds
    + pool_state.pool_info.zombie_share_reserves
)
slundqui commented 9 months ago

The difference here is due to shorts outstanding accumulating variable rate. Solved by using the expected_vault_shares as a lower bound to ensure the pool is always solvent.