From 5e89bd88681da2f331e752ccbf670df77b600789 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Fri, 15 May 2020 13:23:09 -0600 Subject: [PATCH] Panic if no fee-payer found via Message::new() (#10050) automerge --- cli/src/checks.rs | 2 +- ledger/src/entry.rs | 4 +--- programs/bpf_loader/src/syscalls.rs | 2 +- runtime/src/bank.rs | 8 ++------ runtime/src/message_processor.rs | 15 +++++++++------ runtime/src/nonce_utils.rs | 3 +-- sdk/src/fee_calculator.rs | 2 +- sdk/src/message.rs | 19 ++++++++++++++++--- sdk/src/transaction.rs | 2 +- tokens/src/commands.rs | 6 ++---- tokens/src/db.rs | 6 ++++-- 11 files changed, 39 insertions(+), 30 deletions(-) diff --git a/cli/src/checks.rs b/cli/src/checks.rs index 5ef4aafa61c92b..524a84bbca91cb 100644 --- a/cli/src/checks.rs +++ b/cli/src/checks.rs @@ -169,7 +169,7 @@ mod tests { assert_eq!(calculate_fee(&fee_calculator, &[]), 0); // No signatures, no fee. - let message = Message::new(&[]); + let message = Message::default(); assert_eq!(calculate_fee(&fee_calculator, &[&message, &message]), 0); // One message w/ one signature, a fee. diff --git a/ledger/src/entry.rs b/ledger/src/entry.rs index 21bb6cfb30b26b..cf7869c4364445 100644 --- a/ledger/src/entry.rs +++ b/ledger/src/entry.rs @@ -479,7 +479,6 @@ mod tests { use solana_budget_program::budget_instruction; use solana_sdk::{ hash::{hash, Hash}, - message::Message, signature::{Keypair, Signer}, system_transaction, transaction::Transaction, @@ -684,8 +683,7 @@ mod tests { #[test] fn test_verify_tick_hash_count() { let hashes_per_tick = 10; - let keypairs: Vec<&Keypair> = Vec::new(); - let tx: Transaction = Transaction::new(&keypairs, Message::new(&[]), Hash::default()); + let tx = Transaction::default(); let tx_entry = Entry::new(&Hash::default(), 1, vec![tx]); 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()); diff --git a/programs/bpf_loader/src/syscalls.rs b/programs/bpf_loader/src/syscalls.rs index 3c010fe3e99a70..8717beedaf6fab 100644 --- a/programs/bpf_loader/src/syscalls.rs +++ b/programs/bpf_loader/src/syscalls.rs @@ -689,7 +689,7 @@ fn call<'a>( // Translate data passed from the VM 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 = message.account_keys[callee_program_id_index]; let caller_program_id = invoke_context diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index b6880a9aa50805..469d6ad591f27b 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -5455,12 +5455,8 @@ mod tests { let mut transfer_instruction = system_instruction::transfer(&mint_keypair.pubkey(), &key.pubkey(), 0); transfer_instruction.accounts[0].is_signer = false; - - let tx = Transaction::new_signed_instructions( - &Vec::<&Keypair>::new(), - &[transfer_instruction], - bank.last_blockhash(), - ); + let message = Message::new_with_payer(&[transfer_instruction], None); + let tx = Transaction::new(&[&Keypair::new(); 0], message, bank.last_blockhash()); assert_eq!( bank.process_transaction(&tx), diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 95aa5b8c0d9104..3a533a8df87717 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -611,8 +611,10 @@ mod tests { AccountMeta::new(keys[not_owned_index], false), AccountMeta::new(keys[owned_index], false), ]; - let message = - Message::new(&[Instruction::new(program_ids[owned_index], &[0_u8], metas)]); + let message = Message::new_with_payer( + &[Instruction::new(program_ids[owned_index], &[0_u8], metas)], + None, + ); // modify account owned by the program 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) accounts[0].borrow_mut().data[0] = 1; - let message = Message::new(&[Instruction::new( + let instruction = Instruction::new( callee_program_id, &MockInstruction::NoopSuccess, metas.clone(), - )]); + ); + let message = Message::new_with_payer(&[instruction], None); assert_eq!( MessageProcessor::process_cross_program_instruction( &message, @@ -1469,8 +1472,8 @@ mod tests { ]; for case in cases { - let message = - Message::new(&[Instruction::new(callee_program_id, &case.0, metas.clone())]); + let instruction = Instruction::new(callee_program_id, &case.0, metas.clone()); + let message = Message::new_with_payer(&[instruction], None); assert_eq!( MessageProcessor::process_cross_program_instruction( &message, diff --git a/runtime/src/nonce_utils.rs b/runtime/src/nonce_utils.rs index cdd9eb353cbb56..70e6c5218c0c03 100644 --- a/runtime/src/nonce_utils.rs +++ b/runtime/src/nonce_utils.rs @@ -125,8 +125,7 @@ mod tests { #[test] fn tx_uses_nonce_empty_ix_fail() { - let tx = Transaction::new_signed_instructions(&[&Keypair::new(); 0], &[], Hash::default()); - assert!(transaction_uses_durable_nonce(&tx).is_none()); + assert!(transaction_uses_durable_nonce(&Transaction::default()).is_none()); } #[test] diff --git a/sdk/src/fee_calculator.rs b/sdk/src/fee_calculator.rs index 3ba388febce192..6fb4de9e1f355f 100644 --- a/sdk/src/fee_calculator.rs +++ b/sdk/src/fee_calculator.rs @@ -183,7 +183,7 @@ mod tests { #[test] fn test_fee_calculator_calculate_fee() { // Default: no fee. - let message = Message::new(&[]); + let message = Message::default(); assert_eq!(FeeCalculator::default().calculate_fee(&message), 0); // No signature, no fee. diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 740285ef7121e9..d3493208096f9d 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -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 /// 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, @@ -143,7 +155,7 @@ pub struct MessageHeader { 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")] pub struct Message { /// The message header, identifying signed and read-only `account_keys` @@ -221,7 +233,8 @@ impl Message { } 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 { @@ -465,7 +478,7 @@ mod tests { let program_id = Pubkey::default(); let id0 = Pubkey::default(); 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); let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, true)]); diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index de50d07231c703..b38558d86fd3e0 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -80,7 +80,7 @@ impl std::fmt::Display for TransactionError { } /// An atomic transaction -#[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Default, Eq, Clone, Serialize, Deserialize)] pub struct Transaction { /// A set of digital signatures of `account_keys`, `program_ids`, `recent_blockhash`, and `instructions`, signed by the first /// signatures.len() keys of account_keys diff --git a/tokens/src/commands.rs b/tokens/src/commands.rs index 0bb3612bed4458..6d7a94d85d0535 100644 --- a/tokens/src/commands.rs +++ b/tokens/src/commands.rs @@ -605,7 +605,7 @@ pub fn test_process_distribute_stake_with_client(client: C, sender_ke mod tests { use super::*; 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] fn test_process_distribute_tokens() { @@ -679,9 +679,7 @@ mod tests { let transaction_infos = vec![TransactionInfo { recipient: bob, amount: 1.0, - new_stake_account_address: None, - finalized_date: None, - transaction: Transaction::new_unsigned_instructions(&[]), + ..TransactionInfo::default() }]; apply_previous_transactions(&mut allocations, &transaction_infos); assert_eq!(allocations.len(), 1); diff --git a/tokens/src/db.rs b/tokens/src/db.rs index 1dde7472052e6c..c7829488fb4b60 100644 --- a/tokens/src/db.rs +++ b/tokens/src/db.rs @@ -25,8 +25,10 @@ struct SignedTransactionInfo { impl Default for TransactionInfo { fn default() -> Self { - let mut transaction = Transaction::new_unsigned_instructions(&[]); - transaction.signatures.push(Signature::default()); + let transaction = Transaction { + signatures: vec![Signature::default()], + ..Transaction::default() + }; Self { recipient: Pubkey::default(), amount: 0.0,