From 096febd593fcc504d5679deb9b3030a9474c8b5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20Mei=C3=9Fner?= Date: Wed, 13 Apr 2022 12:17:07 +0200 Subject: [PATCH] Remove KeyedAccount in builtin program "address lookup table" (#24283) * Makes sure that there is only one KeyedAccount at a time. * KeyedAccount by BorrowedAccount in address_lookup_table. * Cleanup unused code. --- .../address-lookup-table/src/processor.rs | 210 ++++++++++-------- 1 file changed, 112 insertions(+), 98 deletions(-) diff --git a/programs/address-lookup-table/src/processor.rs b/programs/address-lookup-table/src/processor.rs index 1131d31c64..f86f1cf89c 100644 --- a/programs/address-lookup-table/src/processor.rs +++ b/programs/address-lookup-table/src/processor.rs @@ -8,11 +8,8 @@ use { }, solana_program_runtime::{ic_msg, invoke_context::InvokeContext}, solana_sdk::{ - account::{ReadableAccount, WritableAccount}, - account_utils::State, clock::Slot, instruction::InstructionError, - keyed_account::keyed_account_at_index, program_utils::limited_deserialize, pubkey::{Pubkey, PUBKEY_BYTES}, system_instruction, @@ -60,34 +57,40 @@ pub struct Processor; impl Processor { fn create_lookup_table( invoke_context: &mut InvokeContext, - first_instruction_account: usize, + _first_instruction_account: usize, untrusted_recent_slot: Slot, bump_seed: u8, ) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; - let _instruction_context = transaction_context.get_current_instruction_context()?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; + let instruction_context = transaction_context.get_current_instruction_context()?; let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; - if lookup_table_account.data_len()? > 0 { + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let lookup_table_lamports = lookup_table_account.get_lamports(); + let table_key = *lookup_table_account.get_key(); + if !lookup_table_account.get_data().is_empty() { ic_msg!(invoke_context, "Table account must not be allocated"); return Err(InstructionError::AccountAlreadyInitialized); } + drop(lookup_table_account); let authority_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; - let authority_key = *authority_account.signer_key().ok_or_else(|| { + instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + let authority_key = *authority_account.get_key(); + if !authority_account.is_signer() { ic_msg!(invoke_context, "Authority account must be a signer"); - InstructionError::MissingRequiredSignature - })?; + return Err(InstructionError::MissingRequiredSignature); + } + drop(authority_account); let payer_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 2)?)?; - let payer_key = *payer_account.signer_key().ok_or_else(|| { + instruction_context.try_borrow_instruction_account(transaction_context, 2)?; + let payer_key = *payer_account.get_key(); + if !payer_account.is_signer() { ic_msg!(invoke_context, "Payer account must be a signer"); - InstructionError::MissingRequiredSignature - })?; + return Err(InstructionError::MissingRequiredSignature); + } + drop(payer_account); let derivation_slot = { let slot_hashes = invoke_context.get_sysvar_cache().get_slot_hashes()?; @@ -114,7 +117,6 @@ impl Processor { &crate::id(), )?; - let table_key = *lookup_table_account.unsigned_key(); if table_key != derived_table_key { ic_msg!( invoke_context, @@ -129,7 +131,7 @@ impl Processor { let required_lamports = rent .minimum_balance(table_account_data_len) .max(1) - .saturating_sub(lookup_table_account.lamports()?); + .saturating_sub(lookup_table_lamports); if required_lamports > 0 { invoke_context.native_invoke( @@ -148,9 +150,10 @@ impl Processor { &[table_key], )?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; - let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; + let transaction_context = &invoke_context.transaction_context; + let instruction_context = transaction_context.get_current_instruction_context()?; + let mut lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; lookup_table_account.set_state(&ProgramState::LookupTable(LookupTableMeta::new( authority_key, )))?; @@ -160,33 +163,37 @@ impl Processor { fn freeze_lookup_table( invoke_context: &mut InvokeContext, - first_instruction_account: usize, + _first_instruction_account: usize, ) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; - let _instruction_context = transaction_context.get_current_instruction_context()?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; + let instruction_context = transaction_context.get_current_instruction_context()?; let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; - if lookup_table_account.owner()? != crate::id() { + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + if *lookup_table_account.get_owner() != crate::id() { return Err(InstructionError::InvalidAccountOwner); } + drop(lookup_table_account); let authority_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; - if authority_account.signer_key().is_none() { + instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + let authority_key = *authority_account.get_key(); + if !authority_account.is_signer() { + ic_msg!(invoke_context, "Authority account must be a signer"); return Err(InstructionError::MissingRequiredSignature); } + drop(authority_account); - let lookup_table_account_ref = lookup_table_account.try_account_ref()?; - let lookup_table_data = lookup_table_account_ref.data(); + let mut lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let lookup_table_data = lookup_table_account.get_data(); let lookup_table = AddressLookupTable::deserialize(lookup_table_data)?; if lookup_table.meta.authority.is_none() { ic_msg!(invoke_context, "Lookup table is already frozen"); return Err(InstructionError::Immutable); } - if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { + if lookup_table.meta.authority != Some(authority_key) { return Err(InstructionError::IncorrectAuthority); } if lookup_table.meta.deactivation_slot != Slot::MAX { @@ -199,13 +206,9 @@ impl Processor { } let mut lookup_table_meta = lookup_table.meta; - drop(lookup_table_account_ref); - lookup_table_meta.authority = None; AddressLookupTable::overwrite_meta_data( - lookup_table_account - .try_account_ref_mut()? - .data_as_mut_slice(), + lookup_table_account.get_data_mut(), lookup_table_meta, )?; @@ -214,33 +217,39 @@ impl Processor { fn extend_lookup_table( invoke_context: &mut InvokeContext, - first_instruction_account: usize, + _first_instruction_account: usize, new_addresses: Vec, ) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; - let _instruction_context = transaction_context.get_current_instruction_context()?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; + let instruction_context = transaction_context.get_current_instruction_context()?; let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; - if lookup_table_account.owner()? != crate::id() { + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let table_key = *lookup_table_account.get_key(); + if *lookup_table_account.get_owner() != crate::id() { return Err(InstructionError::InvalidAccountOwner); } + drop(lookup_table_account); let authority_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; - if authority_account.signer_key().is_none() { + instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + let authority_key = *authority_account.get_key(); + if !authority_account.is_signer() { + ic_msg!(invoke_context, "Authority account must be a signer"); return Err(InstructionError::MissingRequiredSignature); } + drop(authority_account); - let lookup_table_account_ref = lookup_table_account.try_account_ref()?; - let lookup_table_data = lookup_table_account_ref.data(); + let mut lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let lookup_table_data = lookup_table_account.get_data(); + let lookup_table_lamports = lookup_table_account.get_lamports(); let mut lookup_table = AddressLookupTable::deserialize(lookup_table_data)?; if lookup_table.meta.authority.is_none() { return Err(InstructionError::Immutable); } - if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { + if lookup_table.meta.authority != Some(authority_key) { return Err(InstructionError::IncorrectAuthority); } if lookup_table.meta.deactivation_slot != Slot::MAX { @@ -286,42 +295,36 @@ impl Processor { } let lookup_table_meta = lookup_table.meta; - drop(lookup_table_account_ref); - let new_table_data_len = checked_add( LOOKUP_TABLE_META_SIZE, new_table_addresses_len.saturating_mul(PUBKEY_BYTES), )?; { - let mut lookup_table_account_ref_mut = lookup_table_account.try_account_ref_mut()?; - AddressLookupTable::overwrite_meta_data( - lookup_table_account_ref_mut.data_as_mut_slice(), - lookup_table_meta, - )?; - - let table_data = lookup_table_account_ref_mut.data_mut(); + let mut table_data = lookup_table_account.get_data_mut().to_vec(); + AddressLookupTable::overwrite_meta_data(&mut table_data, lookup_table_meta)?; for new_address in new_addresses { table_data.extend_from_slice(new_address.as_ref()); } + lookup_table_account.set_data(&table_data); } + drop(lookup_table_account); let rent = invoke_context.get_sysvar_cache().get_rent()?; let required_lamports = rent .minimum_balance(new_table_data_len) .max(1) - .saturating_sub(lookup_table_account.lamports()?); + .saturating_sub(lookup_table_lamports); - let table_key = *lookup_table_account.unsigned_key(); if required_lamports > 0 { let payer_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 2)?)?; - let payer_key = if let Some(payer_key) = payer_account.signer_key() { - *payer_key - } else { + instruction_context.try_borrow_instruction_account(transaction_context, 2)?; + let payer_key = *payer_account.get_key(); + if !payer_account.is_signer() { ic_msg!(invoke_context, "Payer account must be a signer"); return Err(InstructionError::MissingRequiredSignature); - }; + } + drop(payer_account); invoke_context.native_invoke( system_instruction::transfer(&payer_key, &table_key, required_lamports), @@ -334,33 +337,37 @@ impl Processor { fn deactivate_lookup_table( invoke_context: &mut InvokeContext, - first_instruction_account: usize, + _first_instruction_account: usize, ) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; - let _instruction_context = transaction_context.get_current_instruction_context()?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; + let instruction_context = transaction_context.get_current_instruction_context()?; let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; - if lookup_table_account.owner()? != crate::id() { + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + if *lookup_table_account.get_owner() != crate::id() { return Err(InstructionError::InvalidAccountOwner); } + drop(lookup_table_account); let authority_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; - if authority_account.signer_key().is_none() { + instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + let authority_key = *authority_account.get_key(); + if !authority_account.is_signer() { + ic_msg!(invoke_context, "Authority account must be a signer"); return Err(InstructionError::MissingRequiredSignature); } + drop(authority_account); - let lookup_table_account_ref = lookup_table_account.try_account_ref()?; - let lookup_table_data = lookup_table_account_ref.data(); + let mut lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let lookup_table_data = lookup_table_account.get_data(); let lookup_table = AddressLookupTable::deserialize(lookup_table_data)?; if lookup_table.meta.authority.is_none() { ic_msg!(invoke_context, "Lookup table is frozen"); return Err(InstructionError::Immutable); } - if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { + if lookup_table.meta.authority != Some(authority_key) { return Err(InstructionError::IncorrectAuthority); } if lookup_table.meta.deactivation_slot != Slot::MAX { @@ -369,15 +376,11 @@ impl Processor { } let mut lookup_table_meta = lookup_table.meta; - drop(lookup_table_account_ref); - let clock = invoke_context.get_sysvar_cache().get_clock()?; lookup_table_meta.deactivation_slot = clock.slot; AddressLookupTable::overwrite_meta_data( - lookup_table_account - .try_account_ref_mut()? - .data_as_mut_slice(), + lookup_table_account.get_data_mut(), lookup_table_meta, )?; @@ -386,27 +389,36 @@ impl Processor { fn close_lookup_table( invoke_context: &mut InvokeContext, - first_instruction_account: usize, + _first_instruction_account: usize, ) -> Result<(), InstructionError> { let transaction_context = &invoke_context.transaction_context; - let _instruction_context = transaction_context.get_current_instruction_context()?; - let keyed_accounts = invoke_context.get_keyed_accounts()?; + let instruction_context = transaction_context.get_current_instruction_context()?; let lookup_table_account = - keyed_account_at_index(keyed_accounts, first_instruction_account)?; - if lookup_table_account.owner()? != crate::id() { + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + if *lookup_table_account.get_owner() != crate::id() { return Err(InstructionError::InvalidAccountOwner); } + drop(lookup_table_account); let authority_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; - if authority_account.signer_key().is_none() { + instruction_context.try_borrow_instruction_account(transaction_context, 1)?; + let authority_key = *authority_account.get_key(); + if !authority_account.is_signer() { + ic_msg!(invoke_context, "Authority account must be a signer"); return Err(InstructionError::MissingRequiredSignature); } + drop(authority_account); - let recipient_account = - keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 2)?)?; - if recipient_account.unsigned_key() == lookup_table_account.unsigned_key() { + instruction_context.check_number_of_instruction_accounts(3)?; + if instruction_context + .get_index_in_transaction(instruction_context.get_number_of_program_accounts())? + == instruction_context.get_index_in_transaction( + instruction_context + .get_number_of_program_accounts() + .saturating_add(2), + )? + { ic_msg!( invoke_context, "Lookup table cannot be the recipient of reclaimed lamports" @@ -414,15 +426,17 @@ impl Processor { return Err(InstructionError::InvalidArgument); } - let lookup_table_account_ref = lookup_table_account.try_account_ref()?; - let lookup_table_data = lookup_table_account_ref.data(); + let lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + let withdrawn_lamports = lookup_table_account.get_lamports(); + let lookup_table_data = lookup_table_account.get_data(); let lookup_table = AddressLookupTable::deserialize(lookup_table_data)?; if lookup_table.meta.authority.is_none() { ic_msg!(invoke_context, "Lookup table is frozen"); return Err(InstructionError::Immutable); } - if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { + if lookup_table.meta.authority != Some(authority_key) { return Err(InstructionError::IncorrectAuthority); } @@ -445,16 +459,16 @@ impl Processor { } LookupTableStatus::Deactivated => Ok(()), }?; + drop(lookup_table_account); - drop(lookup_table_account_ref); + let mut recipient_account = + instruction_context.try_borrow_instruction_account(transaction_context, 2)?; + recipient_account.checked_add_lamports(withdrawn_lamports)?; + drop(recipient_account); - let withdrawn_lamports = lookup_table_account.lamports()?; - recipient_account - .try_account_ref_mut()? - .checked_add_lamports(withdrawn_lamports)?; - - let mut lookup_table_account = lookup_table_account.try_account_ref_mut()?; - lookup_table_account.set_data(Vec::new()); + let mut lookup_table_account = + instruction_context.try_borrow_instruction_account(transaction_context, 0)?; + lookup_table_account.set_data(&[]); lookup_table_account.set_lamports(0); Ok(())