From 901f7a43cdfd89a47d6049a865792c82acbe4f13 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald <greg@solana.com> Date: Fri, 29 May 2020 22:16:26 -0600 Subject: [PATCH 1/3] Allow paying to oneself --- runtime/src/accounts.rs | 32 ----- runtime/src/bank.rs | 6 +- runtime/src/system_instruction_processor.rs | 135 ++++++++++---------- 3 files changed, 71 insertions(+), 102 deletions(-) diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 9e7c567a340065..0bfa67533321be 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1367,38 +1367,6 @@ mod tests { } } - #[test] - fn test_load_account_pay_to_self() { - let mut accounts: Vec<(Pubkey, Account)> = Vec::new(); - let mut error_counters = ErrorCounters::default(); - - let keypair = Keypair::new(); - let pubkey = keypair.pubkey(); - - let account = Account::new(10, 1, &Pubkey::default()); - accounts.push((pubkey, account)); - - let instructions = vec![CompiledInstruction::new(0, &(), vec![0, 1])]; - // Simulate pay-to-self transaction, which loads the same account twice - let tx = Transaction::new_with_compiled_instructions( - &[&keypair], - &[pubkey], - Hash::default(), - vec![native_loader::id()], - instructions, - ); - let loaded_accounts = load_accounts(tx, &accounts, &mut error_counters); - - assert_eq!(loaded_accounts.len(), 1); - assert_eq!( - loaded_accounts[0], - ( - Err(TransactionError::InvalidAccountForFee), - Some(HashAgeKind::Extant) - ) - ); - } - #[test] fn test_load_by_program_slot() { let accounts = Accounts::new(Vec::new()); diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 721fca827c3faa..13c6425f60976e 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4820,13 +4820,9 @@ mod tests { let _res = bank.process_transaction(&tx); assert_eq!(bank.get_balance(&key1.pubkey()), 1); - - // TODO: Why do we convert errors to Oks? - //res[0].clone().unwrap_err(); - bank.get_signature_status(&tx.signatures[0]) .unwrap() - .unwrap_err(); + .unwrap(); } fn new_from_parent(parent: &Arc<Bank>) -> Bank { diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index 56a2346f75c6d6..b463ba30c65093 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -122,7 +122,7 @@ fn allocate_and_assign( fn create_account( from: &KeyedAccount, - to: &mut Account, + to: &KeyedAccount, to_address: &Address, lamports: u64, space: u64, @@ -130,19 +130,22 @@ fn create_account( signers: &HashSet<Pubkey>, ) -> Result<(), InstructionError> { // if it looks like the `to` account is already in use, bail - if to.lamports > 0 { - debug!( - "Create Account: invalid argument; account {:?} already in use", - to_address - ); - return Err(SystemError::AccountAlreadyInUse.into()); - } + { + let to = &mut to.try_account_ref_mut()?; + if to.lamports > 0 { + debug!( + "Create Account: invalid argument; account {:?} already in use", + to_address + ); + return Err(SystemError::AccountAlreadyInUse.into()); + } - allocate_and_assign(to, to_address, space, owner, signers)?; + allocate_and_assign(to, to_address, space, owner, signers)?; + } transfer(from, to, lamports) } -fn transfer(from: &KeyedAccount, to: &mut Account, lamports: u64) -> Result<(), InstructionError> { +fn transfer(from: &KeyedAccount, to: &KeyedAccount, lamports: u64) -> Result<(), InstructionError> { if lamports == 0 { return Ok(()); } @@ -166,7 +169,7 @@ fn transfer(from: &KeyedAccount, to: &mut Account, lamports: u64) -> Result<(), } from.try_account_ref_mut()?.lamports -= lamports; - to.lamports += lamports; + to.try_account_ref_mut()?.lamports += lamports; Ok(()) } @@ -191,11 +194,10 @@ pub fn process_instruction( } => { let from = next_keyed_account(keyed_accounts_iter)?; let to = next_keyed_account(keyed_accounts_iter)?; - let mut to_account = to.try_account_ref_mut()?; let to_address = Address::create(to.unsigned_key(), None)?; create_account( from, - &mut to_account, + to, &to_address, lamports, space, @@ -212,11 +214,10 @@ pub fn process_instruction( } => { let from = next_keyed_account(keyed_accounts_iter)?; let to = next_keyed_account(keyed_accounts_iter)?; - let mut to_account = to.try_account_ref_mut()?; let to_address = Address::create(&to.unsigned_key(), Some((&base, &seed, &owner)))?; create_account( from, - &mut to_account, + &to, &to_address, lamports, space, @@ -232,8 +233,8 @@ pub fn process_instruction( } SystemInstruction::Transfer { lamports } => { let from = next_keyed_account(keyed_accounts_iter)?; - let mut to_account = next_keyed_account(keyed_accounts_iter)?.try_account_ref_mut()?; - transfer(from, &mut to_account, lamports) + let to = next_keyed_account(keyed_accounts_iter)?; + transfer(from, to, lamports) } SystemInstruction::AdvanceNonceAccount => { let me = &mut next_keyed_account(keyed_accounts_iter)?; @@ -452,13 +453,13 @@ mod tests { let to = Pubkey::create_with_seed(&from, seed, &new_owner).unwrap(); let from_account = Account::new_ref(100, 0, &system_program::id()); - let mut to_account = Account::new(0, 0, &Pubkey::default()); + let to_account = Account::new_ref(0, 0, &Pubkey::default()); let to_address = Address::create(&to, Some((&from, seed, &new_owner))).unwrap(); assert_eq!( create_account( &KeyedAccount::new(&from, false, &from_account), - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), &to_address, 50, 2, @@ -468,7 +469,7 @@ mod tests { Err(InstructionError::MissingRequiredSignature) ); assert_eq!(from_account.borrow().lamports, 100); - assert_eq!(to_account, Account::default()); + assert_eq!(*to_account.borrow(), Account::default()); } #[test] @@ -479,12 +480,12 @@ mod tests { let from_account = Account::new_ref(100, 1, &Pubkey::new_rand()); // not from system account let to = Pubkey::new_rand(); - let mut to_account = Account::new(0, 0, &Pubkey::default()); + let to_account = Account::new_ref(0, 0, &Pubkey::default()); assert_eq!( create_account( &KeyedAccount::new(&from, false, &from_account), // no signer - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), &to.into(), 0, 2, @@ -495,13 +496,13 @@ mod tests { ); let from_lamports = from_account.borrow().lamports; - let to_lamports = to_account.lamports; - let to_owner = to_account.owner; - let to_data = to_account.data; + let to_lamports = to_account.borrow().lamports; + let to_owner = to_account.borrow().owner; + let to_data = &to_account.borrow().data; assert_eq!(from_lamports, 100); assert_eq!(to_lamports, 0); assert_eq!(to_owner, new_owner); - assert_eq!(to_data, [0, 0]); + assert_eq!(*to_data, [0, 0]); } #[test] @@ -512,11 +513,11 @@ mod tests { let from_account = Account::new_ref(100, 0, &system_program::id()); let to = Pubkey::new_rand(); - let mut to_account = Account::new(0, 0, &Pubkey::default()); + let to_account = Account::new_ref(0, 0, &Pubkey::default()); let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut to_account, + &KeyedAccount::new(&from, false, &to_account), &to.into(), 150, 2, @@ -530,7 +531,7 @@ mod tests { fn test_request_more_than_allowed_data_length() { let from_account = Account::new_ref(100, 0, &system_program::id()); let from = Pubkey::new_rand(); - let mut to_account = Account::default(); + let to_account = Account::new_ref(0, 0, &system_program::id()); let to = Pubkey::new_rand(); let signers = &[from, to].iter().cloned().collect::<HashSet<_>>(); @@ -539,7 +540,7 @@ mod tests { // Trying to request more data length than permitted will result in failure let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), &address, 50, MAX_PERMITTED_DATA_LENGTH + 1, @@ -555,7 +556,7 @@ mod tests { // Trying to request equal or less data length than permitted will be successful let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), &address, 50, MAX_PERMITTED_DATA_LENGTH, @@ -563,8 +564,8 @@ mod tests { &signers, ); assert!(result.is_ok()); - assert_eq!(to_account.lamports, 50); - assert_eq!(to_account.data.len() as u64, MAX_PERMITTED_DATA_LENGTH); + assert_eq!(to_account.borrow().lamports, 50); + assert_eq!(to_account.borrow().data.len() as u64, MAX_PERMITTED_DATA_LENGTH); } #[test] @@ -576,7 +577,7 @@ mod tests { let original_program_owner = Pubkey::new(&[5; 32]); let owned_key = Pubkey::new_rand(); - let mut owned_account = Account::new(0, 0, &original_program_owner); + let owned_account = Account::new_ref(0, 0, &original_program_owner); let unchanged_account = owned_account.clone(); let signers = &[from, owned_key].iter().cloned().collect::<HashSet<_>>(); @@ -584,7 +585,7 @@ mod tests { let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 50, 2, @@ -598,11 +599,11 @@ mod tests { assert_eq!(owned_account, unchanged_account); // Attempt to create system account in account that already has data - let mut owned_account = Account::new(0, 1, &Pubkey::default()); - let unchanged_account = owned_account.clone(); + let owned_account = Account::new_ref(0, 1, &Pubkey::default()); + let unchanged_account = owned_account.borrow().clone(); let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 50, 2, @@ -612,14 +613,14 @@ mod tests { assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); let from_lamports = from_account.borrow().lamports; assert_eq!(from_lamports, 100); - assert_eq!(owned_account, unchanged_account); + assert_eq!(*owned_account.borrow(), unchanged_account); // Attempt to create an account that already has lamports - let mut owned_account = Account::new(1, 0, &Pubkey::default()); - let unchanged_account = owned_account.clone(); + let owned_account = Account::new_ref(1, 0, &Pubkey::default()); + let unchanged_account = owned_account.borrow().clone(); let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 50, 2, @@ -628,7 +629,7 @@ mod tests { ); assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); assert_eq!(from_lamports, 100); - assert_eq!(owned_account, unchanged_account); + assert_eq!(*owned_account.borrow(), unchanged_account); } #[test] @@ -639,14 +640,14 @@ mod tests { let from_account = Account::new_ref(100, 0, &system_program::id()); let owned_key = Pubkey::new_rand(); - let mut owned_account = Account::new(0, 0, &Pubkey::default()); + let owned_account = Account::new_ref(0, 0, &Pubkey::default()); let owned_address = owned_key.into(); // Haven't signed from account let result = create_account( &KeyedAccount::new(&from, false, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 50, 2, @@ -656,10 +657,10 @@ mod tests { assert_eq!(result, Err(InstructionError::MissingRequiredSignature)); // Haven't signed to account - let mut owned_account = Account::new(0, 0, &Pubkey::default()); + let owned_account = Account::new_ref(0, 0, &Pubkey::default()); let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, true, &owned_account), &owned_address, 50, 2, @@ -669,10 +670,10 @@ mod tests { assert_eq!(result, Err(InstructionError::MissingRequiredSignature)); // support creation/assignment with zero lamports (ephemeral account) - let mut owned_account = Account::new(0, 0, &Pubkey::default()); + let owned_account = Account::new_ref(0, 0, &Pubkey::default()); let result = create_account( &KeyedAccount::new(&from, false, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 0, 2, @@ -689,7 +690,7 @@ mod tests { let from_account = Account::new_ref(100, 0, &system_program::id()); let to = Pubkey::new_rand(); - let mut to_account = Account::default(); + let to_account = Account::new_ref(0, 0, &system_program::id()); let signers = [from, to].iter().cloned().collect::<HashSet<_>>(); let to_address = to.into(); @@ -697,7 +698,7 @@ mod tests { // fail to create a sysvar::id() owned account let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), &to_address, 50, 2, @@ -716,10 +717,10 @@ mod tests { let from_account = Account::new_ref(100, 0, &system_program::id()); let populated_key = Pubkey::new_rand(); - let mut populated_account = Account { + let populated_account = Account { data: vec![0, 1, 2, 3], ..Account::default() - }; + }.into(); let signers = [from, populated_key] .iter() @@ -729,7 +730,7 @@ mod tests { let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut populated_account, + &KeyedAccount::new(&populated_key, false, &populated_account), &populated_address, 50, 2, @@ -753,15 +754,16 @@ mod tests { let from = KeyedAccount::new(&nonce, true, &nonce_account); let new = Pubkey::new_rand(); - let mut new_account = Account::default(); + let new_account = Account::new_ref(0, 0, &system_program::id()); let signers = [nonce, new].iter().cloned().collect::<HashSet<_>>(); let new_address = new.into(); + let new_keyed_account = KeyedAccount::new(&new, false, &new_account); assert_eq!( create_account( &from, - &mut new_account, + &new_keyed_account, &new_address, 42, 0, @@ -850,26 +852,28 @@ mod tests { fn test_transfer_lamports() { let from = Pubkey::new_rand(); let from_account = Account::new_ref(100, 0, &Pubkey::new(&[2; 32])); // account owner should not matter - let mut to_account = Account::new(1, 0, &Pubkey::new(&[3; 32])); // account owner should not matter + let to = Pubkey::new(&[3; 32]); + let to_account = Account::new_ref(1, 0, &to); // account owner should not matter let from_keyed_account = KeyedAccount::new(&from, true, &from_account); - transfer(&from_keyed_account, &mut to_account, 50).unwrap(); + let to_keyed_account = KeyedAccount::new(&to, false, &to_account); + transfer(&from_keyed_account, &to_keyed_account, 50).unwrap(); let from_lamports = from_keyed_account.account.borrow().lamports; - let to_lamports = to_account.lamports; + let to_lamports = to_keyed_account.account.borrow().lamports; assert_eq!(from_lamports, 50); assert_eq!(to_lamports, 51); // Attempt to move more lamports than remaining in from_account let from_keyed_account = KeyedAccount::new(&from, true, &from_account); - let result = transfer(&from_keyed_account, &mut to_account, 100); + let result = transfer(&from_keyed_account, &to_keyed_account, 100); assert_eq!(result, Err(SystemError::ResultWithNegativeLamports.into())); assert_eq!(from_keyed_account.account.borrow().lamports, 50); - assert_eq!(to_account.lamports, 51); + 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!(transfer(&from_keyed_account, &mut to_account, 0,).is_ok(),); + assert!(transfer(&from_keyed_account, &to_keyed_account, 0,).is_ok(),); assert_eq!(from_keyed_account.account.borrow().lamports, 50); - assert_eq!(to_account.lamports, 51); + assert_eq!(to_keyed_account.account.borrow().lamports, 51); } #[test] @@ -889,11 +893,12 @@ mod tests { Some(SystemAccountKind::Nonce) ); - let mut to_account = Account::new(1, 0, &Pubkey::new(&[3; 32])); // account owner should not matter + let to = Pubkey::new(&[3; 32]); + let to_account = Account::new_ref(1, 0, &to); // account owner should not matter assert_eq!( transfer( &KeyedAccount::new(&from, true, &from_account), - &mut to_account, + &KeyedAccount::new(&to, false, &to_account), 50, ), Err(InstructionError::InvalidArgument), From 7c4b6debffb84c1849a33aa42c8a56610448f62a Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald <greg@solana.com> Date: Fri, 29 May 2020 22:26:26 -0600 Subject: [PATCH 2/3] cargo fmt --- runtime/src/system_instruction_processor.rs | 28 ++++++--------------- 1 file changed, 8 insertions(+), 20 deletions(-) diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index b463ba30c65093..2f64ab0a09da1e 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -195,15 +195,7 @@ pub fn process_instruction( let from = next_keyed_account(keyed_accounts_iter)?; let to = next_keyed_account(keyed_accounts_iter)?; let to_address = Address::create(to.unsigned_key(), None)?; - create_account( - from, - to, - &to_address, - lamports, - space, - &owner, - &signers, - ) + create_account(from, to, &to_address, lamports, space, &owner, &signers) } SystemInstruction::CreateAccountWithSeed { base, @@ -215,15 +207,7 @@ pub fn process_instruction( let from = next_keyed_account(keyed_accounts_iter)?; let to = next_keyed_account(keyed_accounts_iter)?; let to_address = Address::create(&to.unsigned_key(), Some((&base, &seed, &owner)))?; - create_account( - from, - &to, - &to_address, - lamports, - space, - &owner, - &signers, - ) + create_account(from, &to, &to_address, lamports, space, &owner, &signers) } SystemInstruction::Assign { owner } => { let keyed_account = next_keyed_account(keyed_accounts_iter)?; @@ -565,7 +549,10 @@ mod tests { ); assert!(result.is_ok()); assert_eq!(to_account.borrow().lamports, 50); - assert_eq!(to_account.borrow().data.len() as u64, MAX_PERMITTED_DATA_LENGTH); + assert_eq!( + to_account.borrow().data.len() as u64, + MAX_PERMITTED_DATA_LENGTH + ); } #[test] @@ -720,7 +707,8 @@ mod tests { let populated_account = Account { data: vec![0, 1, 2, 3], ..Account::default() - }.into(); + } + .into(); let signers = [from, populated_key] .iter() From 2bc3d2d5b9d9488e89703c3e3bf0149b79048799 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald <greg@solana.com> Date: Sat, 30 May 2020 08:03:41 -0600 Subject: [PATCH 3/3] Permit pay-to-self in CLI No test here because we're just removing an [untested] special case. Fixes #10339 --- cli/src/cli.rs | 5 ----- 1 file changed, 5 deletions(-) diff --git a/cli/src/cli.rs b/cli/src/cli.rs index e1847cabace9aa..307e4993c15753 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -1530,11 +1530,6 @@ fn process_transfer( ) -> ProcessResult { let from = config.signers[from]; - check_unique_pubkeys( - (&from.pubkey(), "cli keypair".to_string()), - (to, "to".to_string()), - )?; - let (recent_blockhash, fee_calculator) = blockhash_query.get_blockhash_and_fee_calculator(rpc_client)?;