Skip to content

Wip FixedPoint refactor - #184

Closed
ryangoree wants to merge 16 commits into
mainfrom
ryangoree/fixed-point
Closed

ryangoree wants to merge 16 commits into
mainfrom
ryangoree/fixed-point

Conversation

@ryangoree

Copy link
Copy Markdown
Member

Resolved Issues

closes #127

Description

Comment thread crates/fixedpointmath/src/value.rs Outdated
Comment thread crates/fixedpointmath/src/lib.rs Outdated
Comment thread crates/fixedpointmath/src/lib.rs Outdated
Comment thread bindings/hyperdrivepy/src/hyperdrive_state_methods/long/open.rs Outdated
Comment thread bindings/hyperdrivepy/src/hyperdrive_state_methods/lp/add.rs Outdated
Comment thread bindings/hyperdrivepy/src/hyperdrive_state_methods/lp/remove.rs Outdated
Comment thread crates/fixedpointmath/src/lib.rs Outdated
Comment thread crates/fixedpointmath/src/lib.rs Outdated
// Calculate exp(y * ln(x)) to get x^y
Self::exp(ylnx)?.try_into()
}
// FIXME: What's the correct return value here?

@dpaiton dpaiton Jul 31, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

python float handles this by including special values inf and -inf and nan.

We mimic that behavior in the python fixedpointmath lib:
https://github.com/delvtech/fixedpointmath/blob/main/fixedpointmath/fixed_point.py#L164

then we check that member variable whenever people want to perform an operation.

You can see all of the rules here: https://github.com/delvtech/fixedpointmath/blob/main/tests/test_fp_nonfinite_checks.py

Comment thread crates/hyperdrive-math/src/long/max.rs Outdated
Comment thread crates/hyperdrive-math/src/base.rs Outdated
true => self.share_reserves() - long_exposure - self.minimum_share_reserves(),
false => fixed!(0),
}
// FIXME: This function is redundant with `calculate_solvency`.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that we should only have one function, but I think calculate_idle_share_reserves is more clear than calculate_solvency. The former is more explicit in what is being computed.

If we allowed negative idle share reserves, then we could also implement a check_solvency function that returns a bool based on the sign of calculate_idle_share_reserves. Or we do not allow negative idle share reserves and leave it to the developer to know that this is the function to run to check & calculate solvency.

@ryangoree
ryangoree force-pushed the ryangoree/fixed-point branch from 7384dc2 to fe176ac Compare August 7, 2024 15:33
@ryangoree
ryangoree force-pushed the ryangoree/fixed-point branch from 7f20855 to b34edb0 Compare August 9, 2024 16:22
@@ -1,859 +1,35 @@
//! A generic fixed point wrapper around the `U256` type from ethers-rs.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since ethers-rs is being deprecated, I checked out alloy to see what they use for ints now and they're using ruint which looks nice. Also found uint. It might be nice to refactor to something similar soon.

@ryangoree

Copy link
Copy Markdown
Member Author

closed in favor of #186

@ryangoree ryangoree closed this Aug 12, 2024
ryangoree added a commit that referenced this pull request Aug 13, 2024
Related to: #127

