Skip to content

Commit

Permalink
Program: add custom error codes for readonly writes (#43)
Browse files Browse the repository at this point in the history
  • Loading branch information
buffalojoec authored Nov 9, 2024
1 parent cf11c00 commit 8be62dd
Show file tree
Hide file tree
Showing 8 changed files with 151 additions and 3 deletions.
3 changes: 3 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions program/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] }
Expand Down
26 changes: 26 additions & 0 deletions program/benches/compute_units.md
Original file line number Diff line number Diff line change
@@ -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 |
Expand Down
13 changes: 10 additions & 3 deletions program/src/entrypoint.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand All @@ -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::<AddressLookupTableError>();
return Err(error);
}
Ok(())
}
37 changes: 37 additions & 0 deletions program/src/error.rs
Original file line number Diff line number Diff line change
@@ -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, // Avoid collisions with System.
}

impl PrintProgramError for AddressLookupTableError {
fn print<E>(&self) {
msg!(&self.to_string());
}
}

impl From<AddressLookupTableError> for ProgramError {
fn from(e: AddressLookupTableError) -> Self {
ProgramError::Custom(e as u32)
}
}

impl<T> DecodeError<T> for AddressLookupTableError {
fn type_of() -> &'static str {
"AddressLookupTableError"
}
}
1 change: 1 addition & 0 deletions program/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
33 changes: 33 additions & 0 deletions program/src/processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use {
crate::{
check_id,
error::AddressLookupTableError,
instruction::AddressLookupTableInstruction,
state::{
AddressLookupTable, ProgramState, LOOKUP_TABLE_MAX_ADDRESSES, LOOKUP_TABLE_META_SIZE,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -419,6 +451,7 @@ fn process_deactivate_lookup_table(program_id: &Pubkey, accounts: &[AccountInfo]
};

let clock = <Clock as Sysvar>::get()?;

lookup_table_meta.deactivation_slot = clock.slot;

AddressLookupTable::overwrite_meta_data(
Expand Down
38 changes: 38 additions & 0 deletions program/tests/extend_lookup_table_ix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use {
Mollusk,
},
solana_address_lookup_table_program::{
error::AddressLookupTableError,
instruction::extend_lookup_table,
state::{AddressLookupTable, LookupTableMeta},
},
Expand Down Expand Up @@ -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,
))],
);
}

0 comments on commit 8be62dd

Please sign in to comment.