Validate order limit prices

client/src/instructions.rs

@@ -339,7 +339,7 @@ mod tests {
                 prop_assert_eq!(push.source_buffer.address(), &expected.buffer);
                 prop_assert_eq!(push.destination.address(), &expected.destination);
                 prop_assert_eq!(push.bump, expected.bump);
-                prop_assert_eq!(u64::from_le_bytes(*push.amount), expected.amount);
+                prop_assert_eq!(push.amount, expected.amount);
             }
         }
     }

interface/src/instruction/settle/finalize.rs

@@ -7,7 +7,7 @@ use solana_program_error::ProgramError;
 use solana_pubkey::Pubkey;
 
 use crate::instruction::InstructionInputParsing;
-use crate::{SettlementError, SettlementInstruction};
+use crate::{recover_discriminator, SettlementError, SettlementInstruction};
 
 use super::{recover_counterpart, INSTRUCTIONS_SYSVAR_ID, SPL_TOKEN_PROGRAM_ID};
 
@@ -16,6 +16,49 @@ use super::{recover_counterpart, INSTRUCTIONS_SYSVAR_ID, SPL_TOKEN_PROGRAM_ID};
 /// program.
 pub const FINALIZE_FIXED_ACCOUNTS: usize = 3;
 
+/// Split the instruction bytes from `FinalizeSettle` that remain after all
+/// constant-size data has been extracted into the per-push bump bytes and the
+/// raw little-endian amount chunks.
+fn split_push_bytes(body: &[u8]) -> Result<(&[u8], &[[u8; 8]]), ProgramError> {
+    // The body is `9 * n` bytes: `n` bump bytes followed by `n` little-endian
+    // `u64` amounts. A body that isn't a whole number of these 9-byte pushes
+    // can't be parsed into the push layout (and would otherwise leave `bumps`
+    // and `amounts` with mismatched lengths).
+    if !body.len().is_multiple_of(9) {
+        return Err(ProgramError::InvalidInstructionData);
+    }
+    let push_count = body.len() / 9;
+    let (bumps, amount_bytes) = body.split_at(push_count);
+    let (amounts, []) = amount_bytes.as_chunks::<8>() else {
+        return Err(ProgramError::InvalidInstructionData);
+    };
+    Ok((bumps, amounts))
+}
+
+/// Like [`split_push_bytes`], but yields the amounts already decoded as `u64`s
+/// so callers streaming the amounts don't each re-decode the little-endian
+/// bytes. The decoding is lazy, so nothing is allocated.
+fn split_pushes(body: &[u8]) -> Result<(&[u8], impl Iterator<Item = u64> + '_), ProgramError> {
+    let (bumps, amounts) = split_push_bytes(body)?;
+    Ok((bumps, amounts.iter().copied().map(u64::from_le_bytes)))
+}
+
+/// The push amounts, in push order, carried by a `FinalizeSettle` instruction,
+/// recovered from its full instruction data alone (discriminator included, as
+/// returned by instruction introspection).
+///
+/// This function doesn't otherwise check that the instruction is consistent
+/// with a `FinalizeSettle` instruction. For example, the discriminator field is
+/// ignored.
+pub fn finalize_push_amounts(
+    instruction_data: &[u8],
+) -> Result<impl Iterator<Item = u64> + '_, ProgramError> {
+    let (_discriminator, rest) = recover_discriminator(instruction_data)?;
+    let (_begin_ix_index, body) = recover_counterpart(rest)?;
+    let (_bumps, amounts) = split_pushes(body)?;
+    Ok(amounts)
+}
+
 /// Builder for a `FinalizeSettle` instruction pushing the funds described by the
 /// parallel lists:
 /// - `source_buffers[i]` is the buffer token account the funds come from,
@@ -91,14 +134,14 @@ impl From<FinalizeSettle<'_>> for Instruction {
     }
 }
 
-/// A single fund push parsed from `FinalizeSettle`: move `amount` (little-endian
-/// `u64`) from `source_buffer` to `destination`. `bump` is `source_buffer`'s
-/// claimed canonical buffer bump, which the program re-derives against.
+/// A single fund push parsed from `FinalizeSettle`: move `amount` from
+/// `source_buffer` to `destination`. `bump` is `source_buffer`'s claimed
+/// canonical buffer bump, which the program re-derives against.
 pub struct Push<'a, A> {
     pub source_buffer: &'a A,
     pub destination: &'a A,
     pub bump: u8,
-    pub amount: &'a [u8; 8],
+    pub amount: u64,
 }
 
 /// Struct storing accounts, bumps, and amounts from parsing the input of
@@ -129,7 +172,7 @@ impl<'a, A> Pushes<'a, A> {
                 return None;
             }
             let bump = self.bumps[i];
-            let amount = &self.amounts[i];
+            let amount = u64::from_le_bytes(self.amounts[i]);
             i += 1;
 
             let source_buffer = &self.push_accounts[account_offset];
