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
This commit is contained in:
Greg Fitzgerald
2020-05-30 20:12:48 -06:00
committed by GitHub
parent 6013b4b2a7
commit 5d248fe5f8
4 changed files with 75 additions and 123 deletions

View File

@ -1530,11 +1530,6 @@ fn process_transfer(
) -> ProcessResult { ) -> ProcessResult {
let from = config.signers[from]; let from = config.signers[from];
check_unique_pubkeys(
(&from.pubkey(), "cli keypair".to_string()),
(to, "to".to_string()),
)?;
let (recent_blockhash, fee_calculator) = let (recent_blockhash, fee_calculator) =
blockhash_query.get_blockhash_and_fee_calculator(rpc_client)?; blockhash_query.get_blockhash_and_fee_calculator(rpc_client)?;

View File

@ -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] #[test]
fn test_load_by_program_slot() { fn test_load_by_program_slot() {
let accounts = Accounts::new(Vec::new()); let accounts = Accounts::new(Vec::new());

View File

@ -4820,13 +4820,9 @@ mod tests {
let _res = bank.process_transaction(&tx); let _res = bank.process_transaction(&tx);
assert_eq!(bank.get_balance(&key1.pubkey()), 1); 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]) bank.get_signature_status(&tx.signatures[0])
.unwrap() .unwrap()
.unwrap_err(); .unwrap();
} }
fn new_from_parent(parent: &Arc<Bank>) -> Bank { fn new_from_parent(parent: &Arc<Bank>) -> Bank {

View File

@ -122,7 +122,7 @@ fn allocate_and_assign(
fn create_account( fn create_account(
from: &KeyedAccount, from: &KeyedAccount,
to: &mut Account, to: &KeyedAccount,
to_address: &Address, to_address: &Address,
lamports: u64, lamports: u64,
space: u64, space: u64,
@ -130,19 +130,22 @@ fn create_account(
signers: &HashSet<Pubkey>, signers: &HashSet<Pubkey>,
) -> Result<(), InstructionError> { ) -> Result<(), InstructionError> {
// if it looks like the `to` account is already in use, bail // if it looks like the `to` account is already in use, bail
if to.lamports > 0 { {
debug!( let to = &mut to.try_account_ref_mut()?;
"Create Account: invalid argument; account {:?} already in use", if to.lamports > 0 {
to_address debug!(
); "Create Account: invalid argument; account {:?} already in use",
return Err(SystemError::AccountAlreadyInUse.into()); 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) 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 { if lamports == 0 {
return Ok(()); return Ok(());
} }
@ -166,7 +169,7 @@ fn transfer(from: &KeyedAccount, to: &mut Account, lamports: u64) -> Result<(),
} }
from.try_account_ref_mut()?.lamports -= lamports; from.try_account_ref_mut()?.lamports -= lamports;
to.lamports += lamports; to.try_account_ref_mut()?.lamports += lamports;
Ok(()) Ok(())
} }
@ -191,17 +194,8 @@ pub fn process_instruction(
} => { } => {
let from = next_keyed_account(keyed_accounts_iter)?; let from = next_keyed_account(keyed_accounts_iter)?;
let to = 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)?; let to_address = Address::create(to.unsigned_key(), None)?;
create_account( create_account(from, to, &to_address, lamports, space, &owner, &signers)
from,
&mut to_account,
&to_address,
lamports,
space,
&owner,
&signers,
)
} }
SystemInstruction::CreateAccountWithSeed { SystemInstruction::CreateAccountWithSeed {
base, base,
@ -212,17 +206,8 @@ pub fn process_instruction(
} => { } => {
let from = next_keyed_account(keyed_accounts_iter)?; let from = next_keyed_account(keyed_accounts_iter)?;
let to = 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)))?; let to_address = Address::create(&to.unsigned_key(), Some((&base, &seed, &owner)))?;
create_account( create_account(from, &to, &to_address, lamports, space, &owner, &signers)
from,
&mut to_account,
&to_address,
lamports,
space,
&owner,
&signers,
)
} }
SystemInstruction::Assign { owner } => { SystemInstruction::Assign { owner } => {
let keyed_account = next_keyed_account(keyed_accounts_iter)?; let keyed_account = next_keyed_account(keyed_accounts_iter)?;
@ -232,8 +217,8 @@ pub fn process_instruction(
} }
SystemInstruction::Transfer { lamports } => { SystemInstruction::Transfer { lamports } => {
let from = next_keyed_account(keyed_accounts_iter)?; let from = next_keyed_account(keyed_accounts_iter)?;
let mut to_account = next_keyed_account(keyed_accounts_iter)?.try_account_ref_mut()?; let to = next_keyed_account(keyed_accounts_iter)?;
transfer(from, &mut to_account, lamports) transfer(from, to, lamports)
} }
SystemInstruction::AdvanceNonceAccount => { SystemInstruction::AdvanceNonceAccount => {
let me = &mut next_keyed_account(keyed_accounts_iter)?; let me = &mut next_keyed_account(keyed_accounts_iter)?;
@ -452,13 +437,13 @@ mod tests {
let to = Pubkey::create_with_seed(&from, seed, &new_owner).unwrap(); let to = Pubkey::create_with_seed(&from, seed, &new_owner).unwrap();
let from_account = Account::new_ref(100, 0, &system_program::id()); 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(); let to_address = Address::create(&to, Some((&from, seed, &new_owner))).unwrap();
assert_eq!( assert_eq!(
create_account( create_account(
&KeyedAccount::new(&from, false, &from_account), &KeyedAccount::new(&from, false, &from_account),
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
&to_address, &to_address,
50, 50,
2, 2,
@ -468,7 +453,7 @@ mod tests {
Err(InstructionError::MissingRequiredSignature) Err(InstructionError::MissingRequiredSignature)
); );
assert_eq!(from_account.borrow().lamports, 100); assert_eq!(from_account.borrow().lamports, 100);
assert_eq!(to_account, Account::default()); assert_eq!(*to_account.borrow(), Account::default());
} }
#[test] #[test]
@ -479,12 +464,12 @@ mod tests {
let from_account = Account::new_ref(100, 1, &Pubkey::new_rand()); // not from system account let from_account = Account::new_ref(100, 1, &Pubkey::new_rand()); // not from system account
let to = Pubkey::new_rand(); 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!( assert_eq!(
create_account( create_account(
&KeyedAccount::new(&from, false, &from_account), // no signer &KeyedAccount::new(&from, false, &from_account), // no signer
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
&to.into(), &to.into(),
0, 0,
2, 2,
@ -495,13 +480,13 @@ mod tests {
); );
let from_lamports = from_account.borrow().lamports; let from_lamports = from_account.borrow().lamports;
let to_lamports = to_account.lamports; let to_lamports = to_account.borrow().lamports;
let to_owner = to_account.owner; let to_owner = to_account.borrow().owner;
let to_data = to_account.data; let to_data = &to_account.borrow().data;
assert_eq!(from_lamports, 100); assert_eq!(from_lamports, 100);
assert_eq!(to_lamports, 0); assert_eq!(to_lamports, 0);
assert_eq!(to_owner, new_owner); assert_eq!(to_owner, new_owner);
assert_eq!(to_data, [0, 0]); assert_eq!(*to_data, [0, 0]);
} }
#[test] #[test]
@ -512,11 +497,11 @@ mod tests {
let from_account = Account::new_ref(100, 0, &system_program::id()); let from_account = Account::new_ref(100, 0, &system_program::id());
let to = Pubkey::new_rand(); 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( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut to_account, &KeyedAccount::new(&from, false, &to_account),
&to.into(), &to.into(),
150, 150,
2, 2,
@ -530,7 +515,7 @@ mod tests {
fn test_request_more_than_allowed_data_length() { fn test_request_more_than_allowed_data_length() {
let from_account = Account::new_ref(100, 0, &system_program::id()); let from_account = Account::new_ref(100, 0, &system_program::id());
let from = Pubkey::new_rand(); 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 to = Pubkey::new_rand();
let signers = &[from, to].iter().cloned().collect::<HashSet<_>>(); let signers = &[from, to].iter().cloned().collect::<HashSet<_>>();
@ -539,7 +524,7 @@ mod tests {
// Trying to request more data length than permitted will result in failure // Trying to request more data length than permitted will result in failure
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
&address, &address,
50, 50,
MAX_PERMITTED_DATA_LENGTH + 1, MAX_PERMITTED_DATA_LENGTH + 1,
@ -555,7 +540,7 @@ mod tests {
// Trying to request equal or less data length than permitted will be successful // Trying to request equal or less data length than permitted will be successful
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
&address, &address,
50, 50,
MAX_PERMITTED_DATA_LENGTH, MAX_PERMITTED_DATA_LENGTH,
@ -563,8 +548,11 @@ mod tests {
&signers, &signers,
); );
assert!(result.is_ok()); assert!(result.is_ok());
assert_eq!(to_account.lamports, 50); assert_eq!(to_account.borrow().lamports, 50);
assert_eq!(to_account.data.len() as u64, MAX_PERMITTED_DATA_LENGTH); assert_eq!(
to_account.borrow().data.len() as u64,
MAX_PERMITTED_DATA_LENGTH
);
} }
#[test] #[test]
@ -576,7 +564,7 @@ mod tests {
let original_program_owner = Pubkey::new(&[5; 32]); let original_program_owner = Pubkey::new(&[5; 32]);
let owned_key = Pubkey::new_rand(); 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 unchanged_account = owned_account.clone();
let signers = &[from, owned_key].iter().cloned().collect::<HashSet<_>>(); let signers = &[from, owned_key].iter().cloned().collect::<HashSet<_>>();
@ -584,7 +572,7 @@ mod tests {
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, false, &owned_account),
&owned_address, &owned_address,
50, 50,
2, 2,
@ -598,11 +586,11 @@ mod tests {
assert_eq!(owned_account, unchanged_account); assert_eq!(owned_account, unchanged_account);
// Attempt to create system account in account that already has data // Attempt to create system account in account that already has data
let mut owned_account = Account::new(0, 1, &Pubkey::default()); let owned_account = Account::new_ref(0, 1, &Pubkey::default());
let unchanged_account = owned_account.clone(); let unchanged_account = owned_account.borrow().clone();
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, false, &owned_account),
&owned_address, &owned_address,
50, 50,
2, 2,
@ -612,14 +600,14 @@ mod tests {
assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
let from_lamports = from_account.borrow().lamports; let from_lamports = from_account.borrow().lamports;
assert_eq!(from_lamports, 100); 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 // Attempt to create an account that already has lamports
let mut owned_account = Account::new(1, 0, &Pubkey::default()); let owned_account = Account::new_ref(1, 0, &Pubkey::default());
let unchanged_account = owned_account.clone(); let unchanged_account = owned_account.borrow().clone();
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, false, &owned_account),
&owned_address, &owned_address,
50, 50,
2, 2,
@ -628,7 +616,7 @@ mod tests {
); );
assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into())); assert_eq!(result, Err(SystemError::AccountAlreadyInUse.into()));
assert_eq!(from_lamports, 100); assert_eq!(from_lamports, 100);
assert_eq!(owned_account, unchanged_account); assert_eq!(*owned_account.borrow(), unchanged_account);
} }
#[test] #[test]
@ -639,14 +627,14 @@ mod tests {
let from_account = Account::new_ref(100, 0, &system_program::id()); let from_account = Account::new_ref(100, 0, &system_program::id());
let owned_key = Pubkey::new_rand(); 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(); let owned_address = owned_key.into();
// Haven't signed from account // Haven't signed from account
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, false, &from_account), &KeyedAccount::new(&from, false, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, false, &owned_account),
&owned_address, &owned_address,
50, 50,
2, 2,
@ -656,10 +644,10 @@ mod tests {
assert_eq!(result, Err(InstructionError::MissingRequiredSignature)); assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
// Haven't signed to account // 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( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, true, &owned_account),
&owned_address, &owned_address,
50, 50,
2, 2,
@ -669,10 +657,10 @@ mod tests {
assert_eq!(result, Err(InstructionError::MissingRequiredSignature)); assert_eq!(result, Err(InstructionError::MissingRequiredSignature));
// support creation/assignment with zero lamports (ephemeral account) // 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( let result = create_account(
&KeyedAccount::new(&from, false, &from_account), &KeyedAccount::new(&from, false, &from_account),
&mut owned_account, &KeyedAccount::new(&owned_key, false, &owned_account),
&owned_address, &owned_address,
0, 0,
2, 2,
@ -689,7 +677,7 @@ mod tests {
let from_account = Account::new_ref(100, 0, &system_program::id()); let from_account = Account::new_ref(100, 0, &system_program::id());
let to = Pubkey::new_rand(); 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 signers = [from, to].iter().cloned().collect::<HashSet<_>>();
let to_address = to.into(); let to_address = to.into();
@ -697,7 +685,7 @@ mod tests {
// fail to create a sysvar::id() owned account // fail to create a sysvar::id() owned account
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
&to_address, &to_address,
50, 50,
2, 2,
@ -716,10 +704,11 @@ mod tests {
let from_account = Account::new_ref(100, 0, &system_program::id()); let from_account = Account::new_ref(100, 0, &system_program::id());
let populated_key = Pubkey::new_rand(); let populated_key = Pubkey::new_rand();
let mut populated_account = Account { let populated_account = Account {
data: vec![0, 1, 2, 3], data: vec![0, 1, 2, 3],
..Account::default() ..Account::default()
}; }
.into();
let signers = [from, populated_key] let signers = [from, populated_key]
.iter() .iter()
@ -729,7 +718,7 @@ mod tests {
let result = create_account( let result = create_account(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut populated_account, &KeyedAccount::new(&populated_key, false, &populated_account),
&populated_address, &populated_address,
50, 50,
2, 2,
@ -753,15 +742,16 @@ mod tests {
let from = KeyedAccount::new(&nonce, true, &nonce_account); let from = KeyedAccount::new(&nonce, true, &nonce_account);
let new = Pubkey::new_rand(); 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 signers = [nonce, new].iter().cloned().collect::<HashSet<_>>();
let new_address = new.into(); let new_address = new.into();
let new_keyed_account = KeyedAccount::new(&new, false, &new_account);
assert_eq!( assert_eq!(
create_account( create_account(
&from, &from,
&mut new_account, &new_keyed_account,
&new_address, &new_address,
42, 42,
0, 0,
@ -850,26 +840,28 @@ mod tests {
fn test_transfer_lamports() { fn test_transfer_lamports() {
let from = Pubkey::new_rand(); let from = Pubkey::new_rand();
let from_account = Account::new_ref(100, 0, &Pubkey::new(&[2; 32])); // account owner should not matter 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); 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 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!(from_lamports, 50);
assert_eq!(to_lamports, 51); assert_eq!(to_lamports, 51);
// Attempt to move more lamports than remaining in from_account // Attempt to move more lamports than remaining in from_account
let from_keyed_account = KeyedAccount::new(&from, true, &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!(result, Err(SystemError::ResultWithNegativeLamports.into()));
assert_eq!(from_keyed_account.account.borrow().lamports, 50); 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 // test unsigned transfer of zero
let from_keyed_account = KeyedAccount::new(&from, false, &from_account); 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!(from_keyed_account.account.borrow().lamports, 50);
assert_eq!(to_account.lamports, 51); assert_eq!(to_keyed_account.account.borrow().lamports, 51);
} }
#[test] #[test]
@ -889,11 +881,12 @@ mod tests {
Some(SystemAccountKind::Nonce) 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!( assert_eq!(
transfer( transfer(
&KeyedAccount::new(&from, true, &from_account), &KeyedAccount::new(&from, true, &from_account),
&mut to_account, &KeyedAccount::new(&to, false, &to_account),
50, 50,
), ),
Err(InstructionError::InvalidArgument), Err(InstructionError::InvalidArgument),