Skip to content

Commit

Permalink
Panic if no fee-payer found via Message::new() (#10050)
Browse files Browse the repository at this point in the history
automerge
  • Loading branch information
garious authored May 15, 2020
1 parent 7080fb9 commit 5e89bd8
Show file tree
Hide file tree
Showing 11 changed files with 39 additions and 30 deletions.
2 changes: 1 addition & 1 deletion cli/src/checks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 1 addition & 3 deletions ledger/src/entry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion programs/bpf_loader/src/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 2 additions & 6 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
15 changes: 9 additions & 6 deletions runtime/src/message_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
3 changes: 1 addition & 2 deletions runtime/src/nonce_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/fee_calculator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
19 changes: 16 additions & 3 deletions sdk/src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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`
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)]);
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions tokens/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ pub fn test_process_distribute_stake_with_client<C: 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() {
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions tokens/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 5e89bd8

Please sign in to comment.