@@ -176,20 +219,8 @@ impl<'a, A> InstructionInputParsing<'a, A> for FinalizeSettleInput<'a, A> {
             return Err(ProgramError::NotEnoughAccountKeys);
         };
 
-        // The body after the begin index is, per push, a bump byte (all `n`
-        // first) then a little-endian `u64` amount: `9 * n` bytes. Unlike
-        // `BeginSettle`, there's no explicit count byte, so `n` is recovered as
-        // `body.len() / 9`; a body that isn't a whole number of these 9-byte
-        // pushes can't be parsed into the push layout at all (and would otherwise
-        // leave `bumps` and `amounts` with mismatched lengths).
-        if body.len() % 9 != 0 {
-            return Err(ProgramError::InvalidInstructionData);
-        }
-        let push_count = body.len() / 9;
-        let (bumps, amount_bytes) = body.split_at(push_count);
-        let (amounts, []) = amount_bytes.as_chunks::<8>() else {
-            return Err(ProgramError::InvalidInstructionData);
-        };
+        let (bumps, amounts) = split_push_bytes(body)?;
+        let push_count = bumps.len();
 
         // Each push contributes a source buffer and a destination account, so
         // the push-account count is `2 * n`.
@@ -222,6 +253,7 @@ mod tests {
     };
     use crate::instruction::settle::tests::ix_data;
     use hex_literal::hex;
+    use proptest::prelude::*;
     use solana_account_view::AccountView;
     use solana_address::Address;
 
@@ -399,7 +431,7 @@ mod tests {
                     push.source_buffer.address(),
                     push.destination.address(),
                     push.bump,
-                    u64::from_le_bytes(*push.amount),
+                    push.amount,
                 )
             })
             .collect();
@@ -467,7 +499,7 @@ mod tests {
             assert_eq!(push.source_buffer.address(), &expected.source);
             assert_eq!(push.destination.address(), &expected.dest);
             assert_eq!(push.bump, expected.bump);
-            assert_eq!(u64::from_le_bytes(*push.amount), expected.amount);
+            assert_eq!(push.amount, expected.amount);
         }
     }
 
@@ -539,4 +571,109 @@ mod tests {
             Some(ProgramError::InvalidInstructionData),
         );
     }
+
+    #[test]
+    fn finalize_push_amounts_extracts_amounts() {
+        let amounts = [0x0102, 0x0304];
+        let ix = Instruction::from(FinalizeSettle {
+            program_id: Pubkey::new_unique(),
+            state_pda: Pubkey::new_unique(),
+            begin_ix_index: 0x1337,
+            source_buffers: &[Pubkey::new_unique(), Pubkey::new_unique()],
+            destinations: &[Pubkey::new_unique(), Pubkey::new_unique()],
+            bumps: &[0xa1, 0xb1],
+            amounts: &amounts,
+        });
+
+        let recovered = finalize_push_amounts(&ix.data).expect("valid finalize data");
+        let decoded: Vec<u64> = recovered.collect();
+        assert_eq!(decoded, amounts);
+    }
+
+    #[test]
+    fn finalize_push_amounts_handles_no_pushes() {
+        let ix = Instruction::from(FinalizeSettle {
+            program_id: Pubkey::new_unique(),
+            state_pda: Pubkey::new_unique(),
+            begin_ix_index: 0,
+            source_buffers: &[],
+            destinations: &[],
+            bumps: &[],
+            amounts: &[],
+        });
+        let mut amounts = finalize_push_amounts(&ix.data).expect("valid empty finalize data");
+        assert!(amounts.next().is_none());
+    }
+
+    #[test]
+    fn finalize_push_amounts_rejects_incorrect_bytes() {
+        let mut ix = Instruction::from(FinalizeSettle {
+            program_id: Pubkey::new_unique(),
+            state_pda: Pubkey::new_unique(),
+            begin_ix_index: 0,
+            source_buffers: &[Pubkey::new_unique()],
+            destinations: &[Pubkey::new_unique()],
+            bumps: &[0xff],
+            amounts: &[31337],
+        });
+        ix.data.pop();
+        assert_eq!(
+            finalize_push_amounts(&ix.data).err(),
+            Some(ProgramError::InvalidInstructionData),
+        );
+    }
+
+    /// An arbitrary well-formed `FinalizeSettle` instruction with `push_count`
+    /// pushes.
+    fn arb_finalize_instruction(
+        push_count: impl Into<prop::collection::SizeRange>,
+    ) -> impl Strategy<Value = Instruction> {
+        (
+            any::<[u8; 32]>().prop_map(Pubkey::new_from_array),
+            any::<[u8; 32]>().prop_map(Pubkey::new_from_array),
+            any::<u16>(),
+            crate::instruction::settle::fixtures::arb_pushes(push_count),
+        )
+            .prop_map(
+                |(
+                    program_id,
+                    state_pda,
+                    begin_ix_index,
+                    (source_buffers, destinations, bumps, amounts),
+                )| {
+                    Instruction::from(FinalizeSettle {
+                        program_id,
+                        state_pda,
+                        begin_ix_index,
+                        source_buffers: &source_buffers,
+                        destinations: &destinations,
+                        bumps: &bumps,
+                        amounts: &amounts,
+                    })
+                },
+            )
+    }
+
+    proptest! {
+        /// For any well-formed `FinalizeSettle`, the amounts `finalize_push_amounts`
+        /// recovers from the instruction data alone (the way `BeginSettle` sees it
+        /// through introspection) must equal the amounts the full
+        /// `FinalizeSettleInput` parser reads from the same data plus its accounts.
+        #[test]
+        fn finalize_push_amounts_matches_parser(ix in arb_finalize_instruction(0..=16usize)) {
+            // Recovered from the instruction data alone.
+            let recovered: Vec<u64> =
+                finalize_push_amounts(&ix.data).expect("well-formed finalize data").collect();
+
+            // Read by the full parser from the same data plus its accounts.
+            let mut accounts: Vec<AccountView> =
+                ix.accounts.iter().map(|meta| fake_account(meta.pubkey)).collect();
+            let parsed = FinalizeSettleInput::parse(&ix.data, &mut accounts)
+                .expect("a well-formed finalize parses");
+            let parsed_amounts: Vec<u64> =
+                parsed.pushes.iter().map(|push| push.amount).collect();
+
+            prop_assert_eq!(recovered, parsed_amounts);
+        }
+    }
 }

