diff --git a/pallets/subtensor/src/staking/helpers.rs b/pallets/subtensor/src/staking/helpers.rs index 1176064e36..11f62b252e 100644 --- a/pallets/subtensor/src/staking/helpers.rs +++ b/pallets/subtensor/src/staking/helpers.rs @@ -207,12 +207,17 @@ impl Pallet { // Add the stake to the coldkey account. Self::add_balance_to_coldkey_account(coldkey, cleared_stake.into()); } else { - // Just clear small alpha + // We should only get here when + // - alpha < min_alpha_stake + // - the unstake fails (for whatever reason) + + // Unstake and clear the amount, burning the alpha. let alpha = Self::get_stake_for_hotkey_and_coldkey_on_subnet(hotkey, coldkey, netuid); - Self::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let alpha_unstaked = Self::decrease_stake_for_hotkey_and_coldkey_on_subnet( hotkey, coldkey, netuid, alpha, ); + Self::burn_subnet_alpha(netuid, alpha_unstaked); } } } diff --git a/pallets/subtensor/src/staking/stake_utils.rs b/pallets/subtensor/src/staking/stake_utils.rs index f61a8a6ce2..bea0aba87e 100644 --- a/pallets/subtensor/src/staking/stake_utils.rs +++ b/pallets/subtensor/src/staking/stake_utils.rs @@ -6,6 +6,8 @@ use substrate_fixed::types::{I64F64, I96F32, U64F64, U96F32}; use subtensor_runtime_common::{AlphaCurrency, Currency, NetUid, TaoCurrency}; use subtensor_swap_interface::{Order, SwapHandler, SwapResult}; +use frame_support::storage::{TransactionOutcome, transactional}; + impl Pallet { /// Retrieves the total alpha issuance for a given subnet. /// @@ -553,6 +555,7 @@ impl Pallet { /// * `netuid` - The unique identifier of the subnet. /// * `amount` - The amount of alpha to be added. /// + #[must_use] pub fn decrease_stake_for_hotkey_and_coldkey_on_subnet( hotkey: &T::AccountId, coldkey: &T::AccountId, @@ -576,6 +579,43 @@ impl Pallet { actual_alpha.neg().max(0).unsigned_abs().into() } + pub fn try_decrease_stake_for_hotkey_and_coldkey_on_subnet( + hotkey: &T::AccountId, + coldkey: &T::AccountId, + netuid: NetUid, + amount: AlphaCurrency, + ) -> bool { + let alpha_share_pool = Self::get_alpha_share_pool(hotkey.clone(), netuid); + + if let Ok(value) = alpha_share_pool.try_get_value(coldkey) { + if value < amount.to_u64() { + return false; + } + + // Try to decrease the stake, if we unstake too much, fail + // Rollback both operations + let result = transactional::with_transaction( + || -> TransactionOutcome> { + let result = Self::decrease_stake_for_hotkey_and_coldkey_on_subnet( + hotkey, coldkey, netuid, amount, + ); + if result > amount { + return TransactionOutcome::Rollback(Err( + Error::::NotEnoughStakeToWithdraw.into(), + )); + } + + TransactionOutcome::Rollback(Ok(())) + }, + ) + .is_ok(); + + return result; + } + + false + } + /// Swaps TAO for the alpha token on the subnet. /// /// Updates TaoIn, AlphaIn, and AlphaOut diff --git a/pallets/subtensor/src/tests/children.rs b/pallets/subtensor/src/tests/children.rs index e8be57f021..017a5db2ba 100644 --- a/pallets/subtensor/src/tests/children.rs +++ b/pallets/subtensor/src/tests/children.rs @@ -2334,7 +2334,7 @@ fn test_do_set_child_cooldown_period() { ); wait_and_set_pending_children(netuid); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &parent, &coldkey, netuid, @@ -2475,7 +2475,7 @@ fn test_revoke_child_no_min_stake_check() { assert_eq!(children_before, vec![]); wait_and_set_pending_children(netuid); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &parent, &coldkey, NetUid::ROOT, @@ -2545,7 +2545,7 @@ fn test_do_set_child_registration_disabled() { )); wait_and_set_pending_children(netuid); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &parent, &coldkey, netuid, diff --git a/pallets/subtensor/src/tests/staking.rs b/pallets/subtensor/src/tests/staking.rs index 4146786709..bb4504d263 100644 --- a/pallets/subtensor/src/tests/staking.rs +++ b/pallets/subtensor/src/tests/staking.rs @@ -1240,7 +1240,7 @@ fn test_remove_stake_from_hotkey_account() { ); // Remove stake - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &hotkey_id, &coldkey_id, netuid, @@ -1293,7 +1293,7 @@ fn test_remove_stake_from_hotkey_account_registered_in_various_networks() { ); // Remove all stake - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &hotkey_id, &coldkey_id, netuid, diff --git a/pallets/subtensor/src/tests/staking2.rs b/pallets/subtensor/src/tests/staking2.rs index 7d6cc6ad3c..cc0e703c55 100644 --- a/pallets/subtensor/src/tests/staking2.rs +++ b/pallets/subtensor/src/tests/staking2.rs @@ -247,7 +247,7 @@ fn test_share_based_staking() { // Test Case 7: Stake Removal // Verify correct stake removal from both accounts - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &primary_hotkey, &primary_coldkey, netuid, @@ -273,7 +273,7 @@ fn test_share_based_staking() { "Stake removal should decrease balance by exact amount" ); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &primary_hotkey, &secondary_coldkey, netuid, @@ -346,7 +346,7 @@ fn test_share_based_staking() { log::info!( "Attempting to remove excessive stake: {available_stake} + 1000 = {excessive_amount}" ); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &primary_hotkey, &primary_coldkey, netuid, @@ -383,7 +383,7 @@ fn test_share_based_staking() { // Test removing stake from non-existent coldkey let non_existent_coldkey = U256::from(5); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &primary_hotkey, &non_existent_coldkey, netuid, @@ -435,7 +435,7 @@ fn test_share_based_staking_denominator_precision() { .to_num::() .into(), ); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &hotkey1, &coldkey1, netuid, @@ -490,7 +490,7 @@ fn test_share_based_staking_stake_unstake_inject() { netuid, stake_amount, ); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &hotkey1, &coldkey1, netuid, @@ -502,7 +502,7 @@ fn test_share_based_staking_stake_unstake_inject() { netuid, stake_amount, ); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &hotkey1, &coldkey2, netuid, diff --git a/pallets/transaction-fee/src/lib.rs b/pallets/transaction-fee/src/lib.rs index fc2a16a409..0ab3d5e2a5 100644 --- a/pallets/transaction-fee/src/lib.rs +++ b/pallets/transaction-fee/src/lib.rs @@ -67,7 +67,7 @@ pub trait AlphaFeeHandler { coldkey: &AccountIdOf, alpha_vec: &[(AccountIdOf, NetUid)], tao_amount: u64, - ); + ) -> Result<(), DispatchError>; fn get_all_netuids_for_coldkey_and_hotkey( coldkey: &AccountIdOf, hotkey: &AccountIdOf, @@ -151,7 +151,22 @@ where ), ); let alpha_price = pallet_subtensor_swap::Pallet::::current_alpha_price(*netuid); - alpha_price.saturating_mul(alpha_balance) >= tao_per_entry + if alpha_price.saturating_mul(alpha_balance) < tao_per_entry { + return false; + } + + let alpha_fee = U96F32::saturating_from_num(tao_per_entry) + .checked_div(alpha_price) + .unwrap_or(alpha_balance) + .min(alpha_balance) + .saturating_to_num::(); + + pallet_subtensor::Pallet::::try_decrease_stake_for_hotkey_and_coldkey_on_subnet( + hotkey, + coldkey, + *netuid, + alpha_fee.into(), + ) }) } @@ -159,14 +174,14 @@ where coldkey: &AccountIdOf, alpha_vec: &[(AccountIdOf, NetUid)], tao_amount: u64, - ) { + ) -> Result<(), DispatchError> { if alpha_vec.is_empty() { - return; + return Ok(()); } let tao_per_entry = tao_amount.checked_div(alpha_vec.len() as u64).unwrap_or(0); - alpha_vec.iter().for_each(|(hotkey, netuid)| { + for (hotkey, netuid) in alpha_vec.iter() { // Divide tao_amount evenly among all alpha entries let alpha_balance = U96F32::saturating_from_num( pallet_subtensor::Pallet::::get_stake_for_hotkey_and_coldkey_on_subnet( @@ -180,13 +195,20 @@ where .min(alpha_balance) .saturating_to_num::(); - pallet_subtensor::Pallet::::decrease_stake_for_hotkey_and_coldkey_on_subnet( - hotkey, - coldkey, - *netuid, - alpha_fee.into(), + let alpha_removed = + pallet_subtensor::Pallet::::decrease_stake_for_hotkey_and_coldkey_on_subnet( + hotkey, + coldkey, + *netuid, + alpha_fee.into(), + ); + ensure!( + alpha_removed == alpha_fee.into(), + "Alpha is not greater than alpha fee" ); - }); + } + + Ok(()) } fn get_all_netuids_for_coldkey_and_hotkey( @@ -308,7 +330,7 @@ where fn withdraw_fee( who: &AccountIdOf, - _call: &CallOf, + call: &CallOf, _dispatch_info: &DispatchInfoOf>, fee: Self::Balance, _tip: Self::Balance, @@ -327,12 +349,14 @@ where ) { Ok(imbalance) => Ok(Some(WithdrawnFee::Tao(imbalance))), Err(_) => { - // let alpha_vec = Self::fees_in_alpha::(who, call); - // if !alpha_vec.is_empty() { - // let fee_u64: u64 = fee.into(); - // OU::withdraw_in_alpha(who, &alpha_vec, fee_u64); - // return Ok(Some(WithdrawnFee::Alpha)); - // } + let alpha_vec = Self::fees_in_alpha::(who, call); + if !alpha_vec.is_empty() { + let fee_u64: u64 = fee.into(); + OU::withdraw_in_alpha(who, &alpha_vec, fee_u64).map_err(|_| { + TransactionValidityError::Invalid(InvalidTransaction::Payment) + })?; + return Ok(Some(WithdrawnFee::Alpha)); + } Err(InvalidTransaction::Payment.into()) } } @@ -340,7 +364,7 @@ where fn can_withdraw_fee( who: &AccountIdOf, - _call: &CallOf, + call: &CallOf, _dispatch_info: &DispatchInfoOf>, fee: Self::Balance, _tip: Self::Balance, @@ -353,14 +377,14 @@ where match F::can_withdraw(who, fee) { WithdrawConsequence::Success => Ok(()), _ => { - // // Fallback to fees in Alpha if possible - // let alpha_vec = Self::fees_in_alpha::(who, call); - // if !alpha_vec.is_empty() { - // let fee_u64: u64 = fee.into(); - // if OU::can_withdraw_in_alpha(who, &alpha_vec, fee_u64) { - // return Ok(()); - // } - // } + // Fallback to fees in Alpha if possible + let alpha_vec = Self::fees_in_alpha::(who, call); + if !alpha_vec.is_empty() { + let fee_u64: u64 = fee.into(); + if OU::can_withdraw_in_alpha(who, &alpha_vec, fee_u64) { + return Ok(()); + } + } Err(InvalidTransaction::Payment.into()) } } diff --git a/pallets/transaction-fee/src/tests/mod.rs b/pallets/transaction-fee/src/tests/mod.rs index b6697e87f0..737c45d902 100644 --- a/pallets/transaction-fee/src/tests/mod.rs +++ b/pallets/transaction-fee/src/tests/mod.rs @@ -74,7 +74,6 @@ fn test_remove_stake_fees_tao() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_fees_alpha --exact --show-output #[test] -#[ignore] fn test_remove_stake_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -143,7 +142,6 @@ fn test_remove_stake_fees_alpha() { // // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_root --exact --show-output #[test] -#[ignore] fn test_remove_stake_root() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -202,7 +200,6 @@ fn test_remove_stake_root() { // // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_completely_root --exact --show-output #[test] -#[ignore] fn test_remove_stake_completely_root() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -254,7 +251,6 @@ fn test_remove_stake_completely_root() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_completely_fees_alpha --exact --show-output #[test] -#[ignore] fn test_remove_stake_completely_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -346,7 +342,7 @@ fn test_remove_stake_not_enough_balance_for_fees() { // For-set Alpha balance to low let new_current_stake = AlphaCurrency::from(1_000); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &sn.hotkeys[0], &sn.coldkey, sn.subnets[0].netuid, @@ -385,7 +381,6 @@ fn test_remove_stake_not_enough_balance_for_fees() { // // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_edge_alpha --exact --show-output #[test] -#[ignore] fn test_remove_stake_edge_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -413,7 +408,7 @@ fn test_remove_stake_edge_alpha() { // For-set Alpha balance to low, but enough to pay tx fees at the current Alpha price let new_current_stake = AlphaCurrency::from(1_000_000); - SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( + let _ = SubtensorModule::decrease_stake_for_hotkey_and_coldkey_on_subnet( &sn.hotkeys[0], &sn.coldkey, sn.subnets[0].netuid, @@ -524,7 +519,6 @@ fn test_remove_stake_failing_transaction_tao_fees() { // // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_failing_transaction_alpha_fees --exact --show-output #[test] -#[ignore] fn test_remove_stake_failing_transaction_alpha_fees() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -590,7 +584,6 @@ fn test_remove_stake_failing_transaction_alpha_fees() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_remove_stake_limit_fees_alpha --exact --show-output #[test] -#[ignore] fn test_remove_stake_limit_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -658,7 +651,6 @@ fn test_remove_stake_limit_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_unstake_all_fees_alpha --exact --show-output #[test] -#[ignore] fn test_unstake_all_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -731,7 +723,6 @@ fn test_unstake_all_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_unstake_all_alpha_fees_alpha --exact --show-output #[test] -#[ignore] fn test_unstake_all_alpha_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -799,7 +790,6 @@ fn test_unstake_all_alpha_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_move_stake_fees_alpha --exact --show-output #[test] -#[ignore] fn test_move_stake_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -871,7 +861,6 @@ fn test_move_stake_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_transfer_stake_fees_alpha --exact --show-output #[test] -#[ignore] fn test_transfer_stake_fees_alpha() { new_test_ext().execute_with(|| { let destination_coldkey = U256::from(100000); @@ -944,7 +933,6 @@ fn test_transfer_stake_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_swap_stake_fees_alpha --exact --show-output #[test] -#[ignore] fn test_swap_stake_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -1015,7 +1003,6 @@ fn test_swap_stake_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_swap_stake_limit_fees_alpha --exact --show-output #[test] -#[ignore] fn test_swap_stake_limit_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -1088,7 +1075,6 @@ fn test_swap_stake_limit_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_burn_alpha_fees_alpha --exact --show-output #[test] -#[ignore] fn test_burn_alpha_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -1150,7 +1136,6 @@ fn test_burn_alpha_fees_alpha() { // cargo test --package subtensor-transaction-fee --lib -- tests::test_recycle_alpha_fees_alpha --exact --show-output #[test] -#[ignore] fn test_recycle_alpha_fees_alpha() { new_test_ext().execute_with(|| { let stake_amount = TAO; @@ -1209,3 +1194,77 @@ fn test_recycle_alpha_fees_alpha() { assert!(actual_alpha_fee > 0.into()); }); } + +#[test] +fn test_remove_stake_alpha_clears_stake_pool_entry() { + new_test_ext().execute_with(|| { + let stake_amount = TAO; + let unstake_amount = AlphaCurrency::from(TAO / 50); + let sn = setup_subnets(1, 1); + setup_stake( + sn.subnets[0].netuid, + &sn.coldkey, + &sn.hotkeys[0], + stake_amount, + ); + + // Force-set signer balance to ED + let current_balance = Balances::free_balance(sn.coldkey); + let _ = SubtensorModule::remove_balance_from_coldkey_account( + &sn.coldkey, + current_balance - ExistentialDeposit::get(), + ); + + // Modify the alpha pool denominator so it's low-precision, coldkey still have 100% of the stake + let denominator = U64F64::from_num(0.0000001); + TotalHotkeyShares::::insert(sn.hotkeys[0], sn.subnets[0].netuid, denominator); + Alpha::::insert( + (&sn.hotkeys[0], &sn.coldkey, sn.subnets[0].netuid), + denominator, + ); + + // Remove stake limit + let balance_before = Balances::free_balance(sn.coldkey); + let alpha_before = SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet( + &sn.hotkeys[0], + &sn.coldkey, + sn.subnets[0].netuid, + ); + let call = RuntimeCall::SubtensorModule(pallet_subtensor::Call::remove_stake_limit { + hotkey: sn.hotkeys[0], + netuid: sn.subnets[0].netuid, + amount_unstaked: unstake_amount, + limit_price: 1_000.into(), + allow_partial: false, + }); + + // Dispatch the extrinsic with ChargeTransactionPayment extension + let info = call.get_dispatch_info(); + let ext = pallet_transaction_payment::ChargeTransactionPayment::::from(0); + let result = ext.validate( + RuntimeOrigin::signed(sn.coldkey).into(), + &call.clone(), + &info, + 10, + (), + &TxBaseImplication(()), + TransactionSource::External, + ); + + assert_eq!( + result.unwrap_err(), + TransactionValidityError::Invalid(InvalidTransaction::Payment) + ); + + // Nothing changes, because it fails validation + let final_balance = Balances::free_balance(sn.coldkey); + let alpha_after = SubtensorModule::get_stake_for_hotkey_and_coldkey_on_subnet( + &sn.hotkeys[0], + &sn.coldkey, + sn.subnets[0].netuid, + ); + + assert_eq!(alpha_after, alpha_before); + assert_eq!(final_balance, balance_before); + }); +}