From e517e6604f76e6d778c2a3f982dbd8187607d90f Mon Sep 17 00:00:00 2001 From: Evgeny Svirsky Date: Fri, 5 Jun 2026 13:16:38 +0200 Subject: [PATCH 1/3] Fix for arithmetic side effect --- pallets/limit-orders/src/lib.rs | 26 +++++++++++++++++--------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/pallets/limit-orders/src/lib.rs b/pallets/limit-orders/src/lib.rs index 9acbe8338d..cc546ca10e 100644 --- a/pallets/limit-orders/src/lib.rs +++ b/pallets/limit-orders/src/lib.rs @@ -1015,7 +1015,8 @@ pub mod pallet { }; Ok((OrderSide::Buy, actual_alpha)) } else { - let total_buy_alpha_equiv = Self::tao_to_alpha(total_buy_net, current_price); + let total_buy_alpha_equiv = + Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(u128::MAX); let net_alpha = (total_sell_net.saturating_sub(total_buy_alpha_equiv)) as u64; let actual_tao = if net_alpha > 0 { let out = T::SwapInterface::sell_alpha( @@ -1054,7 +1055,9 @@ pub mod pallet { ) -> DispatchResult { let total_alpha: u128 = match net_side { OrderSide::Buy => actual_out.saturating_add(total_sell_net), - OrderSide::Sell => Self::tao_to_alpha(total_buy_net, current_price), + OrderSide::Sell => { + Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(0u128) + } }; for e in buys.iter() { @@ -1210,26 +1213,31 @@ pub mod pallet { match net_side { OrderSide::Buy => (total_buy_net.saturating_sub(total_sell_tao_equiv)) as u64, OrderSide::Sell => { - let buy_alpha_equiv = Self::tao_to_alpha(total_buy_net, current_price) as u64; + let buy_alpha_equiv = + Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(0u128) as u64; (total_sell_net as u64).saturating_sub(buy_alpha_equiv) } } } /// Convert a TAO amount to alpha at `price` (TAO/alpha). - /// Returns 0 when `price` is zero. - #[allow(clippy::arithmetic_side_effects)] - fn tao_to_alpha(tao: u128, price: U96F32) -> u128 { + /// + /// A zero `price` yields `Some(0)` (no alpha is purchasable). `None` + /// signals a genuine fixed-point overflow, which each caller must + /// saturate in the direction that fails closed for its own use. + fn tao_to_alpha(tao: u128, price: U96F32) -> Option { if price == U96F32::from_num(0u32) { - return 0u128; + return Some(0u128); } - (U96F32::from_num(tao) / price).saturating_to_num::() + U96F32::saturating_from_num(tao) + .checked_div(price) + .map(|alpha| alpha.saturating_to_num::()) } /// Convert an alpha amount to TAO at `price` (TAO/alpha). fn alpha_to_tao(alpha: u128, price: U96F32) -> u128 { price - .saturating_mul(U96F32::from_num(alpha)) + .saturating_mul(U96F32::saturating_from_num(alpha)) .saturating_to_num::() } } From 1bad8be7aa72a00d1ac718368079bba1320d12cc Mon Sep 17 00:00:00 2001 From: Evgeny Svirsky Date: Fri, 5 Jun 2026 13:32:40 +0200 Subject: [PATCH 2/3] Fix PR comment - throw error instead of saturating the value --- pallets/limit-orders/src/lib.rs | 31 ++++++++++----------- pallets/limit-orders/src/tests/auxiliary.rs | 9 ++++-- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/pallets/limit-orders/src/lib.rs b/pallets/limit-orders/src/lib.rs index cc546ca10e..820550e638 100644 --- a/pallets/limit-orders/src/lib.rs +++ b/pallets/limit-orders/src/lib.rs @@ -346,6 +346,8 @@ pub mod pallet { /// Call on_runtime_upgrade or wait for genesis to complete registration /// before enabling the pallet. PalletHotkeyNotRegistered, + /// A TAO -> alpha conversion overflowed the fixed-point range. + ArithmeticOverflow, } // ── Hooks ───────────────────────────────────────────────────────────────── @@ -867,7 +869,7 @@ pub mod pallet { total_sell_net, total_sell_tao_equiv, current_price, - ); + )?; Self::deposit_event(Event::GroupExecutionSummary { netuid, net_side, @@ -1015,8 +1017,7 @@ pub mod pallet { }; Ok((OrderSide::Buy, actual_alpha)) } else { - let total_buy_alpha_equiv = - Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(u128::MAX); + let total_buy_alpha_equiv = Self::tao_to_alpha(total_buy_net, current_price)?; let net_alpha = (total_sell_net.saturating_sub(total_buy_alpha_equiv)) as u64; let actual_tao = if net_alpha > 0 { let out = T::SwapInterface::sell_alpha( @@ -1055,9 +1056,7 @@ pub mod pallet { ) -> DispatchResult { let total_alpha: u128 = match net_side { OrderSide::Buy => actual_out.saturating_add(total_sell_net), - OrderSide::Sell => { - Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(0u128) - } + OrderSide::Sell => Self::tao_to_alpha(total_buy_net, current_price)?, }; for e in buys.iter() { @@ -1209,29 +1208,29 @@ pub mod pallet { total_sell_net: u128, total_sell_tao_equiv: u128, current_price: U96F32, - ) -> u64 { + ) -> Result { match net_side { - OrderSide::Buy => (total_buy_net.saturating_sub(total_sell_tao_equiv)) as u64, + OrderSide::Buy => Ok((total_buy_net.saturating_sub(total_sell_tao_equiv)) as u64), OrderSide::Sell => { - let buy_alpha_equiv = - Self::tao_to_alpha(total_buy_net, current_price).unwrap_or(0u128) as u64; - (total_sell_net as u64).saturating_sub(buy_alpha_equiv) + let buy_alpha_equiv = Self::tao_to_alpha(total_buy_net, current_price)? as u64; + Ok((total_sell_net as u64).saturating_sub(buy_alpha_equiv)) } } } /// Convert a TAO amount to alpha at `price` (TAO/alpha). /// - /// A zero `price` yields `Some(0)` (no alpha is purchasable). `None` - /// signals a genuine fixed-point overflow, which each caller must - /// saturate in the direction that fails closed for its own use. - fn tao_to_alpha(tao: u128, price: U96F32) -> Option { + /// A zero `price` yields `Ok(0)` (no alpha is purchasable). A genuine + /// fixed-point overflow returns `Err(ArithmeticOverflow)` so the caller + /// aborts the batch. + fn tao_to_alpha(tao: u128, price: U96F32) -> Result { if price == U96F32::from_num(0u32) { - return Some(0u128); + return Ok(0u128); } U96F32::saturating_from_num(tao) .checked_div(price) .map(|alpha| alpha.saturating_to_num::()) + .ok_or(Error::::ArithmeticOverflow.into()) } /// Convert an alpha amount to TAO at `price` (TAO/alpha). diff --git a/pallets/limit-orders/src/tests/auxiliary.rs b/pallets/limit-orders/src/tests/auxiliary.rs index 4cd1090737..b80df468f0 100644 --- a/pallets/limit-orders/src/tests/auxiliary.rs +++ b/pallets/limit-orders/src/tests/auxiliary.rs @@ -31,7 +31,8 @@ fn net_amount_for_event_buy_dominant() { 150u128, // total_sell_net (alpha) ← not used in Buy branch 300u128, // total_sell_tao_equiv price, - ); + ) + .expect("conversion does not overflow"); assert_eq!(net, 700u64); }); } @@ -48,7 +49,8 @@ fn net_amount_for_event_sell_dominant() { 500u128, // total_sell_net (alpha) 400u128, // total_sell_tao_equiv (not used in Sell branch directly) price, - ); + ) + .expect("conversion does not overflow"); // buy_alpha_equiv = 200 / 2 = 100; net = 500 - 100 = 400 assert_eq!(net, 400u64); }); @@ -65,7 +67,8 @@ fn net_amount_for_event_perfectly_offset() { 100u128, 200u128, price, - ); + ) + .expect("conversion does not overflow"); assert_eq!(net, 0u64); }); } From 81763681694d840d139ee501d6eef824f1127d95 Mon Sep 17 00:00:00 2001 From: Evgeny Svirsky Date: Fri, 5 Jun 2026 15:29:28 +0200 Subject: [PATCH 3/3] Added test for overflow error --- pallets/limit-orders/src/tests/auxiliary.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/pallets/limit-orders/src/tests/auxiliary.rs b/pallets/limit-orders/src/tests/auxiliary.rs index b80df468f0..851260d02c 100644 --- a/pallets/limit-orders/src/tests/auxiliary.rs +++ b/pallets/limit-orders/src/tests/auxiliary.rs @@ -73,6 +73,23 @@ fn net_amount_for_event_perfectly_offset() { }); } +#[test] +fn net_amount_for_event_sell_overflow_returns_error() { + new_test_ext().execute_with(|| { + let tiny_price = U96F32::from_bits(1); + assert_eq!( + LimitOrders::::net_amount_for_event( + &OrderSide::Sell, + u128::MAX, + 500u128, + 0u128, + tiny_price, + ), + Err(Error::::ArithmeticOverflow.into()), + ); + }); +} + // ───────────────────────────────────────────────────────────────────────────── // validate_and_classify // ─────────────────────────────────────────────────────────────────────────────