Skip to content

Commit

Permalink
Fix fuzz max long test (#101)
Browse files Browse the repository at this point in the history
# Resolved Issues
Partially resolves these two:
#88
#45

# Description
The fee rounding behavior was adjusted recently to more closely match
solidity, but the order of operations was not exactly the same. This
caused an occasional slight variation in output (usually off by 1e1),
that compounded in situations where we were iteratively assessing fees
such as `max_long`. In that case, the error would grow to as much as
1e5.

During my effort to find this bug I
- modified some math to use the e.g. `.mul_down` syntax instead of `*`
where it was sufficiently complicated to require this to easily pattern
match against Solidity.
- modified `calculate_max_long` to return errors instead of panic. This
makes checks in the tests ugly, but that is a temporary problem until we
purge the panics all-together
(#20)
- purged the use of `traced` and `trace_test` which is helpful for
debugging but was not actively being used and can slow down tests.

# Review Checklists

Please check each item **before approving** the pull request. While
going
through the checklist, it is recommended to leave comments on items that
are
referenced in the checklist to make sure that they are reviewed.

- [ ] **Testing**
    - [ ] Are there new or updated unit or integration tests?
    - [ ] Do the tests cover the happy paths?
    - [ ] Do the tests cover the unhappy paths?
- [ ] Are there an adequate number of fuzz tests to ensure that we are
          covering the full input space?
- [ ] If matching Solidity behavior, are there differential fuzz tests
that
          ensure that Rust matches Solidity?
  • Loading branch information
dpaiton authored May 16, 2024
1 parent 8bcaa63 commit c28bcdd
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 101 deletions.
9 changes: 6 additions & 3 deletions bindings/hyperdrivepy/src/hyperdrive_state_methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,12 @@ impl HyperdriveState {
let checkpoint_exposure_i = I256::from_dec_str(checkpoint_exposure).map_err(|_| {
PyErr::new::<PyValueError, _>("Failed to convert checkpoint_exposure string to I256")
})?;
let result_fp =
self.state
.calculate_max_long(budget_fp, checkpoint_exposure_i, maybe_max_iterations);
let result_fp = self
.state
.calculate_max_long(budget_fp, checkpoint_exposure_i, maybe_max_iterations)
.map_err(|e| {
PyErr::new::<PyValueError, _>(format!("Failed to calculate max long: {}", e))
})?;
let result = U256::from(result_fp).to_string();
Ok(result)
}
Expand Down
1 change: 0 additions & 1 deletion crates/hyperdrive-math/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ impl YieldSpace for State {

#[cfg(test)]
mod tests {
use eyre::Result;
use rand::thread_rng;

use super::*;
Expand Down
8 changes: 4 additions & 4 deletions crates/hyperdrive-math/src/long/fees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ impl State {
/// $$
pub fn open_long_curve_fee(&self, base_amount: FixedPoint) -> FixedPoint {
// NOTE: Round up to overestimate the curve fee.
(fixed!(1e18).div_up(self.calculate_spot_price()) - fixed!(1e18))
.mul_up(self.curve_fee())
self.curve_fee()
.mul_up(fixed!(1e18).div_up(self.calculate_spot_price()) - fixed!(1e18))
.mul_up(base_amount)
}

Expand All @@ -38,9 +38,9 @@ impl State {
None => self.open_long_curve_fee(base_amount),
};
// NOTE: Round down to underestimate the governance curve fee.
self.calculate_spot_price()
.mul_down(curve_fee)
curve_fee
.mul_down(self.governance_lp_fee())
.mul_down(self.calculate_spot_price())
}

/// Calculates the curve fee paid when closing longs for a given bond amount.
Expand Down
Loading

0 comments on commit c28bcdd

Please sign in to comment.