Check order amounts

Author: fedgiac

DESIGN.md

@@ -126,7 +126,7 @@ created_by: Pubkey
 intent: OrderIntent
 ```
 
-There are also other parameters, like `snapshot`, used internally during the settlement.
+`amount_withdrawn` and `amount_received` are cumulative across all of the order's settlements. They cap how much of the order can still be traded, so it can't be filled beyond its sell and buy amounts.
 
 An order PDA can only exist and hold data if the order has been [authenticated](#authenticating-an-order). If this account exists and is not cancelled, filled, or expired, then the order can be traded.
 
@@ -255,9 +255,9 @@ Differences with Ethereum:
 
 A settlement transaction is split into multiple instructions. All settlement operations occur between a `BeginSettle` and a `FinalizeSettle` instruction with the exception of arbitrary interactions, which can take place at any point of a transaction. Except for that, the order of instructions in the transaction is arbitrary.
 
-- `BeginSettle`: Snapshots each order's receiver token account, spender token account, and withdrawal balances. Pulls funds from each order’s sell token account to the solver-specified destination accounts, using the settlement state PDA’s token delegation. Carries an explicit `finalize_ix_index` pointing to its paired `FinalizeSettle`.
+- `BeginSettle`: Pulls funds from each order’s sell token account to the solver-specified destination accounts, using the settlement state PDA’s token delegation. Validates each order's limit price and that its cumulative fill stays within the order's sell and buy amounts (fully filling a fill-or-kill order), and updates the order's `amount_withdrawn`/`amount_received`. Carries an explicit `finalize_ix_index` pointing to its paired `FinalizeSettle`.
 - (arbitrary interactions): Any instruction from the solver. This could be a token transfer, an AMM swap, or anything else.
-- `FinalizeSettle`: Pushes the proceeds of each order from the settlement’s buffer accounts to the order’s buy token account, using the settlement state PDA’s authority over the buffers. Reads balances again, computes deltas against the snapshots, validates clearing/limit prices, updates `amount_received` and order status, revokes solver approvals. Carries an explicit `begin_ix_index` pointing to its paired `BeginSettle`.
+- `FinalizeSettle`: Pushes the proceeds of each order from the settlement’s buffer accounts to the order’s buy token account, using the settlement state PDA’s authority over the buffers. Carries an explicit `begin_ix_index` pointing to its paired `BeginSettle`.
 
 Additionally, a settlement transaction will include the batch number as part of the instruction bytes of `BeginSettle`.
 

client/src/instructions.rs

@@ -237,7 +237,7 @@ mod tests {
                 .iter()
                 .map(|meta| fake_account_from_array(meta.pubkey.to_bytes()))
                 .collect();
-            let parsed = BeginSettleInput::parse(&ix.data, &mut accounts)
+            let mut parsed = BeginSettleInput::parse(&ix.data, &mut accounts)
                 .map_err(|e| TestCaseError::fail(format!("parse failed: {e:?}")))?;
 
             prop_assert_eq!(parsed.finalize_ix_index, finalize_ix_index);
@@ -246,13 +246,18 @@ mod tests {
                 &INSTRUCTIONS_SYSVAR_ID,
             );
 
-            let parsed_orders: Vec<_> = parsed.orders.iter().collect();
-            prop_assert_eq!(parsed_orders.len(), expected.len());
-            for (order, (order_pda, sell_token, bump)) in parsed_orders.iter().zip(&expected) {
+            // Each order borrows the iterator, so only one is live at a time:
+            // compare it against `expected` as it's yielded, without collecting.
+            let mut orders = parsed.orders.iter_mut();
+            for (order_pda, sell_token, bump) in &expected {
+                let order = orders
+                    .next()
+                    .ok_or_else(|| TestCaseError::fail("fewer parsed orders than expected"))?;
                 prop_assert_eq!(order.order_pda.address(), order_pda);
                 prop_assert_eq!(order.sell_token_account.address(), sell_token);
                 prop_assert_eq!(order.bump, *bump);
             }
+            prop_assert!(orders.next().is_none());
         }
 
         // `FinalizeSettle` derives each order's source buffer from its mint and

interface/src/instruction/settle/begin.rs

@@ -36,7 +36,7 @@ pub struct Pull {
 /// `[discriminator=0][finalize_ix_index: u16 LE][n: u8][bump×n][transfer_count×n]
 /// [amount: u64 LE ×T]`.
 /// Required accounts: `[instructions_sysvar (R), state_pda (R), token_program
