Fix tests of Staking for Crab

[clearloop]

#325

---

Update Steps

• update tests from v2.0.0.alpha.3
• seperate tests
• fix tests failed by LockFor
• fix tests failed by ValidatorPrefs
• fix tests failed by BondDurationInEra

Need to separate into other prs

Both

• check the calculation of power, reward.(marked)
• unbond method(marked)

darwinia_tests

• slash_validators issue(marked)

[clearloop]

I found here are various TODOs need to be done in Staking's tests:

Discussion Category

TODOs in this category, I think they need some discussion.

1. outdated problem: disused API
2. WithdrawLock problem, doesn't have this enum, and doesn't support unbond_withdraw fn

Fixed Category

Some TODOs I just fixed without changing any code, unconsciously, need to be reviewed also.

Trying Category

I'll try to fix these in next 3 commits.

1. NotSlash problem
2. overflow problem(this can refer to substrate's code)
3. unit problem (u128 or u64 in code)

[yanganto]

unbond_withdraw is removed, we will automatically withdraw and delete the staking ledge when there is no value in the account. So I remove the test case and leave notes here.
https://github.com/clearloop/darwinia/commit/9c016417ad0fe55aa21cb238610fb6f26b0f4463#diff-6031f6ad0d6b579272b1748548a364ddR1391
reference #154

[AurevoirXavier]

• The Staking struct in darwinia-staking-pallet is different from the substrate's staking-pallet, so the fix process mainly follows the last version of darwinia-staking-pallet.

Base on the latest substrate tests. And do some modification.

[clearloop]

unbond_withdraw is removed, we will automatically withdraw and delete the staking ledge when there is no value in the account. So I remove the test case and leave notes here.

Yep, and this series can not just referring substrate's test cases in v2.0.0-alpha.3, the WithdrawLock in darwinia is required to be discussed.

[clearloop]

Base on the latest substrate tests. And do some modifications.

Yep, this is the last step of this pr.

[AurevoirXavier]

unbond_withdraw is removed, we will automatically withdraw and delete the staking ledge when there is no value in the account. So I remove the test case and leave notes here.

Yep, and this series can not just referring substrate's test cases in v2.0.0-alpha.3, the WithdrawLock in darwinia is required to be discussed.

But our apis (set_lock, remove_lock...) is the same as substrate's apis.

[clearloop]

But our APIs (set_lock, remove_lock...) are the same as substrate's APIs.

Okay, I'll try to fix the withdraw part after I fixed the trying category

[clearloop]

3 Questions

1. Why use u128 here?

darwinia/frame/staking/src/mock.rs

Line 27 in 19f9355

pub type Balance = u128;

2. Why we got a CAP instead of u64::max_value() in our mock.rs?

darwinia/frame/staking/src/mock.rs

Line 42 in 19f9355

pub const CAP: Balance = 10_000_000_000 * COIN;

3. Why use Power instead of Balance here?

darwinia/frame/staking/src/mock.rs

Line 44 in 19f9355

pub const TOTAL_POWER: Power = 1_000_000_000;

darwinia/frame/staking/src/lib.rs

Line 628 in 19f9355

pub total_power: Power,

Note

I'm not sure if the questions before cause the overflow problem.

[AurevoirXavier]

1. x * COIN will overflow the u64
2. CAP is a hardcoded total circulation of RING
3. • RING provide 50% Power
   • KTON provide 50% Power

[clearloop]

The reward of validating is decided by the votes(power) but not funds(balances) in staking, actual is this a part of darwinia's design...? If not, I'll fix it.

The direct effect of using votes to decide the reward is:

If an Exposure has no power, the validators will get nothing even they got involved in validating.

For nominator

darwinia/frame/staking/src/lib.rs

Line 1852 in 19f9355

let per_u64 = Perbill::from_rational_approximation(i.power, total);

For validator

darwinia/frame/staking/src/lib.rs

Line 1862 in 19f9355

let per_u64 = Perbill::from_rational_approximation(exposure.own_power, total);

[AurevoirXavier]

Please read the gene paper. And understand what power is.

[clearloop]

Please read the gene paper. And understand what power is.

Well, I know the power in gene paper means computing power, the votes above means, power in some Exposure and IndividualExposure are converted from darwinia_phragmen::Votes which is original from darwinia_phragmen::elect, and the doc in darwinia_phragmen::elect says validators are candidates and nominators are voters.

Please checkout this function:

darwinia/frame/staking/src/lib.rs

Line 1840 in 19f9355

fn reward_validator(stash: &T::AccountId, reward: RingBalance<T>) -> (RingPositiveImbalance<T>, Rewards<T>) {

This concept decides how I fix the reward tests(because this part is different from substrate), if there is no problem, I'll fix them to follow the design.

[hackfisher]

Please read the gene paper. And understand what power is.

Well, I know the power in gene paper means computing power, the votes above means, power in some Exposure and IndividualExposure are converted from darwinia_phragmen::Votes which is original from darwinia_phragmen::elect, and the doc in darwinia_phragmen::elect says validators are candidates and nominators are voters.

Please checkout this function:

darwinia/frame/staking/src/lib.rs

Line 1840 in 19f9355

fn reward_validator(stash: &T::AccountId, reward: RingBalance<T>) -> (RingPositiveImbalance<T>, Rewards<T>) {

This concept decides how I fix the reward tests(because this part is different from substrate), if there is no problem, I'll fix them to follow the design.

We use power instead of balance value to represent the contribution of validators/nominators(validators can contribute too), so does the rewards.

It's by design, not sure this answers your question?

[clearloop]

Review Guide

All un-fixed problems have been tagged by @ prefix, here are 3 types of @ family:

• @review: need to be reviewed, most of them are about the calculation of power, reward
  • unbond problem is also under this tag, the unlock problem, what should happen when unbond all stakes?
• @TODO: this tag mainly used for BondingDurationInEra problem in staking tests, these might should be fixed, because substrate v2.0.0.alpha.3 can pass these tests.
• @darwinia: this comments the related tests different from the substrate's tests.

Note

The left problems are all about darwinia features, considering the difficulty of review the whole test files, I think we might merge this pr after passing the Travis' tests, then @yakio and I can divide the tests(whatever you choose, I prefer to fix the easier part...)

[yanganto]

Thanks for the help. I will work on these after merging.

[AurevoirXavier]

All <Module<Test>> can be replace with Staking, but it's ok.