Add increase_take and decrease_take extrinsics, rate limiting, and tests #380
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
orriin
approved these changes
Apr 30, 2024
pallets/subtensor/tests/mock.rs
Outdated
| @@ -123,10 +123,11 @@ parameter_types! { | |||
| pub const InitialBondsMovingAverage: u64 = 900_000; | |||
| pub const InitialStakePruningMin: u16 = 0; | |||
| pub const InitialFoundationDistribution: u64 = 0; | |||
| pub const InitialDefaultTake: u16 = 11_796; // 18% honest number. | |||
| pub const InitialDefaultTake: u16 = 32_767; // 50% for tests (18% honest number is used in production (see runtime)) | |||
There was a problem hiding this comment.
why use a different number in tests and runtime?
There was a problem hiding this comment.
yeah I would also prefer consistency here without a good reason not to have it
orriin
reviewed
May 1, 2024
pallets/subtensor/src/staking.rs
Outdated
Comment on lines
95
to
238
| } | ||
|
|
||
| // --- 4. Set the new take value. | ||
| Delegates::<T>::insert(hotkey.clone(), take); | ||
|
|
||
| // --- 5. Emit the take value. | ||
| log::info!( | ||
| "TakeDecreased( coldkey:{:?}, hotkey:{:?}, take:{:?} )", | ||
| coldkey, | ||
| hotkey, | ||
| take | ||
| ); | ||
| Self::deposit_event(Event::TakeDecreased(coldkey, hotkey, take)); | ||
|
|
||
| // --- 6. Ok and return. | ||
| Ok(()) | ||
| } | ||
|
|
||
| // ---- The implementation for the extrinsic increase_take | ||
| // | ||
| // # Args: | ||
| // * 'origin': (<T as frame_system::Config>::RuntimeOrigin): | ||
| // - The signature of the caller's coldkey. | ||
| // | ||
| // * 'hotkey' (T::AccountId): | ||
| // - The hotkey we are delegating (must be owned by the coldkey.) | ||
| // | ||
| // * 'take' (u16): | ||
| // - The stake proportion that this hotkey takes from delegations for subnet ID. | ||
| // | ||
| // # Event: | ||
| // * TakeDecreased; | ||
| // - On successfully setting a decreased take for this hotkey. | ||
| // | ||
| // # Raises: | ||
| // * 'NotRegistered': | ||
| // - The hotkey we are delegating is not registered on the network. | ||
| // | ||
| // * 'NonAssociatedColdKey': | ||
| // - The hotkey we are delegating is not owned by the calling coldket. | ||
| // | ||
| // * 'TxRateLimitExceeded': | ||
| // - Thrown if key has hit transaction rate limit | ||
| // | ||
| pub fn do_increase_take( | ||
| origin: T::RuntimeOrigin, | ||
| hotkey: T::AccountId, | ||
| take: u16, | ||
| ) -> dispatch::DispatchResult { | ||
| // --- 1. We check the coldkey signature. | ||
| let coldkey = ensure_signed(origin)?; | ||
| log::info!( | ||
| "do_increase_take( origin:{:?} hotkey:{:?}, take:{:?} )", | ||
| coldkey, | ||
| hotkey, | ||
| take | ||
| ); | ||
|
|
||
| // --- 2. Ensure we are delegating a known key. | ||
| // Ensure that the coldkey is the owner. | ||
| Self::do_take_checks(&coldkey, &hotkey)?; | ||
|
|
||
| // --- 3. Ensure we are strinctly increasing take | ||
| if let Ok(current_take) = Delegates::<T>::try_get(&hotkey) { | ||
| ensure!(take > current_take, Error::<T>::InvalidTake); | ||
| } | ||
|
|
||
| // --- 4. Ensure take is within the 0 ..= InitialDefaultTake (18%) range | ||
| let max_take = T::InitialDefaultTake::get(); | ||
| ensure!(take <= max_take, Error::<T>::InvalidTake); | ||
|
|
||
| // --- 5. Enforce the rate limit (independently on do_add_stake rate limits) | ||
| let block: u64 = Self::get_current_block_as_u64(); | ||
| ensure!( | ||
| !Self::exceeds_tx_delegate_take_rate_limit( | ||
| Self::get_last_tx_block_delegate_take(&coldkey), | ||
| block | ||
| ), | ||
| Error::<T>::TxRateLimitExceeded | ||
| ); | ||
|
|
||
| // Set last block for rate limiting | ||
| Self::set_last_tx_block_delegate_take(&coldkey, block); | ||
|
|
||
| // --- 6. Set the new take value. | ||
| Delegates::<T>::insert(hotkey.clone(), take); | ||
|
|
||
| // --- 7. Emit the take value. | ||
| log::info!( | ||
| "TakeIncreased( coldkey:{:?}, hotkey:{:?}, take:{:?} )", | ||
| coldkey, | ||
| hotkey, | ||
| take | ||
| ); | ||
| Self::deposit_event(Event::TakeIncreased(coldkey, hotkey, take)); | ||
|
|
||
| // --- 8. Ok and return. | ||
| Ok(()) | ||
| } | ||
|
|
There was a problem hiding this comment.
If these both just set the take value, is there a reason they shouldn't be combined into a single extrinsic?
sam0x17
approved these changes
May 1, 2024
sam0x17
approved these changes
May 2, 2024
orriin
approved these changes
May 2, 2024
| @@ -88,7 +88,7 @@ pub mod pallet { | |||
| #[pallet::weight(T::WeightInfo::sudo_set_default_take())] | |||
| pub fn sudo_set_default_take(origin: OriginFor<T>, default_take: u16) -> DispatchResult { | |||
There was a problem hiding this comment.
Should this be renamed to reflect the changed functionality?
Suggested change
| pub fn sudo_set_default_take(origin: OriginFor<T>, default_take: u16) -> DispatchResult { | |
| pub fn sudo_set_max_delegate_take(origin: OriginFor<T>, max_take: u16) -> DispatchResult { |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Added:
Related Issue(s)
Type of Change
Checklist
cargo fmtandcargo clippyto ensure my code is formatted and linted correctly