From dcbb0062d074b3dc5a41a8272794e8410ea4c5a6 Mon Sep 17 00:00:00 2001 From: Malte Kliemann Date: Tue, 23 Jan 2024 13:13:37 +0100 Subject: [PATCH] Extend neo-swaps tests and clean up `math.rs` (#1238) * Add tests to neo-swaps that check large buy/sell amounts at high liquidity * Use new implementation of HydraDX's `exp` * Add failure tests to neo-swaps's `math.rs` * Fix formatting * Update copyright notices --------- Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- Cargo.lock | 4 +- Cargo.toml | 2 +- zrml/neo-swaps/src/consts.rs | 3 +- zrml/neo-swaps/src/math.rs | 136 +++++++++++++++++++++++++++-------- 4 files changed, 110 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3363f08a2..56a6c80f1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3606,8 +3606,8 @@ checksum = "9a3a5bfb195931eeb336b2a7b4d761daec841b97f947d34394601737a7bba5e4" [[package]] name = "hydra-dx-math" -version = "7.4.3" -source = "git+https://github.com/galacticcouncil/HydraDX-node?tag=v18.0.0#6173a8b0661582247eed774330aa8fa6d99d524d" +version = "7.7.0" +source = "git+https://github.com/galacticcouncil/HydraDX-node?tag=v21.1.1#564553c30caa1eccfa1b6504ec8d5130201c8765" dependencies = [ "fixed", "num-traits", diff --git a/Cargo.toml b/Cargo.toml index 1b0769fc1..ccb25afea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -263,7 +263,7 @@ arrayvec = { version = "0.7.4", default-features = false } cfg-if = { version = "1.0.0" } fixed = { version = "=1.15.0", default-features = false, features = ["num-traits"] } # Using math code directly from the HydraDX node repository as https://github.com/galacticcouncil/hydradx-math is outdated and has been archived in May 2023. -hydra-dx-math = { git = "https://github.com/galacticcouncil/HydraDX-node", package = "hydra-dx-math", tag = "v18.0.0", default-features = false } +hydra-dx-math = { git = "https://github.com/galacticcouncil/HydraDX-node", package = "hydra-dx-math", tag = "v21.1.1", default-features = false } # Hashbrown works in no_std by default and default features are used in Rikiddo hashbrown = { version = "0.12.3", default-features = true } hex-literal = { version = "0.3.4", default-features = false } diff --git a/zrml/neo-swaps/src/consts.rs b/zrml/neo-swaps/src/consts.rs index 0a12edf64..19d0af5f7 100644 --- a/zrml/neo-swaps/src/consts.rs +++ b/zrml/neo-swaps/src/consts.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Forecasting Technologies LTD. +// Copyright 2023-2024 Forecasting Technologies LTD. // // This file is part of Zeitgeist. // @@ -70,3 +70,4 @@ pub(crate) const _1_10: u128 = _1 / 10; pub(crate) const _2_10: u128 = _2 / 10; pub(crate) const _3_10: u128 = _3 / 10; pub(crate) const _4_10: u128 = _4 / 10; +pub(crate) const _9_10: u128 = _9 / 10; diff --git a/zrml/neo-swaps/src/math.rs b/zrml/neo-swaps/src/math.rs index 7e6a7d2db..276b20fc8 100644 --- a/zrml/neo-swaps/src/math.rs +++ b/zrml/neo-swaps/src/math.rs @@ -1,4 +1,4 @@ -// Copyright 2023 Forecasting Technologies LTD. +// Copyright 2023-2024 Forecasting Technologies LTD. // // This file is part of Zeitgeist. // @@ -34,8 +34,7 @@ // // Original source: https://github.com/encointer/substrate-fixed // -// The changes applied are: 1) Used same design for definition of `exp` -// as in the source. 2) Re-used and extended tests for `exp` and other +// The changes applied are: Re-used and extended tests for `exp` and other // functions. use crate::{ @@ -277,6 +276,10 @@ mod detail { amount: FixedType, spot_prices: Vec, ) -> Option<(FixedType, Vec)> { + if amount.is_zero() { + // Ensure that if the amount is zero, we don't accidentally return meaningless results. + return None; + } let tmp_reserves = spot_prices .iter() // Drop the bool (second tuple component) as ln(p) is always negative. @@ -308,22 +311,7 @@ mod detail { } mod transcendental { - use fixed::traits::FixedUnsigned; - pub(crate) use hydra_dx_math::transcendental::{exp as inner_exp, ln}; - use sp_runtime::traits::One; - - pub(crate) fn exp(operand: S, neg: bool) -> Result - where - S: FixedUnsigned + PartialOrd + One, - D: FixedUnsigned + PartialOrd + From + One, - { - if operand == S::one() && neg { - let e_inverse = - S::from_str("0.367879441171442321595523770161460867445").map_err(|_| ())?; - return Ok(D::from(e_inverse)); - } - inner_exp(operand, neg) - } + pub(crate) use hydra_dx_math::transcendental::{exp, ln}; #[cfg(test)] mod tests { @@ -337,7 +325,7 @@ mod transcendental { #[test_case("0", false, "1")] #[test_case("0", true, "1")] - #[test_case("1", false, "2.718281828459045235360287471352662497757")] + #[test_case("1", false, "2.7182818284590452353")] #[test_case("1", true, "0.367879441171442321595523770161460867445")] #[test_case("2", false, "7.3890560989306502265")] #[test_case("2", true, "0.13533528323661269186")] @@ -411,6 +399,7 @@ mod tests { use super::*; use crate::{consts::*, mock::Runtime as MockRuntime}; use alloc::str::FromStr; + use frame_support::assert_err; use test_case::test_case; type MockBalance = BalanceOf; @@ -431,6 +420,19 @@ mod tests { #[test_case(_30, _30, _1 - 100_000, _30)] #[test_case(_1_10, _30, _1 - 100_000, _1_10)] #[test_case(_30, _1_10, _1 - 100_000, 276_478_645_689)] + #[test_case(10_000_000 * _1, _1, 144_269_504_088_896_352, 9_999_999_307)] + #[test_case( + 100_000_000 * _1, + 100_000_000 * _1, + 434_294_481_903_251_840, + 959_041_392_321_093_596 + )] + #[test_case( + 45_757_490_560_675_120, + 100_000_000 * _1, + 434_294_481_903_251_840, + 41_392_685_158_225_036 + )] fn calculate_swap_amount_out_for_buy_works( reserve: MockBalance, amount_in: MockBalance, @@ -443,6 +445,22 @@ mod tests { ); } + #[test_case(_1, _1, 0)] // Division by zero + #[test_case(_1, 1_000 * _1, _1)] // Overflow + #[test_case(u128::MAX, _1, _1)] // to_fixed error + #[test_case(_1, u128::MAX, _1)] // to_fixed error + #[test_case(_1, _1, u128::MAX)] // to_fixed error + fn calculate_swap_amount_out_for_buy_throws_math_error( + reserve: MockBalance, + amount_in: MockBalance, + liquidity: MockBalance, + ) { + assert_err!( + MockMath::calculate_swap_amount_out_for_buy(reserve, amount_in, liquidity), + Error::::MathError + ); + } + #[test_case(_10, _10, 144_269_504_088, 41_503_749_928)] #[test_case(_1, _1, _1, 2_646_743_359)] #[test_case(_2, _2, _2, 5_293_486_719)] @@ -454,6 +472,19 @@ mod tests { #[test_case(_30, _30, _1 - 100_000, 0)] #[test_case(_1_10, _30, _1 - 100_000, 23_521_354_311)] #[test_case(_30, _1_10, _1 - 100_000, 0)] + #[test_case(10_000_000 * _1, _1, 144_269_504_088_896_352, 4_999_999_913)] + #[test_case( + 100_000_000 * _1, + 100_000_000 * _1, + 434_294_481_903_251_840, + 40_958_607_678_906_404 + )] + #[test_case( + 45_757_490_560_675_120, + 100_000_000 * _1, + 434_294_481_903_251_840, + 721_246_399_047_171_074 + )] fn calculate_swap_amount_out_for_sell_works( reserve: MockBalance, amount_in: MockBalance, @@ -466,9 +497,21 @@ mod tests { ); } - #[test] - fn calculate_swap_amount_out_for_sell_fails_if_reserve_is_zero() { - assert!(MockMath::calculate_swap_amount_out_for_sell(0, _1, _1).is_err()); + #[test_case(0, _1, _1)] + #[test_case(_1, _1, 0)] // Division by zero + #[test_case(1000 * _1, _1, _1)] // Overflow + #[test_case(u128::MAX, _1, _1)] // to_fixed error + #[test_case(_1, u128::MAX, _1)] // to_fixed error + #[test_case(_1, _1, u128::MAX)] // to_fixed error + fn calculate_swap_amount_out_for_sell_throws_math_error( + reserve: MockBalance, + amount_in: MockBalance, + liquidity: MockBalance, + ) { + assert_err!( + MockMath::calculate_swap_amount_out_for_sell(reserve, amount_in, liquidity), + Error::::MathError + ); } #[test_case(_10, 144_269_504_088, _1_2)] @@ -482,6 +525,17 @@ mod tests { assert_eq!(MockMath::calculate_spot_price(reserve, liquidity).unwrap(), expected); } + #[test_case(_1, 0)] // Division by zero + #[test_case(1_000 * _1, _1)] // Overflow + #[test_case(u128::MAX, _1)] // to_fixed error + #[test_case(_1, u128::MAX)] // to_fixed error + fn calculate_spot_price_throws_math_error(reserve: MockBalance, liquidity: MockBalance) { + assert_err!( + MockMath::calculate_spot_price(reserve, liquidity), + Error::::MathError + ); + } + #[test_case(_10, vec![_1_2, _1_2], vec![_10, _10], 144_269_504_089)] #[test_case(_20, vec![_3_4, _1_4], vec![_10 - 58_496_250_072, _20], 144_269_504_089)] #[test_case( @@ -496,6 +550,12 @@ mod tests { vec![_100, _100, _100, 30_673_687_183], 188_739_165_818 )] + #[test_case( + 100_000_000 * _1, + vec![_1_10, _9_10], + vec![100_000_000 * _1, 45_757_490_560_675_125], + 434_294_481_903_251_828 + )] fn calculate_reserves_from_spot_prices_works( amount: MockBalance, spot_prices: Vec, @@ -508,19 +568,33 @@ mod tests { assert_eq!(reserves, expected_reserves); } - #[test_case(_10, _10, 144_269_504_088, _1 + _1_2)] - #[test_case(_10, _1, 144_269_504_088, 5_717_734_625)] - #[test_case(_1, _1, _1, 20_861_612_696)] - #[test_case(_444, _1, _11, 951_694_399; "underflow_to_zero")] + #[test_case(0, vec![_1_10, _2_10, _3_10, _4_10])] + #[test_case(u128::MAX, vec![_1_10, _2_10, _3_10, _4_10])] // to_fixed error + #[test_case(_1, vec![u128::MAX, 0, 0])] // to_fixed error + #[test_case(_1, vec![_1, 0])] // ln out of range + fn calculate_reserves_from_spot_prices_throws_math_error( + amount: MockBalance, + spot_prices: Vec, + ) { + assert_err!( + MockMath::calculate_reserves_from_spot_prices(amount, spot_prices), + Error::::MathError + ); + } + + #[test_case(_1, _1, 0)] // Division by zero + #[test_case(_1, 1_000 * _1, _1)] // Overflow + #[test_case(u128::MAX, _1, _1)] // to_fixed error + #[test_case(_1, u128::MAX, _1)] // to_fixed error + #[test_case(_1, _1, u128::MAX)] // to_fixed error fn calculate_buy_ln_argument_fixed_works( reserve: MockBalance, amount_in: MockBalance, liquidity: MockBalance, - expected: MockBalance, ) { - assert_eq!( - MockMath::calculate_buy_ln_argument(reserve, amount_in, liquidity).unwrap(), - expected + assert_err!( + MockMath::calculate_buy_ln_argument(reserve, amount_in, liquidity), + Error::::MathError ); }