Skip to content

Hotfix/filter nested batches #1662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: devnet-ready
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions pallets/subtensor/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2165,6 +2165,20 @@ where
Self::get_priority_staking(who, hotkey, *amount_unstaked),
)
}
Some(Call::unstake_all { hotkey }) => {
// Fully validate the user input
Self::result_to_validity(
Pallet::<T>::validate_unstake_all(who, hotkey, false),
Self::get_priority_vanilla(),
)
}
Some(Call::unstake_all_alpha { hotkey }) => {
// Fully validate the user input
Self::result_to_validity(
Pallet::<T>::validate_unstake_all(who, hotkey, true),
Self::get_priority_vanilla(),
)
}
Some(Call::remove_stake_limit {
hotkey,
netuid,
Expand Down
41 changes: 41 additions & 0 deletions pallets/subtensor/src/staking/stake_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -995,6 +995,47 @@ impl<T: Config> Pallet<T> {
Ok(())
}

/// Validate if unstake_all can be executed
///
pub fn validate_unstake_all(
coldkey: &T::AccountId,
hotkey: &T::AccountId,
only_alpha: bool,
) -> Result<(), Error<T>> {
// Get all netuids (filter out root)
let subnets: Vec<u16> = Self::get_all_subnet_netuids();

// Ensure that the hotkey account exists this is only possible through registration.
ensure!(
Self::hotkey_account_exists(hotkey),
Error::<T>::HotKeyAccountNotExists
);

let mut unstaking_any = false;
for netuid in subnets.iter() {
if !SubtokenEnabled::<T>::get(netuid) {
continue;
}

if only_alpha && (*netuid == Self::get_root_netuid()) {
continue;
}

// Get user's stake in this subnet
let alpha = Self::get_stake_for_hotkey_and_coldkey_on_subnet(hotkey, coldkey, *netuid);

if Self::validate_remove_stake(coldkey, hotkey, *netuid, alpha, alpha, false).is_ok()
{
unstaking_any = true;
}
}

// If no unstaking happens, return error
ensure!(unstaking_any, Error::<T>::AmountTooLow);

Ok(())
}

/// Validate stake transition user input
/// That works for move_stake, transfer_stake, and swap_stake
///
Expand Down
31 changes: 30 additions & 1 deletion pallets/subtensor/src/tests/batch_tx.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use super::mock::*;
use frame_support::{assert_ok, traits::Currency};
use frame_support::{
assert_ok,
traits::{Contains, Currency},
};
use frame_system::Config;
use sp_core::U256;

Expand Down Expand Up @@ -33,3 +36,29 @@ fn test_batch_txs() {
assert_eq!(Balances::total_balance(&charlie), 2_000_000_000);
});
}

#[test]
fn test_cant_nest_batch_txs() {
let _alice = U256::from(0);
let bob = U256::from(1);
let charlie = U256::from(2);

new_test_ext(1).execute_with(|| {
let call = RuntimeCall::Utility(pallet_utility::Call::batch {
calls: vec![
RuntimeCall::Balances(BalanceCall::transfer_allow_death {
dest: bob,
value: 1_000_000_000,
}),
RuntimeCall::Utility(pallet_utility::Call::force_batch {
calls: vec![RuntimeCall::Balances(BalanceCall::transfer_allow_death {
dest: charlie,
value: 1_000_000_000,
})],
}),
],
});

assert!(!<Test as Config>::BaseCallFilter::contains(&call));
});
}
27 changes: 25 additions & 2 deletions pallets/subtensor/src/tests/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@
use crate::utils::rate_limiting::TransactionType;
use frame_support::derive_impl;
use frame_support::dispatch::DispatchResultWithPostInfo;
use frame_support::traits::{Contains, Everything, InsideBoth};
use frame_support::weights::Weight;
use frame_support::weights::constants::RocksDbWeight;
use frame_support::{
assert_ok, parameter_types,
traits::{Everything, Hooks, PrivilegeCmp},
traits::{Hooks, PrivilegeCmp},
};
use frame_system as system;
use frame_system::{EnsureNever, EnsureRoot, RawOrigin, limits};
Expand Down Expand Up @@ -88,9 +89,31 @@ impl pallet_balances::Config for Test {
type MaxFreezes = ();
}

pub struct NoNestingCallFilter;

impl Contains<RuntimeCall> for NoNestingCallFilter {
fn contains(call: &RuntimeCall) -> bool {
match call {
RuntimeCall::Utility(inner) => {
let calls = match inner {
pallet_utility::Call::force_batch { calls } => calls,
pallet_utility::Call::batch { calls } => calls,
pallet_utility::Call::batch_all { calls } => calls,
_ => &Vec::new(),
};

!calls.iter().any(|call| {
matches!(call, RuntimeCall::Utility(inner) if matches!(inner, pallet_utility::Call::force_batch { .. } | pallet_utility::Call::batch_all { .. } | pallet_utility::Call::batch { .. }))
})
}
_ => true,
}
}
}

