From 473037db8628a5ad52d4ed087bc9ade543bece69 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Sat, 30 May 2020 00:17:44 -0600 Subject: [PATCH] Remove WithSigner (#10325) --- programs/stake/src/stake_instruction.rs | 29 +++++++------- programs/vote/src/vote_instruction.rs | 26 ++++++------- sdk/src/instruction.rs | 52 ------------------------- sdk/src/message.rs | 47 +++++++++++++++++++++- sdk/src/system_instruction.rs | 29 +++++++++----- 5 files changed, 94 insertions(+), 89 deletions(-) diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 95e1ae9975..f270d27b62 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -9,7 +9,7 @@ use solana_sdk::{ account::{get_signers, next_keyed_account, KeyedAccount}, clock::{Epoch, UnixTimestamp}, decode_error::DecodeError, - instruction::{AccountMeta, Instruction, InstructionError, WithSigner}, + instruction::{AccountMeta, Instruction, InstructionError}, program_utils::limited_deserialize, pubkey::Pubkey, system_instruction, @@ -202,8 +202,8 @@ fn _split( let account_metas = vec![ AccountMeta::new(*stake_pubkey, false), AccountMeta::new(*split_stake_pubkey, false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new(id(), &StakeInstruction::Split(lamports), account_metas) } @@ -331,8 +331,8 @@ pub fn authorize( let account_metas = vec![ AccountMeta::new(*stake_pubkey, false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new( id(), @@ -352,8 +352,8 @@ pub fn delegate_stake( AccountMeta::new_readonly(sysvar::clock::id(), false), AccountMeta::new_readonly(sysvar::stake_history::id(), false), AccountMeta::new_readonly(crate::config::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new(id(), &StakeInstruction::DelegateStake, account_metas) } @@ -369,11 +369,11 @@ pub fn withdraw( AccountMeta::new(*to_pubkey, false), AccountMeta::new_readonly(sysvar::clock::id(), false), AccountMeta::new_readonly(sysvar::stake_history::id(), false), - ] - .with_signer(withdrawer_pubkey); + AccountMeta::new_readonly(*withdrawer_pubkey, true), + ]; if let Some(custodian_pubkey) = custodian_pubkey { - account_metas = account_metas.with_signer(custodian_pubkey) + account_metas.push(AccountMeta::new_readonly(*custodian_pubkey, true)); } Instruction::new(id(), &StakeInstruction::Withdraw(lamports), account_metas) @@ -383,8 +383,8 @@ pub fn deactivate_stake(stake_pubkey: &Pubkey, authorized_pubkey: &Pubkey) -> In let account_metas = vec![ AccountMeta::new(*stake_pubkey, false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new(id(), &StakeInstruction::Deactivate, account_metas) } @@ -393,7 +393,10 @@ pub fn set_lockup( lockup: &LockupArgs, custodian_pubkey: &Pubkey, ) -> Instruction { - let account_metas = vec![AccountMeta::new(*stake_pubkey, false)].with_signer(custodian_pubkey); + let account_metas = vec![ + AccountMeta::new(*stake_pubkey, false), + AccountMeta::new_readonly(*custodian_pubkey, true), + ]; Instruction::new(id(), &StakeInstruction::SetLockup(*lockup), account_metas) } diff --git a/programs/vote/src/vote_instruction.rs b/programs/vote/src/vote_instruction.rs index 465eb87b00..5df237758d 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -13,7 +13,7 @@ use solana_sdk::{ account::{get_signers, next_keyed_account, KeyedAccount}, decode_error::DecodeError, hash::Hash, - instruction::{AccountMeta, Instruction, InstructionError, WithSigner}, + instruction::{AccountMeta, Instruction, InstructionError}, program_utils::limited_deserialize, pubkey::Pubkey, system_instruction, @@ -116,8 +116,8 @@ fn initialize_account(vote_pubkey: &Pubkey, vote_init: &VoteInit) -> Instruction AccountMeta::new(*vote_pubkey, false), AccountMeta::new_readonly(sysvar::rent::id(), false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(&vote_init.node_pubkey); + AccountMeta::new_readonly(vote_init.node_pubkey, true), + ]; Instruction::new( id(), @@ -170,8 +170,8 @@ pub fn authorize( let account_metas = vec![ AccountMeta::new(*vote_pubkey, false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new( id(), @@ -188,8 +188,8 @@ pub fn update_validator_identity( let account_metas = vec![ AccountMeta::new(*vote_pubkey, false), AccountMeta::new_readonly(*node_pubkey, true), - ] - .with_signer(authorized_withdrawer_pubkey); + AccountMeta::new_readonly(*authorized_withdrawer_pubkey, true), + ]; Instruction::new( id(), @@ -220,8 +220,8 @@ pub fn vote(vote_pubkey: &Pubkey, authorized_voter_pubkey: &Pubkey, vote: Vote) AccountMeta::new(*vote_pubkey, false), AccountMeta::new_readonly(sysvar::slot_hashes::id(), false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(authorized_voter_pubkey); + AccountMeta::new_readonly(*authorized_voter_pubkey, true), + ]; Instruction::new(id(), &VoteInstruction::Vote(vote), account_metas) } @@ -236,8 +236,8 @@ pub fn vote_switch( AccountMeta::new(*vote_pubkey, false), AccountMeta::new_readonly(sysvar::slot_hashes::id(), false), AccountMeta::new_readonly(sysvar::clock::id(), false), - ] - .with_signer(authorized_voter_pubkey); + AccountMeta::new_readonly(*authorized_voter_pubkey, true), + ]; Instruction::new( id(), @@ -255,8 +255,8 @@ pub fn withdraw( let account_metas = vec![ AccountMeta::new(*vote_pubkey, false), AccountMeta::new(*to_pubkey, false), - ] - .with_signer(authorized_withdrawer_pubkey); + AccountMeta::new_readonly(*authorized_withdrawer_pubkey, true), + ]; Instruction::new(id(), &VoteInstruction::Withdraw(lamports), account_metas) } diff --git a/sdk/src/instruction.rs b/sdk/src/instruction.rs index 23643c0f20..a6b13b4981 100644 --- a/sdk/src/instruction.rs +++ b/sdk/src/instruction.rs @@ -209,28 +209,6 @@ impl AccountMeta { } } -/// Trait for adding a signer Pubkey to an existing data structure -pub trait WithSigner { - /// Add a signer Pubkey - fn with_signer(self, signer: &Pubkey) -> Self; -} - -impl WithSigner for Vec { - fn with_signer(mut self, signer: &Pubkey) -> Self { - for meta in self.iter_mut() { - // signer might already appear in parameters - if &meta.pubkey == signer { - meta.is_signer = true; // found it, we're done - return self; - } - } - - // signer wasn't in metas, append it after normal parameters - self.push(AccountMeta::new_readonly(*signer, true)); - self - } -} - /// An instruction to execute a program #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Clone)] #[serde(rename_all = "camelCase")] @@ -286,36 +264,6 @@ impl CompiledInstruction { mod test { use super::*; - #[test] - fn test_account_meta_list_with_signer() { - let account_pubkey = Pubkey::new_rand(); - let signer_pubkey = Pubkey::new_rand(); - - let account_meta = AccountMeta::new(account_pubkey, false); - let signer_account_meta = AccountMeta::new(signer_pubkey, false); - - let metas = vec![].with_signer(&signer_pubkey); - assert_eq!(metas.len(), 1); - assert!(metas[0].is_signer); - - let metas = vec![account_meta.clone()].with_signer(&signer_pubkey); - assert_eq!(metas.len(), 2); - assert!(!metas[0].is_signer); - assert!(metas[1].is_signer); - assert_eq!(metas[1].pubkey, signer_pubkey); - - let metas = vec![signer_account_meta.clone()].with_signer(&signer_pubkey); - assert_eq!(metas.len(), 1); - assert!(metas[0].is_signer); - assert_eq!(metas[0].pubkey, signer_pubkey); - - let metas = vec![account_meta, signer_account_meta].with_signer(&signer_pubkey); - assert_eq!(metas.len(), 2); - assert!(!metas[0].is_signer); - assert!(metas[1].is_signer); - assert_eq!(metas[1].pubkey, signer_pubkey); - } - #[test] fn test_visit_each_account() { let do_work = |accounts: &[u8]| -> (usize, usize) { diff --git a/sdk/src/message.rs b/sdk/src/message.rs index 2b08fdaa58..f82f1b0e24 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -91,11 +91,24 @@ fn get_keys(instructions: &[Instruction], payer: Option<&Pubkey>) -> Instruction keys_and_signed.insert(0, &payer_account_meta); } + let mut unique_metas: Vec = vec![]; + for account_meta in keys_and_signed { + // Promote to writable if a later AccountMeta requires it + if let Some(x) = unique_metas + .iter_mut() + .find(|x| x.pubkey == account_meta.pubkey) + { + x.is_writable |= account_meta.is_writable; + continue; + } + unique_metas.push(account_meta.clone()); + } + let mut signed_keys = vec![]; let mut unsigned_keys = vec![]; let mut num_readonly_signed_accounts = 0; let mut num_readonly_unsigned_accounts = 0; - for account_meta in keys_and_signed.into_iter().unique_by(|x| x.pubkey) { + for account_meta in unique_metas { if account_meta.is_signer { signed_keys.push(account_meta.pubkey); if !account_meta.is_writable { @@ -409,6 +422,38 @@ mod tests { assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0)); } + #[test] + fn test_message_unique_keys_one_readonly_signed() { + let program_id = Pubkey::default(); + let id0 = Pubkey::default(); + let keys = get_keys( + &[ + Instruction::new(program_id, &0, vec![AccountMeta::new_readonly(id0, true)]), + Instruction::new(program_id, &0, vec![AccountMeta::new(id0, true)]), + ], + None, + ); + + // Ensure the key is no longer readonly + assert_eq!(keys, InstructionKeys::new(vec![id0], vec![], 0, 0)); + } + + #[test] + fn test_message_unique_keys_one_readonly_unsigned() { + let program_id = Pubkey::default(); + let id0 = Pubkey::default(); + let keys = get_keys( + &[ + Instruction::new(program_id, &0, vec![AccountMeta::new_readonly(id0, false)]), + Instruction::new(program_id, &0, vec![AccountMeta::new(id0, false)]), + ], + None, + ); + + // Ensure the key is no longer readonly + assert_eq!(keys, InstructionKeys::new(vec![], vec![id0], 0, 0)); + } + #[test] fn test_message_unique_keys_order_preserved() { let program_id = Pubkey::default(); diff --git a/sdk/src/system_instruction.rs b/sdk/src/system_instruction.rs index e78ba86672..dabc716fdb 100644 --- a/sdk/src/system_instruction.rs +++ b/sdk/src/system_instruction.rs @@ -1,6 +1,6 @@ use crate::{ decode_error::DecodeError, - instruction::{AccountMeta, Instruction, WithSigner}, + instruction::{AccountMeta, Instruction}, nonce, pubkey::Pubkey, system_program, @@ -234,8 +234,8 @@ pub fn create_account_with_seed( let account_metas = vec![ AccountMeta::new(*from_pubkey, true), AccountMeta::new(*to_pubkey, false), - ] - .with_signer(base); + AccountMeta::new_readonly(*base, true), + ]; Instruction::new( system_program::id(), @@ -265,7 +265,10 @@ pub fn assign_with_seed( seed: &str, owner: &Pubkey, ) -> Instruction { - let account_metas = vec![AccountMeta::new(*address, false)].with_signer(base); + let account_metas = vec![ + AccountMeta::new(*address, false), + AccountMeta::new_readonly(*base, true), + ]; Instruction::new( system_program::id(), &SystemInstruction::AssignWithSeed { @@ -305,7 +308,10 @@ pub fn allocate_with_seed( space: u64, owner: &Pubkey, ) -> Instruction { - let account_metas = vec![AccountMeta::new(*address, false)].with_signer(base); + let account_metas = vec![ + AccountMeta::new(*address, false), + AccountMeta::new_readonly(*base, true), + ]; Instruction::new( system_program::id(), &SystemInstruction::AllocateWithSeed { @@ -386,8 +392,8 @@ pub fn advance_nonce_account(nonce_pubkey: &Pubkey, authorized_pubkey: &Pubkey) let account_metas = vec![ AccountMeta::new(*nonce_pubkey, false), AccountMeta::new_readonly(recent_blockhashes::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new( system_program::id(), &SystemInstruction::AdvanceNonceAccount, @@ -406,8 +412,8 @@ pub fn withdraw_nonce_account( AccountMeta::new(*to_pubkey, false), AccountMeta::new_readonly(recent_blockhashes::id(), false), AccountMeta::new_readonly(rent::id(), false), - ] - .with_signer(authorized_pubkey); + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new( system_program::id(), &SystemInstruction::WithdrawNonceAccount(lamports), @@ -420,7 +426,10 @@ pub fn authorize_nonce_account( authorized_pubkey: &Pubkey, new_authority: &Pubkey, ) -> Instruction { - let account_metas = vec![AccountMeta::new(*nonce_pubkey, false)].with_signer(authorized_pubkey); + let account_metas = vec![ + AccountMeta::new(*nonce_pubkey, false), + AccountMeta::new_readonly(*authorized_pubkey, true), + ]; Instruction::new( system_program::id(), &SystemInstruction::AuthorizeNonceAccount(*new_authority),