From e538804c28d1471066b5d89ae7db21fd31c258e8 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Wed, 12 Feb 2025 16:46:49 -0500 Subject: [PATCH 1/4] dont let ck-in-swap-sched move any stake/register --- pallets/subtensor/src/lib.rs | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/pallets/subtensor/src/lib.rs b/pallets/subtensor/src/lib.rs index 58406e516a..4399b25a31 100644 --- a/pallets/subtensor/src/lib.rs +++ b/pallets/subtensor/src/lib.rs @@ -1885,6 +1885,12 @@ where netuid, amount_staked, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } // Fully validate the user input Self::result_to_validity(Pallet::::validate_add_stake( who, @@ -1902,6 +1908,13 @@ where limit_price, allow_partial, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + // Calcaulate the maximum amount that can be executed with price limit let max_amount = Pallet::::get_max_amount_add(*netuid, *limit_price); @@ -1957,6 +1970,13 @@ where destination_netuid, alpha_amount, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + // Fully validate the user input Self::result_to_validity(Pallet::::validate_stake_transition( who, @@ -1978,6 +1998,13 @@ where destination_netuid, alpha_amount, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + // Fully validate the user input Self::result_to_validity(Pallet::::validate_stake_transition( who, @@ -1998,6 +2025,13 @@ where destination_netuid, alpha_amount, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + // Fully validate the user input Self::result_to_validity(Pallet::::validate_stake_transition( who, @@ -2020,6 +2054,13 @@ where limit_price, allow_partial, }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + // Get the max amount possible to exchange let max_amount = Pallet::::get_max_amount_move( *origin_netuid, @@ -2042,6 +2083,13 @@ where )) } Some(Call::register { netuid, .. } | Call::burned_register { netuid, .. }) => { + if ColdkeySwapScheduled::::contains_key(who) { + return InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into(), + ) + .into(); + } + let registrations_this_interval = Pallet::::get_registrations_this_interval(*netuid); let max_registrations_per_interval = From 8f197a29cf66a72703e6014bdf50ab810e60b4f9 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Wed, 12 Feb 2025 17:32:50 -0500 Subject: [PATCH 2/4] add tests for validation filter --- pallets/subtensor/src/tests/swap_coldkey.rs | 327 +++++++++++++++++++- 1 file changed, 326 insertions(+), 1 deletion(-) diff --git a/pallets/subtensor/src/tests/swap_coldkey.rs b/pallets/subtensor/src/tests/swap_coldkey.rs index 7761005780..d7f7123341 100644 --- a/pallets/subtensor/src/tests/swap_coldkey.rs +++ b/pallets/subtensor/src/tests/swap_coldkey.rs @@ -1726,7 +1726,7 @@ fn test_schedule_swap_coldkey_duplicate() { }); } -// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --test swap_coldkey -- test_schedule_swap_coldkey_execution --exact --nocapture +// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --package pallet-subtensor --lib -- tests::swap_coldkey::test_schedule_swap_coldkey_execution --exact --show-output --nocapture #[test] fn test_schedule_swap_coldkey_execution() { new_test_ext(1).execute_with(|| { @@ -2022,3 +2022,328 @@ fn test_cant_schedule_swap_without_enough_to_burn() { ); }); } + +// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --package pallet-subtensor --lib -- tests::swap_coldkey::test_coldkey_in_swap_schedule_prevents_funds_usage --exact --show-output --nocapture +#[test] +fn test_coldkey_in_swap_schedule_prevents_funds_usage() { + // Testing the signed extension validate function + // correctly filters transactions that attempt to use funds + // while a coldkey swap is scheduled. + + new_test_ext(0).execute_with(|| { + let netuid: u16 = 1; + let version_key: u64 = 0; + let coldkey = U256::from(0); + let new_coldkey = U256::from(1); + let hotkey: U256 = U256::from(2); // Add the hotkey field + assert_ne!(hotkey, coldkey); // Ensure hotkey is NOT the same as coldkey !!! + let fee = DefaultStakingFee::::get(); + + let who = coldkey; // The coldkey signs this transaction + + // Disallowed transactions are + // - add_stake + // - add_stake_limit + // - swap_stake + // - swap_stake_limit + // - move_stake + // - transfer_stake + // - balances.transfer_all + // - balances.transfer_allow_death + // - balances.transfer_keep_alive + + // Allowed transactions are: + // - remove_stake + // - remove_stake_limit + // others... + + // Create netuid + add_network(netuid, 1, 0); + // Register the hotkey + SubtensorModule::append_neuron(netuid, &hotkey, 0); + crate::Owner::::insert(hotkey, coldkey); + + SubtensorModule::add_balance_to_coldkey_account(&who, u64::MAX); + + // Set the minimum stake to 0. + SubtensorModule::set_stake_threshold(0); + // Add stake to the hotkey + assert_ok!(SubtensorModule::add_stake( + <::RuntimeOrigin>::signed(who), + hotkey, + netuid, + 100_000_000_000 + )); + + // Schedule the coldkey for a swap + assert_ok!(SubtensorModule::schedule_swap_coldkey( + <::RuntimeOrigin>::signed(who), + new_coldkey + )); + + assert!(ColdkeySwapScheduled::::contains_key(who)); + + // Setup the extension + let info: crate::DispatchInfo = + crate::DispatchInfoOf::<::RuntimeCall>::default(); + let extension = crate::SubtensorSignedExtension::::new(); + + // Try each call + + // Add stake + let call = RuntimeCall::SubtensorModule(SubtensorCall::add_stake { + hotkey, + netuid, + amount_staked: 100_000_000_000, + }); + let result: Result = + extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Add stake limit + let call = RuntimeCall::SubtensorModule(SubtensorCall::add_stake_limit { + hotkey, + netuid, + amount_staked: 100_000_000_000, + limit_price: 100_000_000_000, + allow_partial: false, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Swap stake + let call = RuntimeCall::SubtensorModule(SubtensorCall::swap_stake { + hotkey, + origin_netuid: netuid, + destination_netuid: netuid, + alpha_amount: 100_000_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Swap stake limit + let call = RuntimeCall::SubtensorModule(SubtensorCall::swap_stake_limit { + hotkey, + origin_netuid: netuid, + destination_netuid: netuid, + alpha_amount: 100_000_000_000, + limit_price: 100_000_000_000, + allow_partial: false, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Move stake + let call = RuntimeCall::SubtensorModule(SubtensorCall::move_stake { + origin_hotkey: hotkey, + destination_hotkey: hotkey, + origin_netuid: netuid, + destination_netuid: netuid, + alpha_amount: 100_000_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Transfer stake + let call = RuntimeCall::SubtensorModule(SubtensorCall::transfer_stake { + destination_coldkey: new_coldkey, + hotkey, + origin_netuid: netuid, + destination_netuid: netuid, + alpha_amount: 100_000_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Transfer all + let call = RuntimeCall::Balances(BalancesCall::transfer_all { + dest: new_coldkey, + keep_alive: false, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Transfer keep alive + let call = RuntimeCall::Balances(BalancesCall::transfer_keep_alive { + dest: new_coldkey, + value: 100_000_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Transfer allow death + let call = RuntimeCall::Balances(BalancesCall::transfer_allow_death { + dest: new_coldkey, + value: 100_000_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Burned register + let call = RuntimeCall::SubtensorModule(SubtensorCall::burned_register { netuid, hotkey }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + + // Remove stake + let call = RuntimeCall::SubtensorModule(SubtensorCall::remove_stake { + hotkey, + netuid, + amount_unstaked: 1_000_000, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should pass, not in list. + assert_ok!(result); + + // Remove stake limit + let call = RuntimeCall::SubtensorModule(SubtensorCall::remove_stake_limit { + hotkey, + netuid, + amount_unstaked: 1_000_000, + limit_price: 123456789, // should be low enough + allow_partial: true, + }); + let result = extension.validate(&who, &call.clone(), &info, 10); + // Should pass, not in list. + assert_ok!(result); + }); +} + +// SKIP_WASM_BUILD=1 RUST_LOG=info cargo test --package pallet-subtensor --lib -- tests::swap_coldkey::test_coldkey_in_swap_schedule_prevents_critical_calls --exact --show-output --nocapture +#[test] +fn test_coldkey_in_swap_schedule_prevents_critical_calls() { + // Testing the signed extension validate function + // correctly filters transactions that are critical + // while a coldkey swap is scheduled. + + new_test_ext(0).execute_with(|| { + let netuid: u16 = 1; + let version_key: u64 = 0; + let coldkey = U256::from(0); + let new_coldkey = U256::from(1); + let hotkey: U256 = U256::from(2); // Add the hotkey field + assert_ne!(hotkey, coldkey); // Ensure hotkey is NOT the same as coldkey !!! + let fee = DefaultStakingFee::::get(); + + let who = coldkey; // The coldkey signs this transaction + + // Disallowed transactions are + // - dissolve_network + + // Create netuid + add_network(netuid, 1, 0); + // Register the hotkey + SubtensorModule::append_neuron(netuid, &hotkey, 0); + crate::Owner::::insert(hotkey, coldkey); + + SubtensorModule::add_balance_to_coldkey_account(&who, u64::MAX); + + // Set the minimum stake to 0. + SubtensorModule::set_stake_threshold(0); + // Add stake to the hotkey + assert_ok!(SubtensorModule::add_stake( + <::RuntimeOrigin>::signed(who), + hotkey, + netuid, + 100_000_000_000 + )); + + // Schedule the coldkey for a swap + assert_ok!(SubtensorModule::schedule_swap_coldkey( + <::RuntimeOrigin>::signed(who), + new_coldkey + )); + + assert!(ColdkeySwapScheduled::::contains_key(who)); + + // Setup the extension + let info: crate::DispatchInfo = + crate::DispatchInfoOf::<::RuntimeCall>::default(); + let extension = crate::SubtensorSignedExtension::::new(); + + // Try each call + + // Dissolve network + let call = + RuntimeCall::SubtensorModule(SubtensorCall::dissolve_network { netuid, coldkey }); + let result: Result = + extension.validate(&who, &call.clone(), &info, 10); + // Should fail + assert_err!( + // Should get an invalid transaction error + result, + crate::TransactionValidityError::Invalid(crate::InvalidTransaction::Custom( + CustomTransactionError::ColdkeyInSwapSchedule.into() + )) + ); + }); +} From 5b3d4e66e37831932450debd47fd1a6276c58763 Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Wed, 12 Feb 2025 17:33:21 -0500 Subject: [PATCH 3/4] add transfer stake to call nontransfer proxy filter --- runtime/src/lib.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 26bf16206f..f3b7535b55 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -726,7 +726,15 @@ impl InstanceFilter for ProxyType { fn filter(&self, c: &RuntimeCall) -> bool { match self { ProxyType::Any => true, - ProxyType::NonTransfer => !matches!(c, RuntimeCall::Balances(..)), + ProxyType::NonTransfer => !matches!( + c, + RuntimeCall::Balances(..) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::transfer_stake { .. }) + | RuntimeCall::SubtensorModule( + pallet_subtensor::Call::schedule_swap_coldkey { .. } + ) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_coldkey { .. }) + ), ProxyType::NonFungibile => !matches!( c, RuntimeCall::Balances(..) From 45a67fcd88c56de47a3ce6539a0654a8e013e01c Mon Sep 17 00:00:00 2001 From: Cameron Fairchild Date: Wed, 12 Feb 2025 17:40:06 -0500 Subject: [PATCH 4/4] also add proxy filters for new calls --- runtime/src/lib.rs | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index f3b7535b55..b8c48d3802 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -739,12 +739,25 @@ impl InstanceFilter for ProxyType { c, RuntimeCall::Balances(..) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::add_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::add_stake_limit { .. }) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::remove_stake { .. }) + | RuntimeCall::SubtensorModule( + pallet_subtensor::Call::remove_stake_limit { .. } + ) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::unstake_all { .. }) + | RuntimeCall::SubtensorModule( + pallet_subtensor::Call::unstake_all_alpha { .. } + ) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_stake_limit { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::move_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::transfer_stake { .. }) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::burned_register { .. }) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::root_register { .. }) | RuntimeCall::SubtensorModule( pallet_subtensor::Call::schedule_swap_coldkey { .. } ) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_coldkey { .. }) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_hotkey { .. }) ), ProxyType::Transfer => matches!( @@ -752,6 +765,7 @@ impl InstanceFilter for ProxyType { RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { .. }) | RuntimeCall::Balances(pallet_balances::Call::transfer_allow_death { .. }) | RuntimeCall::Balances(pallet_balances::Call::transfer_all { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::transfer_stake { .. }) ), ProxyType::SmallTransfer => match c { RuntimeCall::Balances(pallet_balances::Call::transfer_keep_alive { @@ -761,6 +775,10 @@ impl InstanceFilter for ProxyType { value, .. }) => *value < SMALL_TRANSFER_LIMIT, + RuntimeCall::SubtensorModule(pallet_subtensor::Call::transfer_stake { + alpha_amount, + .. + }) => *alpha_amount < SMALL_TRANSFER_LIMIT, _ => false, }, ProxyType::Owner => matches!(c, RuntimeCall::AdminUtils(..)), @@ -788,6 +806,17 @@ impl InstanceFilter for ProxyType { c, RuntimeCall::SubtensorModule(pallet_subtensor::Call::add_stake { .. }) | RuntimeCall::SubtensorModule(pallet_subtensor::Call::remove_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::unstake_all { .. }) + | RuntimeCall::SubtensorModule( + pallet_subtensor::Call::unstake_all_alpha { .. } + ) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::swap_stake_limit { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::move_stake { .. }) + | RuntimeCall::SubtensorModule(pallet_subtensor::Call::add_stake_limit { .. }) + | RuntimeCall::SubtensorModule( + pallet_subtensor::Call::remove_stake_limit { .. } + ) ), ProxyType::Registration => matches!( c,