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.

Sign up for GitHub

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

Jump to bottom

fix lottery withdraw issue #1331

Merged
merged 5 commits into from
May 9, 2024
Merged

fix lottery withdraw issue #1331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9da7e72
fix lottery withdraw issue
zqhxuyuan Apr 18, 2024
7cf8a63
fmt
zqhxuyuan Apr 19, 2024
786394e
more test
zqhxuyuan Apr 19, 2024
bfcadbf
fmt
zqhxuyuan Apr 19, 2024
aba0295
Merge branch 'manta' into fix_lottery_withdraw
ferrell-code May 7, 2024
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
2 changes: 2 additions & 0 deletions pallets/pallet-lottery/src/staking/deposit_strategies.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,11 @@ pub(super) fn reactivate_bottom_collators<T: Config>(
.expect("is active collator, therefore it has collator info. qed");

// If collator not exist in delegatorState(PotAccount).delegations, ignore
// delegations has other active collator to stake.
if let Some(state) =
pallet_parachain_staking::Pallet::<T>::delegator_state(crate::Pallet::<T>::account_id())
{
log::debug!("delegator PotAccount has state.");
let mut is_kick = true;
for x in &state.delegations.0 {
if x.owner == collator {
Expand Down
21 changes: 19 additions & 2 deletions pallets/pallet-lottery/src/staking/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,9 +209,26 @@ impl<T: Config> Pallet<T> {
if withdrawal_eligible_collators.is_empty() {
return vec![];
}
let mut eligible_collators: Vec<_> = collators_we_are_unstaking_from;
if let Some(state) =
pallet_parachain_staking::Pallet::<T>::delegator_state(crate::Pallet::<T>::account_id())
{
let owners: Vec<_> = state
.delegations
.0
.iter()
.cloned()
.map(|uc| uc.owner)
.collect();
eligible_collators = withdrawal_eligible_collators
.iter()
.filter(|account| owners.contains(account))
.cloned()
.collect();
}
// first concern: If there are inactive collators we are staked with, prefer these
let (mut collators, balance_unstaked) = withdraw_strategies::unstake_inactive_collators::<T>(
&withdrawal_eligible_collators,
&eligible_collators,
remaining_balance,
);
withdrawals.append(&mut collators);
Expand All @@ -222,7 +239,7 @@ impl<T: Config> Pallet<T> {
// If we have balance to withdraw left over, we have to unstake some healthy collator.
// Unstake starting from the highest overallocated collator ( since that yields the lowest APY ) going down until request is satisfied
let (mut collators, balance_unstaked) = withdraw_strategies::unstake_least_apy_collators::<T>(
&withdrawal_eligible_collators
&eligible_collators
.into_iter()
.filter(|collator| !withdrawals.contains(collator))
.collect(),
Expand Down
73 changes: 56 additions & 17 deletions pallets/pallet-lottery/src/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1062,7 +1062,7 @@ fn delegator_kicked_not_in_state_cannot_deposit() {
assert_ok!(Lottery::deposit(Origin::signed(ALICE), 150_000_000 * UNIT));
assert_eq!(crate::StakedCollators::<Test>::iter().count(), 1);

// Check PotAmount state, exist!
// Check PotAccount state, exist!
let pot_state1 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id()).expect("just delegated => exists");
assert_eq!(pot_state1.delegations.0[0].owner, BOB);
assert_eq!(pot_state1.delegations.0[0].amount, 150_000_000 * UNIT);
Expand All @@ -1089,12 +1089,12 @@ fn delegator_kicked_not_in_state_cannot_deposit() {
assert_eq!(charlie_state.delegations.0[0].owner, BOB);
assert_eq!(charlie_state.delegations.0[0].amount, 160_000_000 * UNIT);

// Check PotAmount state, not exist!
// Check PotAccount state, not exist!
let _pot_state2 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id());
assert!(_pot_state2.is_none());

// Delegator: Pot Account is not exist on DelegatorState.
// Now Pot account is kicked, try to deposit, failed!
// Delegator: PotAccount is not exist on DelegatorState.
// Now PotAccount is kicked, try to deposit, failed!
// In this case, we should delegate instead bond more.
assert_noop!(
Lottery::deposit(Origin::signed(ALICE), 150_000_000 * UNIT),
Expand Down Expand Up @@ -1195,7 +1195,7 @@ fn delegator_kicked_when_reactivate_bottom_should_ignored() {
assert_ok!(Lottery::deposit(Origin::signed(ALICE), 51_000_000 * UNIT));
assert_eq!(crate::StakedCollators::<Test>::iter().count(), 2);

// Pot Account delegate to two collators.
// PotAccount delegate to two collators.
let pot_state1 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(pot_state1.delegations.len(), 2);
Expand Down Expand Up @@ -1237,8 +1237,8 @@ fn delegator_kicked_when_reactivate_bottom_should_ignored() {
9,
0
));
// Pot Account now is kicked from collator BOB's delegator list.
// But Pot Account is still on the delegator list of CHARLIE.
// PotAccount now is kicked from collator BOB's delegator list.
// But PotAccount is still on the delegator list of CHARLIE.
let pot_state2 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(pot_state2.delegations.len(), 1);
Expand Down Expand Up @@ -1298,7 +1298,6 @@ fn delegator_kicked_when_reactivate_bottom_should_ignored() {

#[test]
fn delegator_unstaking_then_kicked_should_ignored() {
let reserve = 10_000 * UNIT;
let balance = 500_000_000 * UNIT;
ExtBuilder::default()
.with_balances(vec![
Expand Down Expand Up @@ -1326,7 +1325,7 @@ fn delegator_unstaking_then_kicked_should_ignored() {
(DELEGATOR3, CHARLIE, 60_000_000 * UNIT),
(DELEGATOR4, CHARLIE, 50_000_000 * UNIT),
])
.with_funded_lottery_account(reserve)
.with_funded_lottery_account(balance)
.build()
.execute_with(|| {
assert_ok!(Lottery::deposit(Origin::signed(ALICE), 51_000_000 * UNIT));
Expand All @@ -1343,7 +1342,7 @@ fn delegator_unstaking_then_kicked_should_ignored() {
assert_eq!(crate::StakedCollators::<Test>::iter().count(), 2);
assert_eq!(crate::UnstakingCollators::<Test>::get().len(), 1);

// Pot Account delegate to two collators.
// PotAccount delegate to two collators.
let pot_state1 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(pot_state1.delegations.len(), 2);
Expand Down Expand Up @@ -1392,6 +1391,7 @@ fn delegator_unstaking_then_kicked_should_ignored() {
assert_eq!(pot_state2.delegations.0[0].amount, 51_000_000 * UNIT);

assert_eq!(crate::StakedCollators::<Test>::iter().count(), 2);
assert_eq!(crate::UnstakingCollators::<Test>::get().len(), 1);

let collator1_state = ParachainStaking::candidate_info(BOB).unwrap();
let collator2_state = ParachainStaking::candidate_info(CHARLIE).unwrap();
Expand All @@ -1412,29 +1412,68 @@ fn delegator_unstaking_then_kicked_should_ignored() {
50_000_000 * UNIT
);

// Check PotAmount state, exist!
// Check PotAccount state, exist!
let _pot_state2 =
ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(_pot_state2.delegations.len(), 1);
assert_eq!(_pot_state2.delegations.0[0].owner, CHARLIE);
assert_eq!(_pot_state2.delegations.0[0].amount, 51_000_000 * UNIT);

// Staked to BOB was already kicked Pot Account.
// Staked to BOB was already kicked PotAccount.
// And staked to CHARLIE is unstaking, now there're no collators to deposit.
// If we choose random one which is BOB, then delegator_bond_more has error.
assert_noop!(
Lottery::deposit(Origin::signed(ALICE), 53_000_000 * UNIT),
pallet_parachain_staking::Error::<Test>::DelegationDNE
);

// Check PotAmount state, exist!
// Check PotAccount state, exist!
let _pot_state3 =
ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(_pot_state3.delegations.len(), 1);
assert_eq!(_pot_state3.delegations.0[0].owner, CHARLIE);
assert_eq!(_pot_state3.delegations.0[0].amount, 51_000_000 * UNIT);

assert_eq!(crate::StakedCollators::<Test>::iter().count(), 2);
assert_eq!(crate::UnstakingCollators::<Test>::get().len(), 1);

// All collator not eligible, withdraw failed.
assert_noop!(
Lottery::request_withdraw(Origin::signed(ALICE), 51_000_000 * UNIT),
sp_runtime::DispatchError::Other(
"FATAL: Didn't unstake the full requested balance (or more)"
)
);

// draw lottery to make unstaking collator removed.
pallet_parachain_staking::AwardedPts::<Test>::insert(2, BOB, 20);
pallet_parachain_staking::AwardedPts::<Test>::insert(2, CHARLIE, 20);
roll_to_round_begin(3);
assert_ok!(Lottery::process_matured_withdrawals(RawOrigin::Root.into()));

assert_eq!(crate::StakedCollators::<Test>::iter().count(), 1);
assert_eq!(crate::UnstakingCollators::<Test>::get().len(), 0);
assert_eq!(crate::StakedCollators::<Test>::get(BOB), 51_000_000 * UNIT);

// DelegatorState of PotAccount is empty
let _pot_state4 =
ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id());
assert!(_pot_state4.is_none());

// The only one staked collator BOB is not exist in parachain staking DelegatorState.
assert_noop!(
Lottery::request_withdraw(Origin::signed(ALICE), 51_000_000 * UNIT),
sp_runtime::DispatchError::Other(
"FATAL: Didn't unstake the full requested balance (or more)"
)
);

assert_noop!(
Lottery::deposit(Origin::signed(ALICE), 53_000_000 * UNIT),
pallet_parachain_staking::Error::<Test>::DelegatorDNE
);
});
}

Expand Down Expand Up @@ -1485,7 +1524,7 @@ fn delegator_unstaking_kicked_same_collator_should_ignored() {
assert_eq!(crate::StakedCollators::<Test>::iter().count(), 2);
assert_eq!(crate::UnstakingCollators::<Test>::get().len(), 1);

// Pot Account delegate to two collators.
// PotAccount delegate to two collators.
let pot_state1 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(pot_state1.delegations.len(), 2);
Expand Down Expand Up @@ -1539,7 +1578,7 @@ fn delegator_unstaking_kicked_same_collator_should_ignored() {
let pot_state2 = ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
assert_eq!(pot_state2.delegations.len(), 1);
// We can ensure that Pot Account now only staked to Charlie, but can't ensure the staked amount.
// We can ensure that PotAccount now only staked to Charlie, but can't ensure the staked amount.
assert_eq!(pot_state2.delegations.0[0].owner, CHARLIE);
if first_deposit_staked_to_bob {
assert_eq!(pot_state2.delegations.0[0].amount, 51_000_000 * UNIT);
Expand Down Expand Up @@ -1589,7 +1628,7 @@ fn delegator_unstaking_kicked_same_collator_should_ignored() {
);
}

// Check PotAmount state, exist!
// Check PotAccount state, exist!
let _pot_state2 =
ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
Expand All @@ -1610,7 +1649,7 @@ fn delegator_unstaking_kicked_same_collator_should_ignored() {
);
}

// Check PotAmount state, exist!
// Check PotAccount state, exist!
let _pot_state3 =
ParachainStaking::delegator_state(crate::Pallet::<Test>::account_id())
.expect("just delegated => exists");
Expand Down
Loading