Stake merge inactive lockup (backport #17376) (#17390)

* stake: plumb `can_merge_expired_lockups` feature flag

(cherry picked from commit 74ac6ab80f)

* stake: merge accounts with mismatched, but expired lockups

(cherry picked from commit 019bccab51)

Co-authored-by: Trent Nelson <trent@solana.com>
This commit is contained in:
mergify[bot]
2021-05-21 20:00:59 +00:00
committed by GitHub
parent 25333abd96
commit 8fe5b41f5f
2 changed files with 217 additions and 24 deletions

View File

@ -591,12 +591,15 @@ pub fn process_instruction(
}
StakeInstruction::Merge => {
let source_stake = &next_keyed_account(keyed_accounts)?;
let can_merge_expired_lockups =
invoke_context.is_feature_active(&feature_set::stake_program_v4::id());
me.merge(
invoke_context,
source_stake,
&from_keyed_account::<Clock>(next_keyed_account(keyed_accounts)?)?,
&from_keyed_account::<StakeHistory>(next_keyed_account(keyed_accounts)?)?,
&signers,
can_merge_expired_lockups,
)
}

View File

@ -888,6 +888,7 @@ pub trait StakeAccount {
clock: &Clock,
stake_history: &StakeHistory,
signers: &HashSet<Pubkey>,
can_merge_expired_lockups: bool,
) -> Result<(), InstructionError>;
fn withdraw(
&self,
@ -1189,6 +1190,7 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
clock: &Clock,
stake_history: &StakeHistory,
signers: &HashSet<Pubkey>,
can_merge_expired_lockups: bool,
) -> Result<(), InstructionError> {
// Ensure source isn't spoofed
if source_account.owner()? != id() {
@ -1211,8 +1213,16 @@ impl<'a> StakeAccount for KeyedAccount<'a> {
let source_merge_kind =
MergeKind::get_if_mergeable(invoke_context, source_account, clock, stake_history)?;
let clock = if can_merge_expired_lockups {
Some(clock)
} else {
None
};
ic_msg!(invoke_context, "Merging stake accounts");
if let Some(merged_state) = stake_merge_kind.merge(invoke_context, source_merge_kind)? {
if let Some(merged_state) =
stake_merge_kind.merge(invoke_context, source_merge_kind, clock)?
{
self.set_state(&merged_state)?;
}
@ -1376,13 +1386,24 @@ impl MergeKind {
invoke_context: &dyn InvokeContext,
stake: &Meta,
source: &Meta,
clock: Option<&Clock>,
) -> Result<(), InstructionError> {
let can_merge_lockups = match clock {
// pre-v4 behavior. lockups must match, even when expired
None => stake.lockup == source.lockup,
// v4 behavior. lockups may mismatch so long as both have expired
Some(clock) => {
stake.lockup == source.lockup
|| (!stake.lockup.is_in_force(clock, None)
&& !source.lockup.is_in_force(clock, None))
}
};
// `rent_exempt_reserve` has no bearing on the mergeability of accounts,
// as the source account will be culled by runtime once the operation
// succeeds. Considering it here would needlessly prevent merging stake
// accounts with differing data lengths, which already exist in the wild
// due to an SDK bug
if stake.authorized == source.authorized && stake.lockup == source.lockup {
if stake.authorized == source.authorized && can_merge_lockups {
Ok(())
} else {
ic_msg!(invoke_context, "Unable to merge due to metadata mismatch");
@ -1435,8 +1456,9 @@ impl MergeKind {
self,
invoke_context: &dyn InvokeContext,
source: Self,
clock: Option<&Clock>,
) -> Result<Option<StakeState>, InstructionError> {
Self::metas_can_merge(invoke_context, self.meta(), source.meta())?;
Self::metas_can_merge(invoke_context, self.meta(), source.meta(), clock)?;
self.active_stake()
.zip(source.active_stake())
.map(|(stake, source)| Self::active_stakes_can_merge(invoke_context, stake, source))
@ -5362,7 +5384,8 @@ mod tests {
&source_stake_keyed_account,
&Clock::default(),
&StakeHistory::default(),
&HashSet::new()
&HashSet::new(),
false,
),
Err(InstructionError::MissingRequiredSignature)
);
@ -5373,7 +5396,8 @@ mod tests {
&source_stake_keyed_account,
&Clock::default(),
&StakeHistory::default(),
&signers
&signers,
false,
),
Ok(())
);
@ -5465,6 +5489,7 @@ mod tests {
&Clock::default(),
&StakeHistory::default(),
&signers,
false,
),
Err(InstructionError::InvalidArgument),
);
@ -5522,6 +5547,7 @@ mod tests {
&Clock::default(),
&StakeHistory::default(),
&wrong_signers,
false,
),
Err(InstructionError::MissingRequiredSignature)
);
@ -5533,6 +5559,7 @@ mod tests {
&Clock::default(),
&StakeHistory::default(),
&signers,
false,
),
Err(StakeError::MergeMismatch.into())
);
@ -5585,6 +5612,7 @@ mod tests {
&Clock::default(),
&StakeHistory::default(),
&signers,
false,
),
Err(InstructionError::InvalidAccountData)
);
@ -5633,7 +5661,8 @@ mod tests {
&source_stake_keyed_account,
&Clock::default(),
&StakeHistory::default(),
&signers
&signers,
false,
),
Err(InstructionError::IncorrectProgramId)
);
@ -5730,6 +5759,7 @@ mod tests {
clock,
stake_history,
signers,
false,
);
if result.is_ok() {
assert_eq!(test_source_keyed.state(), Ok(StakeState::Uninitialized),);
@ -6461,11 +6491,19 @@ mod tests {
&good_delegation
)
.is_err());
}
#[test]
fn test_metas_can_merge_pre_v4() {
let invoke_context = MockInvokeContext::default();
// Identical Metas can merge
assert!(
MergeKind::metas_can_merge(&invoke_context, &Meta::default(), &Meta::default()).is_ok()
);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&Meta::default(),
None,
)
.is_ok());
let mismatched_rent_exempt_reserve_ok = Meta {
rent_exempt_reserve: 42,
@ -6478,13 +6516,15 @@ mod tests {
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&mismatched_rent_exempt_reserve_ok
&mismatched_rent_exempt_reserve_ok,
None,
)
.is_ok());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&mismatched_rent_exempt_reserve_ok,
&Meta::default()
&Meta::default(),
None,
)
.is_ok());
@ -6502,13 +6542,15 @@ mod tests {
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&mismatched_authorized_fails
&mismatched_authorized_fails,
None,
)
.is_err());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&mismatched_authorized_fails,
&Meta::default()
&Meta::default(),
None,
)
.is_err());
@ -6524,17 +6566,165 @@ mod tests {
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&mismatched_lockup_fails
&mismatched_lockup_fails,
None,
)
.is_err());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&mismatched_lockup_fails,
&Meta::default()
&Meta::default(),
None,
)
.is_err());
}
#[test]
fn test_metas_can_merge_v4() {
let invoke_context = MockInvokeContext::default();
// Identical Metas can merge
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&Meta::default(),
Some(&Clock::default())
)
.is_ok());
let mismatched_rent_exempt_reserve_ok = Meta {
rent_exempt_reserve: 42,
..Meta::default()
};
assert_ne!(
mismatched_rent_exempt_reserve_ok.rent_exempt_reserve,
Meta::default().rent_exempt_reserve,
);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&mismatched_rent_exempt_reserve_ok,
Some(&Clock::default())
)
.is_ok());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&mismatched_rent_exempt_reserve_ok,
&Meta::default(),
Some(&Clock::default())
)
.is_ok());
let mismatched_authorized_fails = Meta {
authorized: Authorized {
staker: Pubkey::new_unique(),
withdrawer: Pubkey::new_unique(),
},
..Meta::default()
};
assert_ne!(
mismatched_authorized_fails.authorized,
Meta::default().authorized,
);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&Meta::default(),
&mismatched_authorized_fails,
Some(&Clock::default())
)
.is_err());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&mismatched_authorized_fails,
&Meta::default(),
Some(&Clock::default())
)
.is_err());
let lockup1_timestamp = 42;
let lockup2_timestamp = 4242;
let lockup1_epoch = 4;
let lockup2_epoch = 42;
let metas_with_lockup1 = Meta {
lockup: Lockup {
unix_timestamp: lockup1_timestamp,
epoch: lockup1_epoch,
custodian: Pubkey::new_unique(),
},
..Meta::default()
};
let metas_with_lockup2 = Meta {
lockup: Lockup {
unix_timestamp: lockup2_timestamp,
epoch: lockup2_epoch,
custodian: Pubkey::new_unique(),
},
..Meta::default()
};
// Mismatched lockups fail when both in force
assert_ne!(metas_with_lockup1.lockup, Meta::default().lockup);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup1,
&metas_with_lockup2,
Some(&Clock::default())
)
.is_err());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup2,
&metas_with_lockup1,
Some(&Clock::default())
)
.is_err());
let clock = Clock {
epoch: lockup1_epoch + 1,
unix_timestamp: lockup1_timestamp + 1,
..Clock::default()
};
// Mismatched lockups fail when either in force
assert_ne!(metas_with_lockup1.lockup, Meta::default().lockup);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup1,
&metas_with_lockup2,
Some(&clock)
)
.is_err());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup2,
&metas_with_lockup1,
Some(&clock)
)
.is_err());
let clock = Clock {
epoch: lockup2_epoch + 1,
unix_timestamp: lockup2_timestamp + 1,
..Clock::default()
};
// Mismatched lockups succeed when both expired
assert_ne!(metas_with_lockup1.lockup, Meta::default().lockup);
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup1,
&metas_with_lockup2,
Some(&clock)
)
.is_ok());
assert!(MergeKind::metas_can_merge(
&invoke_context,
&metas_with_lockup2,
&metas_with_lockup1,
Some(&clock)
)
.is_ok());
}
#[test]
fn test_merge_kind_get_if_mergeable() {
let invoke_context = MockInvokeContext::default();
@ -6790,37 +6980,37 @@ mod tests {
assert_eq!(
inactive
.clone()
.merge(&invoke_context, inactive.clone())
.merge(&invoke_context, inactive.clone(), None)
.unwrap(),
None
);
assert_eq!(
inactive
.clone()
.merge(&invoke_context, activation_epoch.clone())
.merge(&invoke_context, activation_epoch.clone(), None)
.unwrap(),
None
);
assert!(inactive
.clone()
.merge(&invoke_context, fully_active.clone())
.merge(&invoke_context, fully_active.clone(), None)
.is_err());
assert!(activation_epoch
.clone()
.merge(&invoke_context, fully_active.clone())
.merge(&invoke_context, fully_active.clone(), None)
.is_err());
assert!(fully_active
.clone()
.merge(&invoke_context, inactive.clone())
.merge(&invoke_context, inactive.clone(), None)
.is_err());
assert!(fully_active
.clone()
.merge(&invoke_context, activation_epoch.clone())
.merge(&invoke_context, activation_epoch.clone(), None)
.is_err());
let new_state = activation_epoch
.clone()
.merge(&invoke_context, inactive)
.merge(&invoke_context, inactive, None)
.unwrap()
.unwrap();
let delegation = new_state.delegation().unwrap();
@ -6828,7 +7018,7 @@ mod tests {
let new_state = activation_epoch
.clone()
.merge(&invoke_context, activation_epoch)
.merge(&invoke_context, activation_epoch, None)
.unwrap()
.unwrap();
let delegation = new_state.delegation().unwrap();
@ -6839,7 +7029,7 @@ mod tests {
let new_state = fully_active
.clone()
.merge(&invoke_context, fully_active)
.merge(&invoke_context, fully_active, None)
.unwrap()
.unwrap();
let delegation = new_state.delegation().unwrap();