diff --git a/pallets/subtensor/src/macros/errors.rs b/pallets/subtensor/src/macros/errors.rs index 6c3d7a35df..5a15330075 100644 --- a/pallets/subtensor/src/macros/errors.rs +++ b/pallets/subtensor/src/macros/errors.rs @@ -266,7 +266,5 @@ mod errors { InvalidRootClaimThreshold, /// Exceeded subnet limit number or zero. InvalidSubnetNumber, - /// Unintended precision loss when unstaking alpha - PrecisionLoss, } } diff --git a/pallets/subtensor/src/staking/recycle_alpha.rs b/pallets/subtensor/src/staking/recycle_alpha.rs index 5229971ed0..d8af9e1afd 100644 --- a/pallets/subtensor/src/staking/recycle_alpha.rs +++ b/pallets/subtensor/src/staking/recycle_alpha.rs @@ -55,8 +55,6 @@ impl Pallet { &hotkey, &coldkey, netuid, amount, ); - ensure!(actual_alpha_decrease <= amount, Error::::PrecisionLoss); - // Recycle means we should decrease the alpha issuance tracker. Self::recycle_subnet_alpha(netuid, actual_alpha_decrease); @@ -122,8 +120,6 @@ impl Pallet { &hotkey, &coldkey, netuid, amount, ); - ensure!(actual_alpha_decrease <= amount, Error::::PrecisionLoss); - Self::burn_subnet_alpha(netuid, actual_alpha_decrease); // Deposit event diff --git a/pallets/subtensor/src/tests/recycle_alpha.rs b/pallets/subtensor/src/tests/recycle_alpha.rs index 32a95c700d..35215bb373 100644 --- a/pallets/subtensor/src/tests/recycle_alpha.rs +++ b/pallets/subtensor/src/tests/recycle_alpha.rs @@ -545,8 +545,10 @@ fn test_burn_errors() { }); } +// Test that partial unstake works even with low-precision denominators +// Previously this would fail with PrecisionLoss error, now it should succeed #[test] -fn test_recycle_precision_loss() { +fn test_recycle_with_low_precision_denominator() { new_test_ext(1).execute_with(|| { let coldkey = U256::from(1); let hotkey = U256::from(2); @@ -565,25 +567,27 @@ fn test_recycle_precision_loss() { let recycle_amount = AlphaCurrency::from(stake / 2); // Modify the alpha pool denominator so it's low-precision + // With the fix, this should still work (partial unstake succeeds) let denominator = U64F64::from_num(0.00000001); TotalHotkeyShares::::insert(hotkey, netuid, denominator); Alpha::::insert((&hotkey, &coldkey, netuid), denominator); - // recycle, expect error due to precision loss - assert_noop!( - SubtensorModule::recycle_alpha( - RuntimeOrigin::signed(coldkey), - hotkey, - recycle_amount, - netuid - ), - Error::::PrecisionLoss - ); + // recycle should now succeed (no more PrecisionLoss error) + // The actual amount recycled may be the full stake due to very low precision, + // but the operation should not fail + assert_ok!(SubtensorModule::recycle_alpha( + RuntimeOrigin::signed(coldkey), + hotkey, + recycle_amount, + netuid + )); }); } +// Test that partial burn works even with low-precision denominators +// Previously this would fail with PrecisionLoss error, now it should succeed #[test] -fn test_burn_precision_loss() { +fn test_burn_with_low_precision_denominator() { new_test_ext(1).execute_with(|| { let coldkey = U256::from(1); let hotkey = U256::from(2); @@ -598,23 +602,23 @@ fn test_burn_precision_loss() { let stake = 200_000; increase_stake_on_coldkey_hotkey_account(&coldkey, &hotkey, stake.into(), netuid); - // amount to recycle + // amount to burn let burn_amount = AlphaCurrency::from(stake / 2); // Modify the alpha pool denominator so it's low-precision + // With the fix, this should still work (partial unstake succeeds) let denominator = U64F64::from_num(0.00000001); TotalHotkeyShares::::insert(hotkey, netuid, denominator); Alpha::::insert((&hotkey, &coldkey, netuid), denominator); - // burn, expect error due to precision loss - assert_noop!( - SubtensorModule::burn_alpha( - RuntimeOrigin::signed(coldkey), - hotkey, - burn_amount, - netuid - ), - Error::::PrecisionLoss - ); + // burn should now succeed (no more PrecisionLoss error) + // The actual amount burned may be the full stake due to very low precision, + // but the operation should not fail + assert_ok!(SubtensorModule::burn_alpha( + RuntimeOrigin::signed(coldkey), + hotkey, + burn_amount, + netuid + )); }); } diff --git a/primitives/share-pool/src/lib.rs b/primitives/share-pool/src/lib.rs index d43f36259c..0bb9af6d9a 100644 --- a/primitives/share-pool/src/lib.rs +++ b/primitives/share-pool/src/lib.rs @@ -1,7 +1,6 @@ #![cfg_attr(not(feature = "std"), no_std)] #![allow(clippy::result_unit_err)] -use safe_math::*; use sp_std::marker; use sp_std::ops::Neg; use substrate_fixed::types::{I64F64, U64F64}; @@ -161,28 +160,28 @@ where current_share.saturating_add(U64F64::saturating_from_num(shares_per_update)), ); } else { - // Check if this entry is about to break precision - let mut new_denominator = denominator - .saturating_sub(U64F64::saturating_from_num(shares_per_update.neg())); - let mut new_share = current_share - .saturating_sub(U64F64::saturating_from_num(shares_per_update.neg())); - - // The condition here is either the share remainder is too little OR - // the new_denominator is too low compared to what shared_value + year worth of emissions would be - if (new_share.safe_div(current_share) < U64F64::saturating_from_num(0.00001)) - || shared_value - .saturating_add(U64F64::saturating_from_num(2_628_000_000_000_000_u64)) - .checked_div(new_denominator) - .is_none() + // Calculate new share and denominator after the decrease + let shares_decrease = U64F64::saturating_from_num(shares_per_update.neg()); + let new_denominator = denominator.saturating_sub(shares_decrease); + let new_share = current_share.saturating_sub(shares_decrease); + + // Only force full unstake if the remaining share would be essentially zero + // (less than 1 unit in the smallest representable value) + // This preserves precision for partial unstakes while still cleaning up + // truly negligible remainders + if new_share < U64F64::saturating_from_num(1u64) + || new_denominator < U64F64::saturating_from_num(1u64) { - // yes, precision is low, just remove all - new_share = U64F64::saturating_from_num(0); - new_denominator = denominator.saturating_sub(current_share); + // Remaining share is negligible, remove all to clean up storage + self.state_ops + .set_denominator(denominator.saturating_sub(current_share)); + self.state_ops + .set_share(key, U64F64::saturating_from_num(0)); actual_update = initial_value.neg(); + } else { + self.state_ops.set_denominator(new_denominator); + self.state_ops.set_share(key, new_share); } - - self.state_ops.set_denominator(new_denominator); - self.state_ops.set_share(key, new_share); } }