-/// (R)]` followed, per order, by `[order_pda (R), sell_token_account (W),
+/// (R)]` followed, per order, by `[order_pda (W), sell_token_account (W),
 /// destination (W)...]`.
 ///
 /// The program requires the order PDAs to be strictly increasing by address.
@@ -98,8 +98,9 @@ impl From<BeginSettle<'_>> for Instruction {
             AccountMeta::new_readonly(SPL_TOKEN_PROGRAM_ID, false),
         ];
         for &i in &order {
-            // Read-only account for the order.
-            accounts.push(AccountMeta::new_readonly(order_pdas[i], false));
+            // Writable account for the order: `BeginSettle` updates its filled
+            // amounts (`amount_withdrawn`/`amount_received`).
+            accounts.push(AccountMeta::new(order_pdas[i], false));
             // Writable accounts settling the order: its sell token account and the
             // recipient of each transfer.
             accounts.push(AccountMeta::new(sell_token_accounts[i], false));
@@ -119,7 +120,7 @@ impl From<BeginSettle<'_>> for Instruction {
 /// A single settled order, resulted from parsing `BeginSettle`, together with
 /// the funds to pull from its sell token account.
 pub struct SettledOrder<'a> {
-    pub order_pda: &'a AccountView,
+    pub order_pda: &'a mut AccountView,
     pub sell_token_account: &'a AccountView,
     pub bump: u8,
     /// Destination accounts for this order's transfers.
@@ -137,7 +138,7 @@ pub struct SettledOrders<'a> {
     /// - each order_accounts is a series of accounts:
     ///   `order_pda_N, sell_token_account_N, destination_N_1, destination_N_2, ..., destination_N_M`
     /// - and M is `counts[N]`
-    order_accounts: &'a [AccountView],
+    order_accounts: &'a mut [AccountView],
     bumps: &'a [u8],
     /// One transfer count per order, parallel to `bumps`.
     counts: &'a [u8],
@@ -147,41 +148,47 @@ pub struct SettledOrders<'a> {
 }
 
 impl<'a> SettledOrders<'a> {
-    /// Returns an iterator yielding one [`SettledOrder`] per step.
+    /// Returns an iterator yielding one [`SettledOrder`] per step. Each order's
+    /// `order_pda` is handed out mutably (so `BeginSettle` can update its filled
+    /// amounts), so the items borrow the iterator and must be processed one at a
+    /// time rather than collected.
     #[allow(
         clippy::arithmetic_side_effects,
         reason = "offsets are bounded by tx limits"
     )]
