From 27a5ba838de3aba3f909d1d15dfd80992aae965b Mon Sep 17 00:00:00 2001 From: Joe C Date: Mon, 25 Nov 2024 21:47:24 +0900 Subject: [PATCH] feature cleanup: address lookup table program authority checks (#3540) --- cli/src/address_lookup_table.rs | 57 +++----------- cli/tests/address_lookup_table.rs | 2 - .../tests/create_lookup_table_ix.rs | 77 +------------------ .../address-lookup-table/src/processor.rs | 23 +----- .../src/address_lookup_table/instruction.rs | 1 + 5 files changed, 14 insertions(+), 146 deletions(-) diff --git a/cli/src/address_lookup_table.rs b/cli/src/address_lookup_table.rs index 74ff99b7f7ed18..2ef415228df628 100644 --- a/cli/src/address_lookup_table.rs +++ b/cli/src/address_lookup_table.rs @@ -11,8 +11,8 @@ use { address_lookup_table::{ self, instruction::{ - close_lookup_table, create_lookup_table, create_lookup_table_signed, - deactivate_lookup_table, extend_lookup_table, freeze_lookup_table, + close_lookup_table, create_lookup_table, deactivate_lookup_table, + extend_lookup_table, freeze_lookup_table, }, state::AddressLookupTable, }, @@ -31,7 +31,6 @@ use { pub enum AddressLookupTableCliCommand { CreateLookupTable { authority_pubkey: Pubkey, - authority_signer_index: Option, payer_signer_index: SignerIndex, }, FreezeLookupTable { @@ -76,26 +75,12 @@ impl AddressLookupTableSubCommands for App<'_, '_> { .arg( Arg::with_name("authority") .long("authority") + .alias("authority-signer") .value_name("AUTHORITY_PUBKEY") .takes_value(true) - .validator(is_pubkey) + .validator(is_pubkey_or_keypair) .help( "Lookup table authority address \ - [default: the default configured keypair]. \ - WARNING: Cannot be used for creating a lookup table for \ - a cluster running v1.11 or earlier which requires the \ - authority to sign for lookup table creation.", - ), - ) - .arg( - Arg::with_name("authority_signer") - .long("authority-signer") - .value_name("AUTHORITY_SIGNER") - .takes_value(true) - .conflicts_with("authority") - .validator(is_valid_signer) - .help( - "Lookup table authority keypair \ [default: the default configured keypair].", ), ) @@ -278,12 +263,7 @@ pub fn parse_address_lookup_table_subcommand( default_signer.signer_from_path(matches, wallet_manager)?, )]; - let authority_pubkey = if let Ok((authority_signer, Some(authority_pubkey))) = - signer_of(matches, "authority_signer", wallet_manager) - { - bulk_signers.push(authority_signer); - authority_pubkey - } else if let Some(authority_pubkey) = pubkey_of(matches, "authority") { + let authority_pubkey = if let Some(authority_pubkey) = pubkey_of(matches, "authority") { authority_pubkey } else { default_signer @@ -311,7 +291,6 @@ pub fn parse_address_lookup_table_subcommand( command: CliCommand::AddressLookupTable( AddressLookupTableCliCommand::CreateLookupTable { authority_pubkey, - authority_signer_index: signer_info.index_of(Some(authority_pubkey)), payer_signer_index: signer_info.index_of(payer_pubkey).unwrap(), }, ), @@ -500,15 +479,10 @@ pub fn process_address_lookup_table_subcommand( match subcommand { AddressLookupTableCliCommand::CreateLookupTable { authority_pubkey, - authority_signer_index, payer_signer_index, - } => process_create_lookup_table( - &rpc_client, - config, - *authority_pubkey, - *authority_signer_index, - *payer_signer_index, - ), + } => { + process_create_lookup_table(&rpc_client, config, *authority_pubkey, *payer_signer_index) + } AddressLookupTableCliCommand::FreezeLookupTable { lookup_table_pubkey, authority_signer_index, @@ -565,10 +539,8 @@ fn process_create_lookup_table( rpc_client: &RpcClient, config: &CliConfig, authority_address: Pubkey, - authority_signer_index: Option, payer_signer_index: usize, ) -> ProcessResult { - let authority_signer = authority_signer_index.map(|index| config.signers[index]); let payer_signer = config.signers[payer_signer_index]; let get_clock_result = rpc_client @@ -579,11 +551,8 @@ fn process_create_lookup_table( })?; let payer_address = payer_signer.pubkey(); - let (create_lookup_table_ix, lookup_table_address) = if authority_signer.is_some() { - create_lookup_table_signed(authority_address, payer_address, clock.slot) - } else { - create_lookup_table(authority_address, payer_address, clock.slot) - }; + let (create_lookup_table_ix, lookup_table_address) = + create_lookup_table(authority_address, payer_address, clock.slot); let blockhash = rpc_client.get_latest_blockhash()?; let mut tx = Transaction::new_unsigned(Message::new( @@ -591,11 +560,7 @@ fn process_create_lookup_table( Some(&config.signers[0].pubkey()), )); - let mut keypairs: Vec<&dyn Signer> = vec![config.signers[0], payer_signer]; - if let Some(authority_signer) = authority_signer { - keypairs.push(authority_signer); - } - + let keypairs: Vec<&dyn Signer> = vec![config.signers[0], payer_signer]; tx.try_sign(&keypairs, blockhash)?; let result = rpc_client.send_and_confirm_transaction_with_spinner_and_config( &tx, diff --git a/cli/tests/address_lookup_table.rs b/cli/tests/address_lookup_table.rs index 9bf7db320cb010..e2f5802fa43e64 100644 --- a/cli/tests/address_lookup_table.rs +++ b/cli/tests/address_lookup_table.rs @@ -43,7 +43,6 @@ fn test_cli_create_extend_and_freeze_address_lookup_table() { config.command = CliCommand::AddressLookupTable(AddressLookupTableCliCommand::CreateLookupTable { authority_pubkey: keypair.pubkey(), - authority_signer_index: None, payer_signer_index: 0, }); let response: CliAddressLookupTableCreated = @@ -158,7 +157,6 @@ fn test_cli_create_and_deactivate_address_lookup_table() { config.command = CliCommand::AddressLookupTable(AddressLookupTableCliCommand::CreateLookupTable { authority_pubkey: keypair.pubkey(), - authority_signer_index: Some(0), payer_signer_index: 0, }); let response: CliAddressLookupTableCreated = 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 5a640448fb16d2..8ab6811c303a05 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 @@ -4,7 +4,7 @@ use { solana_program_test::*, solana_sdk::{ address_lookup_table::{ - instruction::{create_lookup_table, create_lookup_table_signed}, + instruction::create_lookup_table, program::id, state::{AddressLookupTable, LOOKUP_TABLE_META_SIZE}, }, @@ -13,25 +13,12 @@ use { pubkey::Pubkey, rent::Rent, signature::Signer, - signer::keypair::Keypair, transaction::Transaction, }, }; mod common; -pub async fn setup_test_context_without_authority_feature() -> ProgramTestContext { - let mut program_test = ProgramTest::new( - "", - id(), - Some(solana_address_lookup_table_program::processor::Entrypoint::vm), - ); - program_test.deactivate_feature( - solana_feature_set::relax_authority_signer_check_for_lookup_table_creation::id(), - ); - program_test.start_with_context().await -} - #[tokio::test] async fn test_create_lookup_table_idempotent() { let mut context = setup_test_context().await; @@ -92,46 +79,6 @@ async fn test_create_lookup_table_idempotent() { } } -#[tokio::test] -async fn test_create_lookup_table_not_idempotent() { - let mut context = setup_test_context_without_authority_feature().await; - - let test_recent_slot = 123; - overwrite_slot_hashes_with_slots(&context, &[test_recent_slot]); - - let client = &mut context.banks_client; - let payer = &context.payer; - let recent_blockhash = context.last_blockhash; - let authority_keypair = Keypair::new(); - let authority_address = authority_keypair.pubkey(); - let (create_lookup_table_ix, ..) = - create_lookup_table_signed(authority_address, payer.pubkey(), test_recent_slot); - - let transaction = Transaction::new_signed_with_payer( - &[create_lookup_table_ix.clone()], - Some(&payer.pubkey()), - &[payer, &authority_keypair], - recent_blockhash, - ); - - assert_matches!(client.process_transaction(transaction).await, Ok(())); - - // Second create should fail - { - context.last_blockhash = client - .get_new_latest_blockhash(&recent_blockhash) - .await - .unwrap(); - assert_ix_error( - &mut context, - create_lookup_table_ix, - Some(&authority_keypair), - InstructionError::AccountAlreadyInitialized, - ) - .await; - } -} - #[tokio::test] async fn test_create_lookup_table_use_payer_as_authority() { let mut context = setup_test_context().await; @@ -153,28 +100,6 @@ async fn test_create_lookup_table_use_payer_as_authority() { assert_matches!(client.process_transaction(transaction).await, Ok(())); } -#[tokio::test] -async fn test_create_lookup_table_missing_signer() { - let mut context = setup_test_context_without_authority_feature().await; - let unsigned_authority_address = Pubkey::new_unique(); - - let mut ix = create_lookup_table_signed( - unsigned_authority_address, - context.payer.pubkey(), - Slot::MAX, - ) - .0; - ix.accounts[1].is_signer = false; - - assert_ix_error( - &mut context, - ix, - None, - InstructionError::MissingRequiredSignature, - ) - .await; -} - #[tokio::test] async fn test_create_lookup_table_not_recent_slot() { let mut context = setup_test_context().await; diff --git a/programs/address-lookup-table/src/processor.rs b/programs/address-lookup-table/src/processor.rs index 0d8abe8bd2e260..fd80e370d2f92b 100644 --- a/programs/address-lookup-table/src/processor.rs +++ b/programs/address-lookup-table/src/processor.rs @@ -1,5 +1,4 @@ use { - solana_feature_set as feature_set, solana_log_collector::ic_msg, solana_program_runtime::{declare_process_instruction, invoke_context::InvokeContext}, solana_sdk::{ @@ -61,27 +60,11 @@ impl Processor { let lookup_table_lamports = lookup_table_account.get_lamports(); let table_key = *lookup_table_account.get_key(); let lookup_table_owner = *lookup_table_account.get_owner(); - if !invoke_context - .get_feature_set() - .is_active(&feature_set::relax_authority_signer_check_for_lookup_table_creation::id()) - && !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 = instruction_context.try_borrow_instruction_account(transaction_context, 1)?; let authority_key = *authority_account.get_key(); - if !invoke_context - .get_feature_set() - .is_active(&feature_set::relax_authority_signer_check_for_lookup_table_creation::id()) - && !authority_account.is_signer() - { - ic_msg!(invoke_context, "Authority account must be a signer"); - return Err(InstructionError::MissingRequiredSignature); - } drop(authority_account); let payer_account = @@ -127,11 +110,7 @@ impl Processor { return Err(InstructionError::InvalidArgument); } - if invoke_context - .get_feature_set() - .is_active(&feature_set::relax_authority_signer_check_for_lookup_table_creation::id()) - && check_id(&lookup_table_owner) - { + if check_id(&lookup_table_owner) { return Ok(()); } diff --git a/sdk/program/src/address_lookup_table/instruction.rs b/sdk/program/src/address_lookup_table/instruction.rs index 4d73060e046ec3..8fa104d0a71acf 100644 --- a/sdk/program/src/address_lookup_table/instruction.rs +++ b/sdk/program/src/address_lookup_table/instruction.rs @@ -113,6 +113,7 @@ fn create_lookup_table_common( /// This instruction requires the authority to be a signer but /// in v1.12 the address lookup table program will no longer require /// the authority to sign the transaction. +#[deprecated(since = "2.2.0", note = "use `create_lookup_table` instead")] pub fn create_lookup_table_signed( authority_address: Pubkey, payer_address: Pubkey,