#[derive_impl(frame_system::config_preludes::TestDefaultConfig)]
impl system::Config for Test {
type BaseCallFilter = Everything;
type BaseCallFilter = InsideBoth<Everything, NoNestingCallFilter>;
type BlockWeights = BlockWeights;
type BlockLength = ();
type DbWeight = RocksDbWeight;
Expand Down
51 changes: 51 additions & 0 deletions pallets/subtensor/src/tests/staking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3841,6 +3841,57 @@ fn test_unstake_low_liquidity_validate() {
});
}

#[test]
fn test_unstake_all_validate() {
// Testing the signed extension validate function
// correctly filters the `unstake_all` transaction.

new_test_ext(0).execute_with(|| {
let subnet_owner_coldkey = U256::from(1001);
let subnet_owner_hotkey = U256::from(1002);
let hotkey = U256::from(2);
let coldkey = U256::from(3);
let amount_staked = DefaultMinStake::<Test>::get() * 10 + DefaultStakingFee::<Test>::get();

let netuid = add_dynamic_network(&subnet_owner_hotkey, &subnet_owner_coldkey);
SubtensorModule::create_account_if_non_existent(&coldkey, &hotkey);
SubtensorModule::add_balance_to_coldkey_account(&coldkey, amount_staked);

// Simulate stake for hotkey
SubnetTAO::<Test>::insert(netuid, u64::MAX / 1000);
SubnetAlphaIn::<Test>::insert(netuid, u64::MAX / 1000);
SubtensorModule::stake_into_subnet(&hotkey, &coldkey, netuid, amount_staked, 0);

// Set the liquidity at lowest possible value so that all staking requests fail
SubnetTAO::<Test>::insert(
netuid,
DefaultMinimumPoolLiquidity::<Test>::get().to_num::<u64>(),
);
SubnetAlphaIn::<Test>::insert(
netuid,
DefaultMinimumPoolLiquidity::<Test>::get().to_num::<u64>(),
);

// unstake_all call
let call = RuntimeCall::SubtensorModule(SubtensorCall::unstake_all { hotkey });

let info: DispatchInfo =
DispatchInfoOf::<<Test as frame_system::Config>::RuntimeCall>::default();

let extension = SubtensorSignedExtension::<Test>::new();
// Submit to the signed extension validate function
let result_no_stake = extension.validate(&coldkey, &call.clone(), &info, 10);

// Should fail due to insufficient stake
assert_err!(
result_no_stake,
TransactionValidityError::Invalid(InvalidTransaction::Custom(
CustomTransactionError::StakeAmountTooLow.into()
))
);
});
}

#[test]
fn test_max_amount_add_root() {
new_test_ext(0).execute_with(|| {
Expand Down
28 changes: 25 additions & 3 deletions runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub mod check_nonce;
mod migrations;

use codec::{Compact, Decode, Encode};
use frame_support::traits::Imbalance;
use frame_support::traits::{Imbalance, InsideBoth};
use frame_support::{
PalletId,
dispatch::DispatchResultWithPostInfo,
Expand Down Expand Up @@ -209,7 +209,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
// `spec_version`, and `authoring_version` are the same between Wasm and native.
// This value is set to 100 to notify Polkadot-JS App (https://polkadot.js.org/apps) to use
// the compatible custom types.
spec_version: 271,
spec_version: 272,
impl_version: 1,
apis: RUNTIME_API_VERSIONS,
transaction_version: 1,
Expand Down Expand Up @@ -244,11 +244,33 @@ parameter_types! {
pub const SS58Prefix: u8 = 42;
}

pub struct NoNestingCallFilter;

impl Contains<RuntimeCall> for NoNestingCallFilter {
fn contains(call: &RuntimeCall) -> bool {
match call {
RuntimeCall::Utility(inner) => {
let calls = match inner {
pallet_utility::Call::force_batch { calls } => calls,
pallet_utility::Call::batch { calls } => calls,
pallet_utility::Call::batch_all { calls } => calls,
_ => &Vec::new(),
};

!calls.iter().any(|call| {
matches!(call, RuntimeCall::Utility(inner) if matches!(inner, pallet_utility::Call::force_batch { .. } | pallet_utility::Call::batch_all { .. } | pallet_utility::Call::batch { .. }))
})
}
_ => true,
}
}
}

// Configure FRAME pallets to include in runtime.

impl frame_system::Config for Runtime {
// The basic call filter to use in dispatchable.
type BaseCallFilter = SafeMode;
type BaseCallFilter = InsideBoth<SafeMode, NoNestingCallFilter>;
// Block & extrinsics weights: base values and limits.
type BlockWeights = BlockWeights;
// The maximum length of a block (in bytes).
Expand Down
Loading