solana-labs / perpetuals

Solana perpetuals reference implementation
Other
66 stars 36 forks source link

Fix math overflow on get_pnl_usd #3

Closed adrena-orex closed 1 year ago

adrena-orex commented 1 year ago

Issue

The function get_pnl_usd returns a MathOverflow error with $1m position size and $25k position price (BTC-ish usecase).

running 1 test
Error: Overflow in 100000000000 * 250000000
thread 'state::pool::test::test_get_pnl_usd' panicked at 'called `Result::unwrap()` on an `Err` value: AnchorError(AnchorError { error_name: "MathOverflow", error_code_number: 6003, error_msg: "Overflow in arithmetic operation", error_origin: Some(Source(Source { filename: "programs/perpetuals/src/math.rs", line: 209 })), compared_values: None })', programs/perpetuals/src/state/pool.rs:1374:14
stack backtrace:
   0: rust_begin_unwind
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/std/src/panicking.rs:575:5
   1: core::panicking::panic_fmt
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/panicking.rs:65:14
   2: core::result::unwrap_failed
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/result.rs:1791:5
   3: core::result::Result<T,E>::unwrap
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/result.rs:1113:23
   4: perpetuals::state::pool::test::test_get_pnl_usd
             at ./src/state/pool.rs:1366:13
   5: perpetuals::state::pool::test::test_get_pnl_usd::{{closure}}
             at ./src/state/pool.rs:1361:5
   6: core::ops::function::FnOnce::call_once
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/ops/function.rs:251:5
   7: core::ops::function::FnOnce::call_once
             at /rustc/69f9c33d71c871fc16ac445211281c6e7a340943/library/core/src/ops/function.rs:251:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
test state::pool::test::test_get_pnl_usd ... FAILED

failures:

failures:
    state::pool::test::test_get_pnl_usd

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 12 filtered out; finished in 0.02s

Solution

Cast variables to u128 during calculations.

Note: I have changed fixtures values with $25k token price and 9 decimals token to make sure-ish there are no other overflow issues in Pool functions. Note 2: Added comments for unit tests (the why) and added _ separator for numbers.

Tests result with patch

package:   /Users/orex/work/perpetuals/programs/perpetuals/Cargo.toml
workspace: /Users/orex/work/perpetuals/Cargo.toml
   Compiling perpetuals v0.1.0 (/Users/orex/work/perpetuals/programs/perpetuals)
    Finished test [unoptimized + debuginfo] target(s) in 2.16s
     Running unittests src/lib.rs (target/debug/deps/perpetuals-930e8997802f4402)

running 9 tests
test state::pool::test::test_get_interest_amount_usd ... ok
test state::pool::test::test_get_liquidation_price ... ok
test state::pool::test::test_get_entry_fee ... ok
test state::pool::test::test_get_close_amount ... ok
test state::pool::test::test_get_fee ... ok
test state::pool::test::test_get_new_ratio ... ok
test state::pool::test::test_get_leverage ... ok
test state::pool::test::test_get_pnl_usd ... ok
test state::pool::test::test_get_price ... ok

test result: ok. 9 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out; finished in 0.00s