gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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

Browse Source 
* 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

(cherry picked from commit 8f5e773caf)

Co-authored-by: Jon Cinque <jon.cinque@gmail.com>
This commit is contained in:
mergify[bot]
2021-06-06 18:27:29 +00:00
committed by GitHub
parent 6d5a4b5cce
commit e5ea16fad8
6 changed files with 59 additions and 46 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
programs/stake/src/stake_instruction.rs
View File

@ -261,13 +261,8 @@ pub fn split(
split_stake_pubkey: &Pubkey,
) -> Vec<Instruction> {
vec![
system_instruction::create_account(
authorized_pubkey, // Sending 0, so any signer will suffice
split_stake_pubkey,
0,
std::mem::size_of::<StakeState>() as u64,
&id(),
),
system_instruction::allocate(split_stake_pubkey, std::mem::size_of::<StakeState>() as u64),
system_instruction::assign(split_stake_pubkey, &id()),
_split(
stake_pubkey,
authorized_pubkey,
@ -286,12 +281,10 @@ pub fn split_with_seed(
seed: &str, // seed
) -> Vec<Instruction> {
vec![
system_instruction::create_account_with_seed(
authorized_pubkey, // Sending 0, so any signer will suffice
system_instruction::allocate_with_seed(
split_stake_pubkey,
base,
seed,
0,
std::mem::size_of::<StakeState>() as u64,
&id(),
),
@ -766,7 +759,7 @@ mod tests {
&Pubkey::default(),
100,
&invalid_stake_state_pubkey(),
)[1]
)[2]
),
Err(InstructionError::InvalidAccountData),
);
@ -852,7 +845,7 @@ mod tests {
&Pubkey::default(),
100,
&Pubkey::default(),
)[1]
)[2]
),
Err(InstructionError::InvalidAccountOwner),
);
@ -863,7 +856,7 @@ mod tests {
&Pubkey::default(),
100,
&spoofed_stake_state_pubkey(),
)[1]
)[2]
),
Err(InstructionError::IncorrectProgramId),
);

47
runtime/src/system_instruction_processor.rs
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]

18
runtime/tests/stake.rs
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
);
}

5
sdk/src/feature_set.rs
View File

@ -143,6 +143,10 @@ pub mod add_missing_program_error_mappings {
solana_sdk::declare_id!("3QEUpjhgPEt92nz3Mqf6pABkHPGCQwSvKtyGMq4SuQyL");
}
pub mod system_transfer_zero_check {
solana_sdk::declare_id!("BrTR9hzw4WBGFP65AJMbpAo64DcA3U6jdPSga9fMV5cS");
}
lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@ -179,6 +183,7 @@ lazy_static! {
(stake_program_v4::id(), "solana_stake_program v4"),
(memory_ops_syscalls::id(), "add syscalls for memory operations"),
(add_missing_program_error_mappings::id(), "add missing program error mappings"),
(system_transfer_zero_check::id(), "perform all checks for transfers of 0 lamports"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()

2
tokens/src/commands.rs
View File

@ -1467,7 +1467,7 @@ mod tests {
)); // Same recipient, same lockups
}
const SET_LOCKUP_INDEX: usize = 4;
const SET_LOCKUP_INDEX: usize = 5;
#[test]
fn test_set_split_stake_lockup() {

14
transaction-status/src/parse_stake.rs
View File

@ -309,21 +309,25 @@ mod test {
);
assert!(parse_stake(&message.instructions[0], &keys[0..5]).is_err());
let instructions = stake_instruction::split(&keys[2], &keys[0], lamports, &keys[1]);
// This looks wrong, but in an actual compiled instruction, the order is:
// * split account (signer, allocate + assign first)
// * stake authority (signer)
// * stake account
let instructions = stake_instruction::split(&keys[2], &keys[1], lamports, &keys[0]);
let message = Message::new(&instructions, None);
assert_eq!(
parse_stake(&message.instructions[1], &keys[0..3]).unwrap(),
parse_stake(&message.instructions[2], &keys[0..3]).unwrap(),
ParsedInstructionEnum {
instruction_type: "split".to_string(),
info: json!({
"stakeAccount": keys[2].to_string(),
"newSplitAccount": keys[1].to_string(),
"stakeAuthority": keys[0].to_string(),
"newSplitAccount": keys[0].to_string(),
"stakeAuthority": keys[1].to_string(),
"lamports": lamports,
}),
}
);
assert!(parse_stake(&message.instructions[1], &keys[0..2]).is_err());
assert!(parse_stake(&message.instructions[2], &keys[0..2]).is_err());
let instruction = stake_instruction::withdraw(&keys[1], &keys[0], &keys[2], lamports, None);
let message = Message::new(&[instruction], None);