(replaces wip pr: #184)

# Description

This refactors the `FixedPoint` type to be generic with implementations
for `i128`, `u128`, `I256`, and `U256`. For the most part, the library
can be used the same, but with a type param, eg:

```diff
- fn foo() -> FixedPoint {}
+ fn foo() -> FixedPoint<U256> {}
```

However, the API has been expanded and there's plenty code reduction we
can do in future PRs. It's probably also worth rethinking argument types
now that the library can support negative numbers and be bound by
different limits.

## Expanded API

I wrote doc comments throughout to aid in adoption, but below is a
skeleton containing most of the new API to get a quick sense:

```rs
pub struct FixedPoint<T: FixedPointValue> {
    raw: T,
    decimals: u8,
}

impl<T: FixedPointValue> FixedPoint<T> {
    pub const MIN: Self;
    pub const MAX: Self;

    // Constructors //
    pub fn new<V: Into<T>>(value: V) -> Self;
    pub fn try_from<V: TryInto<T> + Debug>(value: V) -> Result<Self>;
    pub fn from_sign_and_abs(sign: FixedPointSign, abs: U256) -> Result<Self>;
    pub fn from_dec_str(s: &str) -> Result<Self>;
    pub fn saturate_sign(sign: FixedPointSign) -> Self;
    pub fn zero() -> Self;
    pub fn one(&self) -> Self;

    // Getters //
    pub fn raw(&self) -> T;
    pub fn decimals(&self) -> u8;
    pub fn sign(&self) -> FixedPointSign;

    // Predicates //
    pub fn is_negative(&self) -> bool;
    pub fn is_positive(&self) -> bool;
    pub fn is_zero(&self) -> bool;

    // Conversion to other FixedPoint types //
    pub fn change_type<U: FixedPointValue + TryFrom<T>>(self) -> Result<FixedPoint<U>>;

    // Conversion to unsigned & signed ethers types //
    pub fn to_u256(self) -> Result<U256>;
    pub fn to_i256(self) -> Result<I256>;

    // Conversion to unsigned and signed std types //
    pub fn to_u128(self) -> Result<u128>;
    pub fn to_i128(self) -> Result<i128>;

    // Formatting //
    pub fn to_scaled_string(&self) -> String;

    // Math //
    pub fn abs(&self) -> Self;
    pub fn unsigned_abs(&self) -> FixedPoint<U256>; // `U256` to avoid overflow.
    pub fn abs_diff(&self, other: Self) -> FixedPoint<U256>;
    pub fn mul_div_down(self, other: Self, divisor: Self) -> Self;
    pub fn mul_div_up(self, other: Self, divisor: Self) -> Self;
    pub fn mul_down(self, other: Self) -> Self;
    pub fn mul_up(self, other: Self) -> Self;
    pub fn div_down(self, other: Self) -> Self;
    pub fn div_up(self, other: Self) -> Self;
    pub fn pow(self, y: Self) -> Result<Self>;
}

// Direct conversions between primitive types and FixedPoint for any
// `FixedPointValue` that can be converted to and from the primitive type.
conversion_impls!(i8, u8, i16, u16, i32, u32, i64, u64, isize, usize, [u8; 32], bool);

// Value //

/// A value that can be used to perform fixed-point math.
pub trait FixedPointValue: /* ... */ {
    const MIN: Self;
    const MAX: Self;
    const MAX_DECIMALS: u8 = 18;

    fn is_signed() -> bool;
    fn is_negative(&self) -> bool;
    fn is_positive(&self) -> bool;
    fn is_zero(&self) -> bool;
    fn flip_sign(self) -> Self;
    fn flip_sign_if(self, condition: bool) -> Self;
    fn abs(self) -> Self;
    fn unsigned_abs(self) -> U256;

    pub fn from_u256(u: U256) -> Result<Self>;
    pub fn from_u128(u: u128) -> Result<Self>;
    
    pub fn to_u256(self) -> Result<U256>;
    pub fn to_u128(self) -> Result<u128>;
}

// Conversions traits //

pub trait Fixed: FixedPointValue {
    fn fixed(self) -> FixedPoint<Self>;
}

// Add `.fixed()` to all types that implement `FixedPointValue`.
impl<T: FixedPointValue> Fixed for T {}

pub trait ToFixed: Sized + Debug {
    fn to_fixed<T: FixedPointValue + From<Self>>(self) -> FixedPoint<T>;
    fn try_to_fixed<T: FixedPointValue + TryFrom<Self>>(self) -> Result<FixedPoint<T>>;
}

// Add `.to_fixed()` & `.try_to_fixed()` to all sized types that implement
// `Debug`.
impl<T: Sized + Debug> ToFixed for T {}

// Macros //

macro_rules! uint256 {}    // -> `U256`
macro_rules! int256 {}     // -> `I256`
macro_rules! fixed {}      // -> `FixedPoint<T>` (where `T` is inferred)
macro_rules! fixed_u256 {} // -> `FixedPoint<U256>`
macro_rules! fixed_i256 {} // -> `FixedPoint<I256>`
macro_rules! fixed_u128 {} // -> `FixedPoint<u128>`
macro_rules! fixed_i128 {} // -> `FixedPoint<i128>`
```
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.

Change FixedPoint to only support From<U128> and adjust tests

3 participants