Panic if no fee-payer found via Message::new() (#10050)

automerge
This commit is contained in:
Greg Fitzgerald
2020-05-15 13:23:09 -06:00
committed by GitHub
parent 7080fb9b37
commit 5e89bd8868
11 changed files with 39 additions and 30 deletions

View File

@ -169,7 +169,7 @@ mod tests {
assert_eq!(calculate_fee(&fee_calculator, &[]), 0); assert_eq!(calculate_fee(&fee_calculator, &[]), 0);
// No signatures, no fee. // No signatures, no fee.
let message = Message::new(&[]); let message = Message::default();
assert_eq!(calculate_fee(&fee_calculator, &[&message, &message]), 0); assert_eq!(calculate_fee(&fee_calculator, &[&message, &message]), 0);
// One message w/ one signature, a fee. // One message w/ one signature, a fee.

View File

@ -479,7 +479,6 @@ mod tests {
use solana_budget_program::budget_instruction; use solana_budget_program::budget_instruction;
use solana_sdk::{ use solana_sdk::{
hash::{hash, Hash}, hash::{hash, Hash},
message::Message,
signature::{Keypair, Signer}, signature::{Keypair, Signer},
system_transaction, system_transaction,
transaction::Transaction, transaction::Transaction,
@ -684,8 +683,7 @@ mod tests {
#[test] #[test]
fn test_verify_tick_hash_count() { fn test_verify_tick_hash_count() {
let hashes_per_tick = 10; let hashes_per_tick = 10;
let keypairs: Vec<&Keypair> = Vec::new(); let tx = Transaction::default();
let tx: Transaction = Transaction::new(&keypairs, Message::new(&[]), Hash::default());
let tx_entry = Entry::new(&Hash::default(), 1, vec![tx]); let tx_entry = Entry::new(&Hash::default(), 1, vec![tx]);
let full_tick_entry = Entry::new_tick(hashes_per_tick, &Hash::default()); let full_tick_entry = Entry::new_tick(hashes_per_tick, &Hash::default());
let partial_tick_entry = Entry::new_tick(hashes_per_tick - 1, &Hash::default()); let partial_tick_entry = Entry::new_tick(hashes_per_tick - 1, &Hash::default());

View File

@ -689,7 +689,7 @@ fn call<'a>(
// Translate data passed from the VM // Translate data passed from the VM
let instruction = syscall.translate_instruction(instruction_addr, ro_regions)?; let instruction = syscall.translate_instruction(instruction_addr, ro_regions)?;
let message = Message::new(&[instruction]); let message = Message::new_with_payer(&[instruction], None);
let callee_program_id_index = message.instructions[0].program_id_index as usize; let callee_program_id_index = message.instructions[0].program_id_index as usize;
let callee_program_id = message.account_keys[callee_program_id_index]; let callee_program_id = message.account_keys[callee_program_id_index];
let caller_program_id = invoke_context let caller_program_id = invoke_context

View File

@ -5455,12 +5455,8 @@ mod tests {
let mut transfer_instruction = let mut transfer_instruction =
system_instruction::transfer(&mint_keypair.pubkey(), &key.pubkey(), 0); system_instruction::transfer(&mint_keypair.pubkey(), &key.pubkey(), 0);
transfer_instruction.accounts[0].is_signer = false; transfer_instruction.accounts[0].is_signer = false;
let message = Message::new_with_payer(&[transfer_instruction], None);
let tx = Transaction::new_signed_instructions( let tx = Transaction::new(&[&Keypair::new(); 0], message, bank.last_blockhash());
&Vec::<&Keypair>::new(),
&[transfer_instruction],
bank.last_blockhash(),
);
assert_eq!( assert_eq!(
bank.process_transaction(&tx), bank.process_transaction(&tx),

View File

@ -611,8 +611,10 @@ mod tests {
AccountMeta::new(keys[not_owned_index], false), AccountMeta::new(keys[not_owned_index], false),
AccountMeta::new(keys[owned_index], false), AccountMeta::new(keys[owned_index], false),
]; ];
let message = let message = Message::new_with_payer(
Message::new(&[Instruction::new(program_ids[owned_index], &[0_u8], metas)]); &[Instruction::new(program_ids[owned_index], &[0_u8], metas)],
None,
);
// modify account owned by the program // modify account owned by the program
accounts[owned_index].borrow_mut().data[0] = (MAX_DEPTH + owned_index) as u8; accounts[owned_index].borrow_mut().data[0] = (MAX_DEPTH + owned_index) as u8;
@ -1437,11 +1439,12 @@ mod tests {
// not owned account modified by the caller (before the invoke) // not owned account modified by the caller (before the invoke)
accounts[0].borrow_mut().data[0] = 1; accounts[0].borrow_mut().data[0] = 1;
let message = Message::new(&[Instruction::new( let instruction = Instruction::new(
callee_program_id, callee_program_id,
&MockInstruction::NoopSuccess, &MockInstruction::NoopSuccess,
metas.clone(), metas.clone(),
)]); );
let message = Message::new_with_payer(&[instruction], None);
assert_eq!( assert_eq!(
MessageProcessor::process_cross_program_instruction( MessageProcessor::process_cross_program_instruction(
&message, &message,
@ -1469,8 +1472,8 @@ mod tests {
]; ];
for case in cases { for case in cases {
let message = let instruction = Instruction::new(callee_program_id, &case.0, metas.clone());
Message::new(&[Instruction::new(callee_program_id, &case.0, metas.clone())]); let message = Message::new_with_payer(&[instruction], None);
assert_eq!( assert_eq!(
MessageProcessor::process_cross_program_instruction( MessageProcessor::process_cross_program_instruction(
&message, &message,

View File

@ -125,8 +125,7 @@ mod tests {
#[test] #[test]
fn tx_uses_nonce_empty_ix_fail() { fn tx_uses_nonce_empty_ix_fail() {
let tx = Transaction::new_signed_instructions(&[&Keypair::new(); 0], &[], Hash::default()); assert!(transaction_uses_durable_nonce(&Transaction::default()).is_none());
assert!(transaction_uses_durable_nonce(&tx).is_none());
} }
#[test] #[test]

View File

@ -183,7 +183,7 @@ mod tests {
#[test] #[test]
fn test_fee_calculator_calculate_fee() { fn test_fee_calculator_calculate_fee() {
// Default: no fee. // Default: no fee.
let message = Message::new(&[]); let message = Message::default();
assert_eq!(FeeCalculator::default().calculate_fee(&message), 0); assert_eq!(FeeCalculator::default().calculate_fee(&message), 0);
// No signature, no fee. // No signature, no fee.

View File

@ -57,6 +57,18 @@ impl InstructionKeys {
} }
} }
/// Return the pubkey of the first writable signer in the given set of instructions.
fn find_writable_signer(instructions: &[Instruction]) -> Option<&Pubkey> {
for instruction in instructions {
for account in &instruction.accounts {
if account.is_signer && account.is_writable {
return Some(&account.pubkey);
}
}
}
None
}
/// Return pubkeys referenced by all instructions, with the ones needing signatures first. If the /// Return pubkeys referenced by all instructions, with the ones needing signatures first. If the
/// payer key is provided, it is always placed first in the list of signed keys. Read-only signed /// payer key is provided, it is always placed first in the list of signed keys. Read-only signed
/// accounts are placed last in the set of signed accounts. Read-only unsigned accounts, /// accounts are placed last in the set of signed accounts. Read-only unsigned accounts,
@ -143,7 +155,7 @@ pub struct MessageHeader {
pub num_readonly_unsigned_accounts: u8, pub num_readonly_unsigned_accounts: u8,
} }
#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] #[derive(Serialize, Deserialize, Default, Debug, PartialEq, Eq, Clone)]
#[serde(rename_all = "camelCase")] #[serde(rename_all = "camelCase")]
pub struct Message { pub struct Message {
/// The message header, identifying signed and read-only `account_keys` /// The message header, identifying signed and read-only `account_keys`
@ -221,7 +233,8 @@ impl Message {
} }
pub fn new(instructions: &[Instruction]) -> Self { pub fn new(instructions: &[Instruction]) -> Self {
Self::new_with_payer(instructions, None) let payer = find_writable_signer(instructions).expect("no suitable key for fee-payer");
Self::new_with_payer(instructions, Some(payer))
} }
pub fn new_with_payer(instructions: &[Instruction], payer: Option<&Pubkey>) -> Self { pub fn new_with_payer(instructions: &[Instruction], payer: Option<&Pubkey>) -> Self {
@ -465,7 +478,7 @@ mod tests {
let program_id = Pubkey::default(); let program_id = Pubkey::default();
let id0 = Pubkey::default(); let id0 = Pubkey::default();
let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, false)]); let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, false)]);
let message = Message::new(&[ix]); let message = Message::new_with_payer(&[ix], None);
assert_eq!(message.header.num_required_signatures, 0); assert_eq!(message.header.num_required_signatures, 0);
let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, true)]); let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, true)]);

View File

@ -80,7 +80,7 @@ impl std::fmt::Display for TransactionError {
} }
/// An atomic transaction /// An atomic transaction
#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] #[derive(Debug, PartialEq, Default, Eq, Clone, Serialize, Deserialize)]
pub struct Transaction { pub struct Transaction {
/// A set of digital signatures of `account_keys`, `program_ids`, `recent_blockhash`, and `instructions`, signed by the first /// A set of digital signatures of `account_keys`, `program_ids`, `recent_blockhash`, and `instructions`, signed by the first
/// signatures.len() keys of account_keys /// signatures.len() keys of account_keys

View File

@ -605,7 +605,7 @@ pub fn test_process_distribute_stake_with_client<C: Client>(client: C, sender_ke
mod tests { mod tests {
use super::*; use super::*;
use solana_runtime::{bank::Bank, bank_client::BankClient}; use solana_runtime::{bank::Bank, bank_client::BankClient};
use solana_sdk::{genesis_config::create_genesis_config, transaction::Transaction}; use solana_sdk::genesis_config::create_genesis_config;
#[test] #[test]
fn test_process_distribute_tokens() { fn test_process_distribute_tokens() {
@ -679,9 +679,7 @@ mod tests {
let transaction_infos = vec![TransactionInfo { let transaction_infos = vec![TransactionInfo {
recipient: bob, recipient: bob,
amount: 1.0, amount: 1.0,
new_stake_account_address: None, ..TransactionInfo::default()
finalized_date: None,
transaction: Transaction::new_unsigned_instructions(&[]),
}]; }];
apply_previous_transactions(&mut allocations, &transaction_infos); apply_previous_transactions(&mut allocations, &transaction_infos);
assert_eq!(allocations.len(), 1); assert_eq!(allocations.len(), 1);

View File

@ -25,8 +25,10 @@ struct SignedTransactionInfo {
impl Default for TransactionInfo { impl Default for TransactionInfo {
fn default() -> Self { fn default() -> Self {
let mut transaction = Transaction::new_unsigned_instructions(&[]); let transaction = Transaction {
transaction.signatures.push(Signature::default()); signatures: vec![Signature::default()],
..Transaction::default()
};
Self { Self {
recipient: Pubkey::default(), recipient: Pubkey::default(),
amount: 0.0, amount: 0.0,