wad representation

[ozgunozerk]

Partially Fixes #438

PR Checklist

• Tests
• Documentation

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.02%. Comparing base (6ceeeba) to head (4a6a070).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #498      +/-   ##
==========================================
+ Coverage   94.91%   95.02%   +0.11%     
==========================================
  Files          76       77       +1     
  Lines        5187     5306     +119     
==========================================
+ Hits         4923     5042     +119     
  Misses        264      264              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[brozorec]

1. Document truncation behavior consistently
   Similarly to to_integer():
   • Add to to_token_amount(): "Truncates toward zero when scaling down (token_decimals < 18)."
   • Add to from_token_amount(): "Truncates toward zero when scaling down (token_decimals > 18)."
   • Add to checked_mul(): "Result is truncated toward zero after division by WAD_SCALE."
   • Add to checked_div(): "Result is truncated toward zero."
2. Negatives are supported so abs() can be useful.
3. Can be helpful to provide:

impl Wad {
    pub const ZERO: Wad = Wad(0);
    pub const ONE: Wad = Wad(WAD_SCALE);
}

4. Precision loss: is this inevitable?

    #[test]
    fn test_precision_loss_operation_order() {
        let e = Env::default();

        let principal = Wad::from_integer(&e, 1000);
        let rate = Wad::from_raw(55_000_000_000_000_000);
        let time_fraction = Wad::from_raw(8_333_333_333_333_333);

        let interest_method1 = principal * rate * time_fraction;

        let rate_time = rate * time_fraction;
        let interest_method2 = principal * rate_time;

        let diff = interest_method1.raw() - interest_method2.raw();

        assert!(diff > 0);
    }

    #[test]
    fn test_precision_loss_compound_interest() {
        let e = Env::default();

        let principal = Wad::from_integer(&e, 100);
        let growth_factor = Wad::from_raw(1_010_000_000_000_000_000);

        let mut amount = principal;
        for _ in 0..10 {
            amount = amount * growth_factor;
        }

        let expected = Wad::from_raw(110_462_212_541_000_000_000);

        let diff = amount.raw() - expected.raw();

        assert!(diff > 120_451_000);
    }

    #[test]
    fn test_precision_loss_cross_multiplication() {
        let a = Wad::from_raw(10_000_000_000_000_000_007);
        let b = Wad::from_raw(3_000_000_000_000_000_000);

        let quotient = a / b;
        assert_eq!(quotient.raw(), 3_333_333_333_333_333_335);

        let reconstructed = quotient * b;
        assert_eq!(reconstructed.raw(), 10_000_000_000_000_000_005);

        let lost = a.raw() - reconstructed.raw();
        assert_eq!(lost, 2);
    }

5. Add additional checks for large decimals

    #[test]
    #[should_panic(expected = "attempt to multiply with overflow")]
    fn test_token_decimals_not_validated() {
        let e = Env::default();

        let amount = 1_000_000;
        let invalid_decimals = 57u8;

        let _ = Wad::from_token_amount(&e, amount, invalid_decimals);
    }

[ozgunozerk]

Add additional checks for large decimals

that's good, added the test

Negatives are supported so abs() can be useful.

I had Neg and abs implementation, but removed them. Can add once more, sure.

Can be helpful to provide:
impl Wad {
pub const ZERO: Wad = Wad(0);
pub const ONE: Wad = Wad(WAD_SCALE);
}

Hmm, maybe zero, but definitely not ONE, because it will create ambiguity. Is it 1 token (full token), or 1 smallest unit for the token? I don't think this is a good idea... And I also had zero in the beginning, but since one does not make much sense, I also removed the zero for consistency. They can easily create zero from from_integer(0). I don't see a huge benefit of having this tbh.

Document truncation behavior consistently

good advice, will do!

Precision loss: is this inevitable?

I think yes, but will spend more mental effort on it just in case.

[ozgunozerk]

For any solution we will have for fixed point math, at some point, there will be precision errors. I think this is the innate unavoidable behavior of fixed points, because ultimately, we are using containers like i128 which have their limits.

The precision loss depicted by the tests above are negligible. Let me explain why.

The purpose of any fixed-point math solution, is to not get rid of the precision errors altogether. The purpose is to get rid of the non-negligible, big precision errors. Or, in other words, make the precision errors negligible. For example, 0.002 for the precision error may be important for DeFi applications, right?

Right now, the precision error is 0.000000000000000325, which is, at 10^-16 range.
Try to imagine losing 0.000000000000000325 cents. Well, I couldn't imagine...

I think this is VERY acceptable and expected behavior.

[brozorec]

I'm still not feeling 100% convinced about the operator overloading, but acknowledging the arguments about this design choice. One last thing, there's one miss from the test cov that can be easily covered.

Great job 🙌