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

[Enhancement] Convert fast-unstake to use StakingInterface, decouplin… #12424

Merged
paritytech-processbot merged 48 commits into from
Oct 29, 2022
Merged

[Enhancement] Convert fast-unstake to use StakingInterface, decouplin… #12424

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
3a11e89
[Enhancement] Convert fast-unstake to use StakingInterface, decouplin…
ruseinov Oct 5, 2022
4f39589
Update primitives/staking/src/lib.rs
ruseinov Oct 6, 2022
9b8e123
Update primitives/staking/src/lib.rs
ruseinov Oct 6, 2022
25520d6
fix validator comment
ruseinov Oct 6, 2022
ad90903
remove todo
ruseinov Oct 10, 2022
89b32b5
ideas from Kian (#12474)
ruseinov Oct 11, 2022
f51c073
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 11, 2022
41d04ad
Rename StakingInterface -> Staking for nomination-pools
ruseinov Oct 12, 2022
7fc52f8
Staking fixes
ruseinov Oct 12, 2022
d3958ba
StakingInterface changes
ruseinov Oct 12, 2022
2ff689c
fix fast-unstake
ruseinov Oct 12, 2022
1fc6510
fix nomination-pools
ruseinov Oct 12, 2022
28d15d7
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 18, 2022
0705029
Fix fast-unstake tests
ruseinov Oct 18, 2022
39cf580
Fix benches for fast-unstake
ruseinov Oct 18, 2022
8093567
fix is_unbonding
ruseinov Oct 18, 2022
30a5882
fix nomination pools
ruseinov Oct 18, 2022
37107ef
fix node code
ruseinov Oct 19, 2022
097a9d1
add mock comments
ruseinov Oct 19, 2022
613a701
fix imports
ruseinov Oct 19, 2022
e785dd5
remove todo
ruseinov Oct 19, 2022
03586d7
more fixes
ruseinov Oct 19, 2022
20a4f29
more fixes
ruseinov Oct 19, 2022
9807cf2
bench fixes
ruseinov Oct 19, 2022
3639af8
more fixes
ruseinov Oct 19, 2022
b7827d5
more fixes
ruseinov Oct 20, 2022
5232b99
import fix
ruseinov Oct 20, 2022
f881456
more fixes
ruseinov Oct 20, 2022
c3cee71
more bench fix
ruseinov Oct 20, 2022
068c565
refix
ruseinov Oct 20, 2022
ff36f90
refix
ruseinov Oct 20, 2022
6f862e4
Update primitives/staking/src/lib.rs
ruseinov Oct 25, 2022
bff58a4
is_unbonding returns a result
ruseinov Oct 20, 2022
bdb525e
fix
ruseinov Oct 25, 2022
4bd57b1
review fixes
ruseinov Oct 25, 2022
f6040ec
more review fixes
ruseinov Oct 25, 2022
8c5604d
more fixes
ruseinov Oct 25, 2022
731fab2
more fixes
ruseinov Oct 25, 2022
b90082a
Update frame/fast-unstake/src/benchmarking.rs
ruseinov Oct 26, 2022
1eee8a5
remove redundant CurrencyBalance from nom-pools
ruseinov Oct 26, 2022
1f4fad2
remove CB
ruseinov Oct 26, 2022
27530dc
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 26, 2022
90cec9f
rephrase
ruseinov Oct 26, 2022
b1560aa
Apply suggestions from code review
kianenigma Oct 27, 2022
75a36bb
Update frame/nomination-pools/src/tests.rs
kianenigma Oct 27, 2022
b3e843e
finish damn renamed
kianenigma Oct 27, 2022
963837b
clippy fix
ruseinov Oct 27, 2022
cbed2f9
fix
ruseinov Oct 27, 2022
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
Prev Previous commit
Next Next commit
fix validator comment
  • Loading branch information
@ruseinov
ruseinov committed Oct 6, 2022
commit 25520d60a5f5a56e40ab809a078d4be5dc87b668
2 changes: 1 addition & 1 deletion frame/staking/src/pallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ pub mod pallet {
type WeightInfo: WeightInfo;
}

/// The ideal number of staking participants.
/// The ideal number of active validators.
#[pallet::storage]
#[pallet::getter(fn validator_count)]
pub type ValidatorCount<T> = StorageValue<_, u32, ValueQuery>;
Expand Down
2 changes: 1 addition & 1 deletion primitives/staking/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ pub trait StakingInterface {
/// Get `StakingLedger` by controller account.
fn get_ledger(ctrl: &Self::AccountId) -> StakingLedger<Self>;
Copy link
Contributor Author

@ruseinov ruseinov Oct 6, 2022

Choose a reason for hiding this comment

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

@kianenigma We might want to get rid of those, seems redundant now.:

	fn active_stake(controller: &Self::AccountId) -> Option<Self::Balance> {
		Self::ledger(controller).map(|l| l.active)
	}

	fn total_stake(controller: &Self::AccountId) -> Option<Self::Balance> {
		Self::ledger(controller).map(|l| l.total)
	}

Copy link
Contributor

@kianenigma kianenigma Oct 9, 2022

Choose a reason for hiding this comment

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

Now I see why the old code was not returning StakingLedger. We cannot use StakingLedger in pallet-nomination-pools or pallet-fast-unstake because it is defined in pallet-stakin.

Putting it in a shared crate is also too much of a stretch in my opinion, because these types are arguably an implementation detail.

I think we should keep these, not have get_ledger and get_exposure and instead create other API function that express a condition, a state or a behavior rather than the explicit type.

Copy link
Contributor Author

@ruseinov ruseinov Oct 10, 2022

Choose a reason for hiding this comment

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

Why don't we introduce a special type that will take values out of StakingLedger? Like type Ledger. It could even be an alias for now and then change if needed.

This makes very little sense to keep as is, because:

			let ledger =
				pallet_staking::Ledger::<T>::get(&ctrl).ok_or(Error::<T>::NotController)?;
			ensure!(!Queue::<T>::contains_key(&ledger.stash), Error::<T>::AlreadyQueued);
			ensure!(
				Head::<T>::get().map_or(true, |UnstakeRequest { stash, .. }| stash != ledger.stash),
				Error::<T>::AlreadyHead
			);
			// second part of the && is defensive.
			ensure!(
				ledger.active == ledger.total && ledger.unlocking.is_empty(),
				Error::<T>::NotFullyBonded
			);

We'd need to call three different methods that operate on exactly the same storage item. Actually no, 4!
At the very least that's not very dry and also a bit of overhead.

Copy link
Contributor

@kianenigma kianenigma Oct 11, 2022

Choose a reason for hiding this comment

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

my answer to this and many other aspects can be found in ru/feature/fast-unstake-istaking...kiz-ru/feature/fast-unstake-istaking


/// The ideal number of staking participants.
/// The ideal number of active validators.
fn validator_count() -> u32;

/// Whether or not there is an ongoing election.
Expand Down