-
Notifications
You must be signed in to change notification settings - Fork 48
program: Updates for better 1 SOL minimum delegation support #756
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
17df93f
c880092
3bb4a26
baafee7
a716dee
916bc44
3693520
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,4 +17,4 @@ check-cfg = [ | |
| ] | ||
|
|
||
| [workspace.metadata.cli] | ||
| solana = "3.1.14" | ||
| solana = "4.0.3" | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -916,6 +916,10 @@ async fn fail_withdraw_from_transient() { | |
| .await | ||
| .unwrap(); | ||
|
|
||
| let stake_minimum_delegation = | ||
| stake_get_minimum_delegation(&mut context.banks_client, &context.payer, &last_blockhash) | ||
| .await; | ||
|
|
||
| let rent = context.banks_client.get_rent().await.unwrap(); | ||
| let stake_rent = rent.minimum_balance(std::mem::size_of::<stake::state::StakeStateV2>()); | ||
|
|
||
|
|
@@ -927,7 +931,7 @@ async fn fail_withdraw_from_transient() { | |
| &last_blockhash, | ||
| &validator_stake_account.stake_account, | ||
| &validator_stake_account.transient_stake_account, | ||
| deposit_info.stake_lamports + stake_rent - 2, | ||
| deposit_info.stake_lamports + stake_rent - stake_minimum_delegation - 2, | ||
| validator_stake_account.transient_stake_seed, | ||
| DecreaseInstruction::Reserve, | ||
| ) | ||
|
|
@@ -1531,3 +1535,85 @@ async fn success_remove_preferred_validator_resets_preference() { | |
| "User should receive all lamports from removed validator" | ||
| ); | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn fail_withdrawal_at_minimum() { | ||
| let stake_pool_accounts = StakePoolAccounts::new_without_fees(); | ||
|
|
||
| let ( | ||
| mut context, | ||
| validator_stake, | ||
| deposit_info, | ||
| user_transfer_authority, | ||
| user_stake_recipient, | ||
| tokens_to_burn, | ||
| ) = setup_for_withdraw_with_accounts(&stake_pool_accounts, 0).await; | ||
|
|
||
| // Warp forward to activation | ||
| let first_normal_slot = context.genesis_config().epoch_schedule.first_normal_slot; | ||
| let slot = first_normal_slot + 1; | ||
| context.warp_to_slot(slot).unwrap(); | ||
| let error = stake_pool_accounts | ||
| .update_all( | ||
| &mut context.banks_client, | ||
| &context.payer, | ||
| &context.last_blockhash, | ||
| false, | ||
| ) | ||
| .await; | ||
| assert!(error.is_none()); | ||
|
|
||
| // Withdraw some, get it down to 2x + 1 | ||
| let stake_minimum_delegation = stake_get_minimum_delegation( | ||
| &mut context.banks_client, | ||
| &context.payer, | ||
| &context.last_blockhash, | ||
| ) | ||
| .await; | ||
|
|
||
| let new_authority = Pubkey::new_unique(); | ||
| let error = stake_pool_accounts | ||
| .withdraw_stake( | ||
| &mut context.banks_client, | ||
| &context.payer, | ||
| &context.last_blockhash, | ||
| &user_stake_recipient.pubkey(), | ||
| &user_transfer_authority, | ||
| &deposit_info.pool_account.pubkey(), | ||
| &validator_stake.stake_account, | ||
| &new_authority, | ||
| tokens_to_burn - stake_minimum_delegation, | ||
| ) | ||
| .await; | ||
| assert!(error.is_none(), "{:?}", error); | ||
|
|
||
| let user_stake_recipient = Keypair::new(); | ||
| create_blank_stake_account( | ||
| &mut context.banks_client, | ||
| &context.payer, | ||
| &context.last_blockhash, | ||
| &user_stake_recipient, | ||
| ) | ||
| .await; | ||
| let error = stake_pool_accounts | ||
| .withdraw_stake( | ||
| &mut context.banks_client, | ||
| &context.payer, | ||
| &context.last_blockhash, | ||
| &user_stake_recipient.pubkey(), | ||
| &user_transfer_authority, | ||
| &deposit_info.pool_account.pubkey(), | ||
| &validator_stake.stake_account, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this actually does not try to withdraw from a second non-preferred validator.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's correct, it's not really about the preferred behavior, it's more to make sure that we properly respect the required minimum. I was mainly surprised that no tests broke after making the change 😅 I'll remove the preferred parts of this test, since they aren't really necessary |
||
| &new_authority, | ||
| stake_minimum_delegation, | ||
| ) | ||
| .await; | ||
| let transaction_error = error.unwrap().unwrap(); | ||
| assert_eq!( | ||
| transaction_error, | ||
| TransactionError::InstructionError( | ||
| 0, | ||
| InstructionError::Custom(StakePoolError::StakeLamportsNotEqualToMinimum as u32) | ||
| ) | ||
| ); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
>=is correct here, since if the stake account has exactlyminimum_lamports_with_tolerance, it should be possible to withdraw a stake accountThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, that >= is correct, but it is inconsistent with our previous check on
has_withdrawable_active_stake, as there we check that it is >.So we can get into the situation, that
preferred_validator_info.active_stake_lamports == minimum_lamports_with_toleranceand this guard will fire. But actually withdrawing this amount will not take the same path, as thehas_withdrawable_active_stakeis false forpreferred_validator, so it will go into validator removal, which is not actually necessary as far as I see.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah that's a great point, thanks for checking. I've made it
>=in the latest commit 0095320