Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Program: add custom error codes for readonly writes #43

Merged
merged 3 commits into from
Nov 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
))],
);
}