interface/src/instruction/settle/mod.rs

@@ -10,7 +10,10 @@ mod begin;
 mod finalize;
 
 pub use begin::{BeginSettle, BeginSettleInput, Pull, SettledOrder};
-pub use finalize::{FinalizeSettle, FinalizeSettleInput, Push, Pushes, FINALIZE_FIXED_ACCOUNTS};
+pub use finalize::{
+    finalize_push_amounts, FinalizeSettle, FinalizeSettleInput, Push, Pushes,
+    FINALIZE_FIXED_ACCOUNTS,
+};
 
 /// Reads the first two bytes of a byte slice (instruction data) and
 /// interprets them as a little-endian u16, returning it together with the
@@ -27,6 +30,36 @@ pub fn recover_counterpart(instruction_data: &[u8]) -> Result<(u16, &[u8]), Prog
     }
 }
 
+#[cfg(any(test, feature = "test-fixtures"))]
+pub mod fixtures {
+    use proptest::prelude::*;
+    use solana_pubkey::Pubkey;
+
+    /// Strategy producing `count` random pushes as the parallel
+    /// `(source_buffers, destinations, bumps, amounts)` lists the
+    /// [`FinalizeSettle`](super::FinalizeSettle) builder takes.
+    pub fn arb_pushes(
+        count: impl Into<prop::collection::SizeRange>,
+    ) -> impl Strategy<Value = (Vec<Pubkey>, Vec<Pubkey>, Vec<u8>, Vec<u64>)> {
+        prop::collection::vec(
+            (
+                any::<[u8; 32]>().prop_map(Pubkey::new_from_array),
+                any::<[u8; 32]>().prop_map(Pubkey::new_from_array),
+                any::<u8>(),
+                any::<u64>(),
+            ),
+            count,
+        )
+        .prop_map(|pushes| {
+            let source_buffers = pushes.iter().map(|&(source, ..)| source).collect();
+            let destinations = pushes.iter().map(|&(_, dest, ..)| dest).collect();
+            let bumps = pushes.iter().map(|&(.., bump, _)| bump).collect();
+            let amounts = pushes.iter().map(|&(.., amount)| amount).collect();
+            (source_buffers, destinations, bumps, amounts)
+        })
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

interface/src/lib.rs

@@ -155,11 +155,16 @@ pub enum SettlementError {
     /// (wrong data length or not owned by the token program), so its mint can't
     /// be read to derive the buffer.
     InvalidBuyTokenAccount = 23,
+    /// `BeginSettle`: a settled order's executed price (`amount_out/amount_in`)
+    /// is worse than the order's limit price (`buy_amount/sell_amount`).
+    LimitPriceViolated = 24,
+    /// `BeginSettle`: an order's pull amounts sum to more than `u64::MAX`.
+    PullAmountOverflow = 25,
     /// `ReclaimOrder` was called before the order's `valid_to` has elapsed.
-    OrderNotExpired = 24,
+    OrderNotExpired = 26,
     /// `ReclaimOrder`'s `reclaim_recipient` account doesn't match the
     /// `created_by` address recorded in the order.
-    ReclaimRecipientMismatch = 25,
+    ReclaimRecipientMismatch = 27,
 }
 
 impl From<SettlementError> for u32 {