-    pub fn iter(&self) -> impl Iterator<Item = SettledOrder<'a>> + '_ {
-        let order_count = self.bumps.len();
+    pub fn iter_mut(&mut self) -> impl Iterator<Item = SettledOrder<'_>> + '_ {
+        let bumps = self.bumps;
+        let counts = self.counts;
+        let amounts = self.amounts;
+        // Cursor over the remaining order accounts; each step splits one order's
+        // `[order_pda, sell_token_account, destinations..count]` off the front.
+        let mut rest: &mut [AccountView] = self.order_accounts;
         let mut i = 0usize;
-        let mut account_offset = 0usize;
         let mut amount_offset = 0usize;
         std::iter::from_fn(move || {
-            if i >= order_count {
+            if i >= bumps.len() {
                 return None;
             }
-            let bump = self.bumps[i];
-            let count = usize::from(self.counts[i]);
+            let bump = bumps[i];
+            let count = usize::from(counts[i]);
             i += 1;
 
-            let order_pda = &self.order_accounts[account_offset];
-            let sell_token_account = &self.order_accounts[account_offset + 1];
-            let dest_start = account_offset + 2;
-            let dest_end = dest_start + count;
-            let destinations = &self.order_accounts[dest_start..dest_end];
-            account_offset = dest_end;
+            let taken = core::mem::take(&mut rest);
+            let (order_pda, tail) = taken.split_first_mut()?;
+            let (sell_token_account, tail) = tail.split_first_mut()?;
+            let (destinations, remainder) = tail.split_at_mut(count);
+            rest = remainder;
 
             let amount_end = amount_offset + count;
-            let amounts = &self.amounts[amount_offset..amount_end];
+            let order_amounts = &amounts[amount_offset..amount_end];
             amount_offset = amount_end;
 
             Some(SettledOrder {
                 order_pda,
                 sell_token_account,
                 bump,
                 destinations,
-                amounts,
+                amounts: order_amounts,
             })
         })
     }
@@ -371,16 +378,21 @@ mod tests {
         ];
         let actual: Vec<Pubkey> = accounts.iter().map(|account| account.pubkey).collect();
         assert_eq!(actual, expected);
-        // The fixed accounts and the order PDAs are read-only; only the sell
-        // token accounts are writable, following the sorted order.
+        // The fixed accounts are read-only; each order PDA (updated with its
+        // filled amounts) and sell token account are writable, in sorted order.
         let writable: Vec<Pubkey> = accounts
             .iter()
             .filter(|account| account.is_writable)
             .map(|account| account.pubkey)
             .collect();
         assert_eq!(
             writable,
-            vec![low_sell_token_account, high_sell_token_account],
+            vec![
+                low_order_pda,
+                low_sell_token_account,
+                high_order_pda,
+                high_sell_token_account,
+            ],
         );
         assert!(accounts.iter().all(|account| !account.is_signer));
     }
@@ -453,14 +465,17 @@ mod tests {
         ];
         let actual: Vec<Pubkey> = accounts.iter().map(|account| account.pubkey).collect();
         assert_eq!(actual, expected);
-        // The fixed accounts and the order PDAs are read-only; sell and
-        // destination accounts are writable for the transfer.
+        // The fixed accounts are read-only; each order PDA (updated with its
+        // filled amounts), sell, and destination accounts are writable.
         let writable: Vec<Pubkey> = accounts
             .iter()
             .filter(|account| account.is_writable)
             .map(|account| account.pubkey)
             .collect();
-        assert_eq!(writable, vec![sell_a, dest_a0, dest_a1, sell_b, dest_b0]);
+        assert_eq!(
+            writable,
+            vec![order_a, sell_a, dest_a0, dest_a1, order_b, sell_b, dest_b0],
+        );
         assert!(accounts.iter().all(|account| !account.is_signer));
     }
 
@@ -483,13 +498,13 @@ mod tests {
         let BeginSettleInput {
             finalize_ix_index,
             instructions_sysvar_account,
-            orders,
+            mut orders,
             token_program_account,
             state_pda_account,
         } = BeginSettleInput::parse(&data, &mut accounts).expect("parse should succeed");
         assert_eq!(finalize_ix_index, 0x1337);
         assert_eq!(instructions_sysvar_account.address(), &sysvar);
-        assert_eq!(orders.iter().count(), 0);
+        assert_eq!(orders.iter_mut().count(), 0);
         assert_eq!(token_program_account.address(), &token_program);
         assert_eq!(state_pda_account.address(), &state);
     }
