system-program: Remove zero lamport check on transfers (#17726)

* system-program: Move lamports == 0 check on transfers

* Address feedback

* Update stake split to explicitly allocate + assign

* Update stake tests referring to split instruction

* Revert whitespace

* Update split instruction index in test

* Remove unnecessary `assign_with_seed` from `split_with_seed`

* Fix stake instruction parser

* Update test to allow splitting into account with lamports
This commit is contained in:
Jon Cinque
2021-06-06 01:45:45 +02:00
committed by GitHub
parent ae0bea1ad3
commit 8f5e773caf
6 changed files with 59 additions and 46 deletions

View File

@ -2,7 +2,7 @@ use log::*;
use solana_sdk::{
account::{AccountSharedData, ReadableAccount, WritableAccount},
account_utils::StateMut,
ic_msg,
feature_set, ic_msg,
instruction::InstructionError,
keyed_account::{from_keyed_account, get_signers, keyed_account_at_index, KeyedAccount},
nonce,
@ -200,7 +200,9 @@ fn transfer(
lamports: u64,
invoke_context: &dyn InvokeContext,
) -> Result<(), InstructionError> {
if lamports == 0 {
if !invoke_context.is_feature_active(&feature_set::system_transfer_zero_check::id())
&& lamports == 0
{
return Ok(());
}
@ -225,7 +227,9 @@ fn transfer_with_seed(
lamports: u64,
invoke_context: &dyn InvokeContext,
) -> Result<(), InstructionError> {
if lamports == 0 {
if !invoke_context.is_feature_active(&feature_set::system_transfer_zero_check::id())
&& lamports == 0
{
return Ok(());
}
@ -666,21 +670,21 @@ mod tests {
fn test_create_with_zero_lamports() {
// create account with zero lamports transferred
let new_owner = Pubkey::new(&[9; 32]);
let from = solana_sdk::pubkey::new_rand();
let from_account = AccountSharedData::new_ref(100, 1, &solana_sdk::pubkey::new_rand()); // not from system account
let from = Pubkey::new_unique();
let from_account = AccountSharedData::new_ref(100, 0, &Pubkey::new_unique()); // not from system account
let to = solana_sdk::pubkey::new_rand();
let to = Pubkey::new_unique();
let to_account = AccountSharedData::new_ref(0, 0, &Pubkey::default());
assert_eq!(
create_account(
&KeyedAccount::new(&from, false, &from_account), // no signer
&KeyedAccount::new(&to, false, &to_account),
&KeyedAccount::new(&from, true, &from_account),
&KeyedAccount::new(&to, true, &to_account),
&to.into(),
0,
2,
&new_owner,
&[to].iter().cloned().collect::<HashSet<_>>(),
&[from, to].iter().cloned().collect::<HashSet<_>>(),
&MockInvokeContext::new(vec![]),
),
Ok(())
@ -870,11 +874,11 @@ mod tests {
);
assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
// support creation/assignment with zero lamports (ephemeral account)
// Don't support unsigned creation with zero lamports (ephemeral account)
let owned_account = AccountSharedData::new_ref(0, 0, &Pubkey::default());
let result = create_account(
&KeyedAccount::new(&from, false, &from_account),
&KeyedAccount::new(&owned_key, false, &owned_account),
&KeyedAccount::new(&owned_key, true, &owned_account),
&owned_address,
0,
2,
@ -882,7 +886,7 @@ mod tests {
&[owned_key].iter().cloned().collect::<HashSet<_>>(),
&MockInvokeContext::new(vec![]),
);
assert_eq!(result, Ok(()));
assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
}
#[test]
@ -1094,9 +1098,7 @@ mod tests {
assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
assert_eq!(to_keyed_account.account.borrow().lamports(), 51);
// test unsigned transfer of zero
let from_keyed_account = KeyedAccount::new(&from, false, &from_account);
// test signed transfer of zero
assert!(transfer(
&from_keyed_account,
&to_keyed_account,
@ -1106,6 +1108,21 @@ mod tests {
.is_ok(),);
assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
assert_eq!(to_keyed_account.account.borrow().lamports(), 51);
// test unsigned transfer of zero
let from_keyed_account = KeyedAccount::new(&from, false, &from_account);
assert_eq!(
transfer(
&from_keyed_account,
&to_keyed_account,
0,
&MockInvokeContext::new(vec![]),
),
Err(InstructionError::MissingRequiredSignature)
);
assert_eq!(from_keyed_account.account.borrow().lamports(), 50);
assert_eq!(to_keyed_account.account.borrow().lamports(), 51);
}
#[test]

View File

@ -11,9 +11,7 @@ use solana_sdk::{
message::Message,
pubkey::Pubkey,
signature::{Keypair, Signer},
system_instruction::SystemError,
sysvar::{self, stake_history::StakeHistory},
transaction::TransactionError,
};
use solana_stake_program::{
stake_instruction::{self},
@ -171,7 +169,7 @@ fn test_stake_create_and_split_single_signature() {
#[test]
fn test_stake_create_and_split_to_existing_system_account() {
// Ensure stake-split does not allow the user to promote an existing system account into
// Ensure stake-split allows the user to promote an existing system account into
// a stake account.
solana_logger::setup();
@ -229,7 +227,7 @@ fn test_stake_create_and_split_to_existing_system_account() {
existing_lamports
);
// Verify the split fails because the account is already in use
// Verify the split succeeds with lamports in the destination account
let message = Message::new(
&stake_instruction::split_with_seed(
&stake_address, // original
@ -241,16 +239,12 @@ fn test_stake_create_and_split_to_existing_system_account() {
),
Some(&staker_keypair.pubkey()),
);
assert_eq!(
bank_client
.send_and_confirm_message(&[&staker_keypair], message)
.unwrap_err()
.unwrap(),
TransactionError::InstructionError(0, SystemError::AccountAlreadyInUse.into())
);
bank_client
.send_and_confirm_message(&[&staker_keypair], message)
.expect("failed to split into account with lamports");
assert_eq!(
bank_client.get_balance(&split_stake_address).unwrap(),
existing_lamports
existing_lamports + lamports / 2
);
}