From 19220dfb953f1d6159573d50ebc64290b716b02c Mon Sep 17 00:00:00 2001 From: camfairchild Date: Wed, 8 Jan 2025 21:06:35 -0500 Subject: [PATCH 1/5] charge for swap before sched --- pallets/subtensor/src/macros/dispatches.rs | 11 +++++++++++ pallets/subtensor/src/swap/swap_coldkey.rs | 14 -------------- pallets/subtensor/src/tests/swap_coldkey.rs | 19 +++++++++++++++++++ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index b62ef4f8c..c0269c8d4 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -1319,6 +1319,17 @@ mod dispatches { Error::::SwapAlreadyScheduled ); + // Calculate the swap cost and ensure sufficient balance + let swap_cost = Self::get_key_swap_cost(); + ensure!( + Self::can_remove_balance_from_coldkey_account(&who, swap_cost), + Error::::NotEnoughBalanceToPaySwapColdKey + ); + + // Remove and burn the swap cost from the old coldkey's account + let actual_burn_amount = Self::remove_balance_from_coldkey_account(&who, swap_cost)?; + Self::burn_tokens(actual_burn_amount); + let current_block: BlockNumberFor = >::block_number(); let duration: BlockNumberFor = ColdkeySwapScheduleDuration::::get(); let when: BlockNumberFor = current_block.saturating_add(duration); diff --git a/pallets/subtensor/src/swap/swap_coldkey.rs b/pallets/subtensor/src/swap/swap_coldkey.rs index 4742c3fca..ab08d2e16 100644 --- a/pallets/subtensor/src/swap/swap_coldkey.rs +++ b/pallets/subtensor/src/swap/swap_coldkey.rs @@ -54,20 +54,6 @@ impl Pallet { Identities::::insert(new_coldkey, identity); } - // 6. Calculate the swap cost and ensure sufficient balance - let swap_cost = Self::get_key_swap_cost(); - ensure!( - Self::can_remove_balance_from_coldkey_account(old_coldkey, swap_cost), - Error::::NotEnoughBalanceToPaySwapColdKey - ); - - // 7. Remove and burn the swap cost from the old coldkey's account - let actual_burn_amount = Self::remove_balance_from_coldkey_account(old_coldkey, swap_cost)?; - Self::burn_tokens(actual_burn_amount); - - // 8. Update the weight for the balance operations - weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); - // 9. Perform the actual coldkey swap let _ = Self::perform_swap_coldkey(old_coldkey, new_coldkey, &mut weight); diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 2f1a71366..7a1911383 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -1666,3 +1666,22 @@ fn test_coldkey_swap_stake_delta() { ); }); } + +// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --test swap_coldkey -- test_cant_schedule_swap_without_enough_to_burn --exact --nocapture +#[test] +fn test_cant_schedule_swap_without_enough_to_burn() { + new_test_ext(1).execute_with(|| { + let old_coldkey = U256::from(3); + let new_coldkey = U256::from(4); + let hotkey = U256::from(5); + + let burn_cost = SubtensorModule::get_key_swap_cost(); + assert_noop!( + SubtensorModule::schedule_swap_coldkey( + <::RuntimeOrigin>::signed(old_coldkey), + new_coldkey + ), + Error::::NotEnoughBalanceToPaySwapColdKey + ); + }); +} From c81f3d82a5f22ff87e96044f784779bc55dfd9e3 Mon Sep 17 00:00:00 2001 From: camfairchild Date: Wed, 8 Jan 2025 21:23:59 -0500 Subject: [PATCH 2/5] add swap cost to the events --- pallets/subtensor/src/macros/dispatches.rs | 1 + pallets/subtensor/src/macros/events.rs | 4 ++++ pallets/subtensor/src/swap/swap_coldkey.rs | 1 + pallets/subtensor/src/tests/swap_coldkey.rs | 8 ++++++++ 4 files changed, 14 insertions(+) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index c0269c8d4..7b8b6c0e3 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -1357,6 +1357,7 @@ mod dispatches { old_coldkey: who.clone(), new_coldkey: new_coldkey.clone(), execution_block: when, + swap_cost, }); Ok(().into()) diff --git a/pallets/subtensor/src/macros/events.rs b/pallets/subtensor/src/macros/events.rs index 668eae7fe..3931ef066 100644 --- a/pallets/subtensor/src/macros/events.rs +++ b/pallets/subtensor/src/macros/events.rs @@ -155,6 +155,8 @@ mod events { old_coldkey: T::AccountId, /// the account ID of new coldkey new_coldkey: T::AccountId, + /// the swap cost + swap_cost: u64, }, /// All balance of a hotkey has been unstaked and transferred to a new coldkey AllBalanceUnstakedAndTransferredToNewColdkey { @@ -175,6 +177,8 @@ mod events { new_coldkey: T::AccountId, /// The arbitration block for the coldkey swap execution_block: BlockNumberFor, + /// The swap cost + swap_cost: u64, }, /// The arbitration period has been extended ArbitrationPeriodExtended { diff --git a/pallets/subtensor/src/swap/swap_coldkey.rs b/pallets/subtensor/src/swap/swap_coldkey.rs index ab08d2e16..061437dd5 100644 --- a/pallets/subtensor/src/swap/swap_coldkey.rs +++ b/pallets/subtensor/src/swap/swap_coldkey.rs @@ -68,6 +68,7 @@ impl Pallet { Self::deposit_event(Event::ColdkeySwapped { old_coldkey: old_coldkey.clone(), new_coldkey: new_coldkey.clone(), + swap_cost, }); // 12. Return the result with the updated weight diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 7a1911383..3d8e552b2 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -1325,6 +1325,9 @@ fn test_schedule_swap_coldkey_success() { // Add balance to the old coldkey account SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, 1000); + + let swap_cost = SubtensorModule::get_key_swap_cost(); + // Schedule the coldkey swap assert_ok!(SubtensorModule::schedule_swap_coldkey( <::RuntimeOrigin>::signed(old_coldkey), @@ -1343,6 +1346,7 @@ fn test_schedule_swap_coldkey_success() { old_coldkey, new_coldkey, execution_block: expected_execution_block, + swap_cost, } .into(), ); @@ -1403,6 +1407,8 @@ fn test_schedule_swap_coldkey_execution() { "Initial ownership check failed" ); + let swap_cost = SubtensorModule::get_key_swap_cost(); + // Schedule the swap assert_ok!(SubtensorModule::schedule_swap_coldkey( <::RuntimeOrigin>::signed(old_coldkey), @@ -1418,6 +1424,7 @@ fn test_schedule_swap_coldkey_execution() { old_coldkey, new_coldkey, execution_block, + swap_cost, } .into(), ); @@ -1455,6 +1462,7 @@ fn test_schedule_swap_coldkey_execution() { Event::ColdkeySwapped { old_coldkey, new_coldkey, + swap_cost, } .into(), ); From 95fbf82fdafed088d282a89418eddd1a9be38de2 Mon Sep 17 00:00:00 2001 From: camfairchild Date: Wed, 8 Jan 2025 21:24:12 -0500 Subject: [PATCH 3/5] pass swap cost into swap_coldkey call --- pallets/subtensor/src/macros/dispatches.rs | 8 +++----- pallets/subtensor/src/swap/swap_coldkey.rs | 14 ++++++++++++++ pallets/subtensor/src/tests/swap_coldkey.rs | 19 +++++++++++++------ 3 files changed, 30 insertions(+), 11 deletions(-) diff --git a/pallets/subtensor/src/macros/dispatches.rs b/pallets/subtensor/src/macros/dispatches.rs index 7b8b6c0e3..0b9b78aeb 100644 --- a/pallets/subtensor/src/macros/dispatches.rs +++ b/pallets/subtensor/src/macros/dispatches.rs @@ -954,12 +954,13 @@ mod dispatches { origin: OriginFor, old_coldkey: T::AccountId, new_coldkey: T::AccountId, + swap_cost: u64, ) -> DispatchResultWithPostInfo { // Ensure it's called with root privileges (scheduler has root privileges) ensure_root(origin)?; log::info!("swap_coldkey: {:?} -> {:?}", old_coldkey, new_coldkey); - Self::do_swap_coldkey(&old_coldkey, &new_coldkey) + Self::do_swap_coldkey(&old_coldkey, &new_coldkey, swap_cost) } /// Sets the childkey take for a given hotkey. @@ -1326,10 +1327,6 @@ mod dispatches { Error::::NotEnoughBalanceToPaySwapColdKey ); - // Remove and burn the swap cost from the old coldkey's account - let actual_burn_amount = Self::remove_balance_from_coldkey_account(&who, swap_cost)?; - Self::burn_tokens(actual_burn_amount); - let current_block: BlockNumberFor = >::block_number(); let duration: BlockNumberFor = ColdkeySwapScheduleDuration::::get(); let when: BlockNumberFor = current_block.saturating_add(duration); @@ -1337,6 +1334,7 @@ mod dispatches { let call = Call::::swap_coldkey { old_coldkey: who.clone(), new_coldkey: new_coldkey.clone(), + swap_cost, }; let bound_call = T::Preimages::bound(LocalCallOf::::from(call.clone())) diff --git a/pallets/subtensor/src/swap/swap_coldkey.rs b/pallets/subtensor/src/swap/swap_coldkey.rs index 061437dd5..f34f1dc49 100644 --- a/pallets/subtensor/src/swap/swap_coldkey.rs +++ b/pallets/subtensor/src/swap/swap_coldkey.rs @@ -32,6 +32,7 @@ impl Pallet { pub fn do_swap_coldkey( old_coldkey: &T::AccountId, new_coldkey: &T::AccountId, + swap_cost: u64, ) -> DispatchResultWithPostInfo { // 2. Initialize the weight for this operation let mut weight: Weight = T::DbWeight::get().reads(2); @@ -54,6 +55,19 @@ impl Pallet { Identities::::insert(new_coldkey, identity); } + // 6. Ensure sufficient balance for the swap cost + ensure!( + Self::can_remove_balance_from_coldkey_account(old_coldkey, swap_cost), + Error::::NotEnoughBalanceToPaySwapColdKey + ); + + // 7. Remove and burn the swap cost from the old coldkey's account + let actual_burn_amount = Self::remove_balance_from_coldkey_account(old_coldkey, swap_cost)?; + Self::burn_tokens(actual_burn_amount); + + // 8. Update the weight for the balance operations + weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1)); + // 9. Perform the actual coldkey swap let _ = Self::perform_swap_coldkey(old_coldkey, new_coldkey, &mut weight); diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 3d8e552b2..4e30f27ca 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -566,7 +566,8 @@ fn test_do_swap_coldkey_success() { assert_ok!(SubtensorModule::do_swap_coldkey( // <::RuntimeOrigin>::signed(old_coldkey), &old_coldkey, - &new_coldkey + &new_coldkey, + swap_cost )); // Log state after swap @@ -630,6 +631,7 @@ fn test_do_swap_coldkey_success() { Event::ColdkeySwapped { old_coldkey, new_coldkey, + swap_cost, } .into(), ); @@ -913,7 +915,7 @@ fn test_do_swap_coldkey_with_subnet_ownership() { OwnedHotkeys::::insert(old_coldkey, vec![hotkey]); // Perform the swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey)); + assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, swap_cost)); // Verify subnet ownership transfer assert_eq!(SubnetOwner::::get(netuid), new_coldkey); @@ -1480,7 +1482,8 @@ fn test_direct_swap_coldkey_call_fails() { SubtensorModule::swap_coldkey( <::RuntimeOrigin>::signed(old_coldkey), old_coldkey, - new_coldkey + new_coldkey, + 0 ), BadOrigin ); @@ -1585,7 +1588,7 @@ fn test_coldkey_swap_no_identity_no_changes() { assert!(Identities::::get(old_coldkey).is_none()); // Perform the coldkey swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey)); + assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, burn_cost)); // Ensure no identities have been changed assert!(Identities::::get(old_coldkey).is_none()); @@ -1629,7 +1632,7 @@ fn test_coldkey_swap_no_identity_no_changes_newcoldkey_exists() { assert!(Identities::::get(old_coldkey).is_none()); // Perform the coldkey swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey)); + assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, burn_cost)); // Ensure no identities have been changed assert!(Identities::::get(old_coldkey).is_none()); @@ -1661,7 +1664,11 @@ fn test_coldkey_swap_stake_delta() { SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, 100e9 as u64); // Perform the coldkey swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey)); + assert_ok!(SubtensorModule::do_swap_coldkey( + &old_coldkey, + &new_coldkey, + burn_cost + )); // Ensure the stake delta is correctly transferred assert_eq!( From 4fc211cfc1d5021f2be22339628ed306a97d7d04 Mon Sep 17 00:00:00 2001 From: camfairchild Date: Wed, 8 Jan 2025 21:24:33 -0500 Subject: [PATCH 4/5] fmt --- pallets/subtensor/src/tests/swap_coldkey.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 4e30f27ca..73c62aa69 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -915,7 +915,11 @@ fn test_do_swap_coldkey_with_subnet_ownership() { OwnedHotkeys::::insert(old_coldkey, vec![hotkey]); // Perform the swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, swap_cost)); + assert_ok!(SubtensorModule::do_swap_coldkey( + &old_coldkey, + &new_coldkey, + swap_cost + )); // Verify subnet ownership transfer assert_eq!(SubnetOwner::::get(netuid), new_coldkey); @@ -1327,7 +1331,6 @@ fn test_schedule_swap_coldkey_success() { // Add balance to the old coldkey account SubtensorModule::add_balance_to_coldkey_account(&old_coldkey, 1000); - let swap_cost = SubtensorModule::get_key_swap_cost(); // Schedule the coldkey swap @@ -1588,7 +1591,11 @@ fn test_coldkey_swap_no_identity_no_changes() { assert!(Identities::::get(old_coldkey).is_none()); // Perform the coldkey swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, burn_cost)); + assert_ok!(SubtensorModule::do_swap_coldkey( + &old_coldkey, + &new_coldkey, + burn_cost + )); // Ensure no identities have been changed assert!(Identities::::get(old_coldkey).is_none()); @@ -1632,7 +1639,11 @@ fn test_coldkey_swap_no_identity_no_changes_newcoldkey_exists() { assert!(Identities::::get(old_coldkey).is_none()); // Perform the coldkey swap - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey, burn_cost)); + assert_ok!(SubtensorModule::do_swap_coldkey( + &old_coldkey, + &new_coldkey, + burn_cost + )); // Ensure no identities have been changed assert!(Identities::::get(old_coldkey).is_none()); From 511fd8678bed739969a4519368b49b4c8af9980c Mon Sep 17 00:00:00 2001 From: camfairchild Date: Wed, 8 Jan 2025 21:25:18 -0500 Subject: [PATCH 5/5] oops, add arg --- pallets/subtensor/src/tests/swap_coldkey.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 73c62aa69..49f6c7bc9 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -1555,7 +1555,11 @@ fn test_coldkey_swap_delegate_identity_updated() { assert!(Identities::::get(old_coldkey).is_some()); assert!(Identities::::get(new_coldkey).is_none()); - assert_ok!(SubtensorModule::do_swap_coldkey(&old_coldkey, &new_coldkey)); + assert_ok!(SubtensorModule::do_swap_coldkey( + &old_coldkey, + &new_coldkey, + burn_cost + )); assert!(Identities::::get(old_coldkey).is_none()); assert!(Identities::::get(new_coldkey).is_some());