Skip to content
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

add sudo call to set duration for schedule call #745

Merged
merged 8 commits into from
Aug 23, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
68 changes: 68 additions & 0 deletions pallets/admin-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ pub use pallet::*;
pub mod weights;
pub use weights::WeightInfo;

use frame_system::pallet_prelude::BlockNumberFor;
use sp_runtime::{traits::Member, RuntimeAppPublic};

mod benchmarking;
Expand Down Expand Up @@ -1128,6 +1129,73 @@ pub mod pallet {

Ok(())
}

/// Sets the duration of the coldkey swap schedule.
///
/// This extrinsic allows the root account to set the duration for the coldkey swap schedule.
/// The coldkey swap schedule determines how long it takes for a coldkey swap operation to complete.
///
/// # Arguments
/// * `origin` - The origin of the call, which must be the root account.
/// * `duration` - The new duration for the coldkey swap schedule, in number of blocks.
///
/// # Errors
/// * `BadOrigin` - If the caller is not the root account.
///
/// # Weight
/// Weight is handled by the `#[pallet::weight]` attribute.
#[pallet::call_index(54)]
#[pallet::weight((0, DispatchClass::Operational, Pays::No))]
pub fn sudo_set_coldkey_swap_schedule_duration(
origin: OriginFor<T>,
duration: BlockNumberFor<T>,
) -> DispatchResult {
// Ensure the call is made by the root account
ensure_root(origin)?;

// Set the new duration of schedule coldkey swap
pallet_subtensor::Pallet::<T>::set_coldkey_swap_schedule_duration(duration);

// Log the change
log::trace!("ColdkeySwapScheduleDurationSet( duration: {:?} )", duration);

Ok(())
}

/// Sets the duration of the dissolve network schedule.
///
/// This extrinsic allows the root account to set the duration for the dissolve network schedule.
/// The dissolve network schedule determines how long it takes for a network dissolution operation to complete.
///
/// # Arguments
/// * `origin` - The origin of the call, which must be the root account.
/// * `duration` - The new duration for the dissolve network schedule, in number of blocks.
///
/// # Errors
/// * `BadOrigin` - If the caller is not the root account.
///
/// # Weight
/// Weight is handled by the `#[pallet::weight]` attribute.
#[pallet::call_index(55)]
#[pallet::weight((0, DispatchClass::Operational, Pays::No))]
pub fn sudo_set_dissolve_network_schedule_duration(
origin: OriginFor<T>,
duration: BlockNumberFor<T>,
) -> DispatchResult {
// Ensure the call is made by the root account
ensure_root(origin)?;

// Set the duration of schedule dissolve network
pallet_subtensor::Pallet::<T>::set_dissolve_network_schedule_duration(duration);

// Log the change
log::trace!(
"DissolveNetworkScheduleDurationSet( duration: {:?} )",
duration
);

Ok(())
}
}
}

Expand Down
76 changes: 75 additions & 1 deletion pallets/admin-utils/tests/tests.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use frame_support::sp_runtime::DispatchError;
use frame_support::{
assert_err, assert_ok,
assert_err, assert_noop, assert_ok,
dispatch::{DispatchClass, GetDispatchInfo, Pays},
};
use frame_system::Config;
Expand Down Expand Up @@ -1361,3 +1361,77 @@ fn test_sudo_get_set_alpha() {
));
});
}

#[test]
fn test_sudo_set_coldkey_swap_schedule_duration() {
new_test_ext().execute_with(|| {
// Arrange
let root = RuntimeOrigin::root();
let non_root = RuntimeOrigin::signed(U256::from(1));
let new_duration = 100u32.into();

// Act & Assert: Non-root account should fail
assert_noop!(
AdminUtils::sudo_set_coldkey_swap_schedule_duration(non_root, new_duration),
DispatchError::BadOrigin
);

// Act: Root account should succeed
assert_ok!(AdminUtils::sudo_set_coldkey_swap_schedule_duration(
root.clone(),
new_duration
));

// Assert: Check if the duration was actually set
assert_eq!(
pallet_subtensor::ColdkeySwapScheduleDuration::<Test>::get(),
new_duration
);

// Act & Assert: Setting the same value again should succeed (idempotent operation)
assert_ok!(AdminUtils::sudo_set_coldkey_swap_schedule_duration(
root,
new_duration
));

// You might want to check for events here if your pallet emits them
System::assert_last_event(Event::ColdkeySwapScheduleDurationSet(new_duration).into());
});
}

#[test]
fn test_sudo_set_dissolve_network_schedule_duration() {
new_test_ext().execute_with(|| {
// Arrange
let root = RuntimeOrigin::root();
let non_root = RuntimeOrigin::signed(U256::from(1));
let new_duration = 200u32.into();

// Act & Assert: Non-root account should fail
assert_noop!(
AdminUtils::sudo_set_dissolve_network_schedule_duration(non_root, new_duration),
DispatchError::BadOrigin
);

// Act: Root account should succeed
assert_ok!(AdminUtils::sudo_set_dissolve_network_schedule_duration(
root.clone(),
new_duration
));

// Assert: Check if the duration was actually set
assert_eq!(
pallet_subtensor::DissolveNetworkScheduleDuration::<Test>::get(),
new_duration
);

// Act & Assert: Setting the same value again should succeed (idempotent operation)
assert_ok!(AdminUtils::sudo_set_dissolve_network_schedule_duration(
root,
new_duration
));

// You might want to check for events here if your pallet emits them
System::assert_last_event(Event::DissolveNetworkScheduleDurationSet(new_duration).into());
});
}
2 changes: 1 addition & 1 deletion pallets/subtensor/src/benchmarks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ benchmarks! {
let amount_to_be_staked = 100_000_000_000_000u64;
Subtensor::<T>::add_balance_to_coldkey_account(&coldkey.clone(), amount_to_be_staked);
assert_ok!(Subtensor::<T>::register_network(RawOrigin::Signed(coldkey.clone()).into()));
}: dissolve_network(RawOrigin::Signed(coldkey), 1)
}: dissolve_network(RawOrigin::Root, coldkey, 1)

