Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Improve documentation for fast-unstake pallet #14101

Merged
merged 41 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
41 commits
Select commit Hold shift + click to select a range
dbf2572
improve documentation of fast-unstake pallet
kianenigma May 7, 2023
9d2a9a2
using Sam's crate now
kianenigma May 9, 2023
bb11dc0
merge
kianenigma May 9, 2023
91e811b
fix
kianenigma May 9, 2023
85f4390
remove stuff not needed
kianenigma May 9, 2023
1a6c63d
Some updates
kianenigma May 9, 2023
dc82e48
use new prelude.
kianenigma May 9, 2023
4973a32
update
kianenigma May 10, 2023
8d0c523
some other fancy docs
kianenigma May 10, 2023
1572dd8
Update frame/fast-unstake/src/lib.rs
kianenigma May 10, 2023
9aa4ede
Update frame/support/procedural/src/pallet/expand/mod.rs
kianenigma May 10, 2023
f24ac3c
update
kianenigma May 10, 2023
048e061
Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…
kianenigma May 10, 2023
91846b9
Update frame/fast-unstake/src/lib.rs
kianenigma May 10, 2023
99cc532
Master.into()
kianenigma May 23, 2023
6b7804a
update
kianenigma May 23, 2023
c6a9dc7
fix no_std issue by updating to latest version of docify
sam0x17 May 25, 2023
6fda397
get things compiling by adding a use for StakingInterface
sam0x17 May 25, 2023
372f14e
fix docify paths to their proper values, still not working because bug
sam0x17 May 25, 2023
f68b1bb
embed working :tada:
sam0x17 May 25, 2023
3b75895
Merge remote-tracking branch 'origin/master' into kiz-improve-fast-un…
sam0x17 May 26, 2023
99ab314
update syn
sam0x17 May 26, 2023
acd122b
upgrade to docify v0.1.10 for some fixes
sam0x17 May 26, 2023
44e34e4
Apply suggestions from code review
kianenigma May 26, 2023
d9c814d
Merge branch 'master' of github.com:paritytech/substrate into kiz-imp…
kianenigma May 26, 2023
7cb1db4
improve docs
kianenigma May 26, 2023
5244952
Update frame/support/procedural/src/pallet/expand/doc_only.rs
kianenigma May 26, 2023
9c9ab28
fmt
kianenigma May 26, 2023
8976e53
Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…
kianenigma May 26, 2023
396cec2
fix
kianenigma May 26, 2023
21fc4df
Update frame/support/procedural/src/pallet/expand/doc_only.rs
kianenigma May 28, 2023
e4939dc
Update frame/support/procedural/src/pallet/expand/config.rs
kianenigma May 28, 2023
ffe579f
Update frame/support/procedural/src/pallet/expand/config.rs
kianenigma May 28, 2023
04a5aff
remove redundant
kianenigma May 28, 2023
9d7fb36
Merge branch 'kiz-improve-fast-unstake-dcos' of github.com:paritytech…
kianenigma May 28, 2023
ff78017
update docify rev
kianenigma May 29, 2023
351ee05
Merge remote-tracking branch 'origin/master' into kiz-improve-fast-un…
May 29, 2023
9602902
update.
kianenigma May 29, 2023
93aca7a
update.
kianenigma May 29, 2023
55fc001
update lock file
kianenigma May 29, 2023
851e4af
Merge branch 'master' of github.com:paritytech/substrate into kiz-imp…
kianenigma May 29, 2023
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
35 changes: 34 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions frame/fast-unstake/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ frame-election-provider-support = { default-features = false, path = "../electio

frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" }

docify = "0.1.1"

[dev-dependencies]
pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../staking/reward-curve" }
sp-core = { version = "7.0.0", default-features = false, path = "../../primitives/core" }
Expand Down
97 changes: 63 additions & 34 deletions frame/fast-unstake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

//! # Fast Unstake Pallet
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
//!
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
//! ## Overview
//!
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
//! A pallet that's designed to JUST do the following:
//!
//! If a nominator is not exposed in any `ErasStakers` (i.e. "has not actively backed any
Expand All @@ -27,16 +31,16 @@
//! Nominator" role explained in the
//! [February Staking Update](https://polkadot.network/blog/staking-update-february-2022/).
//!
//! This pallet works off the basis of `on_idle`, meaning that it provides no guarantee about when
//! it will succeed, if at all. Moreover, the queue implementation is unordered. In case of
//! congestion, no FIFO ordering is provided.
//! This pallet works off the basis of `on_idle`, meaning that it
//! provides no guarantee about when it will succeed, if at all. Moreover, the queue implementation
//! is unordered. In case of congestion, no FIFO ordering is provided.
//!
//! Stakers who are certain about NOT being exposed can register themselves with
//! [`Call::register_fast_unstake`]. This will chill, and fully unbond the staker, and place them in
//! the queue to be checked.
//! [`Pallet::register_fast_unstake`]. This will chill, and fully unbond the staker, and place them
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
//! in the queue to be checked.
//!
//! Once queued, but not being actively processed, stakers can withdraw their request via
//! [`Call::deregister`].
//! [`Pallet::deregister`].
//!
//! Once queued, a staker wishing to unbond can perform no further action in pallet-staking. This is
//! to prevent them from accidentally exposing themselves behind a validator etc.
Expand All @@ -46,16 +50,29 @@
//!
//! If unsuccessful, meaning that the staker was exposed sometime in the last `BondingDuration` eras
//! they will end up being slashed for the amount of wasted work they have inflicted on the chian.
//!
//! ## Details
//!
//! See [`pallet`] module for more information.
//!
//! ## Examples
//!
//! 1. Fast-unstake with multiple participants in the queue.
#![doc = docify::embed!("./frame/fast-unstake/src/tests.rs", successful_multi_queue)]
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
liamaharon marked this conversation as resolved.
Show resolved Hide resolved
//!
//! 2. Fast unstake failing because a nominator is exposed.
#![doc = docify::embed!("./frame/fast-unstake/src/tests.rs", exposed_nominator_cannot_unstake)]
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
//!

#![cfg_attr(not(feature = "std"), no_std)]

pub use pallet::*;

#[cfg(test)]
mod mock;
pub mod mock;

#[cfg(test)]
mod tests;
pub mod tests;

// NOTE: enable benchmarking in tests as well.
#[cfg(feature = "runtime-benchmarks")]
Expand Down Expand Up @@ -91,16 +108,6 @@ pub mod pallet {
use sp_std::{prelude::*, vec::Vec};
pub use weights::WeightInfo;

#[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)]
#[codec(mel_bound(T: Config))]
#[scale_info(skip_type_params(T))]
pub struct MaxChecking<T: Config>(sp_std::marker::PhantomData<T>);
impl<T: Config> frame_support::traits::Get<u32> for MaxChecking<T> {
fn get() -> u32 {
T::Staking::bonding_duration() + 1
}
}

const STORAGE_VERSION: StorageVersion = StorageVersion::new(1);

#[pallet::pallet]
Expand All @@ -122,7 +129,7 @@ pub mod pallet {
#[pallet::constant]
type Deposit: Get<BalanceOf<Self>>;

/// The origin that can control this pallet.
/// The origin that can control this pallet, in other words invoke [`Pallet::control`].
type ControlOrigin: frame_support::traits::EnsureOrigin<Self::RuntimeOrigin>;

/// Batch size.
Expand All @@ -133,40 +140,44 @@ pub mod pallet {
/// The access to staking functionality.
type Staking: StakingInterface<Balance = BalanceOf<Self>, AccountId = Self::AccountId>;

/// Maximum value for `ErasToCheckPerBlock`, checked in [`Pallet::control`].
///
/// This should be as close as possible, but more than the actual value, in order to have
/// accurate benchmarks.
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
type MaxErasToCheckPerBlock: Get<u32>;

/// The weight information of this pallet.
type WeightInfo: WeightInfo;

/// Maximum value for `ErasToCheckPerBlock`. This should be as close as possible, but more
/// than the actual value, in order to have accurate benchmarks.
type MaxErasToCheckPerBlock: Get<u32>;

/// Use only for benchmarking.
#[cfg(feature = "runtime-benchmarks")]
type MaxBackersPerValidator: Get<u32>;
}

/// The current "head of the queue" being unstaked.
///
/// The head in itself can be a batch of up to [`Config::BatchSize`] stakers.
#[pallet::storage]
pub type Head<T: Config> = StorageValue<_, UnstakeRequest<T>, OptionQuery>;

/// The map of all accounts wishing to be unstaked.
///
/// Keeps track of `AccountId` wishing to unstake and it's corresponding deposit.
///
/// TWOX-NOTE: SAFE since `AccountId` is a secure hash.
// Hasher: Twox safe since `AccountId` is a secure hash.
#[pallet::storage]
pub type Queue<T: Config> = CountedStorageMap<_, Twox64Concat, T::AccountId, BalanceOf<T>>;

/// Number of eras to check per block.
///
/// If set to 0, this pallet does absolutely nothing.
/// If set to 0, this pallet does absolutely nothing. Cannot be set to more than
/// [`Config::MaxErasToCheckPerBlock`].
///
/// Based on the amount of weight available at `on_idle`, up to this many eras of a single
/// nominator might be checked.
/// Based on the amount of weight available at [`Pallet::on_idle`], up to this many eras of a
/// [`UnstakeRequest`], stored in [`Head`] might be checked.
Copy link
Contributor

Choose a reason for hiding this comment

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

I find this a bit confusing, can it be rephrased?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

good catch, my english sometimes tends to be way too convoluted :)) hopefully better now.

#[pallet::storage]
#[pallet::getter(fn eras_to_check_per_block)]
pub type ErasToCheckPerBlock<T: Config> = StorageValue<_, u32, ValueQuery>;

/// The events of this pallet.
#[pallet::event]
#[pallet::generate_deposit(pub(super) fn deposit_event)]
pub enum Event<T: Config> {
Expand Down Expand Up @@ -244,8 +255,13 @@ pub mod pallet {
impl<T: Config> Pallet<T> {
/// Register oneself for fast-unstake.
///
/// The dispatch origin of this call must be signed by the controller account, similar to
/// `staking::unbond`.
///
/// ## Dispatch Origin
///
/// The dispatch origin of this call must be *signed* by the whoever is permitted by to call
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
/// unbond funds by the staking system. See [`Config::Staking`].
///
/// ## Details
///
/// The stash associated with the origin must have no ongoing unlocking chunks. If
/// successful, this will fully unbond and chill the stash. Then, it will enqueue the stash
Expand Down Expand Up @@ -285,11 +301,18 @@ pub mod pallet {

/// Deregister oneself from the fast-unstake.
///
/// ## Dispatch Origin
///
/// The dispatch origin of this call must be *signed* by the whoever is permitted by to call
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
/// unbond funds by the staking system. See [`Config::Staking`].
///
/// ## Details
///
/// This is useful if one is registered, they are still waiting, and they change their mind.
///
/// Note that the associated stash is still fully unbonded and chilled as a consequence of
/// calling `register_fast_unstake`. This should probably be followed by a call to
/// `Staking::rebond`.
/// calling [`Pallet::register_fast_unstake`]. THerefore, this should probably be followed
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
/// by a call to `rebond` in the staking system.
#[pallet::call_index(1)]
#[pallet::weight(<T as Config>::WeightInfo::deregister())]
pub fn deregister(origin: OriginFor<T>) -> DispatchResult {
Expand All @@ -315,7 +338,13 @@ pub mod pallet {

/// Control the operation of this pallet.
///
/// Dispatch origin must be signed by the [`Config::ControlOrigin`].
/// ## Dispatch Origin
///
/// The dispatch origin of this call must be [`Config::ControlOrigin`].
///
/// ## Details
///
/// Can set the number of eras to check per block, and potentially other admin work.
#[pallet::call_index(2)]
#[pallet::weight(<T as Config>::WeightInfo::control())]
pub fn control(origin: OriginFor<T>, eras_to_check: EraIndex) -> DispatchResult {
Expand Down
5 changes: 4 additions & 1 deletion frame/fast-unstake/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

use super::*;
use crate::{mock::*, types::*, Event};
use frame_support::{assert_noop, assert_ok, bounded_vec, pallet_prelude::*, traits::Currency};
use frame_support::{pallet_prelude::*, traits::Currency, testing_prelude::*};
use pallet_staking::{CurrentEra, RewardDestination};

use sp_runtime::traits::BadOrigin;
Expand Down Expand Up @@ -303,6 +303,7 @@ mod on_idle {
});
}

#[docify::export]
#[test]
fn successful_multi_queue() {
ExtBuilder::default().build_and_execute(|| {
Expand Down Expand Up @@ -356,6 +357,7 @@ mod on_idle {
});
}

#[docify::export]
#[test]
fn successful_unstake() {
ExtBuilder::default().build_and_execute(|| {
Expand Down Expand Up @@ -694,6 +696,7 @@ mod on_idle {
}

#[test]
#[docify::export]
fn exposed_nominator_cannot_unstake() {
ExtBuilder::default().build_and_execute(|| {
ErasToCheckPerBlock::<T>::put(1);
Expand Down
26 changes: 21 additions & 5 deletions frame/fast-unstake/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

//! Types used in the Fast Unstake pallet.

use crate::{Config, MaxChecking};
use crate::Config;
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
traits::Currency, BoundedVec, EqNoBound, PartialEqNoBound, RuntimeDebugNoBound,
Expand All @@ -26,16 +26,32 @@ use scale_info::TypeInfo;
use sp_staking::EraIndex;
use sp_std::prelude::*;

pub type BalanceOf<T> =
/// Maximum number of eras that we might check for a single staker.
///
/// In effect, it is the bonding duration, coming from [`Config::Staking`], plus one.
#[derive(scale_info::TypeInfo, codec::Encode, codec::Decode, codec::MaxEncodedLen)]
#[codec(mel_bound(T: Config))]
#[scale_info(skip_type_params(T))]
pub struct MaxChecking<T: Config>(sp_std::marker::PhantomData<T>);
impl<T: Config> frame_support::traits::Get<u32> for MaxChecking<T> {
fn get() -> u32 {
T::Staking::bonding_duration() + 1
}
}

pub(crate) type BalanceOf<T> =
<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
/// An unstake request.
///
/// This is stored in [`crate::Head`] storage item and points to the current unstake request that is
/// being processed.
#[derive(
Encode, Decode, EqNoBound, PartialEqNoBound, Clone, TypeInfo, RuntimeDebugNoBound, MaxEncodedLen,
)]
#[scale_info(skip_type_params(T))]
pub struct UnstakeRequest<T: Config> {
/// This list of stashes being processed in this request, and their corresponding deposit.
pub(crate) stashes: BoundedVec<(T::AccountId, BalanceOf<T>), T::BatchSize>,
/// This list of stashes are being processed in this request, and their corresponding deposit.
pub stashes: BoundedVec<(T::AccountId, BalanceOf<T>), T::BatchSize>,
/// The list of eras for which they have been checked.
pub(crate) checked: BoundedVec<EraIndex, MaxChecking<T>>,
pub checked: BoundedVec<EraIndex, MaxChecking<T>>,
}
15 changes: 10 additions & 5 deletions frame/support/procedural/src/pallet/expand/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,13 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
}
debug_assert_eq!(fn_weight.len(), methods.len());

let fn_doc = methods.iter().map(|method| &method.docs).collect::<Vec<_>>();
let fn_doc = methods
.iter()
.map(|method| {
let reference = format!("See [`Pallet::{}`].", method.name);
quote!(#reference)
})
.collect::<Vec<_>>();

let args_name = methods
.iter()
Expand Down Expand Up @@ -175,9 +181,8 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
.collect::<Vec<_>>()
});

let default_docs = [syn::parse_quote!(
r"Contains one variant per dispatchable that can be called by an extrinsic."
)];
let default_docs =
[syn::parse_quote!(r"Contains a variant per dispatchable extrinsic that this pallet has.")];
let docs = if docs.is_empty() { &default_docs[..] } else { &docs[..] };

let maybe_compile_error = if def.call.is_none() {
Expand Down Expand Up @@ -274,7 +279,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
#frame_support::Never,
),
#(
#( #[doc = #fn_doc] )*
#[doc = #fn_doc]
#[codec(index = #call_index)]
#fn_name {
#(
Expand Down
Loading