diff --git a/interface/src/lib.rs b/interface/src/lib.rs index 4a8c269..15d3263 100644 --- a/interface/src/lib.rs +++ b/interface/src/lib.rs @@ -148,11 +148,18 @@ pub enum SettlementError { /// to the order's buy token account; its destination differs from the /// `buy_token_account` in the order's intent. PushDestinationMismatch = 21, + /// `FinalizeSettle`: a push doesn't draw funds from the canonical buffer + /// for its destination's mint. + PushSourceNotBuffer = 22, + /// `FinalizeSettle`: a push's destination isn't a valid SPL token account + /// (wrong data length or not owned by the token program), so its mint can't + /// be read to derive the buffer. + InvalidBuyTokenAccount = 23, /// `ReclaimOrder` was called before the order's `valid_to` has elapsed. - OrderNotExpired = 22, + OrderNotExpired = 24, /// `ReclaimOrder`'s `reclaim_recipient` account doesn't match the /// `created_by` address recorded in the order. - ReclaimRecipientMismatch = 23, + ReclaimRecipientMismatch = 25, } impl From for u32 { diff --git a/interface/src/pda/buffer.rs b/interface/src/pda/buffer.rs index cfc505c..e81abe6 100644 --- a/interface/src/pda/buffer.rs +++ b/interface/src/pda/buffer.rs @@ -9,9 +9,13 @@ //! settlement state PDA (see [`crate::pda::state`]), the single authority //! controlling every buffer. +use solana_account_view::AccountView; +use solana_address::Address; +use solana_program_error::ProgramError; use solana_pubkey::Pubkey; use crate::pda::SETTLEMENT_SEED; +use crate::SettlementError; /// Trailing seed identifying the buffer PDAs. pub const BUFFER_SEED: &[u8] = b"buffer"; @@ -26,11 +30,36 @@ pub fn buffer_pda_seeds(mint: &[u8; 32]) -> [&[u8]; 3] { [SETTLEMENT_SEED, mint, BUFFER_SEED] } +/// Canonical seeds for re-deriving the buffer PDA for `mint` with `bump`. +pub fn buffer_pda_signer_seeds<'a>(mint: &'a [u8; 32], bump: &'a [u8; 1]) -> [&'a [u8]; 4] { + let [s0, s1, s2] = buffer_pda_seeds(mint); + [s0, s1, s2, bump] +} + /// Derive the canonical buffer PDA address (and bump) for the token `mint`. pub fn find_buffer_pda(program_id: &Pubkey, mint: &Pubkey) -> (Pubkey, u8) { Pubkey::find_program_address(&buffer_pda_seeds(mint.as_array()), program_id) } +/// Confirm `buffer` matches the derived buffer PDA for `mint` and `bump`. +#[must_use = "ignoring the output means ignoring the validation result"] +pub fn validate_buffer_pda( + program_id: &Address, + buffer: &AccountView, + mint: &Address, + bump: u8, +) -> Result<(), ProgramError> { + let derived = Address::create_program_address( + &buffer_pda_signer_seeds(&mint.to_bytes(), &[bump]), + program_id, + ) + .map_err(|_| SettlementError::PushSourceNotBuffer)?; + if buffer.address() != &derived { + return Err(SettlementError::PushSourceNotBuffer.into()); + } + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -45,6 +74,43 @@ mod tests { ); } + #[test] + fn accepts_a_valid_pda() { + let program_id = Pubkey::new_unique(); + let mint = Pubkey::new_unique(); + let (pda, bump) = find_buffer_pda(&program_id, &mint); + + let buffer = crate::instruction::fixtures::fake_account(pda); + validate_buffer_pda(&program_id, &buffer, &mint, bump) + .expect("the canonical buffer PDA must be accepted"); + } + + #[test] + fn rejects_an_invalid_address() { + let program_id = Pubkey::new_unique(); + let mint = Pubkey::new_unique(); + let (_, bump) = find_buffer_pda(&program_id, &mint); + + // An account sitting at some other address is not the buffer. + let buffer = crate::instruction::fixtures::fake_account(Pubkey::new_unique()); + let err = validate_buffer_pda(&program_id, &buffer, &mint, bump) + .expect_err("a non-canonical address must be rejected"); + assert_eq!(err, SettlementError::PushSourceNotBuffer.into()); + } + + #[test] + fn rejects_a_wrong_bump() { + let program_id = Pubkey::new_unique(); + let mint = Pubkey::new_unique(); + let (pda, bump) = find_buffer_pda(&program_id, &mint); + + // The address is canonical but the carried bump doesn't derive it. + let buffer = crate::instruction::fixtures::fake_account(pda); + let err = validate_buffer_pda(&program_id, &buffer, &mint, bump ^ 1) + .expect_err("a wrong bump must be rejected"); + assert_eq!(err, SettlementError::PushSourceNotBuffer.into()); + } + mod proptest { use ::proptest::prelude::*; diff --git a/interface/src/pda/state.rs b/interface/src/pda/state.rs index 34d3d64..aebe0dd 100644 --- a/interface/src/pda/state.rs +++ b/interface/src/pda/state.rs @@ -14,6 +14,13 @@ pub fn state_pda_seeds<'a>() -> [&'a [u8]; 1] { [SETTLEMENT_SEED] } +/// Canonical seeds for signing as the settlement state PDA with `bump`. The +/// on-chain settlement handlers use this to construct the CPI signer. +pub fn state_pda_signer_seeds(bump: &[u8; 1]) -> [&[u8]; 2] { + let [seed] = state_pda_seeds(); + [seed, bump] +} + /// Derive the canonical settlement state PDA address (and bump). pub fn find_state_pda(program_id: &Pubkey) -> (Pubkey, u8) { Pubkey::find_program_address(&state_pda_seeds(), program_id) diff --git a/programs/settlement/src/settle/begin.rs b/programs/settlement/src/settle/begin.rs index 07c82d7..6703447 100644 --- a/programs/settlement/src/settle/begin.rs +++ b/programs/settlement/src/settle/begin.rs @@ -3,8 +3,7 @@ use std::ops::Deref; use pinocchio::{ - cpi::{Seed, Signer}, - error::ProgramError, + cpi::Signer, sysvars::{ clock::Clock, instructions::{Instructions, IntrospectedInstruction}, @@ -16,17 +15,15 @@ use pinocchio_token::{instructions::Transfer, state::Account as TokenAccount}; use settlement_interface::{ data::order::OrderAccount, instruction::{ - create_buffer::SPL_TOKEN_PROGRAM_ID, settle::{BeginSettleInput, SettledOrder, FINALIZE_FIXED_ACCOUNTS}, InstructionInputParsing, }, - pda::state::state_pda_seeds, recover_discriminator, Pubkey, SettlementError, SettlementInstruction, }; use crate::processor::is_cpi_call; -use super::validate_counterpart; +use super::{validate_counterpart, validate_token_program_account, with_state_pda_signer}; pub fn process_begin_settle( program_id: &Address, @@ -65,15 +62,17 @@ pub fn process_begin_settle( let finalize_ix = instructions.load_instruction_at(usize::from(input.finalize_ix_index))?; - settle_orders( - program_id, - input.token_program_account, - input.state_pda_account, - input.orders.iter(), - &finalize_ix, - )?; + validate_token_program_account(input.token_program_account)?; - Ok(()) + with_state_pda_signer(program_id, input.state_pda_account, |state_pda_signer| { + settle_orders( + program_id, + input.state_pda_account, + state_pda_signer, + input.orders.iter(), + &finalize_ix, + ) + }) } /// The destination address of each push carried by the paired `FinalizeSettle`, @@ -144,10 +143,9 @@ fn validate_no_nested_settlement>( Ok(()) } -/// Validate each order against its push, and pull user funds. This requires: -/// - the legacy SPL Token program; -/// - the canonical state PDA, which signs each transfer as the user's delegate; -/// - orders strictly increasing by address, rejecting duplicates. +/// Validate each order against its push, and pull user funds, signing the pulls +/// as the canonical state PDA (the user's delegate). Orders must be strictly +/// increasing by address, which rejects duplicates. /// /// Each order is paid by exactly one push. The orders and the finalize's pushes /// are both laid out sorted by order PDA, so order `i` is paid by push `i`, and @@ -160,27 +158,11 @@ fn validate_no_nested_settlement>( #[must_use = "ignoring the output may lead to an unintended on-chain state"] fn settle_orders<'a>( program_id: &Address, - token_program_account: &AccountView, state_pda_account: &AccountView, + state_pda_signer: &Signer, orders: impl IntoIterator>, finalize_ix: &IntrospectedInstruction, ) -> ProgramResult { - if token_program_account.address() != &SPL_TOKEN_PROGRAM_ID { - return Err(ProgramError::IncorrectProgramId); - } - - // Funds are pulled with the state PDA's delegation, so it must be the signer. - let seeds = state_pda_seeds(); - let (state_pda, state_bump) = Address::find_program_address(&seeds, program_id); - if state_pda_account.address() != &state_pda { - return Err(SettlementError::StateAccountMismatch.into()); - } - - let [seed] = seeds; - let state_bump = [state_bump]; - let signer_seeds = [seed, &state_bump].map(Seed::from); - let state_pda_signer = Signer::from(&signer_seeds); - // Orders must be passed strictly increasing by address; this rejects // duplicates (settling the same order twice) without a separate scan. let mut previous: Option<&Address> = None; @@ -208,7 +190,7 @@ fn settle_orders<'a>( push_destination, now, state_pda_account, - &state_pda_signer, + state_pda_signer, )?; } diff --git a/programs/settlement/src/settle/finalize.rs b/programs/settlement/src/settle/finalize.rs index 403e2db..9026ebe 100644 --- a/programs/settlement/src/settle/finalize.rs +++ b/programs/settlement/src/settle/finalize.rs @@ -1,14 +1,21 @@ //! `FinalizeSettle` instruction handler. -use pinocchio::{sysvars::instructions::Instructions, AccountView, Address, ProgramResult}; +use pinocchio::{ + cpi::Signer, sysvars::instructions::Instructions, AccountView, Address, ProgramResult, +}; +use pinocchio_token::{instructions::Transfer, state::Account as TokenAccount}; use settlement_interface::{ - instruction::{settle::FinalizeSettleInput, InstructionInputParsing}, + instruction::{ + settle::{FinalizeSettleInput, Pushes}, + InstructionInputParsing, + }, + pda::buffer::validate_buffer_pda, SettlementError, SettlementInstruction, }; use crate::processor::is_cpi_call; -use super::validate_counterpart; +use super::{validate_counterpart, validate_token_program_account, with_state_pda_signer}; pub fn process_finalize_settle( program_id: &Address, @@ -31,9 +38,53 @@ pub fn process_finalize_settle( current_index, input.begin_ix_index, SettlementInstruction::BeginSettle, - ) + )?; + + // `BeginSettle` (which the counterpart check above guarantees ran) already + // validated the push count and destinations. `push_funds` adds the only + // remaining check: each push draws from the buffer for its mint. + + validate_token_program_account(input.token_program_account)?; + + with_state_pda_signer(program_id, input.state_pda_account, |state_pda_signer| { + push_funds( + program_id, + input.state_pda_account, + state_pda_signer, + input.pushes, + ) + }) +} + +/// Push each order's proceeds out of the settlement's buffers, signing each +/// transfer as the canonical state PDA (the buffers' SPL authority). Each push's +/// source must be the derived buffer for its destination's mint; pairing the +/// destination to an order is `BeginSettle`'s job. +#[must_use = "ignoring the output may lead to an unintended on-chain state"] +fn push_funds<'a>( + program_id: &Address, + state_pda_account: &AccountView, + state_pda_signer: &Signer, + pushes: Pushes<'a>, +) -> ProgramResult { + for push in pushes.iter() { + // Read the destination's mint; the borrow ends with this block, before + // the transfer reuses the account. + let mint = { + let destination = TokenAccount::from_account_view(push.destination) + .map_err(|_| SettlementError::InvalidBuyTokenAccount)?; + *destination.mint() + }; + validate_buffer_pda(program_id, push.source_buffer, &mint, push.bump)?; + + Transfer::new( + push.source_buffer, + push.destination, + state_pda_account, + u64::from_le_bytes(*push.amount), + ) + .invoke_signed(core::slice::from_ref(state_pda_signer))?; + } - // Some checks are carried out by `BeginSettle` and we don't repeat them - // under the assumption that the counterpart exists and, since it's a - // `BeginSettle`, it performs the checks. + Ok(()) } diff --git a/programs/settlement/src/settle/mod.rs b/programs/settlement/src/settle/mod.rs index 9a652e8..5776d25 100644 --- a/programs/settlement/src/settle/mod.rs +++ b/programs/settlement/src/settle/mod.rs @@ -2,10 +2,16 @@ use std::ops::Deref; -use pinocchio::{sysvars::instructions::Instructions, Address, ProgramResult}; +use pinocchio::{ + cpi::{Seed, Signer}, + error::ProgramError, + sysvars::instructions::Instructions, + AccountView, Address, ProgramResult, +}; use settlement_interface::{ - instruction::settle::recover_counterpart, recover_discriminator, SettlementError, - SettlementInstruction, + instruction::{create_buffer::SPL_TOKEN_PROGRAM_ID, settle::recover_counterpart}, + pda::state::{state_pda_seeds, state_pda_signer_seeds}, + recover_discriminator, SettlementError, SettlementInstruction, }; mod begin; @@ -42,3 +48,35 @@ fn validate_counterpart>( } Ok(()) } + +/// Validate that `token_program_account` is the legacy SPL Token program, which +/// every settlement transfer is issued against. +#[must_use = "ignoring the output may lead to an unintended on-chain state"] +fn validate_token_program_account(token_program_account: &AccountView) -> ProgramResult { + if token_program_account.address() != &SPL_TOKEN_PROGRAM_ID { + return Err(ProgramError::IncorrectProgramId); + } + Ok(()) +} + +/// Validate that `state_pda_account` is the canonical state PDA and run `f` with +/// a signer for it. Both settlement transfers move funds under the state PDA's +/// authority, so it must sign each of them. +/// +/// The signer only borrows its seed buffers, which are local to this frame; +/// running `f` here rather than returning the signer keeps them alive for as +/// long as `f` needs it. +fn with_state_pda_signer( + program_id: &Address, + state_pda_account: &AccountView, + f: impl FnOnce(&Signer) -> ProgramResult, +) -> ProgramResult { + let (state_pda, state_bump) = Address::find_program_address(&state_pda_seeds(), program_id); + if state_pda_account.address() != &state_pda { + return Err(SettlementError::StateAccountMismatch.into()); + } + + let state_bump = [state_bump]; + let signer_seeds = state_pda_signer_seeds(&state_bump).map(Seed::from); + f(&Signer::from(&signer_seeds)) +} diff --git a/programs/settlement/tests/begin_settle_orders.rs b/programs/settlement/tests/begin_settle_orders.rs index 0eb327e..e120ad7 100644 --- a/programs/settlement/tests/begin_settle_orders.rs +++ b/programs/settlement/tests/begin_settle_orders.rs @@ -18,9 +18,11 @@ //! whose output is a properly built instruction. use crate::common::{ - assert_instruction_error, assert_settlement_error, buffer, create_account, + assert_instruction_error, buffer, create_account, order::{create_order_pda, sample_intent, OrderBuilder}, - replace_first_matching_account, send, set_unix_timestamp, setup, token, + replace_first_matching_account, send, set_unix_timestamp, + settlement::{build_settlement, BEGIN_INDEX, FINALIZE_INDEX}, + setup, to_instruction_error, token, }; use litesvm::LiteSVM; use litesvm_token::spl_token::error::TokenError; @@ -42,16 +44,22 @@ use solana_sdk::{ instruction::{AccountMeta, InstructionError}, pubkey::Pubkey, signature::{Keypair, Signer}, - transaction::Transaction, + transaction::{Transaction, TransactionError}, }; mod common; -/// Position of `BeginSettle` in the `[BeginSettle, FinalizeSettle]` pair the -/// tests in this file build; the finalize sits right after it. Kept in sync with -/// the tests that reach into `instructions[BEGIN_INDEX]` to corrupt the begin. -const BEGIN_INDEX: u8 = 0; -const FINALIZE_INDEX: u8 = 1; +/// Assert the transaction failed in `BeginSettle` (at [`BEGIN_INDEX`]) with +/// `expected`. +fn assert_begin_error(result: Result, expected: SettlementError) { + assert_eq!( + result.err(), + Some(TransactionError::InstructionError( + BEGIN_INDEX, + to_instruction_error(expected), + )), + ); +} /// A list of empty transfer lists, one per order. Used for settling `n` orders /// without pulling any funds. @@ -165,7 +173,7 @@ fn rejects_wrong_bump() { amounts: &[0], }; let instructions = vec![begin.into(), finalize.into()]; - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::AccountNotDerivable, ); @@ -213,7 +221,7 @@ fn rejects_fabricated_program_owned_account() { }; let instructions = vec![begin.into(), finalize.into()]; - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::AccountNotDerivable, ); @@ -280,7 +288,7 @@ fn rejects_sell_token_account_mismatch() { wrong_sell_token, ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::SellTokenAccountMismatch, ); @@ -311,7 +319,7 @@ fn rejects_sell_token_owner_mismatch() { pulls: &[], }], ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::SellTokenOwnerMismatch, ); @@ -340,7 +348,7 @@ fn rejects_non_token_sell_account() { pulls: &[], }], ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::SellTokenAccountInvalid, ); @@ -366,7 +374,7 @@ fn rejects_duplicate_orders() { }, ], ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::OrdersNotStrictlyIncreasing, ); @@ -450,7 +458,7 @@ fn rejects_orders_in_wrong_address_order() { amounts: &amounts, }); let instructions = vec![begin, finalize]; - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::OrdersNotStrictlyIncreasing, ); @@ -497,7 +505,7 @@ fn rejects_cancelled_order() { pulls: &[], }], ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::OrderCancelled, ); @@ -523,7 +531,7 @@ fn rejects_expired_order() { pulls: &[], }], ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::OrderExpired, ); @@ -781,7 +789,7 @@ fn rejects_wrong_state_pda() { Pubkey::new_unique(), ); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::StateAccountMismatch, ); @@ -917,8 +925,83 @@ fn rejects_extra_account() { .accounts .push(AccountMeta::new_readonly(Pubkey::new_unique(), false)); - assert_settlement_error( + assert_begin_error( send(&mut svm, &payer, instructions), SettlementError::AccountCountNotMatchingOrderCount, ); } + +#[test] +fn rejects_push_to_wrong_destination() { + let (mut svm, program_id, payer) = setup(); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + let orders = [FinalizedIntent { + intent: &intent, + mint: Pubkey::new_unique(), + amount: 100, + }]; + + let mut finalize = Instruction::from(FinalizeSettle { + program_id, + begin_ix_index: BEGIN_INDEX.into(), + orders: &orders, + }); + // Redirect the push to an account that isn't the order's buy token account. + // Accounts: `[sysvar, state, token_program, source, destination]`. + let destination_index = 4; + finalize.accounts[destination_index].pubkey = Pubkey::new_unique(); + + let instructions = build_settlement(&program_id, &orders, finalize); + assert_begin_error( + send(&mut svm, &payer, instructions), + SettlementError::PushDestinationMismatch, + ); +} + +#[test] +fn rejects_fewer_pushes_than_orders() { + let (mut svm, program_id, payer) = setup(); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + let orders = [FinalizedIntent { + intent: &intent, + mint: Pubkey::new_unique(), + amount: 100, + }]; + + // A finalize carrying no pushes, paired with a begin settling one order. + let finalize = FinalizeSettle { + program_id, + begin_ix_index: BEGIN_INDEX.into(), + orders: &[], + }; + + let instructions = build_settlement(&program_id, &orders, finalize); + assert_begin_error( + send(&mut svm, &payer, instructions), + SettlementError::SettledOrderPushCountMismatch, + ); +} + +#[test] +fn rejects_more_pushes_than_orders() { + let (mut svm, program_id, payer) = setup(); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + + // A finalize that pushes to one order, paired with a begin that settles none, + // so the extra push has no order to account for it. + let finalize = FinalizeSettle { + program_id, + begin_ix_index: BEGIN_INDEX.into(), + orders: &[FinalizedIntent { + intent: &intent, + mint: Pubkey::new_unique(), + amount: 0, + }], + }; + + let instructions = build_settlement(&program_id, &[], finalize); + assert_begin_error( + send(&mut svm, &payer, instructions), + SettlementError::SettledOrderPushCountMismatch, + ); +} diff --git a/programs/settlement/tests/common/mod.rs b/programs/settlement/tests/common/mod.rs index 53eb059..6f3d5ca 100644 --- a/programs/settlement/tests/common/mod.rs +++ b/programs/settlement/tests/common/mod.rs @@ -9,6 +9,7 @@ pub mod buffer; pub mod lookup_table; pub mod order; pub mod pda; +pub mod settlement; pub mod token; use litesvm::{types::TransactionMetadata, LiteSVM}; @@ -76,9 +77,6 @@ pub fn assert_instruction_error( Some(TransactionError::InstructionError(0, expected)) ); } -pub fn assert_settlement_error(result: Result, expected: SettlementError) { - assert_instruction_error(result, to_instruction_error(expected)); -} /// Place a fresh, rent-exempt account holding `data` and owned by `owner` at a /// new address, and return it. Lets a test populate an arbitrary account (e.g. diff --git a/programs/settlement/tests/common/settlement.rs b/programs/settlement/tests/common/settlement.rs new file mode 100644 index 0000000..29599c3 --- /dev/null +++ b/programs/settlement/tests/common/settlement.rs @@ -0,0 +1,35 @@ +//! Scaffolding for building `[BeginSettle, FinalizeSettle]` settlement pairs. + +use settlement_client::instructions::{BeginSettle, FinalizedIntent, InitializedIntent}; +use settlement_interface::Instruction; +use solana_sdk::pubkey::Pubkey; + +/// Positions of the two instructions in the `[BeginSettle, FinalizeSettle]` pair +/// the settlement tests build: begin first, finalize right after it. Each +/// instruction points at the other through its `begin_ix_index`/`finalize_ix_index`. +pub const BEGIN_INDEX: u8 = 0; +pub const FINALIZE_INDEX: u8 = 1; + +/// Build the `[begin, finalize]` instructions where `finalize` is a pre-built +/// `FinalizeSettle` at [`FINALIZE_INDEX`] and `begin` settles `orders` (with no +/// pulls) at [`BEGIN_INDEX`], the same orders the finalize is expected to push +/// to. Submit the result with [`send`](super::send). +pub fn build_settlement( + program_id: &Pubkey, + orders: &[FinalizedIntent], + finalize: impl Into, +) -> Vec { + let begin_orders: Vec = orders + .iter() + .map(|order| InitializedIntent { + intent: order.intent, + pulls: &[], + }) + .collect(); + let begin = BeginSettle { + program_id: *program_id, + finalize_ix_index: FINALIZE_INDEX.into(), + orders: &begin_orders, + }; + vec![begin.into(), finalize.into()] +} diff --git a/programs/settlement/tests/finalize_settle_pushes.rs b/programs/settlement/tests/finalize_settle_pushes.rs index 3e108d2..52a06eb 100644 --- a/programs/settlement/tests/finalize_settle_pushes.rs +++ b/programs/settlement/tests/finalize_settle_pushes.rs @@ -1,48 +1,40 @@ -//! Integration tests for the fund-push list carried by `FinalizeSettle`. +//! Integration tests for the fund pushes carried by `FinalizeSettle` and +//! validated by `BeginSettle`. +//! +//! Each settlement transaction is a `[BeginSettle, FinalizeSettle]` pair (begin +//! at [`BEGIN_INDEX`] pointing to finalize at [`FINALIZE_INDEX`], and vice +//! versa). `BeginSettle` settles the orders the finalize pays (created on-chain +//! via `OrderBuilder` with no pulls, so only the push side moves funds) and +//! validates that each order is paid by exactly one push to its buy token +//! account. `FinalizeSettle` then executes the transfers out of the buffers, +//! signed by the settlement state PDA that owns them. -use crate::common::{order::OrderBuilder, send, setup, to_instruction_error, token}; -use settlement_client::instructions::{ - BeginSettle, FinalizeSettle, FinalizedIntent, InitializedIntent, +use crate::common::{ + buffer, create_account, + order::{create_order_pda, sample_intent, OrderBuilder}, + replace_first_matching_account, send, + settlement::{build_settlement, BEGIN_INDEX, FINALIZE_INDEX}, + setup, to_instruction_error, token, +}; +use settlement_client::instructions::{FinalizeSettle, FinalizedIntent}; +use settlement_client::settlement_interface::{ + instruction::settle::SPL_TOKEN_PROGRAM_ID, pda::state::find_state_pda, Instruction, + SettlementError, }; -use settlement_client::settlement_interface::{Instruction, SettlementError}; use solana_sdk::{ - instruction::{AccountMeta, InstructionError}, - program_error::ProgramError, - pubkey::Pubkey, + instruction::InstructionError, program_error::ProgramError, pubkey::Pubkey, signature::Signer, transaction::TransactionError, }; mod common; -/// Position of `BeginSettle` in the `[BeginSettle, FinalizeSettle]` pair the -/// tests in this file build; the finalize sits right after it. Kept in sync with -/// the `assert_eq!(index, FINALIZE_INDEX)` checks that a rejection came from the -/// finalize. -const BEGIN_INDEX: u8 = 0; -const FINALIZE_INDEX: u8 = 1; - -/// Build the `[begin, finalize]` instructions where `finalize` is a pre-built -/// `FinalizeSettle` at [`FINALIZE_INDEX`] and `begin` settles `orders` (with no -/// pulls) at [`BEGIN_INDEX`], the same orders the finalize is expected to push -/// to. Submit the result with [`send`]. -fn build_settlement( - program_id: &Pubkey, - orders: &[FinalizedIntent], - finalize: impl Into, -) -> Vec { - let begin_orders: Vec = orders - .iter() - .map(|order| InitializedIntent { - intent: order.intent, - pulls: &[], - }) - .collect(); - let begin = BeginSettle { - program_id: *program_id, - finalize_ix_index: FINALIZE_INDEX.into(), - orders: &begin_orders, - }; - vec![begin.into(), finalize.into()] +/// Assert the transaction failed in `FinalizeSettle` (at [`FINALIZE_INDEX`]) +/// with `expected`. +fn assert_finalize_error(result: Result, expected: InstructionError) { + assert_eq!( + result.err(), + Some(TransactionError::InstructionError(FINALIZE_INDEX, expected)), + ); } /// Build the minimal `[BeginSettle, FinalizeSettle]` instructions that settle @@ -65,54 +57,76 @@ fn finalizes_with_no_pushes() { } #[test] -fn finalizes_with_single_push() { +fn pushes_a_single_order() { let (mut svm, program_id, payer) = setup(); - let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + let mint = token::create_mint(&mut svm, &payer); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer) + .buy_mint(&mint) + .build(); + let funding = 1_000; + let buffer_pda = buffer::ensure_funded(&mut svm, &program_id, &payer, &mint, funding); + let amount = 400; let instructions = finalize( &program_id, &[FinalizedIntent { intent: &intent, - mint: Pubkey::new_unique(), - amount: 1_000, + mint, + amount, }], ); - send(&mut svm, &payer, instructions).expect("a single push should parse and be accepted"); + send(&mut svm, &payer, instructions).expect("a single push should be paid"); + + assert_eq!(token::balance(&svm, &intent.buy_token_account), amount); + assert_eq!(token::balance(&svm, &buffer_pda), funding - amount); } #[test] -fn finalizes_with_several_pushes_same_mint() { +fn pushes_several_orders_from_one_buffer() { let (mut svm, program_id, payer) = setup(); let mint = token::create_mint(&mut svm, &payer); + // Distinct orders (each `OrderBuilder` makes fresh sell and buy token + // accounts) sharing one buy mint, so both pushes draw from one buffer. let intent0 = OrderBuilder::new(&mut svm, &program_id, &payer) - .salt(1) .buy_mint(&mint) + .salt(0) .build(); let intent1 = OrderBuilder::new(&mut svm, &program_id, &payer) - .salt(2) .buy_mint(&mint) + .salt(1) .build(); + let funding = 10_000; + let buffer_pda = buffer::ensure_funded(&mut svm, &program_id, &payer, &mint, funding); + let amount0 = 1_000; + let amount1 = 2_000; let instructions = finalize( &program_id, &[ FinalizedIntent { intent: &intent0, mint, - amount: 1_000, + amount: amount0, }, FinalizedIntent { intent: &intent1, mint, - amount: 2_000, + amount: amount1, }, ], ); - send(&mut svm, &payer, instructions).expect("several pushes should parse and be accepted"); + send(&mut svm, &payer, instructions).expect("several pushes from one buffer should be paid"); + + assert_eq!(token::balance(&svm, &intent0.buy_token_account), amount0); + assert_eq!(token::balance(&svm, &intent1.buy_token_account), amount1); + assert_eq!( + token::balance(&svm, &buffer_pda), + funding - amount0 - amount1, + ); } #[test] -fn finalizes_with_several_pushes_different_mint() { +fn pushes_several_orders_from_different_buffers() { let (mut svm, program_id, payer) = setup(); let mint0 = token::create_mint(&mut svm, &payer); let mint1 = token::create_mint(&mut svm, &payer); @@ -122,23 +136,137 @@ fn finalizes_with_several_pushes_different_mint() { let intent1 = OrderBuilder::new(&mut svm, &program_id, &payer) .buy_mint(&mint1) .build(); + let funding = 5_000; + let buffer0 = buffer::ensure_funded(&mut svm, &program_id, &payer, &mint0, funding); + let buffer1 = buffer::ensure_funded(&mut svm, &program_id, &payer, &mint1, funding); + let amount0 = 1_000; + let amount1 = 2_000; let instructions = finalize( &program_id, &[ FinalizedIntent { intent: &intent0, mint: mint0, - amount: 1_000, + amount: amount0, }, FinalizedIntent { intent: &intent1, mint: mint1, - amount: 2_000, + amount: amount1, }, ], ); - send(&mut svm, &payer, instructions).expect("several pushes should parse and be accepted"); + send(&mut svm, &payer, instructions).expect("pushes from different buffers should be paid"); + + assert_eq!(token::balance(&svm, &intent0.buy_token_account), amount0); + assert_eq!(token::balance(&svm, &intent1.buy_token_account), amount1); + assert_eq!(token::balance(&svm, &buffer0), funding - amount0); + assert_eq!(token::balance(&svm, &buffer1), funding - amount1); +} + +#[test] +fn rejects_push_if_buffer_does_not_match_mint() { + let (mut svm, program_id, payer) = setup(); + let buy_mint = token::create_mint(&mut svm, &payer); + let other_mint = token::create_mint(&mut svm, &payer); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer) + .buy_mint(&buy_mint) + .build(); + // The push draws from the buffer for `other_mint`, not the buy token's mint, + // which `FinalizeSettle` rejects when it reads the destination's mint. + let orders = [FinalizedIntent { + intent: &intent, + mint: other_mint, + amount: 100, + }]; + + let instructions = finalize(&program_id, &orders); + assert_finalize_error( + send(&mut svm, &payer, instructions), + to_instruction_error(SettlementError::PushSourceNotBuffer), + ); +} + +#[test] +fn rejects_push_from_substituted_source() { + let (mut svm, program_id, payer) = setup(); + let mint = token::create_mint(&mut svm, &payer); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer) + .buy_mint(&mint) + .build(); + buffer::ensure_funded(&mut svm, &program_id, &payer, &mint, 1_000); + let orders = [FinalizedIntent { + intent: &intent, + mint, + amount: 100, + }]; + + let mut finalize = Instruction::from(FinalizeSettle { + program_id, + begin_ix_index: BEGIN_INDEX.into(), + orders: &orders, + }); + // Point the push at an account that isn't the canonical buffer, leaving the + // rest well-formed. Accounts: `[sysvar, state, token_program, source, + // destination]`. `BeginSettle` doesn't validate the source, so it passes; + // `FinalizeSettle` re-derives the buffer from the destination's mint and + // rejects the mismatch before touching the substituted account. + let source_index = 3; + finalize.accounts[source_index].pubkey = Pubkey::new_unique(); + + let instructions = build_settlement(&program_id, &orders, finalize); + assert_finalize_error( + send(&mut svm, &payer, instructions), + to_instruction_error(SettlementError::PushSourceNotBuffer), + ); +} + +#[test] +fn rejects_wrong_token_program() { + let (mut svm, program_id, payer) = setup(); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + let orders = [FinalizedIntent { + intent: &intent, + mint: Pubkey::new_unique(), + amount: 0, + }]; + + let mut instructions = finalize(&program_id, &orders); + replace_first_matching_account( + &mut instructions[usize::from(FINALIZE_INDEX)], + &SPL_TOKEN_PROGRAM_ID, + Pubkey::new_unique(), + ); + + assert_finalize_error( + send(&mut svm, &payer, instructions), + InstructionError::IncorrectProgramId, + ); +} + +#[test] +fn rejects_wrong_state_pda() { + let (mut svm, program_id, payer) = setup(); + let intent = OrderBuilder::new(&mut svm, &program_id, &payer).build(); + let orders = [FinalizedIntent { + intent: &intent, + mint: Pubkey::new_unique(), + amount: 0, + }]; + + let mut instructions = finalize(&program_id, &orders); + let (state_pda, _bump) = find_state_pda(&program_id); + replace_first_matching_account( + &mut instructions[usize::from(FINALIZE_INDEX)], + &state_pda, + Pubkey::new_unique(), + ); + + assert_finalize_error( + send(&mut svm, &payer, instructions), + to_instruction_error(SettlementError::StateAccountMismatch), + ); } #[test] @@ -148,27 +276,27 @@ fn rejects_push_account_count_mismatch() { let orders = [FinalizedIntent { intent: &intent, mint: Pubkey::new_unique(), - amount: 1_000, + amount: 100, }]; - // A well-formed single-push finalize... + // A well-formed single-push finalize (five accounts, a nine-byte push body)... let mut finalize = Instruction::from(FinalizeSettle { program_id, begin_ix_index: BEGIN_INDEX.into(), orders: &orders, }); - // ...with one extra account appended. - finalize - .accounts - .push(AccountMeta::new_readonly(Pubkey::new_unique(), false)); + // ...with another push's worth of data bytes appended but no matching + // accounts. `BeginSettle` derives the push count from the (unchanged) account + // metas (one push, matching its one order and paying the right destination) + // so it passes. Only the finalize reads the data, where it now parses two + // pushes against two push accounts and rejects the mismatch. This is the + // account/data disagreement `BeginSettle` structurally can't see. + finalize.data.extend_from_slice(&[0u8; 9]); let instructions = build_settlement(&program_id, &orders, finalize); - assert_eq!( + assert_finalize_error( send(&mut svm, &payer, instructions), - Err(TransactionError::InstructionError( - FINALIZE_INDEX, - to_instruction_error(SettlementError::AccountCountNotMatchingPushCount), - )), + to_instruction_error(SettlementError::AccountCountNotMatchingPushCount), ); } @@ -176,26 +304,29 @@ fn rejects_push_account_count_mismatch() { fn rejects_too_few_accounts() { let (mut svm, program_id, payer) = setup(); - // A well-formed single-push finalize... + // A well-formed no-push finalize... let mut finalize = Instruction::from(FinalizeSettle { program_id, begin_ix_index: BEGIN_INDEX.into(), orders: &[], }); - // ...with one account popped. + // ...with one of its three fixed accounts popped. `BeginSettle` runs first + // but only reads push destinations off the accounts (finding none, matching + // its zero orders) so it passes. The finalize then can't even destructure + // its fixed accounts and raises `NotEnoughAccountKeys`. finalize.accounts.pop(); let instructions = build_settlement(&program_id, &[], finalize); - let result = send(&mut svm, &payer, instructions); - let Err(TransactionError::InstructionError(index, ix_error)) = result else { - panic!("expected an instruction error, got {result:?}"); + let err = send(&mut svm, &payer, instructions) + .expect_err("a finalize missing a fixed account must be rejected"); + let TransactionError::InstructionError(FINALIZE_INDEX, ix_err) = err else { + panic!("expected the finalize (index {FINALIZE_INDEX}) to fail, got {err:?}"); }; - assert_eq!(index, FINALIZE_INDEX); + // Compare against the non-deprecated `ProgramError` variant the program + // returns; naming the `InstructionError` variant directly would touch a + // deprecated alias. assert_eq!( - // This unusual way to test is because `InstructionError::NotEnoughAccountKeys` - // is deprecated, while `ProgramError::NotEnoughAccountKeys` is not. - // Rather than silencing a linting, let's use the program error. - ProgramError::try_from(ix_error), + ProgramError::try_from(ix_err), Ok(ProgramError::NotEnoughAccountKeys), ); } @@ -229,12 +360,71 @@ fn rejects_two_too_few_accounts() { // destinations: the inconsistency is left for the finalize's own // account-count check to reject. let instructions = build_settlement(&program_id, &[], finalize); - assert_eq!( + assert_finalize_error( + send(&mut svm, &payer, instructions), + to_instruction_error(SettlementError::AccountCountNotMatchingPushCount), + ); +} + +#[test] +fn rejects_invalid_buy_token_account() { + let (mut svm, program_id, payer) = setup(); + let mint = token::create_mint(&mut svm, &payer); + let sell_token = token::create_token_account(&mut svm, &payer, &mint, &payer.pubkey()); + + // The order's buy token account (the push destination) isn't a token account, + // so `FinalizeSettle` can't read its mint to derive the buffer. `BeginSettle` + // accepts it: the push destination still matches the intent's buy token. + let not_a_token_account = Pubkey::new_unique(); + let mut intent = sample_intent(payer.pubkey(), sell_token, 0); + intent.buy_token_account = not_a_token_account; + create_order_pda(&mut svm, &program_id, &payer, &intent); + let orders = [FinalizedIntent { + intent: &intent, + mint, + amount: 0, + }]; + + let instructions = finalize(&program_id, &orders); + assert_finalize_error( send(&mut svm, &payer, instructions), - Err(TransactionError::InstructionError( - FINALIZE_INDEX, - to_instruction_error(SettlementError::AccountCountNotMatchingPushCount), - )), + to_instruction_error(SettlementError::InvalidBuyTokenAccount), + ); +} + +#[test] +fn rejects_buy_token_account_owned_by_wrong_program() { + let (mut svm, program_id, payer) = setup(); + let mint = token::create_mint(&mut svm, &payer); + let sell_token = token::create_token_account(&mut svm, &payer, &mint, &payer.pubkey()); + + // Genuine token-account bytes: right length, a real mint at offset 0, so + // `from_account_view` would gladly read the mint. Assign those exact bytes + // to an account not owned by the token program, so it's not a valid + // token account. + let genuine = token::create_token_account(&mut svm, &payer, &mint, &payer.pubkey()); + let token_shaped = svm + .get_account(&genuine) + .expect("the genuine token account exists") + .data; + let impostor = create_account(&mut svm, &Pubkey::new_unique(), &token_shaped); + + // `BeginSettle` only checks the push destination matches the intent's buy + // token, so it accepts the impostor; `FinalizeSettle` rejects it when the + // owner check in `from_account_view` fails, before the mint is ever read. + let mut intent = sample_intent(payer.pubkey(), sell_token, 0); + intent.buy_token_account = impostor; + create_order_pda(&mut svm, &program_id, &payer, &intent); + let orders = [FinalizedIntent { + intent: &intent, + mint, + amount: 0, + }]; + + let instructions = finalize(&program_id, &orders); + assert_finalize_error( + send(&mut svm, &payer, instructions), + to_instruction_error(SettlementError::InvalidBuyTokenAccount), ); } @@ -245,24 +435,22 @@ fn rejects_partial_push_amount() { let orders = [FinalizedIntent { intent: &intent, mint: Pubkey::new_unique(), - amount: 1_000, + amount: 100, }]; - // A well-formed single-push finalize... let mut finalize = Instruction::from(FinalizeSettle { program_id, begin_ix_index: BEGIN_INDEX.into(), orders: &orders, }); - // ...with one byte popped, so the trailing amount is no longer a whole `u64`. + // Drop one byte so the trailing amount is no longer a whole `u64`. Begin + // validates the push from the (unchanged) account metas and passes; finalize + // then rejects the malformed data. finalize.data.pop(); let instructions = build_settlement(&program_id, &orders, finalize); - assert_eq!( + assert_finalize_error( send(&mut svm, &payer, instructions), - Err(TransactionError::InstructionError( - FINALIZE_INDEX, - InstructionError::InvalidInstructionData, - )), + InstructionError::InvalidInstructionData, ); } diff --git a/programs/settlement/tests/reclaim_order.rs b/programs/settlement/tests/reclaim_order.rs index 7e6ecbc..73fe3c3 100644 --- a/programs/settlement/tests/reclaim_order.rs +++ b/programs/settlement/tests/reclaim_order.rs @@ -9,7 +9,7 @@ use solana_sdk::{ signature::{Keypair, Signer}, }; -use crate::common::signed_tx; +use crate::common::{assert_instruction_error, signed_tx, to_instruction_error}; mod common; @@ -143,9 +143,9 @@ fn rejects_when_order_not_yet_expired() { } .instruction(); let tx = signed_tx(&svm, &owner, &owner, ix); - common::assert_settlement_error( + assert_instruction_error( svm.send_transaction(tx).map_err(|e| e.err), - SettlementError::OrderNotExpired, + to_instruction_error(SettlementError::OrderNotExpired), ); } @@ -168,8 +168,8 @@ fn rejects_when_reclaim_recipient_mismatch() { } .instruction(); let tx = signed_tx(&svm, &owner, &owner, ix); - common::assert_settlement_error( + assert_instruction_error( svm.send_transaction(tx).map_err(|e| e.err), - SettlementError::ReclaimRecipientMismatch, + to_instruction_error(SettlementError::ReclaimRecipientMismatch), ); }