// swap_hotkey {
// let seed: u32 = 1;
Expand Down
3 changes: 1 addition & 2 deletions pallets/subtensor/src/coinbase/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -992,9 +992,8 @@ impl<T: Config> Pallet<T> {
/// * 'SubNetworkDoesNotExist': If the specified network does not exist.
/// * 'NotSubnetOwner': If the caller does not own the specified subnet.
///
pub fn user_remove_network(origin: T::RuntimeOrigin, netuid: u16) -> dispatch::DispatchResult {
pub fn user_remove_network(coldkey: T::AccountId, netuid: u16) -> dispatch::DispatchResult {
// --- 1. Ensure the function caller is a signed user.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will fix it.

let coldkey = ensure_signed(origin)?;

// --- 2. Ensure this subnet exists.
ensure!(
Expand Down
18 changes: 13 additions & 5 deletions pallets/subtensor/src/macros/dispatches.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ mod dispatches {
new_coldkey: T::AccountId,
) -> DispatchResultWithPostInfo {
// Ensure it's called with root privileges (scheduler has root privileges)
ensure_root(origin.clone())?;
ensure_root(origin)?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nevermind , the check is the method

log::info!("swap_coldkey: {:?} -> {:?}", old_coldkey, new_coldkey);

Self::do_swap_coldkey(&old_coldkey, &new_coldkey)
Expand Down Expand Up @@ -930,8 +930,13 @@ mod dispatches {
#[pallet::weight((Weight::from_parts(119_000_000, 0)
.saturating_add(T::DbWeight::get().reads(6))
.saturating_add(T::DbWeight::get().writes(31)), DispatchClass::Operational, Pays::No))]
pub fn dissolve_network(origin: OriginFor<T>, netuid: u16) -> DispatchResult {
Self::user_remove_network(origin, netuid)
pub fn dissolve_network(
origin: OriginFor<T>,
coldkey: T::AccountId,
netuid: u16,
) -> DispatchResult {
ensure_root(origin)?;
Self::user_remove_network(coldkey, netuid)
}

/// Set a single child for a given hotkey on a specified network.
Expand Down Expand Up @@ -1100,7 +1105,10 @@ mod dispatches {
let duration: BlockNumberFor<T> = DissolveNetworkScheduleDuration::<T>::get();
let when: BlockNumberFor<T> = current_block.saturating_add(duration);

let call = Call::<T>::dissolve_network { netuid };
let call = Call::<T>::dissolve_network {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need to check that the user owns the network they are trying to dissolve here ? From the looks of it , anyone can dissolve a network , so I think we should just check if the user is the owner of the network

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The owner could be updated after the call scheduled. You can check my test case. We could let the dissolve_network extrinsic to check when it is executed.

coldkey: who.clone(),
netuid,
};

let bound_call = T::Preimages::bound(LocalCallOf::<T>::from(call.clone()))
.map_err(|_| Error::<T>::FailedToSchedule)?;
Expand All @@ -1109,7 +1117,7 @@ mod dispatches {
DispatchTime::At(when),
None,
63,
frame_system::RawOrigin::Signed(who.clone()).into(),
frame_system::RawOrigin::Root.into(),
bound_call,
)
.map_err(|_| Error::<T>::FailedToSchedule)?;
Expand Down
4 changes: 4 additions & 0 deletions pallets/subtensor/src/macros/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,5 +196,9 @@ mod events {
/// extrinsic execution block number
execution_block: BlockNumberFor<T>,
},
/// The duration of schedule coldkey swap has been set
ColdkeySwapScheduleDurationSet(BlockNumberFor<T>),
/// The duration of dissolve network has been set
DissolveNetworkScheduleDurationSet(BlockNumberFor<T>),
}
}
32 changes: 31 additions & 1 deletion pallets/subtensor/src/utils/misc.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::*;
use crate::{
system::{ensure_root, ensure_signed_or_root},
system::{ensure_root, ensure_signed_or_root, pallet_prelude::BlockNumberFor},
Error,
};
use sp_core::Get;
Expand Down Expand Up @@ -750,4 +750,34 @@ impl<T: Config> Pallet<T> {
// Emit an event to notify listeners about the change
Self::deposit_event(Event::NetworkMaxStakeSet(netuid, max_stake));
}

/// Set the duration for coldkey swap
///
/// # Arguments
///
/// * `duration` - The blocks for coldkey swap execution.
///
/// # Effects
///
/// * Update the ColdkeySwapScheduleDuration storage.
/// * Emits a ColdkeySwapScheduleDurationSet evnet.
pub fn set_coldkey_swap_schedule_duration(duration: BlockNumberFor<T>) {
ColdkeySwapScheduleDuration::<T>::set(duration);
Self::deposit_event(Event::ColdkeySwapScheduleDurationSet(duration));
}

/// Set the duration for dissolve network
///
/// # Arguments
///
/// * `duration` - The blocks for dissolve network execution.
///
/// # Effects
///
/// * Update the DissolveNetworkScheduleDuration storage.
/// * Emits a DissolveNetworkScheduleDurationSet evnet.
pub fn set_dissolve_network_schedule_duration(duration: BlockNumberFor<T>) {
DissolveNetworkScheduleDuration::<T>::set(duration);
Self::deposit_event(Event::DissolveNetworkScheduleDurationSet(duration));
}
}
Loading
Loading