From f5d1115468a4fc55cef39cdeb65c02a3550fe403 Mon Sep 17 00:00:00 2001 From: Justin Starry Date: Mon, 20 Dec 2021 17:33:46 -0600 Subject: [PATCH] Add deactivation cooldown before address lookup tables can be closed (#22011) --- .../tests/close_lookup_table_ix.rs | 44 ++++- .../tests/create_lookup_table_ix.rs | 2 +- .../tests/deactivate_lookup_table_ix.rs | 145 +++++++++++++++++ .../tests/extend_lookup_table_ix.rs | 154 ++++++++++++------ .../tests/freeze_lookup_table_ix.rs | 24 +++ .../address-lookup-table/src/instruction.rs | 27 ++- .../address-lookup-table/src/processor.rs | 76 ++++++++- programs/address-lookup-table/src/state.rs | 24 ++- 8 files changed, 427 insertions(+), 69 deletions(-) create mode 100644 programs/address-lookup-table-tests/tests/deactivate_lookup_table_ix.rs diff --git a/programs/address-lookup-table-tests/tests/close_lookup_table_ix.rs b/programs/address-lookup-table-tests/tests/close_lookup_table_ix.rs index 0ffc88455e28f2..71a369c5ce59ee 100644 --- a/programs/address-lookup-table-tests/tests/close_lookup_table_ix.rs +++ b/programs/address-lookup-table-tests/tests/close_lookup_table_ix.rs @@ -21,9 +21,13 @@ async fn test_close_lookup_table() { let mut context = setup_test_context().await; overwrite_slot_hashes_with_slots(&mut context, &[]); - let authority_keypair = Keypair::new(); - let initialized_table = new_address_lookup_table(Some(authority_keypair.pubkey()), 0); let lookup_table_address = Pubkey::new_unique(); + let authority_keypair = Keypair::new(); + let initialized_table = { + let mut table = new_address_lookup_table(Some(authority_keypair.pubkey()), 0); + table.meta.deactivation_slot = 0; + table + }; add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; let client = &mut context.banks_client; @@ -49,7 +53,7 @@ async fn test_close_lookup_table() { } #[tokio::test] -async fn test_close_lookup_table_too_recent() { +async fn test_close_lookup_table_not_deactivated() { let mut context = setup_test_context().await; let authority_keypair = Keypair::new(); @@ -63,10 +67,38 @@ async fn test_close_lookup_table_too_recent() { context.payer.pubkey(), ); + // The ix should fail because the table hasn't been deactivated yet + assert_ix_error( + &mut context, + ix, + Some(&authority_keypair), + InstructionError::InvalidArgument, + ) + .await; +} + +#[tokio::test] +async fn test_close_lookup_table_recently_deactivated() { + let mut context = setup_test_context().await; + + let authority_keypair = Keypair::new(); + let initialized_table = { + let mut table = new_address_lookup_table(Some(authority_keypair.pubkey()), 0); + table.meta.deactivation_slot = 0; + table + }; + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let ix = close_lookup_table( + lookup_table_address, + authority_keypair.pubkey(), + context.payer.pubkey(), + ); + // Context sets up the slot hashes sysvar to have an entry - // for slot 0 which is what the default initialized table - // has as its derivation slot. Because that slot is present, - // the ix should fail. + // for slot 0 which is when the table was deactivated. + // Because that slot is present, the ix should fail. assert_ix_error( &mut context, ix, diff --git a/programs/address-lookup-table-tests/tests/create_lookup_table_ix.rs b/programs/address-lookup-table-tests/tests/create_lookup_table_ix.rs index 7f4da6f279dddf..b91a318028e1df 100644 --- a/programs/address-lookup-table-tests/tests/create_lookup_table_ix.rs +++ b/programs/address-lookup-table-tests/tests/create_lookup_table_ix.rs @@ -52,7 +52,7 @@ async fn test_create_lookup_table() { Rent::default().minimum_balance(LOOKUP_TABLE_META_SIZE) ); let lookup_table = AddressLookupTable::deserialize(&lookup_table_account.data).unwrap(); - assert_eq!(lookup_table.meta.derivation_slot, test_recent_slot); + assert_eq!(lookup_table.meta.deactivation_slot, Slot::MAX); assert_eq!(lookup_table.meta.authority, Some(authority_address)); assert_eq!(lookup_table.meta.last_extended_slot, 0); assert_eq!(lookup_table.meta.last_extended_slot_start_index, 0); diff --git a/programs/address-lookup-table-tests/tests/deactivate_lookup_table_ix.rs b/programs/address-lookup-table-tests/tests/deactivate_lookup_table_ix.rs new file mode 100644 index 00000000000000..81050aca123150 --- /dev/null +++ b/programs/address-lookup-table-tests/tests/deactivate_lookup_table_ix.rs @@ -0,0 +1,145 @@ +use { + assert_matches::assert_matches, + common::{ + add_lookup_table_account, assert_ix_error, new_address_lookup_table, setup_test_context, + }, + solana_address_lookup_table_program::{ + instruction::deactivate_lookup_table, state::AddressLookupTable, + }, + solana_program_test::*, + solana_sdk::{ + instruction::InstructionError, + pubkey::Pubkey, + signature::{Keypair, Signer}, + transaction::Transaction, + }, +}; + +mod common; + +#[tokio::test] +async fn test_deactivate_lookup_table() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let mut initialized_table = new_address_lookup_table(Some(authority.pubkey()), 10); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account( + &mut context, + lookup_table_address, + initialized_table.clone(), + ) + .await; + + let client = &mut context.banks_client; + let payer = &context.payer; + let recent_blockhash = context.last_blockhash; + let transaction = Transaction::new_signed_with_payer( + &[deactivate_lookup_table( + lookup_table_address, + authority.pubkey(), + )], + Some(&payer.pubkey()), + &[payer, &authority], + recent_blockhash, + ); + + assert_matches!(client.process_transaction(transaction).await, Ok(())); + let table_account = client + .get_account(lookup_table_address) + .await + .unwrap() + .unwrap(); + let lookup_table = AddressLookupTable::deserialize(&table_account.data).unwrap(); + assert_eq!(lookup_table.meta.deactivation_slot, 1); + + // Check that only the deactivation slot changed + initialized_table.meta.deactivation_slot = 1; + assert_eq!(initialized_table, lookup_table); +} + +#[tokio::test] +async fn test_deactivate_immutable_lookup_table() { + let mut context = setup_test_context().await; + + let initialized_table = new_address_lookup_table(None, 10); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let authority = Keypair::new(); + let ix = deactivate_lookup_table(lookup_table_address, authority.pubkey()); + + assert_ix_error( + &mut context, + ix, + Some(&authority), + InstructionError::Immutable, + ) + .await; +} + +#[tokio::test] +async fn test_deactivate_already_deactivated() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let initialized_table = { + let mut table = new_address_lookup_table(Some(authority.pubkey()), 0); + table.meta.deactivation_slot = 0; + table + }; + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let ix = deactivate_lookup_table(lookup_table_address, authority.pubkey()); + + assert_ix_error( + &mut context, + ix, + Some(&authority), + InstructionError::InvalidArgument, + ) + .await; +} + +#[tokio::test] +async fn test_deactivate_lookup_table_with_wrong_authority() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let wrong_authority = Keypair::new(); + let initialized_table = new_address_lookup_table(Some(authority.pubkey()), 10); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let ix = deactivate_lookup_table(lookup_table_address, wrong_authority.pubkey()); + + assert_ix_error( + &mut context, + ix, + Some(&wrong_authority), + InstructionError::IncorrectAuthority, + ) + .await; +} + +#[tokio::test] +async fn test_deactivate_lookup_table_without_signing() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let initialized_table = new_address_lookup_table(Some(authority.pubkey()), 10); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let mut ix = deactivate_lookup_table(lookup_table_address, authority.pubkey()); + ix.accounts[1].is_signer = false; + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::MissingRequiredSignature, + ) + .await; +} diff --git a/programs/address-lookup-table-tests/tests/extend_lookup_table_ix.rs b/programs/address-lookup-table-tests/tests/extend_lookup_table_ix.rs index ffed5c619f66a0..576895a0c44a44 100644 --- a/programs/address-lookup-table-tests/tests/extend_lookup_table_ix.rs +++ b/programs/address-lookup-table-tests/tests/extend_lookup_table_ix.rs @@ -1,6 +1,8 @@ use { assert_matches::assert_matches, - common::{add_lookup_table_account, new_address_lookup_table, setup_test_context}, + common::{ + add_lookup_table_account, assert_ix_error, new_address_lookup_table, setup_test_context, + }, solana_address_lookup_table_program::{ instruction::extend_lookup_table, state::{AddressLookupTable, LookupTableMeta}, @@ -130,7 +132,7 @@ async fn test_extend_lookup_table() { } else { num_existing_addresses as u8 }, - derivation_slot: lookup_table.meta.derivation_slot, + deactivation_slot: lookup_table.meta.deactivation_slot, authority: lookup_table.meta.authority, _padding: 0u16, }, @@ -156,59 +158,111 @@ async fn test_extend_lookup_table() { } #[tokio::test] -async fn test_extend_addresses_authority_errors() { +async fn test_extend_lookup_table_with_wrong_authority() { let mut context = setup_test_context().await; + let authority = Keypair::new(); + let wrong_authority = Keypair::new(); + let initialized_table = new_address_lookup_table(Some(authority.pubkey()), 0); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; - for (existing_authority, ix_authority, use_signer, expected_err) in [ - ( - Some(authority.pubkey()), - Keypair::new(), - true, - InstructionError::IncorrectAuthority, - ), - ( - Some(authority.pubkey()), - authority, - false, - InstructionError::MissingRequiredSignature, - ), - (None, Keypair::new(), true, InstructionError::Immutable), - ] { - let lookup_table = new_address_lookup_table(existing_authority, 0); - let lookup_table_address = Pubkey::new_unique(); - let _ = add_lookup_table_account(&mut context, lookup_table_address, lookup_table.clone()) - .await; - - let num_new_addresses = 1; - let mut new_addresses = Vec::with_capacity(num_new_addresses); - new_addresses.resize_with(num_new_addresses, Pubkey::new_unique); - let mut instruction = extend_lookup_table( - lookup_table_address, - ix_authority.pubkey(), - context.payer.pubkey(), - new_addresses.clone(), - ); - if !use_signer { - instruction.accounts[1].is_signer = false; - } + let new_addresses = vec![Pubkey::new_unique()]; + let ix = extend_lookup_table( + lookup_table_address, + wrong_authority.pubkey(), + context.payer.pubkey(), + new_addresses, + ); - let mut expected_addresses: Vec = lookup_table.addresses.to_vec(); - expected_addresses.extend(new_addresses); + assert_ix_error( + &mut context, + ix, + Some(&wrong_authority), + InstructionError::IncorrectAuthority, + ) + .await; +} - let extra_signer = if use_signer { - Some(&ix_authority) - } else { - None - }; +#[tokio::test] +async fn test_extend_lookup_table_without_signing() { + let mut context = setup_test_context().await; - let test_case = TestCase { - lookup_table_address, - instruction, - extra_signer, - expected_result: Err(expected_err), - }; + let authority = Keypair::new(); + let initialized_table = new_address_lookup_table(Some(authority.pubkey()), 10); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; - run_test_case(&mut context, test_case).await; - } + let new_addresses = vec![Pubkey::new_unique()]; + let mut ix = extend_lookup_table( + lookup_table_address, + authority.pubkey(), + context.payer.pubkey(), + new_addresses, + ); + ix.accounts[1].is_signer = false; + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::MissingRequiredSignature, + ) + .await; +} + +#[tokio::test] +async fn test_extend_deactivated_lookup_table() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let initialized_table = { + let mut table = new_address_lookup_table(Some(authority.pubkey()), 0); + table.meta.deactivation_slot = 0; + table + }; + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let new_addresses = vec![Pubkey::new_unique()]; + let ix = extend_lookup_table( + lookup_table_address, + authority.pubkey(), + context.payer.pubkey(), + new_addresses, + ); + + assert_ix_error( + &mut context, + ix, + Some(&authority), + InstructionError::InvalidArgument, + ) + .await; +} + +#[tokio::test] +async fn test_extend_immutable_lookup_table() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let initialized_table = new_address_lookup_table(None, 1); + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let new_addresses = vec![Pubkey::new_unique()]; + let ix = extend_lookup_table( + lookup_table_address, + authority.pubkey(), + context.payer.pubkey(), + new_addresses, + ); + + assert_ix_error( + &mut context, + ix, + Some(&authority), + InstructionError::Immutable, + ) + .await; } diff --git a/programs/address-lookup-table-tests/tests/freeze_lookup_table_ix.rs b/programs/address-lookup-table-tests/tests/freeze_lookup_table_ix.rs index acb638a39fb4ab..bb169fda293b0f 100644 --- a/programs/address-lookup-table-tests/tests/freeze_lookup_table_ix.rs +++ b/programs/address-lookup-table-tests/tests/freeze_lookup_table_ix.rs @@ -78,6 +78,30 @@ async fn test_freeze_immutable_lookup_table() { .await; } +#[tokio::test] +async fn test_freeze_deactivated_lookup_table() { + let mut context = setup_test_context().await; + + let authority = Keypair::new(); + let initialized_table = { + let mut table = new_address_lookup_table(Some(authority.pubkey()), 10); + table.meta.deactivation_slot = 0; + table + }; + let lookup_table_address = Pubkey::new_unique(); + add_lookup_table_account(&mut context, lookup_table_address, initialized_table).await; + + let ix = freeze_lookup_table(lookup_table_address, authority.pubkey()); + + assert_ix_error( + &mut context, + ix, + Some(&authority), + InstructionError::InvalidArgument, + ) + .await; +} + #[tokio::test] async fn test_freeze_lookup_table_with_wrong_authority() { let mut context = setup_test_context().await; diff --git a/programs/address-lookup-table/src/instruction.rs b/programs/address-lookup-table/src/instruction.rs index 6ba3dfe808bd46..5fca13b290e828 100644 --- a/programs/address-lookup-table/src/instruction.rs +++ b/programs/address-lookup-table/src/instruction.rs @@ -31,7 +31,7 @@ pub enum ProgramInstruction { bump_seed: u8, }, - /// Permanently freeze a address lookup table, making it immutable. + /// Permanently freeze an address lookup table, making it immutable. /// /// # Account references /// 0. `[WRITE]` Address lookup table account to freeze @@ -47,6 +47,14 @@ pub enum ProgramInstruction { /// 3. `[]` System program for CPI. ExtendLookupTable { new_addresses: Vec }, + /// Deactivate an address lookup table, making it unusable and + /// eligible for closure after a short period of time. + /// + /// # Account references + /// 0. `[WRITE]` Address lookup table account to deactivate + /// 1. `[SIGNER]` Current authority + DeactivateLookupTable, + /// Close an address lookup table account /// /// # Account references @@ -127,6 +135,23 @@ pub fn extend_lookup_table( ) } +/// Constructs an instruction that deactivates an address lookup +/// table so that it cannot be extended again and will be unusable +/// and eligible for closure after a short amount of time. +pub fn deactivate_lookup_table( + lookup_table_address: Pubkey, + authority_address: Pubkey, +) -> Instruction { + Instruction::new_with_bincode( + id(), + &ProgramInstruction::DeactivateLookupTable, + vec![ + AccountMeta::new(lookup_table_address, false), + AccountMeta::new_readonly(authority_address, true), + ], + ) +} + /// Returns an instruction that closes an address lookup table /// account. The account will be deallocated and the lamports /// will be drained to the recipient address. diff --git a/programs/address-lookup-table/src/processor.rs b/programs/address-lookup-table/src/processor.rs index ce2b9f161af8ae..824955d4641c00 100644 --- a/programs/address-lookup-table/src/processor.rs +++ b/programs/address-lookup-table/src/processor.rs @@ -47,6 +47,9 @@ pub fn process_instruction( ProgramInstruction::ExtendLookupTable { new_addresses } => { Processor::extend_lookup_table(invoke_context, first_instruction_account, new_addresses) } + ProgramInstruction::DeactivateLookupTable => { + Processor::deactivate_lookup_table(invoke_context, first_instruction_account) + } ProgramInstruction::CloseLookupTable => { Processor::close_lookup_table(invoke_context, first_instruction_account) } @@ -152,7 +155,6 @@ impl Processor { keyed_account_at_index(keyed_accounts, first_instruction_account)?; lookup_table_account.set_state(&ProgramState::LookupTable(LookupTableMeta::new( authority_key, - derivation_slot, )))?; Ok(()) @@ -187,6 +189,10 @@ impl Processor { if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { return Err(InstructionError::IncorrectAuthority); } + if lookup_table.meta.deactivation_slot != Slot::MAX { + ic_msg!(invoke_context, "Deactivated tables cannot be frozen"); + return Err(InstructionError::InvalidArgument); + } if lookup_table.addresses.is_empty() { ic_msg!(invoke_context, "Empty lookup tables cannot be frozen"); return Err(InstructionError::InvalidInstructionData); @@ -244,6 +250,10 @@ impl Processor { if lookup_table.meta.authority != Some(*authority_account.unsigned_key()) { return Err(InstructionError::IncorrectAuthority); } + if lookup_table.meta.deactivation_slot != Slot::MAX { + ic_msg!(invoke_context, "Deactivated tables cannot be extended"); + return Err(InstructionError::InvalidArgument); + } if lookup_table.addresses.len() >= LOOKUP_TABLE_MAX_ADDRESSES { ic_msg!( invoke_context, @@ -320,6 +330,56 @@ impl Processor { Ok(()) } + fn deactivate_lookup_table( + invoke_context: &mut InvokeContext, + first_instruction_account: usize, + ) -> Result<(), InstructionError> { + let keyed_accounts = invoke_context.get_keyed_accounts()?; + + let lookup_table_account = + keyed_account_at_index(keyed_accounts, first_instruction_account)?; + if lookup_table_account.owner()? != crate::id() { + return Err(InstructionError::InvalidAccountOwner); + } + + let authority_account = + keyed_account_at_index(keyed_accounts, checked_add(first_instruction_account, 1)?)?; + if authority_account.signer_key().is_none() { + return Err(InstructionError::MissingRequiredSignature); + } + + let lookup_table_account_ref = lookup_table_account.try_account_ref()?; + let lookup_table_data = lookup_table_account_ref.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()) { + return Err(InstructionError::IncorrectAuthority); + } + if lookup_table.meta.deactivation_slot != Slot::MAX { + ic_msg!(invoke_context, "Lookup table is already deactivated"); + return Err(InstructionError::InvalidArgument); + } + + let mut lookup_table_meta = lookup_table.meta; + drop(lookup_table_account_ref); + + let clock: Clock = invoke_context.get_sysvar(&clock::id())?; + lookup_table_meta.deactivation_slot = clock.slot; + + AddressLookupTable::overwrite_meta_data( + lookup_table_account + .try_account_ref_mut()? + .data_as_mut_slice(), + lookup_table_meta, + )?; + + Ok(()) + } + fn close_lookup_table( invoke_context: &mut InvokeContext, first_instruction_account: usize, @@ -353,16 +413,24 @@ impl Processor { 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()) { return Err(InstructionError::IncorrectAuthority); } + if lookup_table.meta.deactivation_slot == Slot::MAX { + ic_msg!(invoke_context, "Lookup table is not deactivated"); + return Err(InstructionError::InvalidArgument); + } - // Assert that the slot used in the derivation path of the lookup table address - // is no longer recent and can't be reused to initialize an account at the same address. + // Assert that the deactivation slot is no longer recent to give in-flight transactions + // enough time to land and to remove indeterminism caused by transactions loading + // addresses in the same slot when a table is closed. This enforced delay has a side + // effect of not allowing lookup tables to be recreated at the same derived address + // because tables must be created at an address derived from a recent slot. let slot_hashes: SlotHashes = invoke_context.get_sysvar(&slot_hashes::id())?; - if let Some(position) = slot_hashes.position(&lookup_table.meta.derivation_slot) { + if let Some(position) = slot_hashes.position(&lookup_table.meta.deactivation_slot) { let expiration = MAX_ENTRIES.saturating_sub(position); ic_msg!( invoke_context, diff --git a/programs/address-lookup-table/src/state.rs b/programs/address-lookup-table/src/state.rs index 906487962bebed..673ab111230521 100644 --- a/programs/address-lookup-table/src/state.rs +++ b/programs/address-lookup-table/src/state.rs @@ -22,12 +22,11 @@ pub enum ProgramState { } /// Address lookup table metadata -#[derive(Debug, Default, Serialize, Deserialize, PartialEq, Clone, AbiExample)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Clone, AbiExample)] pub struct LookupTableMeta { - /// The slot used to derive the table's address. The table cannot - /// be closed until the derivation slot is no longer "recent" - /// (not accessible in the `SlotHashes` sysvar). - pub derivation_slot: Slot, + /// Lookup tables cannot be closed until the deactivation slot is + /// no longer "recent" (not accessible in the `SlotHashes` sysvar). + pub deactivation_slot: Slot, /// The slot that the table was last extended. Address tables may /// only be used to lookup addresses that were extended before /// the current bank's slot. @@ -43,10 +42,21 @@ pub struct LookupTableMeta { // the account's data, starting from `LOOKUP_TABLE_META_SIZE`. } +impl Default for LookupTableMeta { + fn default() -> Self { + Self { + deactivation_slot: Slot::MAX, + last_extended_slot: 0, + last_extended_slot_start_index: 0, + authority: None, + _padding: 0, + } + } +} + impl LookupTableMeta { - pub fn new(authority: Pubkey, derivation_slot: Slot) -> Self { + pub fn new(authority: Pubkey) -> Self { LookupTableMeta { - derivation_slot, authority: Some(authority), ..LookupTableMeta::default() }