From 8e04fadb05910ca85e499d5404efd1e29020d82c Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Fri, 29 Mar 2019 13:21:32 -0600 Subject: [PATCH] Cleanup magic numbers Rename `num_signatures` to `num_required_signatures` to disambiguate it from `tx.signatures.len()`. --- core/src/sigverify.rs | 5 ++++- runtime/src/runtime.rs | 2 +- sdk/src/message.rs | 17 +++++++++-------- sdk/src/transaction.rs | 23 ++++++++++++++--------- 4 files changed, 28 insertions(+), 19 deletions(-) diff --git a/core/src/sigverify.rs b/core/src/sigverify.rs index 043e4d276a..800347267f 100644 --- a/core/src/sigverify.rs +++ b/core/src/sigverify.rs @@ -16,6 +16,9 @@ use std::mem::size_of; type TxOffsets = (Vec, Vec, Vec, Vec, Vec>); +// The serialized size of Message::num_required_signatures. +const NUM_REQUIRED_SIGNATURES_SIZE: usize = 1; + #[cfg(feature = "cuda")] #[repr(C)] struct Elems { @@ -128,7 +131,7 @@ pub fn get_packet_offsets(packet: &Packet, current_offset: u32) -> (u32, u32, u3 let sig_start = current_offset as usize + sig_size; let msg_start = current_offset as usize + msg_start_offset; - let pubkey_start = msg_start + 1 + pubkey_size; + let pubkey_start = msg_start + NUM_REQUIRED_SIGNATURES_SIZE + pubkey_size; ( sig_len as u32, diff --git a/runtime/src/runtime.rs b/runtime/src/runtime.rs index 7e4355c659..3046443733 100644 --- a/runtime/src/runtime.rs +++ b/runtime/src/runtime.rs @@ -129,7 +129,7 @@ impl Runtime { .map(|&index| { let index = index as usize; let key = &message.account_keys[index]; - (key, index < message.num_signatures as usize) + (key, index < message.num_required_signatures as usize) }) .zip(program_accounts.iter_mut()) .map(|((key, is_signer), account)| KeyedAccount::new(key, is_signer, account)) diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 333353513c..ddb4d9410e 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -70,7 +70,9 @@ fn get_program_ids(instructions: &[Instruction]) -> Vec { #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] pub struct Message { - pub num_signatures: u8, + /// The number of signatures required for this message to be considered valid. The + /// signatures must match the first `num_required_signatures` of `account_keys`. + pub num_required_signatures: u8, /// All the account keys used by this transaction #[serde(with = "short_vec")] @@ -94,7 +96,7 @@ pub struct Message { impl Message { pub fn new_with_compiled_instructions( - num_signatures: u8, + num_required_signatures: u8, account_keys: Vec, recent_blockhash: Hash, fee: u64, @@ -102,7 +104,7 @@ impl Message { instructions: Vec, ) -> Self { Self { - num_signatures, + num_required_signatures, account_keys, recent_blockhash, fee, @@ -111,15 +113,14 @@ impl Message { } } - /// Return an unsigned transaction with space for requires signatures. pub fn new(instructions: Vec) -> Self { let program_ids = get_program_ids(&instructions); let (mut signed_keys, unsigned_keys) = get_keys(&instructions); - let num_signatures = signed_keys.len() as u8; + let num_required_signatures = signed_keys.len() as u8; signed_keys.extend(&unsigned_keys); let instructions = compile_instructions(instructions, &signed_keys, &program_ids); Self::new_with_compiled_instructions( - num_signatures, + num_required_signatures, signed_keys, Hash::default(), 0, @@ -240,11 +241,11 @@ mod tests { let id0 = Pubkey::default(); let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, false)]); let message = Message::new(vec![ix]); - assert_eq!(message.num_signatures, 0); + assert_eq!(message.num_required_signatures, 0); let ix = Instruction::new(program_id, &0, vec![AccountMeta::new(id0, true)]); let message = Message::new(vec![ix]); - assert_eq!(message.num_signatures, 1); + assert_eq!(message.num_required_signatures, 1); } #[test] diff --git a/sdk/src/transaction.rs b/sdk/src/transaction.rs index ceb9529f12..63a0493672 100644 --- a/sdk/src/transaction.rs +++ b/sdk/src/transaction.rs @@ -61,7 +61,7 @@ pub struct Transaction { impl Transaction { pub fn new_unsigned(message: Message) -> Self { Self { - signatures: Vec::with_capacity(message.num_signatures as usize), + signatures: Vec::with_capacity(message.num_required_signatures as usize), message, } } @@ -176,7 +176,8 @@ impl Transaction { /// Check keys and keypair lengths, then sign this transaction. pub fn sign(&mut self, keypairs: &[&T], recent_blockhash: Hash) { - let signed_keys = &self.message.account_keys[0..self.message.num_signatures as usize]; + let signed_keys = + &self.message.account_keys[0..self.message.num_required_signatures as usize]; for (i, keypair) in keypairs.iter().enumerate() { assert_eq!(keypair.pubkey(), signed_keys[i], "keypair-pubkey mismatch"); } @@ -348,16 +349,20 @@ mod tests { let tx = Transaction::new(&[&alice_keypair], message, Hash::default()); - let expected_transaction_size = 1 + let len_size = 1; + let num_required_sigs_size = 1; + let blockhash_size = size_of::(); + let fee_size = size_of::(); + let expected_transaction_size = len_size + (tx.signatures.len() * size_of::()) - + 1 - + 1 + + num_required_sigs_size + + len_size + (tx.message.account_keys.len() * size_of::()) - + size_of::() - + size_of::() - + 1 + + blockhash_size + + fee_size + + len_size + (tx.message.program_ids.len() * size_of::()) - + 1 + + len_size + expected_instruction_size; assert_eq!(expected_transaction_size, 222);