diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 354cdd0454..b8aee7ed8a 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -1,36 +1,38 @@ use log::*; use solana_sdk::account::KeyedAccount; use solana_sdk::instruction::InstructionError; +use solana_sdk::instruction_processor_utils::next_keyed_account; use solana_sdk::pubkey::Pubkey; use solana_sdk::system_instruction::{SystemError, SystemInstruction}; use solana_sdk::system_program; use solana_sdk::sysvar; -const FROM_ACCOUNT_INDEX: usize = 0; -const TO_ACCOUNT_INDEX: usize = 1; - fn create_system_account( - keyed_accounts: &mut [KeyedAccount], + from: &mut KeyedAccount, + to: &mut KeyedAccount, lamports: u64, space: u64, program_id: &Pubkey, -) -> Result<(), SystemError> { - if !system_program::check_id(&keyed_accounts[FROM_ACCOUNT_INDEX].account.owner) { - debug!( - "CreateAccount: invalid account[from] owner {} ", - &keyed_accounts[FROM_ACCOUNT_INDEX].account.owner - ); - return Err(SystemError::SourceNotSystemAccount); +) -> Result<(), InstructionError> { + if from.signer_key().is_none() { + debug!("from is unsigned"); + return Err(InstructionError::MissingRequiredSignature); } - if !keyed_accounts[TO_ACCOUNT_INDEX].account.data.is_empty() - || !system_program::check_id(&keyed_accounts[TO_ACCOUNT_INDEX].account.owner) - { + if !system_program::check_id(&from.account.owner) { + debug!( + "CreateAccount: invalid account[from] owner {} ", + &from.account.owner + ); + return Err(SystemError::SourceNotSystemAccount.into()); + } + + if !to.account.data.is_empty() || !system_program::check_id(&to.account.owner) { debug!( "CreateAccount: invalid argument; account {} already in use", - keyed_accounts[TO_ACCOUNT_INDEX].unsigned_key() + to.unsigned_key() ); - return Err(SystemError::AccountAlreadyInUse); + return Err(SystemError::AccountAlreadyInUse.into()); } if sysvar::check_id(&program_id) { @@ -38,52 +40,67 @@ fn create_system_account( "CreateAccount: invalid argument; program id {} invalid", program_id ); - return Err(SystemError::InvalidProgramId); + return Err(SystemError::InvalidProgramId.into()); } - if sysvar::is_sysvar_id(&keyed_accounts[TO_ACCOUNT_INDEX].unsigned_key()) { + if sysvar::is_sysvar_id(&to.unsigned_key()) { debug!( "CreateAccount: invalid argument; account id {} invalid", program_id ); - return Err(SystemError::InvalidAccountId); + return Err(SystemError::InvalidAccountId.into()); } - if lamports > keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports { + if lamports > from.account.lamports { debug!( "CreateAccount: insufficient lamports ({}, need {})", - keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports, lamports + from.account.lamports, lamports ); - return Err(SystemError::ResultWithNegativeLamports); + return Err(SystemError::ResultWithNegativeLamports.into()); } - keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports -= lamports; - keyed_accounts[TO_ACCOUNT_INDEX].account.lamports += lamports; - keyed_accounts[TO_ACCOUNT_INDEX].account.owner = *program_id; - keyed_accounts[TO_ACCOUNT_INDEX].account.data = vec![0; space as usize]; - keyed_accounts[TO_ACCOUNT_INDEX].account.executable = false; + from.account.lamports -= lamports; + to.account.lamports += lamports; + to.account.owner = *program_id; + to.account.data = vec![0; space as usize]; + to.account.executable = false; Ok(()) } fn assign_account_to_program( - keyed_accounts: &mut [KeyedAccount], + account: &mut KeyedAccount, program_id: &Pubkey, -) -> Result<(), SystemError> { - keyed_accounts[FROM_ACCOUNT_INDEX].account.owner = *program_id; +) -> Result<(), InstructionError> { + if !system_program::check_id(&account.account.owner) { + return Err(InstructionError::IncorrectProgramId); + } + + if account.signer_key().is_none() { + debug!("account is unsigned"); + return Err(InstructionError::MissingRequiredSignature); + } + + account.account.owner = *program_id; Ok(()) } fn transfer_lamports( - keyed_accounts: &mut [KeyedAccount], + from: &mut KeyedAccount, + to: &mut KeyedAccount, lamports: u64, -) -> Result<(), SystemError> { - if lamports > keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports { +) -> Result<(), InstructionError> { + if from.signer_key().is_none() { + debug!("from is unsigned"); + return Err(InstructionError::MissingRequiredSignature); + } + + if lamports > from.account.lamports { debug!( "Transfer: insufficient lamports ({}, need {})", - keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports, lamports + from.account.lamports, lamports ); - return Err(SystemError::ResultWithNegativeLamports); + return Err(SystemError::ResultWithNegativeLamports.into()); } - keyed_accounts[FROM_ACCOUNT_INDEX].account.lamports -= lamports; - keyed_accounts[TO_ACCOUNT_INDEX].account.lamports += lamports; + from.account.lamports -= lamports; + to.account.lamports += lamports; Ok(()) } @@ -92,43 +109,33 @@ pub fn process_instruction( keyed_accounts: &mut [KeyedAccount], data: &[u8], ) -> Result<(), InstructionError> { - if let Ok(instruction) = bincode::deserialize(data) { - trace!("process_instruction: {:?}", instruction); - trace!("keyed_accounts: {:?}", keyed_accounts); + let instruction = + bincode::deserialize(data).map_err(|_| InstructionError::InvalidInstructionData)?; - if keyed_accounts.is_empty() { - debug!("Invalid instruction data: {:?}", data); - return Err(InstructionError::NotEnoughAccountKeys); - } - // All system instructions require that accounts_keys[0] be a signer - if keyed_accounts[FROM_ACCOUNT_INDEX].signer_key().is_none() { - debug!("account[from] is unsigned"); - return Err(InstructionError::MissingRequiredSignature); - } + trace!("process_instruction: {:?}", instruction); + trace!("keyed_accounts: {:?}", keyed_accounts); - match instruction { - SystemInstruction::CreateAccount { - lamports, - space, - program_id, - } if keyed_accounts.len() >= 2 => { - create_system_account(keyed_accounts, lamports, space, &program_id) - } - SystemInstruction::Assign { program_id } => { - if !system_program::check_id(&keyed_accounts[FROM_ACCOUNT_INDEX].account.owner) { - return Err(InstructionError::IncorrectProgramId); - } - assign_account_to_program(keyed_accounts, &program_id) - } - SystemInstruction::Transfer { lamports } if keyed_accounts.len() >= 2 => { - transfer_lamports(keyed_accounts, lamports) - } - _ => return Err(InstructionError::NotEnoughAccountKeys), + let keyed_accounts_iter = &mut keyed_accounts.iter_mut(); + + match instruction { + SystemInstruction::CreateAccount { + lamports, + space, + program_id, + } => { + let from = next_keyed_account(keyed_accounts_iter)?; + let to = next_keyed_account(keyed_accounts_iter)?; + create_system_account(from, to, lamports, space, &program_id) + } + SystemInstruction::Assign { program_id } => { + let account = next_keyed_account(keyed_accounts_iter)?; + assign_account_to_program(account, &program_id) + } + SystemInstruction::Transfer { lamports } => { + let from = next_keyed_account(keyed_accounts_iter)?; + let to = next_keyed_account(keyed_accounts_iter)?; + transfer_lamports(from, to, lamports) } - .map_err(|e| InstructionError::CustomError(e as u32)) - } else { - debug!("Invalid instruction data: {:?}", data); - Err(InstructionError::InvalidInstructionData) } } @@ -155,11 +162,14 @@ mod tests { let to = Pubkey::new_rand(); let mut to_account = Account::new(0, 0, &Pubkey::default()); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&to, false, &mut to_account), - ]; - create_system_account(&mut keyed_accounts, 50, 2, &new_program_owner).unwrap(); + create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&to, false, &mut to_account), + 50, + 2, + &new_program_owner, + ) + .unwrap(); let from_lamports = from_account.lamports; let to_lamports = to_account.lamports; let to_owner = to_account.owner; @@ -181,12 +191,14 @@ mod tests { let mut to_account = Account::new(0, 0, &Pubkey::default()); let unchanged_account = to_account.clone(); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&to, false, &mut to_account), - ]; - let result = create_system_account(&mut keyed_accounts, 150, 2, &new_program_owner); - assert_eq!(result, Err(SystemError::ResultWithNegativeLamports)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&to, false, &mut to_account), + 150, + 2, + &new_program_owner, + ); + assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into())); let from_lamports = from_account.lamports; assert_eq!(from_lamports, 100); assert_eq!(to_account, unchanged_account); @@ -204,12 +216,14 @@ mod tests { let mut owned_account = Account::new(0, 0, &original_program_owner); let unchanged_account = owned_account.clone(); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&owned_key, false, &mut owned_account), - ]; - let result = create_system_account(&mut keyed_accounts, 50, 2, &new_program_owner); - assert_eq!(result, Err(SystemError::AccountAlreadyInUse)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&owned_key, false, &mut owned_account), + 50, + 2, + &new_program_owner, + ); + assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); let from_lamports = from_account.lamports; assert_eq!(from_lamports, 100); assert_eq!(owned_account, unchanged_account); @@ -224,24 +238,28 @@ mod tests { let to = Pubkey::new_rand(); let mut to_account = Account::default(); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&to, false, &mut to_account), - ]; // fail to create a sysvar::id() owned account - let result = create_system_account(&mut keyed_accounts, 50, 2, &sysvar::id()); - assert_eq!(result, Err(SystemError::InvalidProgramId)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&to, false, &mut to_account), + 50, + 2, + &sysvar::id(), + ); + assert_eq!(result, Err(SystemError::InvalidProgramId.into())); let to = sysvar::fees::id(); let mut to_account = Account::default(); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&to, false, &mut to_account), - ]; // fail to create an account with a sysvar id - let result = create_system_account(&mut keyed_accounts, 50, 2, &system_program::id()); - assert_eq!(result, Err(SystemError::InvalidAccountId)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&to, false, &mut to_account), + 50, + 2, + &system_program::id(), + ); + assert_eq!(result, Err(SystemError::InvalidAccountId.into())); let from_lamports = from_account.lamports; assert_eq!(from_lamports, 100); @@ -261,12 +279,14 @@ mod tests { }; let unchanged_account = populated_account.clone(); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&populated_key, false, &mut populated_account), - ]; - let result = create_system_account(&mut keyed_accounts, 50, 2, &new_program_owner); - assert_eq!(result, Err(SystemError::AccountAlreadyInUse)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&populated_key, false, &mut populated_account), + 50, + 2, + &new_program_owner, + ); + assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); assert_eq!(from_account.lamports, 100); assert_eq!(populated_account, unchanged_account); } @@ -279,12 +299,14 @@ mod tests { let mut from_account = Account::new(100, 0, &other_program); let to = Pubkey::new_rand(); let mut to_account = Account::new(0, 0, &Pubkey::default()); - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new(&to, false, &mut to_account), - ]; - let result = create_system_account(&mut keyed_accounts, 50, 2, &other_program); - assert_eq!(result, Err(SystemError::SourceNotSystemAccount)); + let result = create_system_account( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new(&to, false, &mut to_account), + 50, + 2, + &other_program, + ); + assert_eq!(result, Err(SystemError::SourceNotSystemAccount.into())); } #[test] @@ -293,14 +315,17 @@ mod tests { let from = Pubkey::new_rand(); let mut from_account = Account::new(100, 0, &system_program::id()); - let mut keyed_accounts = [KeyedAccount::new(&from, true, &mut from_account)]; - assign_account_to_program(&mut keyed_accounts, &new_program_owner).unwrap(); + assign_account_to_program( + &mut KeyedAccount::new(&from, true, &mut from_account), + &new_program_owner, + ) + .unwrap(); let from_owner = from_account.owner; assert_eq!(from_owner, new_program_owner); // Attempt to assign account not owned by system program let another_program_owner = Pubkey::new(&[8; 32]); - keyed_accounts = [KeyedAccount::new(&from, true, &mut from_account)]; + let mut keyed_accounts = [KeyedAccount::new(&from, true, &mut from_account)]; let instruction = SystemInstruction::Assign { program_id: another_program_owner, }; @@ -339,23 +364,24 @@ mod tests { let mut from_account = Account::new(100, 0, &Pubkey::new(&[2; 32])); // account owner should not matter let to = Pubkey::new_rand(); let mut to_account = Account::new(1, 0, &Pubkey::new(&[3; 32])); // account owner should not matter - let mut keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new_credit_only(&to, false, &mut to_account), - ]; - transfer_lamports(&mut keyed_accounts, 50).unwrap(); + transfer_lamports( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new_credit_only(&to, false, &mut to_account), + 50, + ) + .unwrap(); let from_lamports = from_account.lamports; let to_lamports = to_account.lamports; assert_eq!(from_lamports, 50); assert_eq!(to_lamports, 51); // Attempt to move more lamports than remaining in from_account - keyed_accounts = [ - KeyedAccount::new(&from, true, &mut from_account), - KeyedAccount::new_credit_only(&to, false, &mut to_account), - ]; - let result = transfer_lamports(&mut keyed_accounts, 100); - assert_eq!(result, Err(SystemError::ResultWithNegativeLamports)); + let result = transfer_lamports( + &mut KeyedAccount::new(&from, true, &mut from_account), + &mut KeyedAccount::new_credit_only(&to, false, &mut to_account), + 100, + ); + assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into())); assert_eq!(from_account.lamports, 50); assert_eq!(to_account.lamports, 51); } diff --git a/sdk/src/system_instruction.rs b/sdk/src/system_instruction.rs index 0a5151d1fa..4ddb87b509 100644 --- a/sdk/src/system_instruction.rs +++ b/sdk/src/system_instruction.rs @@ -2,9 +2,9 @@ use crate::instruction::{AccountMeta, Instruction}; use crate::instruction_processor_utils::DecodeError; use crate::pubkey::Pubkey; use crate::system_program; -use num_derive::FromPrimitive; +use num_derive::{FromPrimitive, ToPrimitive}; -#[derive(Serialize, Debug, Clone, PartialEq, FromPrimitive)] +#[derive(Serialize, Debug, Clone, PartialEq, FromPrimitive, ToPrimitive)] pub enum SystemError { AccountAlreadyInUse, ResultWithNegativeLamports,