From 4b1290284f3baedca134596582c604704474adcd Mon Sep 17 00:00:00 2001 From: Nisheeth Barthwal Date: Thu, 21 Jul 2022 20:05:59 +0200 Subject: [PATCH] [MOON-1745] re-add status Leaving as deprecated (#1702) * merge master * fix tests * fix toJSON --- .../src/delegation_requests.rs | 48 ++++++- pallets/parachain-staking/src/tests.rs | 120 +++++++++++++++++- pallets/parachain-staking/src/types.rs | 4 + .../tests/test-staking/test-delegator-join.ts | 2 +- 4 files changed, 171 insertions(+), 3 deletions(-) diff --git a/pallets/parachain-staking/src/delegation_requests.rs b/pallets/parachain-staking/src/delegation_requests.rs index 42f96febaf..07cef17781 100644 --- a/pallets/parachain-staking/src/delegation_requests.rs +++ b/pallets/parachain-staking/src/delegation_requests.rs @@ -20,7 +20,7 @@ use crate::pallet::{ BalanceOf, CandidateInfo, Config, DelegationScheduledRequests, DelegatorState, Error, Event, Pallet, Round, RoundIndex, Total, }; -use crate::Delegator; +use crate::{Delegator, DelegatorStatus}; use frame_support::ensure; use frame_support::traits::Get; use frame_support::{dispatch::DispatchResultWithPostInfo, RuntimeDebug}; @@ -342,6 +342,13 @@ impl Pallet { let now = >::get().current; let when = now.saturating_add(T::LeaveDelegatorsDelay::get()); + // lazy migration for DelegatorStatus::Leaving + #[allow(deprecated)] + if matches!(state.status, DelegatorStatus::Leaving(_)) { + state.status = DelegatorStatus::Active; + >::insert(delegator.clone(), state.clone()); + } + // it is assumed that a multiple delegations to the same collator does not exist, else this // will cause a bug - the last duplicate delegation update will be the only one applied. let mut existing_revoke_count = 0; @@ -398,6 +405,15 @@ impl Pallet { let mut state = >::get(&delegator).ok_or(>::DelegatorDNE)?; let mut updated_scheduled_requests = vec![]; + // backwards compatible handling for DelegatorStatus::Leaving + #[allow(deprecated)] + if matches!(state.status, DelegatorStatus::Leaving(_)) { + state.status = DelegatorStatus::Active; + >::insert(delegator.clone(), state.clone()); + Self::deposit_event(Event::DelegatorExitCancelled { delegator }); + return Ok(().into()); + } + // pre-validate that all delegations have a Revoke request. for bond in &state.delegations.0 { let collator = bond.owner.clone(); @@ -444,6 +460,36 @@ impl Pallet { ); let now = >::get().current; + // backwards compatible handling for DelegatorStatus::Leaving + #[allow(deprecated)] + if let DelegatorStatus::Leaving(when) = state.status { + ensure!( + >::get().current >= when, + Error::::DelegatorCannotLeaveYet + ); + + for bond in state.delegations.0.clone() { + if let Err(error) = Self::delegator_leaves_candidate( + bond.owner.clone(), + delegator.clone(), + bond.amount, + ) { + log::warn!( + "STORAGE CORRUPTED \nDelegator leaving collator failed with error: {:?}", + error + ); + } + + Self::delegation_remove_request_with_state(&bond.owner, &delegator, &mut state); + } + >::remove(&delegator); + Self::deposit_event(Event::DelegatorLeft { + delegator, + unstaked_amount: state.total, + }); + return Ok(().into()); + } + let mut validated_scheduled_requests = vec![]; // pre-validate that all delegations have a Revoke request that can be executed now. for bond in &state.delegations.0 { diff --git a/pallets/parachain-staking/src/tests.rs b/pallets/parachain-staking/src/tests.rs index 30571991f5..a26d987cdb 100644 --- a/pallets/parachain-staking/src/tests.rs +++ b/pallets/parachain-staking/src/tests.rs @@ -30,7 +30,8 @@ use crate::{ assert_eq_events, assert_eq_last_events, assert_event_emitted, assert_last_event, assert_tail_eq, set::OrderedSet, AtStake, Bond, BottomDelegations, CandidateInfo, CandidateMetadata, CandidatePool, CapacityStatus, CollatorStatus, DelegationScheduledRequests, - Delegations, DelegatorAdded, Error, Event, Range, TopDelegations, DELEGATOR_LOCK_ID, + Delegations, DelegatorAdded, DelegatorState, DelegatorStatus, Error, Event, Range, + TopDelegations, DELEGATOR_LOCK_ID, }; use frame_support::{assert_noop, assert_ok}; use sp_runtime::{traits::Zero, DispatchError, ModuleError, Perbill, Percent}; @@ -9451,3 +9452,120 @@ mod jit_migrate_reserve_to_locks_tests { // * other tests around lquidity checks (can't bond_more if not enough unlocked amount, etc) // * request cancellation } +#[allow(deprecated)] +#[test] +fn test_delegator_with_deprecated_status_leaving_can_schedule_leave_delegators_as_fix() { + ExtBuilder::default() + .with_balances(vec![(1, 20), (2, 40)]) + .with_candidates(vec![(1, 20)]) + .with_delegations(vec![(2, 1, 10)]) + .build() + .execute_with(|| { + >::mutate(2, |value| { + value.as_mut().map(|mut state| { + state.status = DelegatorStatus::Leaving(2); + }) + }); + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Leaving(_))); + + assert_ok!(ParachainStaking::schedule_leave_delegators(Origin::signed( + 2 + ))); + assert!(>::get(1) + .iter() + .any(|r| r.delegator == 2 && matches!(r.action, DelegationAction::Revoke(_)))); + assert_last_event!(MetaEvent::ParachainStaking(Event::DelegatorExitScheduled { + round: 1, + delegator: 2, + scheduled_exit: 3 + })); + + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Active)); + }); +} + +#[allow(deprecated)] +#[test] +fn test_delegator_with_deprecated_status_leaving_can_cancel_leave_delegators_as_fix() { + ExtBuilder::default() + .with_balances(vec![(1, 20), (2, 40)]) + .with_candidates(vec![(1, 20)]) + .with_delegations(vec![(2, 1, 10)]) + .build() + .execute_with(|| { + >::mutate(2, |value| { + value.as_mut().map(|mut state| { + state.status = DelegatorStatus::Leaving(2); + }) + }); + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Leaving(_))); + + assert_ok!(ParachainStaking::cancel_leave_delegators(Origin::signed(2))); + assert_last_event!(MetaEvent::ParachainStaking(Event::DelegatorExitCancelled { + delegator: 2 + })); + + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Active)); + }); +} + +#[allow(deprecated)] +#[test] +fn test_delegator_with_deprecated_status_leaving_can_execute_leave_delegators_as_fix() { + ExtBuilder::default() + .with_balances(vec![(1, 20), (2, 40)]) + .with_candidates(vec![(1, 20)]) + .with_delegations(vec![(2, 1, 10)]) + .build() + .execute_with(|| { + >::mutate(2, |value| { + value.as_mut().map(|mut state| { + state.status = DelegatorStatus::Leaving(2); + }) + }); + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Leaving(_))); + + roll_to(10); + assert_ok!(ParachainStaking::execute_leave_delegators( + Origin::signed(2), + 2, + 1 + )); + assert_event_emitted!(Event::DelegatorLeft { + delegator: 2, + unstaked_amount: 10 + }); + + let state = >::get(2); + assert!(state.is_none()); + }); +} + +#[allow(deprecated)] +#[test] +fn test_delegator_with_deprecated_status_leaving_cannot_execute_leave_delegators_early_no_fix() { + ExtBuilder::default() + .with_balances(vec![(1, 20), (2, 40)]) + .with_candidates(vec![(1, 20)]) + .with_delegations(vec![(2, 1, 10)]) + .build() + .execute_with(|| { + >::mutate(2, |value| { + value.as_mut().map(|mut state| { + state.status = DelegatorStatus::Leaving(2); + }) + }); + let state = >::get(2); + assert!(matches!(state.unwrap().status, DelegatorStatus::Leaving(_))); + + assert_noop!( + ParachainStaking::execute_leave_delegators(Origin::signed(2), 2, 1), + Error::::DelegatorCannotLeaveYet + ); + }); +} diff --git a/pallets/parachain-staking/src/types.rs b/pallets/parachain-staking/src/types.rs index 72b362a1c6..f371a26f5a 100644 --- a/pallets/parachain-staking/src/types.rs +++ b/pallets/parachain-staking/src/types.rs @@ -1211,10 +1211,14 @@ impl From> for CollatorSnapshot } } +#[allow(deprecated)] #[derive(Clone, PartialEq, Encode, Decode, RuntimeDebug, TypeInfo)] pub enum DelegatorStatus { /// Active with no scheduled exit Active, + /// Schedule exit to revoke all ongoing delegations + #[deprecated(note = "must only be used for backwards compatibility reasons")] + Leaving(RoundIndex), } #[derive(Clone, Encode, Decode, RuntimeDebug, TypeInfo)] diff --git a/tests/tests/test-staking/test-delegator-join.ts b/tests/tests/test-staking/test-delegator-join.ts index c18f206c5a..a11790d9b1 100644 --- a/tests/tests/test-staking/test-delegator-join.ts +++ b/tests/tests/test-staking/test-delegator-join.ts @@ -165,7 +165,7 @@ describeDevMoonbeam("Staking - Delegator Join - valid request", (context) => { ], id: ethan.address, lessTotal: 0, - status: "Active", + status: { active: null }, total: numberToHex(MIN_GLMR_DELEGATOR), }); });