From e7b0234a988e74e0e76473fa494ac5bda6b6b55f Mon Sep 17 00:00:00 2001 From: Kaze <230549489+kaze-cow@users.noreply.github.com> Date: Fri, 9 Oct 2026 22:21:43 +0900 Subject: [PATCH 01/10] Add WithdrawNativeSol instruction Lets the settlement-owned-order authority move lamports out of the native SOL buffer to a recipient of its choosing. The buffer's rent stays put: the runtime rejects leaving it under-funded, and emptying it is rejected with the existing NativeSolBufferEmptied. Co-Authored-By: Claude Opus 5.5 (1M context) --- DESIGN.md | 4 +- SECURITY_DESIGN.md | 2 +- bench-report.json | 37 +-- client/src/instruction/mod.rs | 2 + client/src/instruction/withdraw_native_sol.rs | 30 +++ client/src/parse.rs | 15 ++ interface/src/error.rs | 11 +- interface/src/instruction/mod.rs | 6 + .../src/instruction/withdraw_native_sol.rs | 239 ++++++++++++++++++ interface/src/role.rs | 3 +- programs/settlement/idl/cow_settlement.json | 138 +++++++++- programs/settlement/src/processor/mod.rs | 5 + .../src/processor/withdraw_native_sol.rs | 209 +++++++++++++++ programs/settlement/tests/common/benchmark.rs | 2 + programs/settlement/tests/idl/generate.rs | 19 +- programs/settlement/tests/idl/parse_rust.rs | 5 + .../settlement/tests/withdraw_native_sol.rs | 212 ++++++++++++++++ 17 files changed, 915 insertions(+), 24 deletions(-) create mode 100644 client/src/instruction/withdraw_native_sol.rs create mode 100644 interface/src/instruction/withdraw_native_sol.rs create mode 100644 programs/settlement/src/processor/withdraw_native_sol.rs create mode 100644 programs/settlement/tests/withdraw_native_sol.rs diff --git a/DESIGN.md b/DESIGN.md index 7c041502..4b98b2e5 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -39,7 +39,7 @@ The program grant privileged roles to specific accounts (_authorities_). They ar - Manager: the account that can update the address of all other roles. - Solver Authority: the account that can add and remove solvers. - Reclaim Authority: the account authorized to close buffer accounts, reclaim their rent, and choose where that rent goes. -- Settlement-Owned-Order Authority: the account authorized to place arbitrary orders that sell the protocol's own buffer balances (for fee withdrawals). +- Settlement-Owned-Order Authority: the account authorized to place arbitrary orders that sell the protocol's own buffer balances (for fee withdrawals), and to withdraw native SOL from the native SOL buffer. ### Updating authorities @@ -91,6 +91,8 @@ Fees are withdrawn by placing an order, owned by the settlement state PDA, that The order is placed through the `CreateSettlementOwnedOrder` instruction. The settlement-owned-order authority can specify arbitrary order parameters, as long as the owner is the state PDA. +Native SOL isn't a token, so it can't be sold through an order. The settlement-owned-order authority instead withdraws it directly from the native SOL buffer with the `WithdrawNativeSol` instruction, to a recipient of its choosing. The buffer's rent can't be withdrawn. + Differences with Ethereum: - Solvers can't access the content of the buffers directly anymore (though they can do so indirectly by creating a dedicated order just to sweep the buffer). diff --git a/SECURITY_DESIGN.md b/SECURITY_DESIGN.md index 4a65bc5c..2c7d4d24 100644 --- a/SECURITY_DESIGN.md +++ b/SECURITY_DESIGN.md @@ -18,7 +18,7 @@ All roles, other than Solvers, are defined and manipulated using the same patter | Manager | Ability to assign every role, its own included. Transfers take one step, with no acceptance or zero-address check | | Solver authority | Adding and removing solvers, so it decides who can settle | | Reclaim authority | Closing buffers and choosing where their rent goes. Can also burn tokens from the buffer to allow closing to happen | -| Settlement-owned-order authority | Ability to place orders that are owned by the settlement program, used to withdraw fees | +| Settlement-owned-order authority | Ability to place orders that are owned by the settlement program, used to withdraw fees, and to withdraw the native SOL buffer's lamports above rent | | Solvers | Ability to settle user orders | ## Users diff --git a/bench-report.json b/bench-report.json index a2ddbff4..d0e892b2 100644 --- a/bench-report.json +++ b/bench-report.json @@ -48,13 +48,14 @@ "transfer_authority/manager_can_transfer_solver_authority": 4, "transfer_authority/reclaim_authority_can_transfer_itself": 4, "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 4, - "transfer_authority/solver_authority_can_transfer_itself": 4 + "transfer_authority/solver_authority_can_transfer_itself": 4, + "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 6 }, "compute_units": { - "add_solver/add_with_many_existing_solvers": 2061, - "add_solver/adds_a_solver": 1609, - "cancel_order/cancels_existing_active_order": 2011, - "cancel_order/cancels_existing_order_without_intent_bytes": 1982, + "add_solver/add_with_many_existing_solvers": 2060, + "add_solver/adds_a_solver": 1608, + "cancel_order/cancels_existing_active_order": 2009, + "cancel_order/cancels_existing_order_without_intent_bytes": 1980, "cancel_order/creates_cancelled_pda_for_nonexistent_order": 4931, "create_buffers/happy_path_creates_initialized_buffer_token_account": 5846, "create_buffers/happy_path_creates_initialized_buffer_token_account_token_2022": 10653, @@ -64,7 +65,7 @@ "create_buffers/max_buffers_in_one_instruction_token_2022": 224501, "create_order/happy_path_creates_order_pda_with_expected_body": 6361, "create_settlement_owned_order/places_an_order_owned_by_the_state_pda": 3441, - "initialize/happy_path_initializes_state_pda_with_expected_data": 7567, + "initialize/happy_path_initializes_state_pda_with_expected_data": 7564, "reclaim_buffer/funded_buffer_is_skipped": 4831, "reclaim_buffer/funded_buffer_is_skipped_token_2022": 4841, "reclaim_buffer/happy_path_reclaims_empty_buffer_to_the_authority_itself": 5994, @@ -73,9 +74,9 @@ "reclaim_buffer/max_buffers_in_one_instruction_token_2022": 173550, "reclaim_buffer/reclaims_multiple_buffers_skipping_funded": 7621, "reclaim_buffer/reclaims_multiple_buffers_skipping_funded_token_2022": 9119, - "reclaim_order/happy_path_expired_returns_lamports_and_closes_pda": 2049, - "reclaim_order/happy_path_order_cancelled_is_reclaimable_by_owner_before_expiry": 2089, - "reclaim_order/happy_path_order_fully_filled_is_reclaimable_before_expiry": 1929, + "reclaim_order/happy_path_expired_returns_lamports_and_closes_pda": 2048, + "reclaim_order/happy_path_order_cancelled_is_reclaimable_by_owner_before_expiry": 2088, + "reclaim_order/happy_path_order_fully_filled_is_reclaimable_before_expiry": 1928, "reclaim_order/on_chain_order_partially_filled_is_not_reclaimable_before_expiry": null, "remove_solver/remove_with_many_existing_solvers": 754, "remove_solver/removes_a_solver": 489, @@ -93,13 +94,14 @@ "settle/settles_both_token_programs_with_transfer_checked_token_2022": 10835, "settle/settles_multiple_orders": 16050, "settle/settling_a_settlement_owned_order_withdraws_the_buffered_fees": 7265, - "transfer_authority/manager_can_transfer_manager": 178, - "transfer_authority/manager_can_transfer_reclaim_authority": 178, - "transfer_authority/manager_can_transfer_settlement_owned_order_authority": 177, - "transfer_authority/manager_can_transfer_solver_authority": 178, - "transfer_authority/reclaim_authority_can_transfer_itself": 203, - "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 201, - "transfer_authority/solver_authority_can_transfer_itself": 203 + "transfer_authority/manager_can_transfer_manager": 176, + "transfer_authority/manager_can_transfer_reclaim_authority": 176, + "transfer_authority/manager_can_transfer_settlement_owned_order_authority": 175, + "transfer_authority/manager_can_transfer_solver_authority": 176, + "transfer_authority/reclaim_authority_can_transfer_itself": 201, + "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 199, + "transfer_authority/solver_authority_can_transfer_itself": 201, + "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 233 }, "transaction_bytes": { "add_solver/add_with_many_existing_solvers": 366, @@ -150,6 +152,7 @@ "transfer_authority/manager_can_transfer_solver_authority": 333, "transfer_authority/reclaim_authority_can_transfer_itself": 333, "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 333, - "transfer_authority/solver_authority_can_transfer_itself": 333 + "transfer_authority/solver_authority_can_transfer_itself": 333, + "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 374 } } diff --git a/client/src/instruction/mod.rs b/client/src/instruction/mod.rs index 2cb7d691..e0e20a70 100644 --- a/client/src/instruction/mod.rs +++ b/client/src/instruction/mod.rs @@ -16,6 +16,7 @@ pub mod initialize; pub mod reclaim_buffer; pub mod remove_solver; pub mod transfer_authority; +pub mod withdraw_native_sol; pub use add_solver::AddSolver; pub use begin_settle::{BeginSettle, InitializedIntent, Pull, TokenProgram}; @@ -28,3 +29,4 @@ pub use initialize::Initialize; pub use reclaim_buffer::ReclaimBuffer; pub use remove_solver::RemoveSolver; pub use transfer_authority::TransferAuthority; +pub use withdraw_native_sol::WithdrawNativeSol; diff --git a/client/src/instruction/withdraw_native_sol.rs b/client/src/instruction/withdraw_native_sol.rs new file mode 100644 index 00000000..d624e340 --- /dev/null +++ b/client/src/instruction/withdraw_native_sol.rs @@ -0,0 +1,30 @@ +//! Builder for the `WithdrawNativeSol` instruction. + +use cow_settlement_interface::{ + pda::{buffer::find_native_sol_buffer_pda, state::find_state_pda}, + Instruction, Pubkey, +}; + +/// Moves `amount` lamports out of the native SOL buffer to `recipient`. +/// Authorized by `authority`, the settlement-owned-order authority, which picks +/// `recipient` freely and may set it to itself. +pub struct WithdrawNativeSol { + pub program_id: Pubkey, + pub authority: Pubkey, + pub recipient: Pubkey, + pub amount: u64, +} + +impl From for Instruction { + fn from(builder: WithdrawNativeSol) -> Self { + cow_settlement_interface::instruction::withdraw_native_sol::WithdrawNativeSol { + program_id: builder.program_id, + state_pda: find_state_pda(&builder.program_id).0, + authority: builder.authority, + native_sol_buffer: find_native_sol_buffer_pda(&builder.program_id).0, + recipient: builder.recipient, + amount: builder.amount, + } + .into() + } +} diff --git a/client/src/parse.rs b/client/src/parse.rs index 24b99501..564d70b3 100644 --- a/client/src/parse.rs +++ b/client/src/parse.rs @@ -16,6 +16,7 @@ use cow_settlement_interface::{ remove_solver::RemoveSolverInput, settle::{BeginSettleInput, FinalizeSettleInput}, transfer_authority::TransferAuthorityInput, + withdraw_native_sol::WithdrawNativeSolInput, InstructionInputParsing, }, recover_discriminator, SettlementInstruction, @@ -36,6 +37,7 @@ pub enum ParsedInstruction<'a, A> { TransferAuthority(TransferAuthorityInput<'a, A>), AddSolver(AddSolverInput<'a, A>), RemoveSolver(RemoveSolverInput<'a, A>), + WithdrawNativeSol(WithdrawNativeSolInput<'a, A>), } /// Parses any settlement instruction by its discriminator. @@ -83,6 +85,9 @@ pub fn parse_instruction<'a, A>( SettlementInstruction::RemoveSolver => ParsedInstruction::RemoveSolver( RemoveSolverInput::parse_body(remaining_data, accounts)?, ), + SettlementInstruction::WithdrawNativeSol => ParsedInstruction::WithdrawNativeSol( + WithdrawNativeSolInput::parse_body(remaining_data, accounts)?, + ), }) } @@ -92,6 +97,7 @@ mod tests { use crate::instruction::{ AddSolver, BeginSettle, CancelOrder, CreateBuffers, CreateOrder, CreateSettlementOwnedOrder, FinalizeSettle, Initialize, InitializedIntent, RemoveSolver, + WithdrawNativeSol, }; use cow_settlement_interface::{ data::intent::fixtures::sample_intent, @@ -206,6 +212,13 @@ mod tests { solver: pubkey_from_seed("solver"), } .into(), + SettlementInstruction::WithdrawNativeSol => WithdrawNativeSol { + program_id, + authority: payer, + recipient: payer, + amount: 42, + } + .into(), } } @@ -227,6 +240,7 @@ mod tests { SettlementInstruction::TransferAuthority, SettlementInstruction::AddSolver, SettlementInstruction::RemoveSolver, + SettlementInstruction::WithdrawNativeSol, ] { let ix = build(expected); let accounts: Vec<_> = ix @@ -251,6 +265,7 @@ mod tests { ParsedInstruction::TransferAuthority(_) => SettlementInstruction::TransferAuthority, ParsedInstruction::AddSolver(_) => SettlementInstruction::AddSolver, ParsedInstruction::RemoveSolver(_) => SettlementInstruction::RemoveSolver, + ParsedInstruction::WithdrawNativeSol(_) => SettlementInstruction::WithdrawNativeSol, }; assert_eq!(actual, expected); } diff --git a/interface/src/error.rs b/interface/src/error.rs index c83e91d9..6d4ec7c2 100644 --- a/interface/src/error.rs +++ b/interface/src/error.rs @@ -147,9 +147,16 @@ pub enum SettlementError { /// buffer the push draws from, so settling it would move no funds yet still /// record the declared amount as received. PushSourceIsDestination = 44, - /// `FinalizeSettle`: the native SOL payouts would empty the native SOL - /// buffer, which the runtime would then delete. + /// `FinalizeSettle`'s native SOL payouts or `WithdrawNativeSol` would empty + /// the native SOL buffer, which the runtime would then delete. NativeSolBufferEmptied = 45, + /// `WithdrawNativeSol`'s authority account isn't a signer, or doesn't + /// match the `settlement_owned_order_authority` recorded in the settlement + /// state PDA. + UnauthorizedNativeSolWithdrawal = 46, + /// `WithdrawNativeSol`'s native SOL buffer account isn't the canonical + /// native SOL buffer PDA. + NativeSolBufferMismatch = 47, } impl From for u32 { diff --git a/interface/src/instruction/mod.rs b/interface/src/instruction/mod.rs index 5aa92333..bbe5c680 100644 --- a/interface/src/instruction/mod.rs +++ b/interface/src/instruction/mod.rs @@ -17,6 +17,7 @@ pub mod reclaim_order; pub mod remove_solver; pub mod settle; pub mod transfer_authority; +pub mod withdraw_native_sol; #[derive(Clone, Copy, Debug, Eq, PartialEq, num_enum::TryFromPrimitive)] #[repr(u8)] @@ -69,6 +70,11 @@ pub enum SettlementInstruction { /// If the order PDA doesn't exist yet, it's created already cancelled. For /// this, some arbitrary account needs to pay the rent and sign. CancelOrder = 11, + /// Moves lamports out of the native SOL buffer to a recipient of the + /// caller's choosing. Only the current holder of the + /// SettlementOwnedOrderAuthority role may authorize this, and the buffer's + /// rent can't be withdrawn. + WithdrawNativeSol = 12, } impl SettlementInstruction { diff --git a/interface/src/instruction/withdraw_native_sol.rs b/interface/src/instruction/withdraw_native_sol.rs new file mode 100644 index 00000000..163b6c05 --- /dev/null +++ b/interface/src/instruction/withdraw_native_sol.rs @@ -0,0 +1,239 @@ +//! `WithdrawNativeSol` instruction builder and parser. +//! +//! Moves lamports out of the native SOL buffer (see +//! [`crate::pda::buffer::NATIVE_SOL_BUFFER_PDA`]) to a `recipient` of the +//! caller's choosing. Only the settlement's configured +//! `settlement_owned_order_authority` (see +//! [`crate::data::state::StateAccount`]) may authorize this. +//! +//! The buffer's rent is never withdrawable: the buffer must stay alive, since +//! only `Initialize` can create it. +//! +//! Wire format: `[discriminator=12, amount (u64 LE)]`, 9 bytes. +//! Required accounts: +//! `[state_pda (R), authority (R,S), native_sol_buffer (W), recipient (W)]`. + +use core::mem::size_of; + +use solana_instruction::{AccountMeta, Instruction}; +use solana_program_error::ProgramError; +use solana_pubkey::Pubkey; + +use super::InstructionInputParsing; +use crate::SettlementInstruction; + +/// Builder for a `WithdrawNativeSol` instruction. +/// +/// `state_pda` must be [`crate::pda::state::STATE_PDA`]. `authority` must sign +/// and must match the `settlement_owned_order_authority` recorded in the state +/// PDA's data. `native_sol_buffer` must be +/// [`crate::pda::buffer::NATIVE_SOL_BUFFER_PDA`]. `recipient` receives +/// `amount` lamports. +pub struct WithdrawNativeSol { + pub program_id: Pubkey, + pub state_pda: Pubkey, + pub authority: Pubkey, + pub native_sol_buffer: Pubkey, + pub recipient: Pubkey, + pub amount: u64, +} + +impl From for Instruction { + fn from(builder: WithdrawNativeSol) -> Self { + let mut data = vec![SettlementInstruction::WithdrawNativeSol.discriminator()]; + data.extend_from_slice(&builder.amount.to_le_bytes()); + Instruction { + program_id: builder.program_id, + accounts: vec![ + AccountMeta::new_readonly(builder.state_pda, false), + AccountMeta::new_readonly(builder.authority, true), + AccountMeta::new(builder.native_sol_buffer, false), + AccountMeta::new(builder.recipient, false), + ], + data, + } + } +} + +/// Parsed inputs of a `WithdrawNativeSol` instruction. +pub struct WithdrawNativeSolInput<'a, A> { + pub state_pda: &'a A, + pub authority: &'a A, + pub native_sol_buffer: &'a A, + pub recipient: &'a A, + pub amount: u64, +} + +impl<'a, A> InstructionInputParsing<'a, A> for WithdrawNativeSolInput<'a, A> { + const DISCRIMINATOR: SettlementInstruction = SettlementInstruction::WithdrawNativeSol; + + fn parse_body(instruction_data: &[u8], accounts: &'a [A]) -> Result { + let amount: &[u8; size_of::()] = instruction_data + .try_into() + .map_err(|_| ProgramError::InvalidInstructionData)?; + let amount = u64::from_le_bytes(*amount); + + // Accounts: [state_pda (R), authority (R,S), native_sol_buffer (W), + // recipient (W)]. + let [state_pda, authority, native_sol_buffer, recipient, ..] = accounts else { + return Err(ProgramError::NotEnoughAccountKeys); + }; + + Ok(Self { + state_pda, + authority, + native_sol_buffer, + recipient, + amount, + }) + } +} + +/// Test scaffolding for `WithdrawNativeSol` 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 super::{Instruction, WithdrawNativeSol}; + + /// Number of accounts `WithdrawNativeSol` expects: state PDA, authority, + /// native SOL buffer, and recipient. + pub const NUM_ACCOUNTS: usize = 4; + + /// `WithdrawNativeSol` instruction data withdrawing `amount`, with + /// placeholder addresses since they don't end up in the data. + pub fn withdraw_native_sol_data(amount: u64) -> Vec { + let zero = Address::new_from_array([0; 32]); + Instruction::from(WithdrawNativeSol { + program_id: zero, + state_pda: zero, + authority: zero, + native_sol_buffer: zero, + recipient: zero, + amount, + }) + .data + } +} + +#[cfg(test)] +mod tests { + use super::fixtures::{withdraw_native_sol_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, + }; + + fn sample() -> WithdrawNativeSol { + WithdrawNativeSol { + program_id: pubkey_from_seed("program id"), + state_pda: pubkey_from_seed("state pda"), + authority: pubkey_from_seed("authority"), + native_sol_buffer: pubkey_from_seed("native sol buffer"), + recipient: pubkey_from_seed("recipient"), + amount: 0x0102_0304_0506_0708, + } + } + + #[test] + fn withdraw_native_sol_input_parses_valid_input() { + let builder = sample(); + let accounts = [ + fake_account(builder.state_pda), + fake_account(builder.authority), + fake_account(builder.native_sol_buffer), + fake_account(builder.recipient), + ]; + let (state_pda, authority, native_sol_buffer, recipient, amount) = ( + builder.state_pda, + builder.authority, + builder.native_sol_buffer, + builder.recipient, + builder.amount, + ); + let data = Instruction::from(builder).data; + + let parsed = WithdrawNativeSolInput::parse(&data, &accounts).expect("parse should succeed"); + + assert_eq!(*parsed.state_pda.address(), state_pda); + assert_eq!(*parsed.authority.address(), authority); + assert_eq!(*parsed.native_sol_buffer.address(), native_sol_buffer); + assert_eq!(*parsed.recipient.address(), recipient); + assert_eq!(parsed.amount, amount); + } + + #[test] + fn withdraw_native_sol_input_rejects_short_data() { + let mut data = withdraw_native_sol_data(1); + data.pop(); + let accounts = fake_sequential_accounts::(); + assert_eq!( + WithdrawNativeSolInput::parse(&data, &accounts).err(), + Some(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn withdraw_native_sol_input_rejects_long_data() { + let mut data = withdraw_native_sol_data(1); + data.push(0); + let accounts = fake_sequential_accounts::(); + assert_eq!( + WithdrawNativeSolInput::parse(&data, &accounts).err(), + Some(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn withdraw_native_sol_input_rejects_missing_accounts() { + let data = withdraw_native_sol_data(1); + let accounts = fake_sequential_accounts::<{ NUM_ACCOUNTS - 1 }>(); + assert_eq!( + WithdrawNativeSolInput::parse(&data, &accounts).err(), + Some(ProgramError::NotEnoughAccountKeys), + ); + } + + #[test] + fn instruction_data_has_expected_layout() { + let Instruction { data, .. } = sample().into(); + assert_eq!( + data, + [ + SettlementInstruction::WithdrawNativeSol.discriminator(), + 8, + 7, + 6, + 5, + 4, + 3, + 2, + 1, + ] + ); + } + + #[test] + fn instruction_has_expected_accounts() { + let builder = sample(); + let expected = [ + builder.state_pda, + builder.authority, + builder.native_sol_buffer, + builder.recipient, + ]; + let program_id = builder.program_id; + let ix = Instruction::from(builder); + + assert_eq!(ix.program_id, program_id); + assert_eq!(ix.accounts.len(), NUM_ACCOUNTS); + assert_readonly_nonsigner(&ix.accounts[0], expected[0]); + assert_readonly_signer(&ix.accounts[1], expected[1]); + assert_writable_nonsigner(&ix.accounts[2], expected[2]); + assert_writable_nonsigner(&ix.accounts[3], expected[3]); + } +} diff --git a/interface/src/role.rs b/interface/src/role.rs index 3983cdc4..7d239cef 100644 --- a/interface/src/role.rs +++ b/interface/src/role.rs @@ -21,7 +21,8 @@ pub enum Role { ReclaimAuthority, /// The account authorized to place orders that sell the protocol's own /// buffer balances (e.g., fee withdrawals). It chooses the destination, the - /// bought token, and the price of those orders. + /// bought token, and the price of those orders. It may also withdraw the + /// native SOL buffer's lamports above rent. SettlementOwnedOrderAuthority, } diff --git a/programs/settlement/idl/cow_settlement.json b/programs/settlement/idl/cow_settlement.json index 9c1eba4a..42def231 100644 --- a/programs/settlement/idl/cow_settlement.json +++ b/programs/settlement/idl/cow_settlement.json @@ -902,6 +902,132 @@ ] } ] + }, + { + "name": "withdraw_native_sol", + "docs": [ + "Moves lamports out of the native SOL buffer to a recipient of the caller's choosing. Only the current holder of the SettlementOwnedOrderAuthority role may authorize this, and the buffer's rent can't be withdrawn.", + "Lamports move by a direct debit from the program-owned buffer, so no system program is involved. Leaving the buffer below its rent-exempt minimum fails with the runtime's InsufficientFundsForRent, and emptying it fails with NativeSolBufferEmptied." + ], + "discriminator": [ + 12 + ], + "accounts": [ + { + "name": "state_pda", + "pda": { + "seeds": [ + { + "kind": "const", + "value": [ + 115, + 101, + 116, + 116, + 108, + 101, + 109, + 101, + 110, + 116, + 32, + 118, + 48, + 46, + 53, + 32, + 32, + 32, + 32 + ] + } + ] + }, + "docs": [ + "Must be the canonical state PDA; holds the settlement_owned_order_authority the signer is checked against. Canonical PDA seeded by SETTLEMENT_SEED alone. SETTLEMENT_SEED is the ASCII prefix \"settlement v\" followed by the program's major.minor version, right-padded with spaces to a fixed 19 bytes; the fixed width prevents one version's seeds from being a prefix of another's." + ] + }, + { + "name": "authority", + "signer": true, + "docs": [ + "Must match the settlement_owned_order_authority address recorded in the state PDA." + ] + }, + { + "name": "native_sol_buffer", + "writable": true, + "pda": { + "seeds": [ + { + "kind": "const", + "value": [ + 115, + 101, + 116, + 116, + 108, + 101, + 109, + 101, + 110, + 116, + 32, + 118, + 48, + 46, + 53, + 32, + 32, + 32, + 32 + ] + }, + { + "kind": "const", + "value": [ + 98, + 117, + 102, + 102, + 101, + 114, + 32, + 110, + 97, + 116, + 105, + 118, + 101, + 32, + 115, + 111, + 108 + ] + } + ] + }, + "docs": [ + "The native SOL buffer created by Initialize; debited by amount." + ] + }, + { + "name": "recipient", + "writable": true, + "docs": [ + "Receives the withdrawn lamports." + ] + } + ], + "args": [ + { + "name": "amount", + "type": "u64", + "docs": [ + "The lamports to withdraw." + ] + } + ] } ], "accounts": [ @@ -1320,7 +1446,17 @@ { "code": 45, "name": "NativeSolBufferEmptied", - "msg": "FinalizeSettle: the native SOL payouts would empty the native SOL buffer, which the runtime would then delete." + "msg": "FinalizeSettle's native SOL payouts or WithdrawNativeSol would empty the native SOL buffer, which the runtime would then delete." + }, + { + "code": 46, + "name": "UnauthorizedNativeSolWithdrawal", + "msg": "WithdrawNativeSol's authority account isn't a signer, or doesn't match the settlement_owned_order_authority recorded in the settlement state PDA." + }, + { + "code": 47, + "name": "NativeSolBufferMismatch", + "msg": "WithdrawNativeSol's native SOL buffer account isn't the canonical native SOL buffer PDA." } ] } diff --git a/programs/settlement/src/processor/mod.rs b/programs/settlement/src/processor/mod.rs index c636de51..a7d23da0 100644 --- a/programs/settlement/src/processor/mod.rs +++ b/programs/settlement/src/processor/mod.rs @@ -13,6 +13,7 @@ mod reclaim_order; mod remove_solver; mod transfer_authority; pub mod utils; +mod withdraw_native_sol; use add_solver::process_add_solver; use begin_settle::process_begin_settle; @@ -26,6 +27,7 @@ use reclaim_buffer::process_reclaim_buffer; use reclaim_order::process_reclaim_order; use remove_solver::process_remove_solver; use transfer_authority::process_transfer_authority; +use withdraw_native_sol::process_withdraw_native_sol; use cow_settlement_interface::{recover_discriminator, SettlementInstruction}; use pinocchio::{AccountView, Address, ProgramResult}; @@ -67,5 +69,8 @@ pub fn process_instruction( } SettlementInstruction::AddSolver => process_add_solver(accounts, instruction_data), SettlementInstruction::RemoveSolver => process_remove_solver(accounts, instruction_data), + SettlementInstruction::WithdrawNativeSol => { + process_withdraw_native_sol(accounts, instruction_data) + } } } diff --git a/programs/settlement/src/processor/withdraw_native_sol.rs b/programs/settlement/src/processor/withdraw_native_sol.rs new file mode 100644 index 00000000..53629d47 --- /dev/null +++ b/programs/settlement/src/processor/withdraw_native_sol.rs @@ -0,0 +1,209 @@ +//! `WithdrawNativeSol` instruction handler. +//! +//! The native SOL buffer is owned by this program, so its lamports are moved +//! by editing balances directly, as `FinalizeSettle` does. Leaving it below +//! rent but not empty is rejected by the runtime; emptying it is rejected here, +//! since the runtime would accept it and delete the buffer. + +use cow_settlement_interface::{ + data::state::StateAccount, + instruction::{withdraw_native_sol::WithdrawNativeSolInput, InstructionInputParsing}, + pda::{buffer::NATIVE_SOL_BUFFER_PDA, state::validate_is_state_pda}, + Pubkey, Role, SettlementError, +}; +use pinocchio::{AccountView, ProgramResult}; + +use crate::processor::utils::lamports::move_lamports; + +pub fn process_withdraw_native_sol( + accounts: &mut [AccountView], + instruction_data: &[u8], +) -> ProgramResult { + let WithdrawNativeSolInput { + state_pda, + authority, + native_sol_buffer, + recipient, + amount, + } = WithdrawNativeSolInput::parse(instruction_data, accounts)?; + + validate_is_state_pda(state_pda.address().as_array())?; + + let settlement_owned_order_authority: Pubkey = + StateAccount::from_account(state_pda)?.authority(Role::SettlementOwnedOrderAuthority); + if !authority.is_signer() || authority.address() != &settlement_owned_order_authority { + return Err(SettlementError::UnauthorizedNativeSolWithdrawal.into()); + } + + if native_sol_buffer.address() != &NATIVE_SOL_BUFFER_PDA { + return Err(SettlementError::NativeSolBufferMismatch.into()); + } + + // A copied `AccountView` writes through to the same runtime account. + let mut native_sol_buffer = *native_sol_buffer; + let mut recipient = *recipient; + move_lamports(&mut native_sol_buffer, &mut recipient, amount)?; + + if native_sol_buffer.lamports() == 0 { + return Err(SettlementError::NativeSolBufferEmptied.into()); + } + + Ok(()) +} + +#[cfg(test)] +mod tests { + use cow_settlement_interface::data::state::fixtures::state_account_bytes; + use cow_settlement_interface::data::state::StateInitArgs; + use cow_settlement_interface::fixtures::pubkey_from_seed; + use cow_settlement_interface::instruction::fixtures::{ + fake_account, fake_account_with_data, fake_sequential_accounts, fake_signer, + }; + use cow_settlement_interface::instruction::withdraw_native_sol::fixtures::{ + withdraw_native_sol_data, NUM_ACCOUNTS, + }; + use cow_settlement_interface::pda::state::STATE_PDA; + use pinocchio::error::ProgramError; + use std::sync::LazyLock; + + use super::*; + + // Positions within [`base_accounts`], for the tests that swap one entry. + const STATE_ACCOUNT: usize = 0; + const AUTHORITY: usize = 1; + const NATIVE_SOL_BUFFER: usize = 2; + const RECIPIENT: usize = 3; + + const BUFFER_LAMPORTS: u64 = 1_000; + const RECIPIENT_LAMPORTS: u64 = 50; + + static SETTLEMENT_OWNED_ORDER_AUTHORITY: LazyLock = + LazyLock::new(|| pubkey_from_seed("settlement-owned-order authority")); + + /// The [`StateInitArgs`] planted by [`base_accounts`]. + fn base_init_args() -> StateInitArgs { + StateInitArgs { + manager: pubkey_from_seed("base_init_args's unused manager"), + solver_authority: pubkey_from_seed("base_init_args's unused solver authority"), + reclaim_authority: pubkey_from_seed("base_init_args's unused reclaim authority"), + settlement_owned_order_authority: *SETTLEMENT_OWNED_ORDER_AUTHORITY, + } + } + + /// Accounts for a withdrawal authorized by + /// [`SETTLEMENT_OWNED_ORDER_AUTHORITY`], each one well-formed, with the + /// buffer holding [`BUFFER_LAMPORTS`] and the recipient + /// [`RECIPIENT_LAMPORTS`]. + fn base_accounts() -> [AccountView; NUM_ACCOUNTS] { + let mut accounts = [ + fake_account_with_data(STATE_PDA, &state_account_bytes(&base_init_args(), &[])), + fake_signer(*SETTLEMENT_OWNED_ORDER_AUTHORITY), + fake_account(NATIVE_SOL_BUFFER_PDA), + fake_account(pubkey_from_seed("recipient")), + ]; + accounts[NATIVE_SOL_BUFFER].set_lamports(BUFFER_LAMPORTS); + accounts[RECIPIENT].set_lamports(RECIPIENT_LAMPORTS); + accounts + } + + #[track_caller] + fn assert_rejects( + mut accounts: [AccountView; NUM_ACCOUNTS], + amount: u64, + expected: ProgramError, + ) { + assert_eq!( + process_withdraw_native_sol(&mut accounts, &withdraw_native_sol_data(amount)), + Err(expected), + ); + } + + #[test] + fn process_withdraw_native_sol_propagates_parse_error() { + let mut data = withdraw_native_sol_data(1); + data.push(0); // trailing byte triggers a parse error + let mut accounts = fake_sequential_accounts::(); + assert_eq!( + process_withdraw_native_sol(&mut accounts, &data), + Err(ProgramError::InvalidInstructionData), + ); + } + + #[test] + fn process_withdraw_native_sol_moves_the_amount_to_the_recipient() { + let mut accounts = base_accounts(); + let amount = BUFFER_LAMPORTS - 1; + + process_withdraw_native_sol(&mut accounts, &withdraw_native_sol_data(amount)) + .unwrap_or_else(|err| panic!("withdrawal should succeed: {err}")); + + assert_eq!(accounts[NATIVE_SOL_BUFFER].lamports(), 1); + assert_eq!(accounts[RECIPIENT].lamports(), RECIPIENT_LAMPORTS + amount); + } + + #[test] + fn process_withdraw_native_sol_rejects_wrong_state_pda() { + let mut accounts = base_accounts(); + accounts[STATE_ACCOUNT] = fake_account_with_data( + pubkey_from_seed("unrelated"), + &state_account_bytes(&base_init_args(), &[]), + ); + assert_rejects(accounts, 1, SettlementError::StateAccountMismatch.into()); + } + + #[test] + fn process_withdraw_native_sol_rejects_uninitialized_state_pda() { + let mut accounts = base_accounts(); + accounts[STATE_ACCOUNT] = fake_account(STATE_PDA); + assert_rejects(accounts, 1, ProgramError::InvalidAccountData); + } + + #[test] + fn process_withdraw_native_sol_rejects_wrong_authority() { + let mut accounts = base_accounts(); + accounts[AUTHORITY] = fake_signer(pubkey_from_seed("unrelated")); + assert_rejects( + accounts, + 1, + SettlementError::UnauthorizedNativeSolWithdrawal.into(), + ); + } + + #[test] + fn process_withdraw_native_sol_rejects_nonsigner_authority() { + let mut accounts = base_accounts(); + // `fake_account`, unlike `fake_signer`, leaves the signer flag clear. + accounts[AUTHORITY] = fake_account(*SETTLEMENT_OWNED_ORDER_AUTHORITY); + assert_rejects( + accounts, + 1, + SettlementError::UnauthorizedNativeSolWithdrawal.into(), + ); + } + + #[test] + fn process_withdraw_native_sol_rejects_wrong_native_sol_buffer() { + let mut accounts = base_accounts(); + accounts[NATIVE_SOL_BUFFER] = fake_account(pubkey_from_seed("unrelated")); + accounts[NATIVE_SOL_BUFFER].set_lamports(BUFFER_LAMPORTS); + assert_rejects(accounts, 1, SettlementError::NativeSolBufferMismatch.into()); + } + + #[test] + fn process_withdraw_native_sol_rejects_draining_the_buffer() { + assert_rejects( + base_accounts(), + BUFFER_LAMPORTS, + SettlementError::NativeSolBufferEmptied.into(), + ); + } + + #[test] + fn process_withdraw_native_sol_rejects_more_than_the_balance() { + assert_rejects( + base_accounts(), + BUFFER_LAMPORTS + 1, + ProgramError::ArithmeticOverflow, + ); + } +} diff --git a/programs/settlement/tests/common/benchmark.rs b/programs/settlement/tests/common/benchmark.rs index c280ccee..1321669f 100644 --- a/programs/settlement/tests/common/benchmark.rs +++ b/programs/settlement/tests/common/benchmark.rs @@ -20,6 +20,7 @@ pub enum BenchLabel { TransferAuthority, AddSolver, RemoveSolver, + WithdrawNativeSol, } impl fmt::Display for BenchLabel { @@ -38,6 +39,7 @@ impl fmt::Display for BenchLabel { Self::TransferAuthority => "transfer_authority", Self::AddSolver => "add_solver", Self::RemoveSolver => "remove_solver", + Self::WithdrawNativeSol => "withdraw_native_sol", }) } } diff --git a/programs/settlement/tests/idl/generate.rs b/programs/settlement/tests/idl/generate.rs index e1ee4a54..b5d70ef0 100644 --- a/programs/settlement/tests/idl/generate.rs +++ b/programs/settlement/tests/idl/generate.rs @@ -9,7 +9,10 @@ //! agree with everything that _is_ here. use cow_settlement_interface::{ - pda::{buffer::BUFFER_SEED, SETTLEMENT_SEED}, + pda::{ + buffer::{BUFFER_SEED, NATIVE_SOL_BUFFER_SEED}, + SETTLEMENT_SEED, + }, SettlementInstruction, }; use serde_json::{json, Map, Value}; @@ -46,6 +49,12 @@ const BUFFER_PDA_0: &[Seed] = &[ Seed::Const(BUFFER_SEED), ]; +/// The native SOL buffer PDA, which isn't keyed by a mint. +const NATIVE_SOL_BUFFER_PDA: &[Seed] = &[ + Seed::Const(SETTLEMENT_SEED), + Seed::Const(NATIVE_SOL_BUFFER_SEED), +]; + /// What the Rust source doesn't say about one instruction. struct Instruction { /// The discriminator variant naming it. The IDL calls the instruction by @@ -129,6 +138,14 @@ const INSTRUCTIONS: &[Instruction] = &[ input: &parse_rust::REMOVE_SOLVER_RS, pda_accounts: &[("state_pda", STATE_PDA)], }, + Instruction { + variant: SettlementInstruction::WithdrawNativeSol, + input: &parse_rust::WITHDRAW_NATIVE_SOL_RS, + pda_accounts: &[ + ("state_pda", STATE_PDA), + ("native_sol_buffer", NATIVE_SOL_BUFFER_PDA), + ], + }, ]; /// The struct types the IDL defines, as `(source, Rust name, IDL name)`. The diff --git a/programs/settlement/tests/idl/parse_rust.rs b/programs/settlement/tests/idl/parse_rust.rs index c74eae7d..9cf21689 100644 --- a/programs/settlement/tests/idl/parse_rust.rs +++ b/programs/settlement/tests/idl/parse_rust.rs @@ -115,6 +115,11 @@ pub const REMOVE_SOLVER_RS: Source = Source { text: include_str!("../../../../interface/src/instruction/remove_solver.rs"), }; +pub const WITHDRAW_NATIVE_SOL_RS: Source = Source { + display: "interface/src/instruction/withdraw_native_sol.rs", + text: include_str!("../../../../interface/src/instruction/withdraw_native_sol.rs"), +}; + impl Source { fn parse(&self) -> syn::File { syn::parse_file(self.text) diff --git a/programs/settlement/tests/withdraw_native_sol.rs b/programs/settlement/tests/withdraw_native_sol.rs new file mode 100644 index 00000000..16c614b4 --- /dev/null +++ b/programs/settlement/tests/withdraw_native_sol.rs @@ -0,0 +1,212 @@ +//! Integration tests for withdrawing lamports from the native SOL buffer. + +use cow_settlement_client::instruction::WithdrawNativeSol; +use cow_settlement_interface::{ + instruction::withdraw_native_sol::WithdrawNativeSol as WithdrawNativeSolRaw, + pda::{buffer::NATIVE_SOL_BUFFER_PDA, state::STATE_PDA}, + Instruction, SettlementError, +}; +use solana_sdk::{signature::Signer, transaction::TransactionError}; + +use crate::common::benchmark::{send_transaction_metered, BenchLabel}; +use crate::common::{ + assert_instruction_error, buffer, lamports, send_with_signers, InitializedParams, +}; + +mod common; + +/// Enough that even half of it leaves an unfunded recipient rent-exempt. +const FUNDING: u64 = 10_000_000; + +#[test] +fn happy_path_withdraws_to_a_recipient_chosen_by_the_authority() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let recipient = common::unique_pubkey(); + let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let authority_before = lamports(&svm, &authority.pubkey()); + + let ix = WithdrawNativeSol { + program_id, + authority: authority.pubkey(), + recipient, + amount: FUNDING, + }; + let tx = common::signed_tx(&svm, &payer, &authority, ix); + send_transaction_metered(&mut svm, tx, BenchLabel::WithdrawNativeSol) + .expect("withdrawing everything above rent should succeed"); + + assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded - FUNDING); + assert_eq!(lamports(&svm, &recipient), FUNDING); + assert_eq!( + lamports(&svm, &authority.pubkey()), + authority_before, + "the authority must not be credited when it named someone else" + ); + common::assert_rent_exempt( + &svm, + &svm.get_account(&NATIVE_SOL_BUFFER_PDA) + .expect("the native SOL buffer must survive the withdrawal"), + ); +} + +#[test] +fn withdraws_to_the_authority_itself() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let authority_before = lamports(&svm, &authority.pubkey()); + + let ix = WithdrawNativeSol { + program_id, + authority: authority.pubkey(), + recipient: authority.pubkey(), + amount: FUNDING / 2, + }; + send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) + .expect("a partial withdrawal should succeed"); + + assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded - FUNDING / 2); + assert_eq!( + lamports(&svm, &authority.pubkey()), + authority_before + FUNDING / 2 + ); +} + +#[test] +fn rejects_any_other_role() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + manager, + solver_authority, + reclaim, + .. + }, + ) = common::setup_init(); + let funded = buffer::add_native_lamports(&mut svm, FUNDING); + + for impostor in [&manager, &solver_authority, &reclaim] { + let ix = WithdrawNativeSol { + program_id, + authority: impostor.pubkey(), + recipient: impostor.pubkey(), + amount: 1, + }; + assert_instruction_error( + send_with_signers(&mut svm, &payer, &[impostor], &[ix.into()]), + SettlementError::UnauthorizedNativeSolWithdrawal, + ); + } + + assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); +} + +#[test] +fn rejects_dipping_into_rent() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let funded = buffer::add_native_lamports(&mut svm, FUNDING); + + let ix = WithdrawNativeSol { + program_id, + authority: authority.pubkey(), + recipient: authority.pubkey(), + amount: FUNDING + 1, + }; + let err = send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) + .expect_err("a withdrawal into the native SOL buffer's rent must be rejected"); + assert!( + matches!(err, TransactionError::InsufficientFundsForRent { .. }), + "expected a rent failure, got {err:?}", + ); + + assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); +} + +/// The runtime would happily delete an emptied buffer, and only `Initialize` +/// can create it, so the program refuses. +#[test] +fn rejects_draining_the_buffer() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let funded = buffer::add_native_lamports(&mut svm, FUNDING); + + let ix = WithdrawNativeSol { + program_id, + authority: authority.pubkey(), + recipient: authority.pubkey(), + amount: funded, + }; + assert_instruction_error( + send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]), + SettlementError::NativeSolBufferEmptied, + ); + + assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); +} + +/// Another program-owned account is just as debitable, so the program has to +/// pin the buffer's address itself. +#[test] +fn rejects_a_non_canonical_native_sol_buffer() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let impostor_buffer = common::create_account(&mut svm, &program_id, &[]); + let impostor_funded = lamports(&svm, &impostor_buffer) + FUNDING; + svm.airdrop(&impostor_buffer, FUNDING) + .expect("airdrop should succeed"); + + let ix: Instruction = WithdrawNativeSolRaw { + program_id, + state_pda: STATE_PDA, + authority: authority.pubkey(), + native_sol_buffer: impostor_buffer, + recipient: authority.pubkey(), + amount: 1, + } + .into(); + assert_instruction_error( + send_with_signers(&mut svm, &payer, &[&authority], &[ix]), + SettlementError::NativeSolBufferMismatch, + ); + + assert_eq!(lamports(&svm, &impostor_buffer), impostor_funded); +} From d2d98b07ccdff56700c5f949891d2410f7650eff Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:57:02 +0200 Subject: [PATCH 02/10] Destructure in tests --- .../src/instruction/withdraw_native_sol.rs | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/interface/src/instruction/withdraw_native_sol.rs b/interface/src/instruction/withdraw_native_sol.rs index 163b6c05..e162ab5f 100644 --- a/interface/src/instruction/withdraw_native_sol.rs +++ b/interface/src/instruction/withdraw_native_sol.rs @@ -157,13 +157,19 @@ mod tests { ); let data = Instruction::from(builder).data; - let parsed = WithdrawNativeSolInput::parse(&data, &accounts).expect("parse should succeed"); - - assert_eq!(*parsed.state_pda.address(), state_pda); - assert_eq!(*parsed.authority.address(), authority); - assert_eq!(*parsed.native_sol_buffer.address(), native_sol_buffer); - assert_eq!(*parsed.recipient.address(), recipient); - assert_eq!(parsed.amount, amount); + let WithdrawNativeSolInput { + state_pda: parsed_state_pda, + authority: parsed_authority, + native_sol_buffer: parsed_native_sol_buffer, + recipient: parsed_recipient, + amount: parsed_amount, + } = WithdrawNativeSolInput::parse(&data, &accounts).expect("parse should succeed"); + + assert_eq!(*parsed_state_pda.address(), state_pda); + assert_eq!(*parsed_authority.address(), authority); + assert_eq!(*parsed_native_sol_buffer.address(), native_sol_buffer); + assert_eq!(*parsed_recipient.address(), recipient); + assert_eq!(parsed_amount, amount); } #[test] From 63d415259f40d8e395a5dce47aae48f2607a0649 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Fri, 9 Oct 2026 18:59:05 +0200 Subject: [PATCH 03/10] Explicit amount --- interface/src/instruction/withdraw_native_sol.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/interface/src/instruction/withdraw_native_sol.rs b/interface/src/instruction/withdraw_native_sol.rs index e162ab5f..262b1d5c 100644 --- a/interface/src/instruction/withdraw_native_sol.rs +++ b/interface/src/instruction/withdraw_native_sol.rs @@ -206,7 +206,11 @@ mod tests { #[test] fn instruction_data_has_expected_layout() { - let Instruction { data, .. } = sample().into(); + let Instruction { data, .. } = WithdrawNativeSol { + amount: 0x0102_0304_0506_0708, + ..sample() + } + .into(); assert_eq!( data, [ From 7bebf9e483f6c2388589b83bc47f9033745403d3 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:04:16 +0200 Subject: [PATCH 04/10] Remove expected array --- .../src/instruction/withdraw_native_sol.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/interface/src/instruction/withdraw_native_sol.rs b/interface/src/instruction/withdraw_native_sol.rs index 262b1d5c..4e9bc1bb 100644 --- a/interface/src/instruction/withdraw_native_sol.rs +++ b/interface/src/instruction/withdraw_native_sol.rs @@ -230,20 +230,13 @@ mod tests { #[test] fn instruction_has_expected_accounts() { let builder = sample(); - let expected = [ - builder.state_pda, - builder.authority, - builder.native_sol_buffer, - builder.recipient, - ]; - let program_id = builder.program_id; - let ix = Instruction::from(builder); + let ix = Instruction::from(sample()); - assert_eq!(ix.program_id, program_id); + assert_eq!(ix.program_id, builder.program_id); assert_eq!(ix.accounts.len(), NUM_ACCOUNTS); - assert_readonly_nonsigner(&ix.accounts[0], expected[0]); - assert_readonly_signer(&ix.accounts[1], expected[1]); - assert_writable_nonsigner(&ix.accounts[2], expected[2]); - assert_writable_nonsigner(&ix.accounts[3], expected[3]); + assert_readonly_nonsigner(&ix.accounts[0], builder.state_pda); + assert_readonly_signer(&ix.accounts[1], builder.authority); + assert_writable_nonsigner(&ix.accounts[2], builder.native_sol_buffer); + assert_writable_nonsigner(&ix.accounts[3], builder.recipient); } } From b22dfd0ae5f4b978642f50e246e7650d80686ea0 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:07:47 +0200 Subject: [PATCH 05/10] More generic comment --- interface/src/error.rs | 4 ++-- programs/settlement/idl/cow_settlement.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interface/src/error.rs b/interface/src/error.rs index 2ef49940..7b3d8745 100644 --- a/interface/src/error.rs +++ b/interface/src/error.rs @@ -147,8 +147,8 @@ pub enum SettlementError { /// buffer the push draws from, so settling it would move no funds yet still /// record the declared amount as received. PushSourceIsDestination = 44, - /// `FinalizeSettle`'s native SOL payouts or `WithdrawNativeSol` would empty - /// the native SOL buffer, which the runtime would then delete. + /// Moving lamports out of the native SOL buffer would empty it, which the + /// runtime would then delete. NativeSolBufferEmptied = 45, /// A buffer to be reclaimed holds more tokens than the burn limit supplied /// for it, so its balance can't be cleared for closing. diff --git a/programs/settlement/idl/cow_settlement.json b/programs/settlement/idl/cow_settlement.json index 9cb086eb..0db8ad19 100644 --- a/programs/settlement/idl/cow_settlement.json +++ b/programs/settlement/idl/cow_settlement.json @@ -1446,7 +1446,7 @@ { "code": 45, "name": "NativeSolBufferEmptied", - "msg": "FinalizeSettle's native SOL payouts or WithdrawNativeSol would empty the native SOL buffer, which the runtime would then delete." + "msg": "Moving lamports out of the native SOL buffer would empty it, which the runtime would then delete." }, { "code": 46, From e331010692ca7eed61243d1d5ddcf8c03cc34acd Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:10:04 +0200 Subject: [PATCH 06/10] Identifiably useless number --- .../src/processor/withdraw_native_sol.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/programs/settlement/src/processor/withdraw_native_sol.rs b/programs/settlement/src/processor/withdraw_native_sol.rs index 53629d47..8a78589a 100644 --- a/programs/settlement/src/processor/withdraw_native_sol.rs +++ b/programs/settlement/src/processor/withdraw_native_sol.rs @@ -148,14 +148,18 @@ mod tests { pubkey_from_seed("unrelated"), &state_account_bytes(&base_init_args(), &[]), ); - assert_rejects(accounts, 1, SettlementError::StateAccountMismatch.into()); + assert_rejects( + accounts, + 31337, + SettlementError::StateAccountMismatch.into(), + ); } #[test] fn process_withdraw_native_sol_rejects_uninitialized_state_pda() { let mut accounts = base_accounts(); accounts[STATE_ACCOUNT] = fake_account(STATE_PDA); - assert_rejects(accounts, 1, ProgramError::InvalidAccountData); + assert_rejects(accounts, 31337, ProgramError::InvalidAccountData); } #[test] @@ -164,7 +168,7 @@ mod tests { accounts[AUTHORITY] = fake_signer(pubkey_from_seed("unrelated")); assert_rejects( accounts, - 1, + 31337, SettlementError::UnauthorizedNativeSolWithdrawal.into(), ); } @@ -176,7 +180,7 @@ mod tests { accounts[AUTHORITY] = fake_account(*SETTLEMENT_OWNED_ORDER_AUTHORITY); assert_rejects( accounts, - 1, + 31337, SettlementError::UnauthorizedNativeSolWithdrawal.into(), ); } @@ -186,7 +190,11 @@ mod tests { let mut accounts = base_accounts(); accounts[NATIVE_SOL_BUFFER] = fake_account(pubkey_from_seed("unrelated")); accounts[NATIVE_SOL_BUFFER].set_lamports(BUFFER_LAMPORTS); - assert_rejects(accounts, 1, SettlementError::NativeSolBufferMismatch.into()); + assert_rejects( + accounts, + 31337, + SettlementError::NativeSolBufferMismatch.into(), + ); } #[test] From e9de1083c3a9b13f77be19de27aa481b0190d128 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:46:24 +0200 Subject: [PATCH 07/10] funded -> native_token_balance --- .../settlement/tests/withdraw_native_sol.rs | 37 +++++++++++++------ 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/programs/settlement/tests/withdraw_native_sol.rs b/programs/settlement/tests/withdraw_native_sol.rs index 16c614b4..cc5bf000 100644 --- a/programs/settlement/tests/withdraw_native_sol.rs +++ b/programs/settlement/tests/withdraw_native_sol.rs @@ -30,7 +30,7 @@ fn happy_path_withdraws_to_a_recipient_chosen_by_the_authority() { }, ) = common::setup_init(); let recipient = common::unique_pubkey(); - let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); let authority_before = lamports(&svm, &authority.pubkey()); let ix = WithdrawNativeSol { @@ -43,7 +43,10 @@ fn happy_path_withdraws_to_a_recipient_chosen_by_the_authority() { send_transaction_metered(&mut svm, tx, BenchLabel::WithdrawNativeSol) .expect("withdrawing everything above rent should succeed"); - assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded - FUNDING); + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance - FUNDING + ); assert_eq!(lamports(&svm, &recipient), FUNDING); assert_eq!( lamports(&svm, &authority.pubkey()), @@ -68,7 +71,7 @@ fn withdraws_to_the_authority_itself() { .. }, ) = common::setup_init(); - let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); let authority_before = lamports(&svm, &authority.pubkey()); let ix = WithdrawNativeSol { @@ -80,7 +83,10 @@ fn withdraws_to_the_authority_itself() { send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) .expect("a partial withdrawal should succeed"); - assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded - FUNDING / 2); + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance - FUNDING / 2 + ); assert_eq!( lamports(&svm, &authority.pubkey()), authority_before + FUNDING / 2 @@ -100,7 +106,7 @@ fn rejects_any_other_role() { .. }, ) = common::setup_init(); - let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); for impostor in [&manager, &solver_authority, &reclaim] { let ix = WithdrawNativeSol { @@ -115,7 +121,10 @@ fn rejects_any_other_role() { ); } - assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance + ); } #[test] @@ -129,7 +138,7 @@ fn rejects_dipping_into_rent() { .. }, ) = common::setup_init(); - let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); let ix = WithdrawNativeSol { program_id, @@ -144,7 +153,10 @@ fn rejects_dipping_into_rent() { "expected a rent failure, got {err:?}", ); - assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance + ); } /// The runtime would happily delete an emptied buffer, and only `Initialize` @@ -160,20 +172,23 @@ fn rejects_draining_the_buffer() { .. }, ) = common::setup_init(); - let funded = buffer::add_native_lamports(&mut svm, FUNDING); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); let ix = WithdrawNativeSol { program_id, authority: authority.pubkey(), recipient: authority.pubkey(), - amount: funded, + amount: native_buffer_balance, }; assert_instruction_error( send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]), SettlementError::NativeSolBufferEmptied, ); - assert_eq!(lamports(&svm, &NATIVE_SOL_BUFFER_PDA), funded); + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance + ); } /// Another program-owned account is just as debitable, so the program has to From 1e6b0f7d9ed15baed6d1bcdf0502e3f134054d05 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 00:51:15 +0200 Subject: [PATCH 08/10] Test arbitrary non-authority, not all invalid authorities --- .../settlement/tests/withdraw_native_sol.rs | 32 ++++++++----------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/programs/settlement/tests/withdraw_native_sol.rs b/programs/settlement/tests/withdraw_native_sol.rs index cc5bf000..fdc3c099 100644 --- a/programs/settlement/tests/withdraw_native_sol.rs +++ b/programs/settlement/tests/withdraw_native_sol.rs @@ -94,32 +94,26 @@ fn withdraws_to_the_authority_itself() { } #[test] -fn rejects_any_other_role() { +fn rejects_an_unknown_authority() { let ( mut svm, InitializedParams { - program_id, - payer, - manager, - solver_authority, - reclaim, - .. + program_id, payer, .. }, ) = common::setup_init(); let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); + let impostor = common::unique_keypair(); - for impostor in [&manager, &solver_authority, &reclaim] { - let ix = WithdrawNativeSol { - program_id, - authority: impostor.pubkey(), - recipient: impostor.pubkey(), - amount: 1, - }; - assert_instruction_error( - send_with_signers(&mut svm, &payer, &[impostor], &[ix.into()]), - SettlementError::UnauthorizedNativeSolWithdrawal, - ); - } + let ix = WithdrawNativeSol { + program_id, + authority: impostor.pubkey(), + recipient: impostor.pubkey(), + amount: 1, + }; + assert_instruction_error( + send_with_signers(&mut svm, &payer, &[&impostor], &[ix.into()]), + SettlementError::UnauthorizedNativeSolWithdrawal, + ); assert_eq!( lamports(&svm, &NATIVE_SOL_BUFFER_PDA), From 773c557435d6f9c5d9129937ffff20e4e8022e9f Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:00:03 +0200 Subject: [PATCH 09/10] Test withdrawing more than expected --- .../settlement/tests/withdraw_native_sol.rs | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/programs/settlement/tests/withdraw_native_sol.rs b/programs/settlement/tests/withdraw_native_sol.rs index fdc3c099..42909fee 100644 --- a/programs/settlement/tests/withdraw_native_sol.rs +++ b/programs/settlement/tests/withdraw_native_sol.rs @@ -185,6 +185,36 @@ fn rejects_draining_the_buffer() { ); } +#[test] +fn rejects_more_than_the_balance() { + let ( + mut svm, + InitializedParams { + program_id, + payer, + settlement_owned_order: authority, + .. + }, + ) = common::setup_init(); + let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); + + let ix = WithdrawNativeSol { + program_id, + authority: authority.pubkey(), + recipient: authority.pubkey(), + amount: native_buffer_balance + 1, + }; + assert_instruction_error( + send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]), + solana_sdk::instruction::InstructionError::ArithmeticOverflow, + ); + + assert_eq!( + lamports(&svm, &NATIVE_SOL_BUFFER_PDA), + native_buffer_balance + ); +} + /// Another program-owned account is just as debitable, so the program has to /// pin the buffer's address itself. #[test] From 250b83a572c25c04d767fb8188799c52915b61f0 Mon Sep 17 00:00:00 2001 From: Federico Giacon <58218759+fedgiac@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:33:00 +0000 Subject: [PATCH 10/10] Cap `WithdrawNativeSol` to full balance minus rent (#199) Implements suggestion from [this comment](https://github.com/cowprotocol/solana-programs/pull/198#discussion_r4231854573) to silently cap the amount withdrawn in a `WithdrawNativeSol` instruction to the full balance minus the rent. This is done so that a script that automatically withdraws the fees won't occasionally revert if at the same time a settlement is using funds from the native SOL buffer. ### How to test Existing and modified tests. --- bench-report.json | 36 ++--- .../src/instruction/withdraw_native_sol.rs | 9 +- programs/settlement/idl/cow_settlement.json | 6 +- .../src/processor/withdraw_native_sol.rs | 58 +++----- .../settlement/tests/withdraw_native_sol.rs | 134 ++++++++++-------- 5 files changed, 121 insertions(+), 122 deletions(-) diff --git a/bench-report.json b/bench-report.json index 6e769600..649d6032 100644 --- a/bench-report.json +++ b/bench-report.json @@ -50,10 +50,10 @@ "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 6 }, "compute_units": { - "add_solver/add_with_many_existing_solvers": 2060, - "add_solver/adds_a_solver": 1608, - "cancel_order/cancels_existing_active_order": 2009, - "cancel_order/cancels_existing_order_without_intent_bytes": 1980, + "add_solver/add_with_many_existing_solvers": 2061, + "add_solver/adds_a_solver": 1609, + "cancel_order/cancels_existing_active_order": 2011, + "cancel_order/cancels_existing_order_without_intent_bytes": 1982, "cancel_order/creates_cancelled_pda_for_nonexistent_order": 4931, "create_buffers/happy_path_creates_initialized_buffer_token_account": 5846, "create_buffers/happy_path_creates_initialized_buffer_token_account_token_2022": 10653, @@ -63,19 +63,19 @@ "create_buffers/max_buffers_in_one_instruction_token_2022": 224501, "create_order/happy_path_creates_order_pda_with_expected_body": 6361, "create_settlement_owned_order/places_an_order_owned_by_the_state_pda": 3441, - "initialize/happy_path_initializes_state_pda_with_expected_data": 7564, + "initialize/happy_path_initializes_state_pda_with_expected_data": 7566, "reclaim_buffer/funded_buffer_within_limit_is_burned_and_closed": 7199, "reclaim_buffer/funded_buffer_within_limit_is_burned_and_closed_token_2022": 10518, "reclaim_buffer/happy_path_reclaims_empty_buffer_to_the_authority_itself": 6029, "reclaim_buffer/happy_path_reclaims_empty_buffer_to_the_authority_itself_token_2022": 7518, "reclaim_buffer/max_buffers_in_one_instruction": 129322, "reclaim_buffer/max_buffers_in_one_instruction_token_2022": 173992, - "reclaim_order/happy_path_expired_returns_lamports_and_closes_pda": 2048, - "reclaim_order/happy_path_order_cancelled_is_reclaimable_by_owner_before_expiry": 2088, - "reclaim_order/happy_path_order_fully_filled_is_reclaimable_before_expiry": 1928, + "reclaim_order/happy_path_expired_returns_lamports_and_closes_pda": 2049, + "reclaim_order/happy_path_order_cancelled_is_reclaimable_by_owner_before_expiry": 2089, + "reclaim_order/happy_path_order_fully_filled_is_reclaimable_before_expiry": 1929, "reclaim_order/on_chain_order_partially_filled_is_not_reclaimable_before_expiry": null, - "remove_solver/remove_with_many_existing_solvers": 754, - "remove_solver/removes_a_solver": 489, + "remove_solver/remove_with_many_existing_solvers": 755, + "remove_solver/removes_a_solver": 490, "settle/finalizes_with_no_pushes": 1108, "settle/happy_path_sell_tokens_for_native_sol": 4663, "settle/pulls_from_multiple_orders": 13366, @@ -90,14 +90,14 @@ "settle/settles_both_token_programs_with_transfer_checked_token_2022": 10835, "settle/settles_multiple_orders": 16050, "settle/settling_a_settlement_owned_order_withdraws_the_buffered_fees": 7265, - "transfer_authority/manager_can_transfer_manager": 176, - "transfer_authority/manager_can_transfer_reclaim_authority": 176, - "transfer_authority/manager_can_transfer_settlement_owned_order_authority": 175, - "transfer_authority/manager_can_transfer_solver_authority": 176, - "transfer_authority/reclaim_authority_can_transfer_itself": 201, - "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 199, - "transfer_authority/solver_authority_can_transfer_itself": 201, - "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 233 + "transfer_authority/manager_can_transfer_manager": 178, + "transfer_authority/manager_can_transfer_reclaim_authority": 178, + "transfer_authority/manager_can_transfer_settlement_owned_order_authority": 177, + "transfer_authority/manager_can_transfer_solver_authority": 178, + "transfer_authority/reclaim_authority_can_transfer_itself": 203, + "transfer_authority/settlement_owned_order_authority_can_transfer_itself": 201, + "transfer_authority/solver_authority_can_transfer_itself": 203, + "withdraw_native_sol/happy_path_withdraws_to_a_recipient_chosen_by_the_authority": 369 }, "transaction_bytes": { "add_solver/add_with_many_existing_solvers": 366, diff --git a/interface/src/instruction/withdraw_native_sol.rs b/interface/src/instruction/withdraw_native_sol.rs index 4e9bc1bb..bee015f6 100644 --- a/interface/src/instruction/withdraw_native_sol.rs +++ b/interface/src/instruction/withdraw_native_sol.rs @@ -6,8 +6,9 @@ //! `settlement_owned_order_authority` (see //! [`crate::data::state::StateAccount`]) may authorize this. //! -//! The buffer's rent is never withdrawable: the buffer must stay alive, since -//! only `Initialize` can create it. +//! The `amount` is a maximum: it's capped to the lamports above the buffer's +//! rent-exempt minimum, so the buffer always stays alive (only `Initialize` can +//! create it) and a withdrawal never reverts for asking too much. //! //! Wire format: `[discriminator=12, amount (u64 LE)]`, 9 bytes. //! Required accounts: @@ -27,8 +28,8 @@ use crate::SettlementInstruction; /// `state_pda` must be [`crate::pda::state::STATE_PDA`]. `authority` must sign /// and must match the `settlement_owned_order_authority` recorded in the state /// PDA's data. `native_sol_buffer` must be -/// [`crate::pda::buffer::NATIVE_SOL_BUFFER_PDA`]. `recipient` receives -/// `amount` lamports. +/// [`crate::pda::buffer::NATIVE_SOL_BUFFER_PDA`]. `recipient` receives up to +/// `amount` lamports, capped to the buffer's balance above rent. pub struct WithdrawNativeSol { pub program_id: Pubkey, pub state_pda: Pubkey, diff --git a/programs/settlement/idl/cow_settlement.json b/programs/settlement/idl/cow_settlement.json index 0db8ad19..16f1ab29 100644 --- a/programs/settlement/idl/cow_settlement.json +++ b/programs/settlement/idl/cow_settlement.json @@ -907,7 +907,7 @@ "name": "withdraw_native_sol", "docs": [ "Moves lamports out of the native SOL buffer to a recipient of the caller's choosing. Only the current holder of the SettlementOwnedOrderAuthority role may authorize this, and the buffer's rent can't be withdrawn.", - "Lamports move by a direct debit from the program-owned buffer, so no system program is involved. Leaving the buffer below its rent-exempt minimum fails with the runtime's InsufficientFundsForRent, and emptying it fails with NativeSolBufferEmptied." + "Lamports move by a direct debit from the program-owned buffer, so no system program is involved." ], "discriminator": [ 12 @@ -1008,7 +1008,7 @@ ] }, "docs": [ - "The native SOL buffer created by Initialize; debited by amount." + "The native SOL buffer created by Initialize; debited by the withdrawn amount, never below its rent." ] }, { @@ -1024,7 +1024,7 @@ "name": "amount", "type": "u64", "docs": [ - "The lamports to withdraw." + "The maximum lamports to withdraw; capped to the buffer's balance above rent." ] } ] diff --git a/programs/settlement/src/processor/withdraw_native_sol.rs b/programs/settlement/src/processor/withdraw_native_sol.rs index 8a78589a..5fe396c3 100644 --- a/programs/settlement/src/processor/withdraw_native_sol.rs +++ b/programs/settlement/src/processor/withdraw_native_sol.rs @@ -1,9 +1,10 @@ //! `WithdrawNativeSol` instruction handler. //! //! The native SOL buffer is owned by this program, so its lamports are moved -//! by editing balances directly, as `FinalizeSettle` does. Leaving it below -//! rent but not empty is rejected by the runtime; emptying it is rejected here, -//! since the runtime would accept it and delete the buffer. +//! by editing balances directly, as `FinalizeSettle` does. The requested amount +//! is capped to the lamports above the buffer's rent-exempt minimum, so the +//! buffer always stays alive and a balance that shrank since the transaction +//! was built lowers the payout instead of reverting it. use cow_settlement_interface::{ data::state::StateAccount, @@ -11,7 +12,10 @@ use cow_settlement_interface::{ pda::{buffer::NATIVE_SOL_BUFFER_PDA, state::validate_is_state_pda}, Pubkey, Role, SettlementError, }; -use pinocchio::{AccountView, ProgramResult}; +use pinocchio::{ + sysvars::{rent::Rent, Sysvar}, + AccountView, ProgramResult, +}; use crate::processor::utils::lamports::move_lamports; @@ -39,15 +43,23 @@ pub fn process_withdraw_native_sol( return Err(SettlementError::NativeSolBufferMismatch.into()); } + // Cap the withdrawal to the lamports above the buffer's rent-exempt + // minimum, so the buffer always survives and a balance smaller than the + // caller expected shrinks the payout instead of reverting it. + let withdrawable = native_sol_buffer + .lamports() + // overflow: basically unreachable since an on-chain account will have + // enough rent by the SVM's correctness guarantees. This function is + // designed to avoid reverts for making withdrawing easier, so we + // saturate here instead of erroring out explicitly. + .saturating_sub(Rent::get()?.try_minimum_balance(native_sol_buffer.data_len())?); + let amount = amount.min(withdrawable); + // A copied `AccountView` writes through to the same runtime account. let mut native_sol_buffer = *native_sol_buffer; let mut recipient = *recipient; move_lamports(&mut native_sol_buffer, &mut recipient, amount)?; - if native_sol_buffer.lamports() == 0 { - return Err(SettlementError::NativeSolBufferEmptied.into()); - } - Ok(()) } @@ -129,18 +141,6 @@ mod tests { ); } - #[test] - fn process_withdraw_native_sol_moves_the_amount_to_the_recipient() { - let mut accounts = base_accounts(); - let amount = BUFFER_LAMPORTS - 1; - - process_withdraw_native_sol(&mut accounts, &withdraw_native_sol_data(amount)) - .unwrap_or_else(|err| panic!("withdrawal should succeed: {err}")); - - assert_eq!(accounts[NATIVE_SOL_BUFFER].lamports(), 1); - assert_eq!(accounts[RECIPIENT].lamports(), RECIPIENT_LAMPORTS + amount); - } - #[test] fn process_withdraw_native_sol_rejects_wrong_state_pda() { let mut accounts = base_accounts(); @@ -196,22 +196,4 @@ mod tests { SettlementError::NativeSolBufferMismatch.into(), ); } - - #[test] - fn process_withdraw_native_sol_rejects_draining_the_buffer() { - assert_rejects( - base_accounts(), - BUFFER_LAMPORTS, - SettlementError::NativeSolBufferEmptied.into(), - ); - } - - #[test] - fn process_withdraw_native_sol_rejects_more_than_the_balance() { - assert_rejects( - base_accounts(), - BUFFER_LAMPORTS + 1, - ProgramError::ArithmeticOverflow, - ); - } } diff --git a/programs/settlement/tests/withdraw_native_sol.rs b/programs/settlement/tests/withdraw_native_sol.rs index 42909fee..db61ac50 100644 --- a/programs/settlement/tests/withdraw_native_sol.rs +++ b/programs/settlement/tests/withdraw_native_sol.rs @@ -6,7 +6,7 @@ use cow_settlement_interface::{ pda::{buffer::NATIVE_SOL_BUFFER_PDA, state::STATE_PDA}, Instruction, SettlementError, }; -use solana_sdk::{signature::Signer, transaction::TransactionError}; +use solana_sdk::signature::Signer; use crate::common::benchmark::{send_transaction_metered, BenchLabel}; use crate::common::{ @@ -93,36 +93,12 @@ fn withdraws_to_the_authority_itself() { ); } +/// A buffer holding less than its rent-exempt minimum caps the withdrawal to +/// zero: the cap's `saturating_sub` floors at zero, so the withdrawal succeeds +/// as a no-op rather than reverting. Not expected to be reachable unless the +/// rent mechanism changes. #[test] -fn rejects_an_unknown_authority() { - let ( - mut svm, - InitializedParams { - program_id, payer, .. - }, - ) = common::setup_init(); - let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); - let impostor = common::unique_keypair(); - - let ix = WithdrawNativeSol { - program_id, - authority: impostor.pubkey(), - recipient: impostor.pubkey(), - amount: 1, - }; - assert_instruction_error( - send_with_signers(&mut svm, &payer, &[&impostor], &[ix.into()]), - SettlementError::UnauthorizedNativeSolWithdrawal, - ); - - assert_eq!( - lamports(&svm, &NATIVE_SOL_BUFFER_PDA), - native_buffer_balance - ); -} - -#[test] -fn rejects_dipping_into_rent() { +fn caps_a_below_rent_buffer_to_a_no_op() { let ( mut svm, InitializedParams { @@ -132,51 +108,61 @@ fn rejects_dipping_into_rent() { .. }, ) = common::setup_init(); - let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); + + // Drop the buffer one lamport below its rent-exempt minimum (it holds no + // data), so the cap's `saturating_sub(rent_floor)` floors at zero. + // `set_account` bypasses the runtime rent check that would otherwise forbid + // this state. + let below_rent = svm.minimum_balance_for_rent_exemption(0).strict_sub(1); + let mut buffer = svm + .get_account(&NATIVE_SOL_BUFFER_PDA) + .expect("the native SOL buffer should exist"); + buffer.lamports = below_rent; + svm.set_account(NATIVE_SOL_BUFFER_PDA, buffer) + .expect("set_account should succeed"); + let authority_before = lamports(&svm, &authority.pubkey()); let ix = WithdrawNativeSol { program_id, authority: authority.pubkey(), recipient: authority.pubkey(), - amount: FUNDING + 1, + amount: u64::MAX, }; - let err = send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) - .expect_err("a withdrawal into the native SOL buffer's rent must be rejected"); - assert!( - matches!(err, TransactionError::InsufficientFundsForRent { .. }), - "expected a rent failure, got {err:?}", - ); + send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) + .expect("a below-rent buffer should cap to a no-op, not revert"); assert_eq!( lamports(&svm, &NATIVE_SOL_BUFFER_PDA), - native_buffer_balance + below_rent, + "the buffer balance must be untouched", + ); + assert_eq!( + lamports(&svm, &authority.pubkey()), + authority_before, + "nothing should be credited", ); } -/// The runtime would happily delete an emptied buffer, and only `Initialize` -/// can create it, so the program refuses. #[test] -fn rejects_draining_the_buffer() { +fn rejects_an_unknown_authority() { let ( mut svm, InitializedParams { - program_id, - payer, - settlement_owned_order: authority, - .. + program_id, payer, .. }, ) = common::setup_init(); let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); + let impostor = common::unique_keypair(); let ix = WithdrawNativeSol { program_id, - authority: authority.pubkey(), - recipient: authority.pubkey(), - amount: native_buffer_balance, + authority: impostor.pubkey(), + recipient: impostor.pubkey(), + amount: 1, }; assert_instruction_error( - send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]), - SettlementError::NativeSolBufferEmptied, + send_with_signers(&mut svm, &payer, &[&impostor], &[ix.into()]), + SettlementError::UnauthorizedNativeSolWithdrawal, ); assert_eq!( @@ -185,8 +171,12 @@ fn rejects_draining_the_buffer() { ); } -#[test] -fn rejects_more_than_the_balance() { +/// Withdraw `requested(native_buffer_balance)` (an amount at or beyond the +/// whole balance) to the authority, and assert it's capped to the lamports +/// above rent: the authority is paid exactly those, and the buffer is left +/// alive at its rent floor rather than the withdrawal reverting. +#[track_caller] +fn assert_overdraw_is_capped(requested: impl FnOnce(u64) -> u64) { let ( mut svm, InitializedParams { @@ -197,24 +187,50 @@ fn rejects_more_than_the_balance() { }, ) = common::setup_init(); let native_buffer_balance = buffer::add_native_lamports(&mut svm, FUNDING); + let authority_before = lamports(&svm, &authority.pubkey()); let ix = WithdrawNativeSol { program_id, authority: authority.pubkey(), recipient: authority.pubkey(), - amount: native_buffer_balance + 1, + amount: requested(native_buffer_balance), }; - assert_instruction_error( - send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]), - solana_sdk::instruction::InstructionError::ArithmeticOverflow, - ); + send_with_signers(&mut svm, &payer, &[&authority], &[ix.into()]) + .expect("an overdraw should be capped to the balance above rent, not rejected"); + assert_eq!( + lamports(&svm, &authority.pubkey()), + authority_before.strict_add(FUNDING) + ); assert_eq!( lamports(&svm, &NATIVE_SOL_BUFFER_PDA), - native_buffer_balance + native_buffer_balance.strict_sub(FUNDING) + ); + common::assert_rent_exempt( + &svm, + &svm.get_account(&NATIVE_SOL_BUFFER_PDA) + .expect("the native SOL buffer must survive the capped withdrawal"), ); } +#[test] +fn caps_a_withdrawal_dipping_into_rent() { + // One lamport past what's above rent. + assert_overdraw_is_capped(|_native_buffer_balance| FUNDING + 1); +} + +#[test] +fn caps_a_withdrawal_exactly_draining_the_buffer() { + // The buffer's entire balance, rent included. + assert_overdraw_is_capped(|native_buffer_balance| native_buffer_balance); +} + +#[test] +fn caps_a_grossly_oversized_withdrawal() { + // Far more than the buffer could ever hold. + assert_overdraw_is_capped(|_native_buffer_balance| u64::MAX); +} + /// Another program-owned account is just as debitable, so the program has to /// pin the buffer's address itself. #[test]