From 19d11800bf12368591ad2fc4c9e5cbfcef05e016 Mon Sep 17 00:00:00 2001 From: Greg Fitzgerald Date: Sat, 30 May 2020 00:17:44 -0600 Subject: [PATCH] Remove WithSigner (#10325) automerge --- core/src/rpc.rs | 13 +++---- ledger/src/blockstore.rs | 6 +-- 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 +++++++++----- 7 files changed, 103 insertions(+), 99 deletions(-) diff --git a/core/src/rpc.rs b/core/src/rpc.rs index 1088477072..4078fe9a44 100644 --- a/core/src/rpc.rs +++ b/core/src/rpc.rs @@ -1648,6 +1648,7 @@ pub mod tests { fee_calculator::DEFAULT_BURN_PERCENT, hash::{hash, Hash}, instruction::InstructionError, + message::Message, rpc_port, signature::{Keypair, Signer}, system_transaction, @@ -1734,11 +1735,8 @@ pub mod tests { &leader_vote_keypair.pubkey(), vote, ); - let vote_tx = Transaction::new_signed_instructions( - &[&leader_vote_keypair], - &[vote_ix], - Hash::default(), - ); + let vote_msg = Message::new_with_payer(&[vote_ix], Some(&leader_vote_keypair.pubkey())); + let vote_tx = Transaction::new(&[&leader_vote_keypair], vote_msg, Hash::default()); let shreds = entries_to_test_shreds( vec![next_entry_mut(&mut Hash::default(), 0, vec![vote_tx])], 1, @@ -3303,9 +3301,10 @@ pub mod tests { bank.get_minimum_balance_for_rent_exemption(VoteState::size_of()), ); - let transaction = Transaction::new_signed_instructions( + let message = Message::new_with_payer(&instructions, Some(&alice.pubkey())); + let transaction = Transaction::new( &[&alice, &alice_vote_keypair], - &instructions, + message, bank.last_blockhash(), ); bank.process_transaction(&transaction) diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 5fa3927d68..f838c61c44 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -3087,6 +3087,7 @@ pub mod tests { use solana_sdk::{ hash::{self, hash, Hash}, instruction::CompiledInstruction, + message::Message, native_token::sol_to_lamports, packet::PACKET_DATA_SIZE, pubkey::Pubkey, @@ -5488,9 +5489,8 @@ pub mod tests { timestamp, }; let vote_ix = vote_instruction::vote(&keypair.pubkey(), &keypair.pubkey(), vote); - - let vote_tx = - Transaction::new_signed_instructions(&[keypair], &[vote_ix], Hash::default()); + let vote_msg = Message::new_with_payer(&[vote_ix], Some(&keypair.pubkey())); + let vote_tx = Transaction::new(&[keypair], vote_msg, Hash::default()); vote_entries.push(next_entry_mut(&mut Hash::default(), 0, vec![vote_tx])); let mut tick = create_ticks(1, 0, hash(&serialize(&i).unwrap())); diff --git a/programs/stake/src/stake_instruction.rs b/programs/stake/src/stake_instruction.rs index 9073117b81..e3d11a1395 100644 --- a/programs/stake/src/stake_instruction.rs +++ b/programs/stake/src/stake_instruction.rs @@ -8,7 +8,7 @@ use serde_derive::{Deserialize, Serialize}; use solana_sdk::{ account::{get_signers, KeyedAccount}, clock::{Epoch, UnixTimestamp}, - instruction::{AccountMeta, Instruction, InstructionError, WithSigner}, + instruction::{AccountMeta, Instruction, InstructionError}, program_utils::{limited_deserialize, next_keyed_account, DecodeError}, pubkey::Pubkey, system_instruction, @@ -184,8 +184,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) } @@ -293,8 +293,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(), @@ -314,8 +314,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) } @@ -331,11 +331,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) @@ -345,8 +345,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) } @@ -355,7 +355,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 18db807484..9cfa0ad1b0 100644 --- a/programs/vote/src/vote_instruction.rs +++ b/programs/vote/src/vote_instruction.rs @@ -12,7 +12,7 @@ use solana_metrics::inc_new_counter_info; use solana_sdk::{ account::{get_signers, KeyedAccount}, hash::Hash, - instruction::{AccountMeta, Instruction, InstructionError, WithSigner}, + instruction::{AccountMeta, Instruction, InstructionError}, program_utils::{limited_deserialize, next_keyed_account, DecodeError}, pubkey::Pubkey, system_instruction, @@ -108,8 +108,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(), @@ -162,8 +162,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(), @@ -180,8 +180,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(), @@ -195,8 +195,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) } @@ -211,8 +211,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(), @@ -230,8 +230,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 d349320809..41de809232 100644 --- a/sdk/src/message.rs +++ b/sdk/src/message.rs @@ -103,11 +103,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 { @@ -426,6 +439,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 ea5cfa0193..ad70e16bcb 100644 --- a/sdk/src/system_instruction.rs +++ b/sdk/src/system_instruction.rs @@ -1,5 +1,5 @@ use crate::{ - instruction::{AccountMeta, Instruction, WithSigner}, + instruction::{AccountMeta, Instruction}, nonce, program_utils::DecodeError, pubkey::Pubkey, @@ -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),