@@ -544,7 +559,7 @@ mod tests {
         let BeginSettleInput {
             finalize_ix_index,
             instructions_sysvar_account,
-            orders,
+            mut orders,
             state_pda_account,
             token_program_account,
         } = BeginSettleInput::parse(&data, &mut accounts).expect("parse should succeed");
@@ -553,7 +568,7 @@ mod tests {
         assert_eq!(token_program_account.address(), &token_program);
         assert_eq!(state_pda_account.address(), &state);
 
-        let mut orders = orders.iter();
+        let mut orders = orders.iter_mut();
         let order = orders.next().expect("one settled order");
         assert_eq!(order.order_pda.address(), &order_pda);
         assert_eq!(order.sell_token_account.address(), &sell_token);
@@ -590,10 +605,10 @@ mod tests {
             0x3344u64.to_le_bytes(),
         ];
 
-        let BeginSettleInput { orders, .. } =
+        let BeginSettleInput { mut orders, .. } =
             BeginSettleInput::parse(&data, &mut accounts).expect("parse should succeed");
 
-        let mut orders = orders.iter();
+        let mut orders = orders.iter_mut();
         let order = orders.next().expect("one settled order");
         assert_eq!(order.order_pda.address(), &order_pda);
         assert_eq!(order.sell_token_account.address(), &sell_token);
@@ -643,16 +658,20 @@ mod tests {
             [0u8; ORDER_COUNT],
         ];
 
-        let parsed = BeginSettleInput::parse(&data, &mut accounts).expect("parse should succeed");
-        let orders: Vec<_> = parsed.orders.iter().collect();
+        let mut parsed =
+            BeginSettleInput::parse(&data, &mut accounts).expect("parse should succeed");
 
-        assert_eq!(orders.len(), ORDER_COUNT);
-        for (order, (order_pda, sell_token, bump)) in orders.iter().zip(&expected) {
+        // Each order borrows the iterator, so only one is live at a time:
+        // compare it against `expected` as it's yielded, without collecting.
+        let mut orders = parsed.orders.iter_mut();
+        for (order_pda, sell_token, bump) in &expected {
+            let order = orders.next().expect("an order per expected entry");
             assert_eq!(order.order_pda.address(), order_pda);
             assert_eq!(order.sell_token_account.address(), sell_token);
             assert_eq!(order.bump, *bump);
             assert_eq!(order.destinations.len(), 0);
         }
+        assert!(orders.next().is_none());
     }
 
     #[test]

interface/src/instruction/settle/mod.rs

@@ -9,7 +9,7 @@ pub use spl_token_interface::ID as SPL_TOKEN_PROGRAM_ID;
 mod begin;
 mod finalize;
 
-pub use begin::{BeginSettle, BeginSettleInput, Pull, SettledOrder};
+pub use begin::{BeginSettle, BeginSettleInput, Pull, SettledOrder, SettledOrders};
 pub use finalize::{
     finalize_push_amounts, FinalizeSettle, FinalizeSettleInput, Push, Pushes,
     FINALIZE_FIXED_ACCOUNTS,

interface/src/lib.rs

@@ -160,11 +160,24 @@ pub enum SettlementError {
     LimitPriceViolated = 24,
     /// `BeginSettle`: an order's pull amounts sum to more than `u64::MAX`.
     PullAmountOverflow = 25,
+    /// `BeginSettle`: filling this order would consume more tokens than the
+    /// maximum the user is willing to trade on this intent.
+    /// Sell: `amount_in > sell_amount`; buy: `amount_out > buy_amount`.
+    FillExceedsOrderAmount = 26,
+    /// `BeginSettle`: a non-`partially_fillable` order isn't filled completely
+    /// Sell: `amount_in != sell_amount`; buy: total `amount_out != buy_amount`.
+    OrderNotFullyFilled = 27,
+    /// `BeginSettle`: the order's cumulative `amount_withdrawn` would exceed
+    /// `u64::MAX` once this settlement's pulls are added.
+    AmountWithdrawnOverflow = 28,
+    /// `BeginSettle`: the order's cumulative `amount_received` would exceed
+    /// `u64::MAX` once this settlement's push is added.
+    AmountReceivedOverflow = 29,
     /// `ReclaimOrder` was called before the order's `valid_to` has elapsed.
-    OrderNotExpired = 26,
+    OrderNotExpired = 30,
     /// `ReclaimOrder`'s `reclaim_recipient` account doesn't match the
     /// `created_by` address recorded in the order.
-    ReclaimRecipientMismatch = 27,
+    ReclaimRecipientMismatch = 31,
 }
 
 impl From<SettlementError> for u32 {