diff --git a/bench-report.json b/bench-report.json index 7dee8d7..f7a86db 100644 --- a/bench-report.json +++ b/bench-report.json @@ -1,5 +1,7 @@ { "accounts": { + "add_solver/add_with_many_existing_solvers": 5, + "add_solver/adds_a_solver": 5, "create_buffers/happy_path_creates_initialized_buffer_token_account": 6, "create_buffers/happy_path_creates_multiple_buffers_in_one_instruction": 10, "create_buffers/max_buffers_in_one_instruction": 64, @@ -24,30 +26,34 @@ "transfer_authority/reclaim_authority_can_transfer_itself": 4 }, "compute_units": { - "create_buffers/happy_path_creates_initialized_buffer_token_account": 10340, - "create_buffers/happy_path_creates_multiple_buffers_in_one_instruction": 21731, - "create_buffers/max_buffers_in_one_instruction": 176947, - "create_order/happy_path_creates_order_pda_with_expected_body": 4976, - "initialize/happy_path_initializes_state_pda_with_expected_data": 4526, - "reclaim_buffer/funded_buffer_is_skipped": 6324, - "reclaim_buffer/happy_path_reclaims_empty_buffer_to_the_authority_itself": 7472, - "reclaim_buffer/max_buffers_in_one_instruction": 136526, - "reclaim_buffer/reclaims_multiple_buffers_skipping_funded": 18068, + "add_solver/add_with_many_existing_solvers": 5074, + "add_solver/adds_a_solver": 4622, + "create_buffers/happy_path_creates_initialized_buffer_token_account": 10345, + "create_buffers/happy_path_creates_multiple_buffers_in_one_instruction": 21743, + "create_buffers/max_buffers_in_one_instruction": 177040, + "create_order/happy_path_creates_order_pda_with_expected_body": 4978, + "initialize/happy_path_initializes_state_pda_with_expected_data": 4529, + "reclaim_buffer/funded_buffer_is_skipped": 6333, + "reclaim_buffer/happy_path_reclaims_empty_buffer_to_the_authority_itself": 7481, + "reclaim_buffer/max_buffers_in_one_instruction": 136650, + "reclaim_buffer/reclaims_multiple_buffers_skipping_funded": 18080, "reclaim_order/happy_path_returns_lamports_and_closes_pda": 2183, - "settle/finalizes_with_no_pushes": 7062, - "settle/pulls_from_multiple_orders": 19921, - "settle/pulls_funds_to_destination": 13524, - "settle/pulls_to_multiple_destinations": 14675, - "settle/pushes_a_single_order": 12366, - "settle/pushes_several_orders_from_different_buffers": 17608, - "settle/pushes_several_orders_from_one_buffer": 17609, - "settle/settles_a_single_order": 12384, - "settle/settles_multiple_orders": 22894, - "transfer_authority/manager_can_transfer_manager": 3174, - "transfer_authority/manager_can_transfer_reclaim_authority": 3176, - "transfer_authority/reclaim_authority_can_transfer_itself": 3180 + "settle/finalizes_with_no_pushes": 7070, + "settle/pulls_from_multiple_orders": 19949, + "settle/pulls_funds_to_destination": 13542, + "settle/pulls_to_multiple_destinations": 14694, + "settle/pushes_a_single_order": 12380, + "settle/pushes_several_orders_from_different_buffers": 17632, + "settle/pushes_several_orders_from_one_buffer": 17631, + "settle/settles_a_single_order": 12398, + "settle/settles_multiple_orders": 22926, + "transfer_authority/manager_can_transfer_manager": 3175, + "transfer_authority/manager_can_transfer_reclaim_authority": 3177, + "transfer_authority/reclaim_authority_can_transfer_itself": 3181 }, "transaction_bytes": { + "add_solver/add_with_many_existing_solvers": 366, + "add_solver/adds_a_solver": 366, "create_buffers/happy_path_creates_initialized_buffer_token_account": 303, "create_buffers/happy_path_creates_multiple_buffers_in_one_instruction": 435, "create_buffers/max_buffers_in_one_instruction": 331, diff --git a/client/src/instructions.rs b/client/src/instructions.rs index 6800abe..17cc5a9 100644 --- a/client/src/instructions.rs +++ b/client/src/instructions.rs @@ -245,6 +245,30 @@ impl From for Instruction { } } +/// Inserts `solver` into the state PDA's solver list. `manager` authorizes the +/// change and must be the current manager; `payer` funds the account's growth. +/// Both sign. +pub struct AddSolver { + pub program_id: Pubkey, + pub manager: Pubkey, + pub payer: Pubkey, + pub solver: Pubkey, +} + +impl From for Instruction { + fn from(builder: AddSolver) -> Self { + let (state_pda, _bump) = find_state_pda(&builder.program_id); + cow_settlement_interface::instruction::add_solver::AddSolver { + program_id: builder.program_id, + manager: builder.manager, + payer: builder.payer, + state_pda, + solver: builder.solver, + } + .into() + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/client/src/parse.rs b/client/src/parse.rs index d923e00..d83ea15 100644 --- a/client/src/parse.rs +++ b/client/src/parse.rs @@ -5,6 +5,7 @@ use cow_settlement_interface::{ instruction::{ + add_solver::AddSolverInput, create_buffer::CreateBufferInput, create_order::CreateOrderInput, initialize::InitializeInput, @@ -28,6 +29,7 @@ pub enum ParsedInstruction<'a, A> { ReclaimOrder(ReclaimOrderInput<'a, A>), ReclaimBuffer(ReclaimBufferInput<'a, A>), TransferAuthority(TransferAuthorityInput<'a, A>), + AddSolver(AddSolverInput<'a, A>), } /// Parses any settlement instruction by its discriminator. @@ -61,6 +63,9 @@ pub fn parse_instruction<'a, A>( SettlementInstruction::TransferAuthority => ParsedInstruction::TransferAuthority( TransferAuthorityInput::parse_body(remaining_data, accounts)?, ), + SettlementInstruction::AddSolver => { + ParsedInstruction::AddSolver(AddSolverInput::parse_body(remaining_data, accounts)?) + } }) } @@ -68,7 +73,8 @@ pub fn parse_instruction<'a, A>( mod tests { use super::*; use crate::instructions::{ - BeginSettle, CreateBuffers, CreateOrder, FinalizeSettle, Initialize, InitializedIntent, + AddSolver, BeginSettle, CreateBuffers, CreateOrder, FinalizeSettle, Initialize, + InitializedIntent, }; use cow_settlement_interface::{ data::intent::fixtures::sample_intent, @@ -145,6 +151,13 @@ mod tests { new_authority: payer, } .into(), + SettlementInstruction::AddSolver => AddSolver { + program_id, + manager: payer, + payer, + solver: pubkey_from_seed("solver"), + } + .into(), } } @@ -162,6 +175,7 @@ mod tests { SettlementInstruction::ReclaimOrder, SettlementInstruction::ReclaimBuffer, SettlementInstruction::TransferAuthority, + SettlementInstruction::AddSolver, ] { let ix = build(expected); let accounts: Vec<_> = ix @@ -180,6 +194,7 @@ mod tests { ParsedInstruction::ReclaimOrder(_) => SettlementInstruction::ReclaimOrder, ParsedInstruction::ReclaimBuffer(_) => SettlementInstruction::ReclaimBuffer, ParsedInstruction::TransferAuthority(_) => SettlementInstruction::TransferAuthority, + ParsedInstruction::AddSolver(_) => SettlementInstruction::AddSolver, }; assert_eq!(actual, expected); } diff --git a/interface/src/data/state.rs b/interface/src/data/state.rs index d20cbca..e869d71 100644 --- a/interface/src/data/state.rs +++ b/interface/src/data/state.rs @@ -1,23 +1,24 @@ //! Settlement state account: its byte layout and the zero-copy accessor over it. //! //! The state PDA (see [`crate::pda::state`]) stores the protocol's authority -//! configuration in a fixed header: a discriminator byte followed by the holder -//! of each [`Role`]. +//! configuration in a fixed header (a discriminator byte followed by the holder +//! of each [`Role`]), then the list of approved solvers, packed and sorted +//! ascending by address. //! //! ```text //! ┌──── discriminator -//! ┌┬───────────────────────────────┬───────────────────────────────┐ -//! ││ manager │ reclaim_authority │ -//! └┴───────────────────────────────┴───────────────────────────────┘ -//! 0 1 33 65 -//! └───────────────────────────── header ──────────────────────────┘ +//! ┌┬───────────────────────────────┬───────────────────────────────┬───────────────────────────────┬───── ... ─────┬───────────────────────────────┐ +//! ││ manager │ reclaim_authority │ solver[0] │ other solvers │ solver[N-1] │ +//! └┴───────────────────────────────┴───────────────────────────────┴───────────────────────────────┴───── ... ─────┴───────────────────────────────┘ +//! 0 1 33 65 97 +//! └───────────────────────────── header ──────────────────────────┘└─────────────────────────────── N sorted solvers ─────────────────────────────┘ //! ``` //! //! [`StateAccount`] is a zero-copy accessor over an account's bytes, generic //! over the borrow (`&[u8]`, `&mut [u8]`): the reads are available for any //! borrow, the in-place role write only for a mutable one. It reads and updates -//! the header directly in the account, so the program never copies the account -//! into an owned struct just to inspect a field or flip a role. +//! the account data directly, so the program never copies the account into an +//! owned struct. use core::mem::size_of; use core::ops::{Deref, DerefMut}; @@ -27,7 +28,7 @@ use solana_account_view::{AccountView, Ref}; use solana_program_error::ProgramError; use solana_pubkey::Pubkey; -use crate::{Role, SettlementAccount}; +use crate::{Role, SettlementAccount, SettlementError}; /// Single-byte account discriminator at the front of the header. pub const DISCRIMINATOR: u8 = SettlementAccount::SettlementState.discriminator(); @@ -36,7 +37,7 @@ pub const DISCRIMINATOR: u8 = SettlementAccount::SettlementState.discriminator() const WIDTH_DISCRIMINATOR: usize = size_of::(); /// Byte width of an account address (a [`Pubkey`]): each role holder in the -/// header is one. +/// header and each solver in the list is one. pub const WIDTH_PUBKEY: usize = size_of::(); /// Length of the fixed header: the discriminator byte followed by one holder @@ -127,6 +128,45 @@ impl> StateAccount { }; Pubkey::new_from_array(*holder) } + + /// The sorted solver list that follows the header, as fixed-width entries. + /// A trailing partial entry (only possible on a corrupt account) is ignored + /// and an uninitialized/too-small account returns an empty slice. + fn solver_region(&self) -> &[[u8; WIDTH_PUBKEY]] { + let region = self.0.get(WIDTH_HEADER..).unwrap_or_default(); + let (solvers, _partial) = region.as_chunks::(); + solvers + } + + /// Locate `solver` in the sorted solver list, mirroring + /// [`slice::binary_search`]: `Ok(index)` if present, `Err(index)` with the + /// slot it would occupy otherwise. The list is sorted by address bytes. + pub fn solver_search(&self, solver: &Pubkey) -> Result { + let seek = solver.to_bytes(); + self.solver_region().binary_search(&seek) + } + + /// The stored solvers, in order (sorted ascending by address). + pub fn solvers(&self) -> impl Iterator + '_ { + self.solver_region() + .iter() + .map(|raw| Pubkey::new_from_array(*raw)) + } + + /// The account's data length after growing it by one solver slot: the size + /// it must be resized to before [`insert_solver`](Self::insert_solver) can + /// fill that new slot. + /// + /// Returns [`ProgramError::ArithmeticOverflow`] if that length overflows + /// `usize`, which a caller handling data from a real account can treat as + /// unreachable: the runtime caps account data at + /// [`MAX_PERMITTED_DATA_LENGTH`](solana_system_interface::MAX_PERMITTED_DATA_LENGTH). + pub fn grown_len(&self) -> Result { + self.0 + .len() + .checked_add(WIDTH_PUBKEY) + .ok_or(ProgramError::ArithmeticOverflow) + } } impl<'a> StateAccount> { @@ -171,6 +211,82 @@ impl> StateAccount { }; *holder = new.to_bytes(); } + + /// Insert `solver` into the sorted solver list, or fail with + /// [`SettlementError::SolverAlreadyExists`] if it is already stored. + /// + /// The account must already be grown by one solver slot + /// ([`grown_len`](Self::grown_len)): the trailing slot is spare capacity + /// for the new entry, so the live list is every entry but that last one. + /// The solver is placed in sorted order by shifting all entries and + /// inserting the new solver in the initial slot. + /// An error is returned if the solver is already included. + pub fn insert_solver(&mut self, solver: &Pubkey) -> Result<(), ProgramError> { + let (_spare, occupied) = self + .solver_region() + .split_last() + .expect("account grown by one solver slot before insertion"); + let index = match occupied.binary_search(&solver.to_bytes()) { + Ok(_) => return Err(SettlementError::SolverAlreadyExists.into()), + Err(index) => index, + }; + + // Shift the entries at and after `index` up one slot into the spare, + // then write the solver into the gap that opens at `index`. + let data: &mut [u8] = &mut self.0; + let occupied_end = data + .len() + .checked_sub(WIDTH_PUBKEY) + .expect("account grown by one solver slot before insertion"); + let gap = WIDTH_HEADER + .checked_add( + index + .checked_mul(WIDTH_PUBKEY) + .expect("insertion index bound by account length"), + ) + .expect("insertion offset bound by account length"); + let gap_end = gap + .checked_add(WIDTH_PUBKEY) + .expect("insertion slot bound by account length"); + + data.copy_within(gap..occupied_end, gap_end); + data[gap..gap_end].copy_from_slice(&solver.to_bytes()); + Ok(()) + } +} + +/// Test scaffolding for building state-account bytes, shared by this crate's +/// tests and the settlement program's via the `test-fixtures` feature. +#[cfg(any(test, feature = "test-fixtures"))] +pub mod fixtures { + use proptest::prelude::*; + use solana_pubkey::Pubkey; + + use super::{StateAccount, StateInitArgs, WIDTH_HEADER}; + + /// The bytes of a state account: the `header` followed by `solvers`, stored + /// sorted ascending by address as the on-chain list always is (so callers can + /// pass them in any order). + pub fn state_account_bytes(header: &StateInitArgs, solvers: &[Pubkey]) -> Vec { + let mut sorted = solvers.to_vec(); + sorted.sort(); + let mut bytes = vec![0u8; WIDTH_HEADER]; + StateAccount::initialize(&mut bytes[..], header).expect("header fits"); + for solver in &sorted { + bytes.extend_from_slice(&solver.to_bytes()); + } + bytes + } + + /// Any valid [`StateInitArgs`]. + pub fn arb_init_params() -> impl Strategy { + (any::<[u8; 32]>(), any::<[u8; 32]>()).prop_map(|(manager, reclaim_authority)| { + StateInitArgs { + manager: Pubkey::new_from_array(manager), + reclaim_authority: Pubkey::new_from_array(reclaim_authority), + } + }) + } } #[cfg(test)] @@ -195,6 +311,12 @@ mod tests { bytes } + /// [`SAMPLE_INIT_ARGS`] followed by `solvers`, stored sorted ascending by address + /// as the on-chain list always is, so callers can pass them in any order. + fn state_bytes(solvers: &[Pubkey]) -> Vec { + super::fixtures::state_account_bytes(&SAMPLE_INIT_ARGS, solvers) + } + #[test] fn header_has_the_canonical_wire_layout() { assert_eq!(WIDTH_HEADER, 65); @@ -287,6 +409,63 @@ mod tests { ); } + /// Three solvers in arbitrary order; `state_bytes` stores them sorted, as the + /// on-chain list always is. Returns both the stored bytes and the sorted + /// order the reads should reflect. + fn sample_solvers() -> (Vec, [Pubkey; 3]) { + let solvers = [ + pubkey_from_seed("sample_solver's solver a"), + pubkey_from_seed("sample_solver's solver b"), + pubkey_from_seed("sample_solver's solver c"), + ]; + let bytes = state_bytes(&solvers); + let mut sorted = solvers; + sorted.sort(); + (bytes, sorted) + } + + #[test] + fn solvers_is_empty_without_any_stored() { + let bytes = state_bytes(&[]); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + assert_eq!(state.solvers().count(), 0); + } + + #[test] + fn solvers_lists_stored_solvers_sorted() { + let (bytes, sorted) = sample_solvers(); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + assert_eq!(state.solvers().collect::>(), sorted); + } + + #[test] + fn solver_search_on_empty_list() { + let bytes = state_bytes(&[]); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + let absent = pubkey_from_seed("absent solver"); + assert_eq!(state.solver_search(&absent), Err(0)); + } + + #[test] + fn solver_search_finds_present() { + let (bytes, sorted) = sample_solvers(); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + for (index, solver) in sorted.iter().enumerate() { + assert_eq!(state.solver_search(solver), Ok(index)); + } + } + + #[test] + fn solver_search_locates_absent() { + let (bytes, sorted) = sample_solvers(); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + // An absent solver isn't found; its reported slot is where inserting it + // would keep the list sorted. + let absent = pubkey_from_seed("absent solver"); + let slot = state.solver_search(&absent).expect_err("absent solver"); + assert_eq!(slot, sorted.partition_point(|s| s < &absent)); + } + #[test] fn initialize_rejects_too_small_buffer() { let mut bytes = [0u8; WIDTH_HEADER - 1]; @@ -305,21 +484,76 @@ mod tests { /// The encode roundtrip: any two role holders written with /// `initialize` read back unchanged. #[test] - fn initializes_accounts_as_expected( - manager in any::<[u8; 32]>(), - reclaim_authority in any::<[u8; 32]>(), - ) { - let init_args = StateInitArgs { - manager: Pubkey::new_from_array(manager), - reclaim_authority: Pubkey::new_from_array(reclaim_authority), - }; - + fn account_encode_roundtrip(header in fixtures::arb_init_params()) { let mut bytes = [0u8; WIDTH_HEADER]; - StateAccount::initialize(&mut bytes[..], &init_args).expect("header fits"); + StateAccount::initialize(&mut bytes[..], &header).expect("header fits"); + + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + let StateInitArgs { manager, reclaim_authority } = header; + prop_assert_eq!(state.authority(Role::Manager), manager); + prop_assert_eq!(state.authority(Role::ReclaimAuthority), reclaim_authority); + } + #[test] + fn insert_solver_inserts_an_absent_solver( + header in fixtures::arb_init_params(), + // Unique and already sorted, being a `BTreeSet`. + raw_solvers in prop::collection::btree_set(any::<[u8; 32]>(), 0..50), + raw_new in any::<[u8; 32]>(), + ) { + prop_assume!(!raw_solvers.contains(&raw_new)); + let stored: Vec = + raw_solvers.into_iter().map(Pubkey::new_from_array).collect(); + let new = Pubkey::new_from_array(raw_new); + + // Grow by one slot, exactly as the handler resizes the account + // before delegating the insert. + let mut bytes = fixtures::state_account_bytes(&header, &stored); + let grown_len = StateAccount::attach(&bytes[..]) + .expect("valid header") + .grown_len() + .expect("grown length fits"); + prop_assert_eq!(grown_len, bytes.len().strict_add(WIDTH_PUBKEY)); + bytes.resize(grown_len, 0); + StateAccount::attach(&mut bytes[..]) + .expect("valid header") + .insert_solver(&new) + .expect("absent solver inserts"); + + let mut expected = stored; + expected.push(new); + expected.sort(); + let state = StateAccount::attach(&bytes[..]).expect("valid header"); + prop_assert_eq!(state.solvers().collect::>(), expected); + } + + /// `insert_solver` rejects a solver that is already stored and leaves + /// the live list untouched. + #[test] + fn insert_solver_rejects_an_existing_solver( + header in fixtures::arb_init_params(), + // Unique and already sorted, being a `BTreeSet`. + raw_solvers in prop::collection::btree_set(any::<[u8; 32]>(), 1..50), + pick in any::(), + ) { + let stored: Vec = + raw_solvers.into_iter().map(Pubkey::new_from_array).collect(); + let existing = stored[pick.index(stored.len())]; + + // Grow by one slot as the handler does, then try to re-add. + let mut bytes = fixtures::state_account_bytes(&header, &stored); + bytes.resize(bytes.len().strict_add(WIDTH_PUBKEY), 0); + prop_assert_eq!( + StateAccount::attach(&mut bytes[..]) + .expect("valid header") + .insert_solver(&existing), + Err(SettlementError::SolverAlreadyExists.into()), + ); + + // Nothing was written: the stored solvers still read back in + // order (the trailing spare slot is left as the zero pubkey). let state = StateAccount::attach(&bytes[..]).expect("valid header"); - prop_assert_eq!(state.authority(Role::Manager), init_args.manager); - prop_assert_eq!(state.authority(Role::ReclaimAuthority), init_args.reclaim_authority); + prop_assert_eq!(state.solvers().take(stored.len()).collect::>(), stored); } } } diff --git a/interface/src/instruction/add_solver.rs b/interface/src/instruction/add_solver.rs new file mode 100644 index 0000000..3348fbb --- /dev/null +++ b/interface/src/instruction/add_solver.rs @@ -0,0 +1,257 @@ +//! `AddSolver` instruction builder and parser. +//! +//! It inserts a solver into the sorted solver list stored in the state PDA (see +//! [`crate::data::state`]). Only the manager may authorize it. The state PDA +//! grows by one solver, so a `payer` funds the extra rent through a `Transfer` +//! from the system program. + +use core::mem::size_of; + +use solana_instruction::{AccountMeta, Instruction}; +use solana_program_error::ProgramError; +use solana_pubkey::Pubkey; + +pub use solana_system_interface::program::ID as SYSTEM_PROGRAM_ID; + +use crate::instruction::InstructionInputParsing; +use crate::SettlementInstruction; + +/// Builder for an `AddSolver` instruction. +/// +/// `manager` authorizes the change and must be the state PDA's current manager; +/// it signs but doesn't pay. `payer` funds the extra rent and signs the funding +/// transfer. `solver` is inserted into the sorted solver list; adding one +/// already present fails. +/// +/// Wire format: `[discriminator=8, solver (32 bytes)]`. +/// Required accounts: `[manager (S), payer (W,S), state_pda (W), +/// system_program (R)]`. The system program must be available for the +/// rent-funding `Transfer` CPI but doesn't need to sit at a specific position. +pub struct AddSolver { + pub program_id: Pubkey, + pub manager: Pubkey, + pub payer: Pubkey, + pub state_pda: Pubkey, + pub solver: Pubkey, +} + +impl From for Instruction { + fn from(builder: AddSolver) -> Self { + let mut data = vec![SettlementInstruction::AddSolver.discriminator()]; + data.extend_from_slice(&builder.solver.to_bytes()); + Instruction { + program_id: builder.program_id, + accounts: vec![ + AccountMeta::new_readonly(builder.manager, true), + AccountMeta::new(builder.payer, true), + AccountMeta::new(builder.state_pda, false), + AccountMeta::new_readonly(SYSTEM_PROGRAM_ID, false), + ], + data, + } + } +} + +/// Parsed inputs of an `AddSolver` instruction. +pub struct AddSolverInput<'a, A> { + pub manager: &'a A, + pub payer: &'a A, + pub state_pda: &'a A, + pub solver: Pubkey, +} + +impl<'a, A> InstructionInputParsing<'a, A> for AddSolverInput<'a, A> { + const DISCRIMINATOR: SettlementInstruction = SettlementInstruction::AddSolver; + + fn parse_body(instruction_data: &[u8], accounts: &'a [A]) -> Result { + let solver: &[u8; size_of::()] = instruction_data + .try_into() + .map_err(|_| ProgramError::InvalidInstructionData)?; + let solver = Pubkey::new_from_array(*solver); + + // Accounts: [manager (S), payer (W,S), state_pda (W), system_program (R)]. + // The system program needs to be present for the `Transfer` CPI but + // doesn't need to be referenced directly and can be at any later position. + let [manager, payer, state_pda, _system, ..] = accounts else { + return Err(ProgramError::NotEnoughAccountKeys); + }; + + Ok(Self { + manager, + payer, + state_pda, + solver, + }) + } +} + +/// Test scaffolding for `AddSolver` parsing and handling, shared by this crate's +/// tests and the settlement program's via the `test-fixtures` feature. +#[cfg(any(test, feature = "test-fixtures"))] +pub mod fixtures { + use solana_address::Address; + use solana_instruction::Instruction; + + use super::AddSolver; + + /// Number of accounts `AddSolver` expects: manager, payer, state PDA, system + /// program. + pub const NUM_ACCOUNTS: usize = 4; + + /// `AddSolver` instruction data with placeholder addresses, for failure cases + /// where the actual addresses don't matter. + pub fn add_solver_data() -> Vec { + let zero = Address::default(); + Instruction::from(AddSolver { + program_id: zero, + manager: zero, + payer: zero, + state_pda: zero, + solver: zero, + }) + .data + } +} + +#[cfg(test)] +mod tests { + use super::fixtures::{add_solver_data, NUM_ACCOUNTS}; + use super::*; + use crate::fixtures::pubkey_from_seed; + use crate::instruction::fixtures::{fake_account, fake_sequential_accounts}; + use crate::instruction::tests::{ + assert_readonly_nonsigner, assert_readonly_signer, assert_writable_nonsigner, + assert_writable_signer, + }; + use solana_account_view::AccountView; + + #[test] + fn add_solver_input_parses_valid_input() { + let program_id = pubkey_from_seed("program id"); + let manager = fake_account(pubkey_from_seed("manager")); + let payer = fake_account(pubkey_from_seed("payer")); + let state_pda = fake_account(pubkey_from_seed("state pda")); + let system_program = fake_account(pubkey_from_seed("system program")); + let solver = pubkey_from_seed("solver"); + + let data = Instruction::from(AddSolver { + program_id, + manager: *manager.address(), + payer: *payer.address(), + state_pda: *state_pda.address(), + solver, + }) + .data; + let accounts = [manager, payer, state_pda, system_program]; + + let AddSolverInput { + manager: parsed_manager, + payer: parsed_payer, + state_pda: parsed_state_pda, + solver: parsed_solver, + } = AddSolverInput::parse(&data, &accounts).expect("parse should succeed"); + + assert_eq!(parsed_manager.address(), manager.address()); + assert_eq!(parsed_payer.address(), payer.address()); + assert_eq!(parsed_state_pda.address(), state_pda.address()); + assert_eq!(parsed_solver, solver); + } + + #[test] + fn add_solver_input_rejects_long_data() { + let mut data = add_solver_data(); + data.push(42); // trailing byte + let accounts = fake_sequential_accounts::(); + assert_eq!( + AddSolverInput::parse(&data, &accounts).err(), + Some(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn add_solver_input_rejects_short_data() { + let mut data = add_solver_data(); + data.pop(); // one byte short + let accounts = fake_sequential_accounts::(); + assert_eq!( + AddSolverInput::parse(&data, &accounts).err(), + Some(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn add_solver_input_rejects_missing_accounts() { + let data = add_solver_data(); + let mut accounts: Vec = fake_sequential_accounts::().into(); + accounts.pop(); + assert_eq!( + AddSolverInput::parse(&data, &accounts).err(), + Some(ProgramError::NotEnoughAccountKeys), + ); + } + + #[test] + fn instruction_data_has_expected_layout() { + let solver = pubkey_from_seed("solver"); + let Instruction { data, .. } = AddSolver { + program_id: pubkey_from_seed("program id"), + manager: pubkey_from_seed("manager"), + payer: pubkey_from_seed("payer"), + state_pda: pubkey_from_seed("state pda"), + solver, + } + .into(); + + assert_eq!(data.len(), 1 + size_of::()); + assert_eq!(data[0], SettlementInstruction::AddSolver.discriminator()); + assert_eq!(&data[1..], &solver.to_bytes()); + } + + #[test] + fn instruction_data_regression() { + let solver = Pubkey::new_from_array([0x11; 32]); + let Instruction { data, .. } = AddSolver { + program_id: pubkey_from_seed("program id"), + manager: pubkey_from_seed("manager"), + payer: pubkey_from_seed("payer"), + state_pda: pubkey_from_seed("state pda"), + solver, + } + .into(); + + #[rustfmt::skip] + let expected: [u8; 1 + size_of::()] = [ + // discriminator (AddSolver = 8) + 0x08, + // solver + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, + 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, 0x11, + ]; + assert_eq!(data, expected); + } + + #[test] + fn instruction_has_expected_accounts() { + let manager = pubkey_from_seed("manager"); + let payer = pubkey_from_seed("payer"); + let state_pda = pubkey_from_seed("state pda"); + let Instruction { accounts, .. } = AddSolver { + program_id: pubkey_from_seed("program id"), + manager, + payer, + state_pda, + solver: pubkey_from_seed("solver"), + } + .into(); + + assert_eq!(accounts.len(), 4); + // The manager authorizes the change; the payer funds the extra rent; the + // state PDA is grown and written; the system program is only referenced. + assert_readonly_signer(&accounts[0], manager); + assert_writable_signer(&accounts[1], payer); + assert_writable_nonsigner(&accounts[2], state_pda); + assert_readonly_nonsigner(&accounts[3], SYSTEM_PROGRAM_ID); + } +} diff --git a/interface/src/instruction/mod.rs b/interface/src/instruction/mod.rs index d073f24..6da3a24 100644 --- a/interface/src/instruction/mod.rs +++ b/interface/src/instruction/mod.rs @@ -8,6 +8,7 @@ use solana_program_error::ProgramError; use crate::{recover_discriminator, SettlementInstruction}; +pub mod add_solver; pub mod create_buffer; pub mod create_order; pub mod initialize; diff --git a/interface/src/lib.rs b/interface/src/lib.rs index 3f61256..182a1f4 100644 --- a/interface/src/lib.rs +++ b/interface/src/lib.rs @@ -25,6 +25,7 @@ pub enum SettlementInstruction { ReclaimOrder = 5, ReclaimBuffer = 6, TransferAuthority = 7, + AddSolver = 8, } impl SettlementInstruction { @@ -221,6 +222,11 @@ pub enum SettlementError { /// `TransferAuthority`'s signer is neither the manager nor the current /// holder of the role being transferred, so it may not transfer it. UnauthorizedAuthorityTransfer = 34, + /// `AddSolver`'s manager account isn't a signer, or doesn't match the + /// `manager` recorded in the settlement state PDA. + UnauthorizedSolverManagement = 35, + /// `AddSolver`'s solver is already in the state PDA's solver list. + SolverAlreadyExists = 36, } impl From for u32 { diff --git a/programs/settlement/Cargo.toml b/programs/settlement/Cargo.toml index 7638cf5..723eeb5 100644 --- a/programs/settlement/Cargo.toml +++ b/programs/settlement/Cargo.toml @@ -18,7 +18,7 @@ name = "cow_settlement" crate-type = ["cdylib", "lib"] [dependencies] -pinocchio = { workspace = true, features = ["cpi"] } +pinocchio = { workspace = true, features = ["cpi", "account-resize"] } pinocchio-system.workspace = true pinocchio-token.workspace = true cow-settlement-interface.workspace = true diff --git a/programs/settlement/src/add_solver.rs b/programs/settlement/src/add_solver.rs new file mode 100644 index 0000000..244364b --- /dev/null +++ b/programs/settlement/src/add_solver.rs @@ -0,0 +1,105 @@ +//! `AddSolver` instruction handler. +//! +//! Inserts a solver into the sorted solver list that follows the state PDA +//! header, keeping it sorted so the list stays binary-searchable. Only the +//! manager may authorize it. The account grows by one solver, so the `payer` +//! funds the extra rent through a `Transfer` before the account is resized. + +use cow_settlement_interface::{ + data::state::StateAccount, + instruction::{add_solver::AddSolverInput, InstructionInputParsing}, + Role, SettlementError, +}; +use pinocchio::{ + sysvars::{rent::Rent, Sysvar}, + AccountView, Address, ProgramResult, Resize, +}; +use pinocchio_system::instructions::Transfer; + +use crate::processor::check_state_pda; + +pub fn process_add_solver( + program_id: &Address, + accounts: &mut [AccountView], + instruction_data: &[u8], +) -> ProgramResult { + let AddSolverInput { + manager, + payer, + state_pda, + solver, + } = AddSolverInput::parse(instruction_data, accounts)?; + + check_state_pda(program_id, state_pda)?; + + // Only the manager may change the solver list. Attaching validates the + // account; `grown_len` is the size it must reach to hold one more solver. + let new_len = { + let state = StateAccount::attach(state_pda.try_borrow()?)?; + if !manager.is_signer() || manager.address() != &state.authority(Role::Manager) { + return Err(SettlementError::UnauthorizedSolverManagement.into()); + } + state + .grown_len() + .expect("grown account length fits in usize") + }; + + let shortfall = Rent::get()? + .try_minimum_balance(new_len)? + // why saturating: if there's more balance available than rent needed, + // then there's no shortfall, that is, `shortfall == 0``. + .saturating_sub(state_pda.lamports()); + if shortfall > 0 { + Transfer { + from: payer, + to: state_pda, + lamports: shortfall, + } + .invoke()?; + } + + // Grow the account by one solver slot and insert. A duplicate solver is + // rejected before anything is written; the error reverts the growth and the + // rent transfer along with the rest of the instruction. + let mut state_pda = *state_pda; + state_pda.resize(new_len)?; + let mut state = StateAccount::attach(state_pda.try_borrow_mut()?)?; + state.insert_solver(&solver)?; + + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use cow_settlement_interface::instruction::add_solver::fixtures::{ + add_solver_data, NUM_ACCOUNTS, + }; + use cow_settlement_interface::instruction::fixtures::fake_sequential_accounts; + use pinocchio::error::ProgramError; + + const PROGRAM_ID: Address = Address::new_from_array([0xc0; 32]); + + #[test] + fn process_add_solver_propagates_parse_error() { + let mut data = add_solver_data(); + data.push(0); // trailing byte triggers a parse error + let mut accounts = fake_sequential_accounts::(); + assert_eq!( + process_add_solver(&PROGRAM_ID, &mut accounts, &data), + Err(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn process_add_solver_rejects_non_canonical_state_pda() { + // `fake_sequential_accounts` puts the state PDA at `[3; 32]`, which is + // not the canonical state PDA for this program. + let data = add_solver_data(); + let mut accounts = fake_sequential_accounts::(); + assert_eq!( + process_add_solver(&PROGRAM_ID, &mut accounts, &data), + Err(SettlementError::StateAccountMismatch.into()), + ); + } +} diff --git a/programs/settlement/src/lib.rs b/programs/settlement/src/lib.rs index af2dfa5..d60fa23 100644 --- a/programs/settlement/src/lib.rs +++ b/programs/settlement/src/lib.rs @@ -1,5 +1,6 @@ //! On-chain CoW Protocol settlement program. +mod add_solver; mod create_buffer; mod create_order; mod initialize; @@ -9,6 +10,7 @@ mod reclaim_order; mod settle; mod transfer_authority; +use add_solver::process_add_solver; use cow_settlement_interface::{recover_discriminator, SettlementInstruction}; use create_buffer::process_create_buffer; use create_order::process_create_order; @@ -52,5 +54,8 @@ pub fn process_instruction( SettlementInstruction::TransferAuthority => { process_transfer_authority(program_id, accounts, instruction_data) } + SettlementInstruction::AddSolver => { + process_add_solver(program_id, accounts, instruction_data) + } } } diff --git a/programs/settlement/tests/add_solvers.rs b/programs/settlement/tests/add_solvers.rs new file mode 100644 index 0000000..b76481d --- /dev/null +++ b/programs/settlement/tests/add_solvers.rs @@ -0,0 +1,316 @@ +//! Integration tests for the solver list stored in the state PDA: adding solvers +//! (kept sorted, growing the account and funding the extra rent) and the manager +//! gate on adding them. + +use cow_settlement_client::cow_settlement_interface::{ + data::state::{StateAccount, WIDTH_HEADER, WIDTH_PUBKEY}, + Instruction, SettlementError, +}; +use cow_settlement_client::instructions::AddSolver; +use litesvm::LiteSVM; +use solana_sdk::{ + instruction::InstructionError, + pubkey::Pubkey, + signature::Signer, + transaction::{Transaction, TransactionError}, +}; +use solana_system_interface::MAX_PERMITTED_DATA_LENGTH; + +use crate::common::{ + assert_instruction_error, + benchmark::{send_transaction_metered, BenchLabel}, + create_account_at, lamports, setup_init, to_instruction_error, unique_keypair, + InitializedParams, +}; + +mod common; + +/// Assert the solver list's storage invariant: solvers are stored strictly +/// ascending by address (sorted, with no duplicates). This is what lets the +/// program binary-search the list, so every read below re-checks it. +#[track_caller] +fn assert_solver_invariant(solvers: &[Pubkey]) { + assert!( + // We use `is_sorted_by` here instead of `is_sorted` because that + // doesn't catch duplicates. + solvers.is_sorted_by(|a, b| a < b), + "invariant violated: solver list must be strictly ascending by address: {solvers:?}", + ); +} + +/// The solver list currently stored in the state PDA, in stored order. Reading it +/// also re-checks the storage invariant (see [`assert_solver_invariant`]), so +/// every test that inspects the list enforces it, not just the ones that compare +/// against a sorted expectation. +#[track_caller] +fn solvers(svm: &LiteSVM, state_pda: &Pubkey) -> Vec { + let data = svm + .get_account(state_pda) + .expect("state PDA should exist") + .data; + let solvers: Vec = StateAccount::attach(&data[..]) + .expect("state PDA should be a valid state account") + .solvers() + .collect(); + assert_solver_invariant(&solvers); + solvers +} + +/// Build an `AddSolver` transaction authorized by the manager and paid by the +/// payer, both of which sign. Split from [`add_solver`] so the happy-path test +/// can submit the same transaction through the metered send. +fn add_solver_tx(svm: &LiteSVM, params: &InitializedParams, solver: &Pubkey) -> Transaction { + let ix = AddSolver { + program_id: params.program_id, + manager: params.manager.pubkey(), + payer: params.payer.pubkey(), + solver: *solver, + }; + common::signed_tx(svm, ¶ms.payer, ¶ms.manager, ix) +} + +/// Send an [`add_solver_tx`]. +fn add_solver( + svm: &mut LiteSVM, + params: &InitializedParams, + solver: &Pubkey, +) -> Result<(), TransactionError> { + let tx = add_solver_tx(svm, params, solver); + svm.send_transaction(tx).map(|_| ()).map_err(|e| e.err) +} + +#[test] +fn adds_a_solver() { + let (mut svm, params) = setup_init(); + let before = lamports(&svm, ¶ms.state_pda); + + assert_eq!(solvers(&svm, ¶ms.state_pda), vec![]); + + let solver = unique_keypair().pubkey(); + let tx = add_solver_tx(&svm, ¶ms, &solver); + send_transaction_metered(&mut svm, tx, BenchLabel::AddSolver) + .expect("adding a solver should succeed"); + + assert_eq!(solvers(&svm, ¶ms.state_pda), vec![solver]); + + // The account grew by exactly one solver and stayed rent-exempt (its balance + // rose to the larger rent minimum, funded by the payer). + let account = svm + .get_account(¶ms.state_pda) + .expect("state PDA exists"); + assert_eq!(account.data.len(), WIDTH_HEADER + WIDTH_PUBKEY); + assert_eq!( + account.lamports, + svm.minimum_balance_for_rent_exemption(account.data.len()), + ); + assert!(account.lamports > before, "the payer funded the extra rent"); +} + +#[test] +fn adds_a_solver_without_extra_rent() { + let (mut svm, params) = setup_init(); + + // Pre-fund the state PDA so it holds one more than the rent minimum for the + // grown size. + let mut account = svm + .get_account(¶ms.state_pda) + .expect("state PDA exists"); + account.lamports = svm + .minimum_balance_for_rent_exemption(WIDTH_HEADER + WIDTH_PUBKEY) + .strict_add(1); + svm.set_account(params.state_pda, account) + .expect("set_account should succeed"); + let before = lamports(&svm, ¶ms.state_pda); + + let solver = unique_keypair().pubkey(); + let tx = add_solver_tx(&svm, ¶ms, &solver); + svm.send_transaction(tx) + .expect("adding a solver should succeed"); + + assert_eq!(solvers(&svm, ¶ms.state_pda), vec![solver]); + + // The account grew by one solver and its balance is unchanged: it was already + // rent-exempt for the new size, so the payer funded nothing. + let account = svm + .get_account(¶ms.state_pda) + .expect("state PDA exists"); + assert_eq!(account.data.len(), WIDTH_HEADER + WIDTH_PUBKEY); + assert_eq!(account.lamports, before, "no extra rent was pulled"); +} + +/// Solvers are stored sorted no matter the order they're added in. Adds many +/// solvers in hash order (effectively unsorted) and checks the stored list came +/// out sorted. +#[test] +fn keeps_solvers_sorted() { + let (mut svm, params) = setup_init(); + + const COUNT: usize = 50; + let mut added: Vec = (0..COUNT).map(|_| unique_keypair().pubkey()).collect(); + for solver in &added { + add_solver(&mut svm, ¶ms, solver).expect("adding a solver should succeed"); + } + + added.sort(); + assert_eq!(solvers(&svm, ¶ms.state_pda), added); +} + +#[test] +fn rejects_adding_an_existing_solver() { + let (mut svm, params) = setup_init(); + let solver = unique_keypair().pubkey(); + add_solver(&mut svm, ¶ms, &solver).expect("first add should succeed"); + + // The re-add is an identical message; move past the first transaction's + // blockhash so it isn't rejected as a duplicate before reaching the program. + svm.expire_blockhash(); + assert_instruction_error( + add_solver(&mut svm, ¶ms, &solver), + to_instruction_error(SettlementError::SolverAlreadyExists), + ); +} + +#[test] +fn rejects_adding_solver_if_manager_is_not_signer() { + let (mut svm, params) = setup_init(); + let solver = unique_keypair().pubkey(); + + let mut ix: Instruction = AddSolver { + program_id: params.program_id, + manager: params.manager.pubkey(), + payer: params.payer.pubkey(), + solver, + } + .into(); + assert!( + ix.accounts[MANAGER_INDEX].is_signer && !ix.accounts[MANAGER_INDEX].is_writable, + "test sanity check failed: MANAGER_INDEX should point to the manager signer" + ); + ix.accounts[MANAGER_INDEX].is_signer = false; + + let res = common::send(&mut svm, ¶ms.payer, vec![ix]); + assert_instruction_error( + res, + to_instruction_error(SettlementError::UnauthorizedSolverManagement), + ); +} + +#[test] +fn rejects_adding_solver_by_non_manager() { + let (mut svm, params) = setup_init(); + let solver = unique_keypair().pubkey(); + + let stranger = unique_keypair(); + let ix = AddSolver { + program_id: params.program_id, + manager: stranger.pubkey(), + payer: params.payer.pubkey(), + solver, + }; + let tx = common::signed_tx(&svm, ¶ms.payer, &stranger, ix); + assert_instruction_error( + svm.send_transaction(tx).map(|_| ()).map_err(|e| e.err), + to_instruction_error(SettlementError::UnauthorizedSolverManagement), + ); +} + +/// Adding a solver still works, and stays sorted, when the list is already +/// large. This test also benchmarks moving a lot of account data. +#[test] +fn add_with_many_existing_solvers() { + let (mut svm, params) = setup_init(); + + /// A deterministic solver address holding `index` big-endian in its leading + /// two bytes and the rest zero, so their relative order is their index's + /// order. + fn indexed_solver(index: u16) -> Pubkey { + let mut bytes = [0u8; 32]; + let leading = index.to_be_bytes(); + bytes[0] = leading[0]; + bytes[1] = leading[1]; + Pubkey::new_from_array(bytes) + } + + // Existing solvers 0x0143, 0x0144, …, written straight into the state PDA after + // its header rather than added one transaction at a time. + const EXISTING: u16 = 1_000; + const NEW_INDEX: u16 = 42; + let mut expected: Vec = (0..=EXISTING) + .filter(|i| *i != NEW_INDEX) + .map(indexed_solver) + .collect(); + let mut account = svm + .get_account(¶ms.state_pda) + .expect("state PDA exists"); + for solver in &expected { + account.data.extend_from_slice(&solver.to_bytes()); + } + account.lamports = svm.minimum_balance_for_rent_exemption(account.data.len()); + svm.set_account(params.state_pda, account) + .expect("set_account should succeed"); + + // Insert 0x0142: it sorts ahead of every existing solver, so it lands at the + // front and the program shifts the whole list. + let extra = indexed_solver(NEW_INDEX); + let tx = add_solver_tx(&svm, ¶ms, &extra); + send_transaction_metered(&mut svm, tx, BenchLabel::AddSolver) + .expect("adding into a large list should succeed"); + + expected.push(extra); + expected.sort(); + assert_eq!(solvers(&svm, ¶ms.state_pda), expected); +} + +#[test] +fn rejects_growing_beyond_the_max_account_size() { + let (mut svm, params) = setup_init(); + + let mut data = svm + .get_account(¶ms.state_pda) + .expect("state PDA exists") + .data; + data.resize(MAX_PERMITTED_DATA_LENGTH as usize, 0); + create_account_at(&mut svm, params.state_pda, ¶ms.program_id, &data); + + let solver = unique_keypair().pubkey(); + let tx = add_solver_tx(&svm, ¶ms, &solver); + let err = svm + .send_transaction(tx) + .expect_err("growing past the max account size should revert") + .err; + + // The revert is the runtime enforcing its account-size limit, not our program: + // our settlement errors surface as `InstructionError::Custom`, whereas this is a + // plain `InvalidArgument` from the rent-exemption sizing check. + assert!( + !matches!( + err, + TransactionError::InstructionError(_, InstructionError::Custom(_)) + ), + "the revert must not be one of our program's errors: {err:?}", + ); + assert_eq!( + err, + TransactionError::InstructionError(0, InstructionError::InvalidArgument), + ); +} + +/// Index of the manager account in an `AddSolver` instruction. +const MANAGER_INDEX: usize = 0; + +#[test] +fn rejects_adding_solver_if_state_pda_is_uninitialized() { + let (mut svm, program_id, payer) = common::setup(); + let manager = unique_keypair(); + let solver = unique_keypair().pubkey(); + + let ix = AddSolver { + program_id, + manager: manager.pubkey(), + payer: payer.pubkey(), + solver, + }; + let tx = common::signed_tx(&svm, &payer, &manager, ix); + let res = svm.send_transaction(tx).map_err(|e| e.err); + assert_instruction_error(res, InstructionError::InvalidAccountData); +} diff --git a/programs/settlement/tests/common/benchmark.rs b/programs/settlement/tests/common/benchmark.rs index 4bc42ed..23f0ee3 100644 --- a/programs/settlement/tests/common/benchmark.rs +++ b/programs/settlement/tests/common/benchmark.rs @@ -24,6 +24,7 @@ pub enum BenchLabel { ReclaimOrder, Settle, TransferAuthority, + AddSolver, } impl fmt::Display for BenchLabel { @@ -38,6 +39,7 @@ impl fmt::Display for BenchLabel { Self::ReclaimOrder => "reclaim_order", Self::Settle => "settle", Self::TransferAuthority => "transfer_authority", + Self::AddSolver => "add_solver", }) } } diff --git a/programs/settlement/tests/common/mod.rs b/programs/settlement/tests/common/mod.rs index a5ea200..08cc4f8 100644 --- a/programs/settlement/tests/common/mod.rs +++ b/programs/settlement/tests/common/mod.rs @@ -166,17 +166,6 @@ pub fn assert_instruction_error_at( ); } -/// Convenience wrapper around [`assert_instruction_error_at`] for the common -/// case of asserting a specific [`SettlementError`]. -#[track_caller] -pub fn assert_settlement_error( - ix_idx: u8, - result: Result, - expected: SettlementError, -) { - assert_instruction_error_at(ix_idx, result, to_instruction_error(expected)); -} - pub fn create_account_at(svm: &mut LiteSVM, address: Pubkey, owner: &Pubkey, data: &[u8]) { let lamports = svm.minimum_balance_for_rent_exemption(data.len()); svm.set_account( diff --git a/programs/settlement/tests/reclaim_buffer.rs b/programs/settlement/tests/reclaim_buffer.rs index 22c6975..ad5607d 100644 --- a/programs/settlement/tests/reclaim_buffer.rs +++ b/programs/settlement/tests/reclaim_buffer.rs @@ -13,7 +13,9 @@ use solana_sdk::{ use crate::common::benchmark::{send_transaction_metered, BenchLabel}; use crate::common::buffer::ensure_buffer_exists; -use crate::common::{unique_pubkey, InitializedParams}; +use crate::common::{ + assert_instruction_error, to_instruction_error, unique_pubkey, InitializedParams, +}; mod common; @@ -262,7 +264,7 @@ fn rejects_the_same_buffer_twice_in_one_instruction() { mints: &[mint, mint], }; let tx = common::signed_tx(&svm, &payer, &reclaim_authority, ix); - common::assert_instruction_error( + assert_instruction_error( svm.send_transaction(tx).map_err(|e| e.err), InstructionError::InvalidAccountData, ); @@ -291,10 +293,9 @@ fn rejects_when_signer_is_not_the_configured_reclaim_authority() { mints: &[mint], }; let tx = common::signed_tx(&svm, &payer, &impostor, ix); - common::assert_settlement_error( - 0, + assert_instruction_error( svm.send_transaction(tx).map_err(|e| e.err), - SettlementError::ReclaimAuthorityMismatch, + to_instruction_error(SettlementError::ReclaimAuthorityMismatch), ); } @@ -335,10 +336,9 @@ fn rejects_when_the_reclaim_authority_does_not_sign() { .expect("instruction should reference the reclaim authority"); authority_meta.is_signer = false; - common::assert_settlement_error( - 0, + assert_instruction_error( common::send(&mut svm, &payer, vec![ix]), - SettlementError::ReclaimAuthorityMismatch, + to_instruction_error(SettlementError::ReclaimAuthorityMismatch), ); assert!( svm.get_account(&buffer_pda).is_some(), diff --git a/programs/settlement/tests/settle_limit_prices.rs b/programs/settlement/tests/settle_limit_prices.rs index 86d4eae..e29d36e 100644 --- a/programs/settlement/tests/settle_limit_prices.rs +++ b/programs/settlement/tests/settle_limit_prices.rs @@ -7,7 +7,7 @@ //! succeeds or is rejected with the expected error. use crate::common::{ - assert_settlement_error, buffer, + assert_instruction_error_at, buffer, order::OrderBuilder, settlement::{BEGIN_INDEX, FINALIZE_INDEX}, setup, to_instruction_error, token, unique_pubkey, @@ -30,6 +30,19 @@ use solana_sdk::{ mod common; +/// Convenience wrapper around [`assert_instruction_error_at`] for asserting a +/// specific [`SettlementError`] at the instruction that produced it: settlements +/// run as a `[BeginSettle, FinalizeSettle]` pair, so the failing instruction +/// isn't always the first. +#[track_caller] +fn assert_settlement_error( + ix_idx: u8, + result: Result, + expected: SettlementError, +) { + assert_instruction_error_at(ix_idx, result, to_instruction_error(expected)); +} + /// Read `intent`'s order PDA and return its persisted `(amount_withdrawn, /// amount_received)` cumulative fill totals. fn order_fill(svm: &LiteSVM, program_id: &Pubkey, intent: &OrderIntent) -> (u64, u64) {