From c071dfdc5952cbc9c81aae2a57714741aea9137a Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Fri, 8 Nov 2024 09:28:20 +0000 Subject: [PATCH 1/3] program: add custom error code for ro writes --- Cargo.lock | 3 ++ program/Cargo.toml | 3 ++ program/src/entrypoint.rs | 13 +++++++-- program/src/error.rs | 37 ++++++++++++++++++++++++ program/src/lib.rs | 1 + program/src/processor.rs | 33 +++++++++++++++++++++ program/tests/extend_lookup_table_ix.rs | 38 +++++++++++++++++++++++++ 7 files changed, 125 insertions(+), 3 deletions(-) create mode 100644 program/src/error.rs diff --git a/Cargo.lock b/Cargo.lock index 2a7904b..75d6fd4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2719,6 +2719,8 @@ dependencies = [ "bytemuck", "mollusk-svm", "mollusk-svm-bencher", + "num-derive 0.4.2", + "num-traits", "serde", "solana-frozen-abi 2.0.1", "solana-frozen-abi-macro 2.0.1", @@ -2727,6 +2729,7 @@ dependencies = [ "strum", "strum_macros", "test-case", + "thiserror", ] [[package]] diff --git a/program/Cargo.toml b/program/Cargo.toml index b249005..f7cbf03 100644 --- a/program/Cargo.toml +++ b/program/Cargo.toml @@ -19,10 +19,13 @@ test-sbf = [] [dependencies] bincode = "1.3.3" bytemuck = "1.14.1" +num-derive = "0.4" +num-traits = "0.2" serde = { version = "1.0.193", features = ["derive"] } solana-frozen-abi = { version = "2.0.1", optional = true } solana-frozen-abi-macro = { version = "2.0.1", optional = true } solana-program = "2.0.1" +thiserror = "1.0.61" [dev-dependencies] mollusk-svm = { version = "0.0.5", features = ["fuzz"] } diff --git a/program/src/entrypoint.rs b/program/src/entrypoint.rs index f79f7f5..f9f16f8 100644 --- a/program/src/entrypoint.rs +++ b/program/src/entrypoint.rs @@ -1,8 +1,11 @@ //! Program entrypoint use { - crate::processor, - solana_program::{account_info::AccountInfo, entrypoint::ProgramResult, pubkey::Pubkey}, + crate::{error::AddressLookupTableError, processor}, + solana_program::{ + account_info::AccountInfo, entrypoint::ProgramResult, program_error::PrintProgramError, + pubkey::Pubkey, + }, }; solana_program::entrypoint!(process_instruction); @@ -11,5 +14,9 @@ fn process_instruction( accounts: &[AccountInfo], instruction_data: &[u8], ) -> ProgramResult { - processor::process(program_id, accounts, instruction_data) + if let Err(error) = processor::process(program_id, accounts, instruction_data) { + error.print::(); + return Err(error); + } + Ok(()) } diff --git a/program/src/error.rs b/program/src/error.rs new file mode 100644 index 0000000..9c84c91 --- /dev/null +++ b/program/src/error.rs @@ -0,0 +1,37 @@ +//! Program error types. + +use { + num_derive::FromPrimitive, + solana_program::{ + decode_error::DecodeError, + msg, + program_error::{PrintProgramError, ProgramError}, + }, + thiserror::Error, +}; + +/// Errors that can be returned by the Config program. +#[derive(Error, Clone, Debug, Eq, PartialEq, FromPrimitive)] +pub enum AddressLookupTableError { + /// Instruction modified data of a read-only account. + #[error("Instruction modified data of a read-only account")] + ReadonlyDataModified = 10, +} + +impl PrintProgramError for AddressLookupTableError { + fn print(&self) { + msg!(&self.to_string()); + } +} + +impl From for ProgramError { + fn from(e: AddressLookupTableError) -> Self { + ProgramError::Custom(e as u32) + } +} + +impl DecodeError for AddressLookupTableError { + fn type_of() -> &'static str { + "AddressLookupTableError" + } +} diff --git a/program/src/lib.rs b/program/src/lib.rs index 6bb092a..bae8b5f 100644 --- a/program/src/lib.rs +++ b/program/src/lib.rs @@ -3,6 +3,7 @@ #[cfg(all(target_os = "solana", feature = "bpf-entrypoint"))] mod entrypoint; +pub mod error; pub mod instruction; pub mod processor; pub mod state; diff --git a/program/src/processor.rs b/program/src/processor.rs index 2510439..14234ec 100644 --- a/program/src/processor.rs +++ b/program/src/processor.rs @@ -3,6 +3,7 @@ use { crate::{ check_id, + error::AddressLookupTableError, instruction::AddressLookupTableInstruction, state::{ AddressLookupTable, ProgramState, LOOKUP_TABLE_MAX_ADDRESSES, LOOKUP_TABLE_META_SIZE, @@ -342,6 +343,37 @@ fn process_extend_lookup_table( ) }; + // [Core BPF]: + // When a builtin program attempts to write to an executable or read-only + // account, it will be immediately rejected by the `TransactionContext`. + // For more information, see https://github.com/solana-program/config/pull/21. + // + // However, in the case of the Address Lookup Table program's + // `ExtendLookupTable` instruction, since the processor rejects any + // zero-length "new keys" vectors, and will gladly append the same keys + // again to the table, the issue here is slightly different than the linked + // PR. + // + // The builtin version of the Address Lookup Table program will throw + // when it attempts to overwrite the metadata, while the BPF version will + // continue. In the case where an executable or read-only lookup table + // account is provided, and some other requirement below is violated + // (ie. no payer or system program accounts provided, payer is not a + // signer, payer has insufficent balance, etc.), the BPF version will throw + // based on one of those violations, rather than throwing immediately when + // it encounters the executable or read-only lookup table account. + // + // In order to maximize backwards compatibility between the BPF version and + // its original builtin, we add this check from `TransactionContext` to the + // program directly, to throw even when the data being written is the same + // same as what's currently in the account. + // + // Since the account can never be executable and also owned by the ALT + // program, we'll just focus on readonly. + if !lookup_table_info.is_writable { + return Err(AddressLookupTableError::ReadonlyDataModified.into()); + } + AddressLookupTable::overwrite_meta_data( &mut lookup_table_info.try_borrow_mut_data()?[..], lookup_table_meta, @@ -419,6 +451,7 @@ fn process_deactivate_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo] }; let clock = ::get()?; + lookup_table_meta.deactivation_slot = clock.slot; AddressLookupTable::overwrite_meta_data( diff --git a/program/tests/extend_lookup_table_ix.rs b/program/tests/extend_lookup_table_ix.rs index 9cf642c..f0f7239 100644 --- a/program/tests/extend_lookup_table_ix.rs +++ b/program/tests/extend_lookup_table_ix.rs @@ -10,6 +10,7 @@ use { Mollusk, }, solana_address_lookup_table_program::{ + error::AddressLookupTableError, instruction::extend_lookup_table, state::{AddressLookupTable, LookupTableMeta}, }, @@ -355,3 +356,40 @@ fn test_extend_prepaid_lookup_table_without_payer() { }, ); } + +// Backwards compatibility test case. +#[test] +fn test_extend_readonly() { + let mollusk = setup(); + + let payer = Pubkey::new_unique(); + let authority = Pubkey::new_unique(); + + let initialized_table = new_address_lookup_table(Some(authority), 0); + + let lookup_table_address = Pubkey::new_unique(); + let lookup_table_account = lookup_table_account(initialized_table); + + let new_addresses = vec![Pubkey::new_unique()]; + let mut instruction = + extend_lookup_table(lookup_table_address, authority, Some(payer), new_addresses); + + // Make the lookup table account read-only. + instruction.accounts[0].is_writable = false; + + mollusk.process_and_validate_instruction( + &instruction, + &[ + (lookup_table_address, lookup_table_account), + (authority, AccountSharedData::default()), + ( + payer, + AccountSharedData::new(100_000_000, 0, &system_program::id()), + ), + keyed_account_for_system_program(), + ], + &[Check::err(ProgramError::Custom( + AddressLookupTableError::ReadonlyDataModified as u32, + ))], + ); +} From 85efb5bccbcdba870be89baa60ec2f8edd9f505d Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Fri, 8 Nov 2024 17:17:54 +0400 Subject: [PATCH 2/3] check in new compute unit benchmark --- program/benches/compute_units.md | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/program/benches/compute_units.md b/program/benches/compute_units.md index 0561642..c89af98 100644 --- a/program/benches/compute_units.md +++ b/program/benches/compute_units.md @@ -1,3 +1,29 @@ +#### Compute Units: 2024-11-08 13:17:54.004441 UTC + +| Name | CUs | Delta | +|------|------|-------| +| create_lookup_table | 10759 | +1 | +| freeze_lookup_table | 1518 | +1 | +| extend_lookup_table_from_0_to_1 | 6226 | +4 | +| extend_lookup_table_from_0_to_10 | 8845 | +4 | +| extend_lookup_table_from_0_to_38 | 17081 | +4 | +| extend_lookup_table_from_1_to_2 | 6226 | +4 | +| extend_lookup_table_from_1_to_10 | 8551 | +4 | +| extend_lookup_table_from_1_to_39 | 17081 | +4 | +| extend_lookup_table_from_5_to_6 | 6226 | +4 | +| extend_lookup_table_from_5_to_15 | 8846 | +4 | +| extend_lookup_table_from_5_to_43 | 17081 | +4 | +| extend_lookup_table_from_25_to_26 | 6229 | +4 | +| extend_lookup_table_from_25_to_35 | 8848 | +4 | +| extend_lookup_table_from_25_to_63 | 17084 | +4 | +| extend_lookup_table_from_50_to_88 | 17087 | +4 | +| extend_lookup_table_from_100_to_138 | 17093 | +4 | +| extend_lookup_table_from_150_to_188 | 17100 | +4 | +| extend_lookup_table_from_200_to_238 | 17106 | +4 | +| extend_lookup_table_from_255_to_256 | 6258 | +4 | +| deactivate_lookup_table | 3152 | +1 | +| close_lookup_table | 2227 | +1 | + #### Compute Units: 2024-11-07 16:24:48.059441548 UTC | Name | CUs | Delta | From 99ead095c4252af6322b77482e5fade511df370f Mon Sep 17 00:00:00 2001 From: Joe Caulfield Date: Sat, 9 Nov 2024 13:11:45 +0400 Subject: [PATCH 3/3] add comment --- program/src/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/program/src/error.rs b/program/src/error.rs index 9c84c91..b9511f6 100644 --- a/program/src/error.rs +++ b/program/src/error.rs @@ -15,7 +15,7 @@ use { pub enum AddressLookupTableError { /// Instruction modified data of a read-only account. #[error("Instruction modified data of a read-only account")] - ReadonlyDataModified = 10, + ReadonlyDataModified = 10, // Avoid collisions with System. } impl PrintProgramError for AddressLookupTableError {