From 82ce035e35c0b3243ab23d931bd5e6697bb5b961 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Sat, 30 May 2020 20:12:48 -0600 Subject: [PATCH] Permit paying oneself (#10337) * Allow paying to oneself * cargo fmt * Permit pay-to-self in CLI No test here because we're just removing an [untested] special case. Fixes #10339 (cherry picked from commit 5d248fe5f8cb4f11abc01d2b2d0353b0cbb169ef) --- cli/src/cli.rs | 5 - runtime/src/accounts.rs | 32 ----- runtime/src/bank.rs | 6 +- runtime/src/system_instruction_processor.rs | 123 +++++++++++--------- 4 files changed, 67 insertions(+), 99 deletions(-) diff --git a/cli/src/cli.rs b/cli/src/cli.rs index a14247d4aed81e..e7bda5db27901a 100644 --- a/cli/src/cli.rs +++ b/cli/src/cli.rs @@ -1584,11 +1584,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)?; let ixs = vec![system_instruction::transfer(&from.pubkey(), to, lamports)]; diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index e19a71df87265b..6448db0b262253 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -1393,38 +1393,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 7208f43af18b06..32aaccb6cb0e20 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4973,13 +4973,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 { diff --git a/runtime/src/system_instruction_processor.rs b/runtime/src/system_instruction_processor.rs index a161aa39ab6e3d..e3c20783add99f 100644 --- a/runtime/src/system_instruction_processor.rs +++ b/runtime/src/system_instruction_processor.rs @@ -122,18 +122,21 @@ fn allocate_and_assign( fn create_account( from: &KeyedAccount, - to: &mut Account, + to: &KeyedAccount, to_address: &Address, lamports: u64, space: u64, program_id: &Pubkey, signers: &HashSet, ) -> Result<(), InstructionError> { - allocate_and_assign(to, to_address, space, program_id, signers)?; + { + let mut to_account = to.try_account_ref_mut()?; + allocate_and_assign(&mut to_account, to_address, space, program_id, 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(()); } @@ -157,7 +160,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(()) } @@ -182,11 +185,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, @@ -203,12 +205,11 @@ 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, &program_id)))?; create_account( from, - &mut to_account, + &to, &to_address, lamports, space, @@ -224,8 +225,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_program_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_program_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.clone(); + 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_program_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::>(); @@ -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,11 @@ 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 +580,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::>(); @@ -584,7 +588,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 +602,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,13 +616,13 @@ 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); // Verify that create_account works even if `to` has a non-zero balance - let mut owned_account = Account::new(1, 0, &Pubkey::default()); + let owned_account = Account::new_ref(1, 0, &Pubkey::default()); let result = create_account( &KeyedAccount::new(&from, true, &from_account), - &mut owned_account, + &KeyedAccount::new(&owned_key, false, &owned_account), &owned_address, 50, 2, @@ -627,7 +631,7 @@ mod tests { ); assert_eq!(result, Ok(())); assert_eq!(from_account.borrow().lamports, from_lamports - 50); - assert_eq!(owned_account.lamports, 1 + 50); + assert_eq!(owned_account.borrow().lamports, 1 + 50); } #[test] @@ -638,14 +642,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, @@ -655,10 +659,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, @@ -668,10 +672,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, @@ -688,7 +692,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::>(); let to_address = to.into(); @@ -696,7 +700,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, @@ -715,10 +719,11 @@ 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() @@ -728,7 +733,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, @@ -752,15 +757,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::>(); 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, @@ -857,26 +863,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] @@ -896,11 +904,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),