Make trade-fetch order-hash 'all' an explicit OrderHashFilter variant

Replace the implicit "empty slice means all trades" footgun in
build_fetch_order_trades_batch_stmt with an explicit OrderHashFilter
enum: All drops the order-hash predicate (every order), while
These(&[..]) filters to exactly those hashes. An empty These now means
*none* — it emits a constant-false 'AND 1=0' predicate (SQLite rejects
'IN ()') so empty filters return zero rows instead of silently fetching
all. All and These(empty) are explicit opposites.

The fetch_order_trades_batch empty-input short-circuit is retained but
re-documented as a perf optimization (skip the DB round-trip) rather
than a correctness guard, since These(&[]) would already return no rows.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Author: thedavidmeister

crates/common/src/local_db/query/fetch_order_trades/mod.rs

@@ -39,37 +39,76 @@ pub struct LocalDbOrderTrade {
 
 const ORDER_HASH_CLAUSE: &str = "/*ORDER_HASH_CLAUSE*/";
 const ORDER_HASH_LIST_BODY: &str = "AND tws.order_hash IN ({list})";
+/// Match-NONE predicate emitted for an empty `These` filter. SQLite rejects the
+/// degenerate `IN ()` form, so we splice a constant-false predicate that the
+/// query optimizer prunes to zero rows. This makes "filter to exactly these
+/// (none) hashes" return no rows — the deliberate opposite of `All`.
+const ORDER_HASH_MATCH_NONE_BODY: &str = "AND 1=0";
 
 const START_TS_CLAUSE: &str = "/*START_TS_CLAUSE*/";
 const START_TS_BODY: &str = "\n  AND tws.block_timestamp >= {param}\n";
 
 const END_TS_CLAUSE: &str = "/*END_TS_CLAUSE*/";
 const END_TS_BODY: &str = "\n  AND tws.block_timestamp <= {param}\n";
 
+/// Explicit selection of which orders' trades a fetch covers, so the builder
+/// never has to read "all" out of an empty slice.
+///
+/// - [`OrderHashFilter::All`] emits no order-hash predicate at all, so every
+///   order's trades for the chain/raindex (within the optional time window) are
+///   returned.
+/// - [`OrderHashFilter::These`] filters to exactly the given hashes via
+///   `WHERE order_hash IN (...)`. An empty slice means *none*: it emits a
+///   match-NONE predicate and returns zero rows. It is the deliberate opposite
+///   of `All`, not a synonym for it.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum OrderHashFilter<'a> {
+    /// No order-hash predicate: trades for every order are returned.
+    All,
+    /// Trades for exactly these order hashes. Empty = none (zero rows), never
+    /// all.
+    These(&'a [B256]),
+}
+
 /// Builds the SQL statement for retrieving order trades within the specified
-/// window. Accepts a slice of order hashes and emits a single query with a
-/// `WHERE order_hash IN (...)` clause, so trades for one or many orders are
-/// fetched in a single query (eliminating the N+1 query pattern and per-query
-/// connection overhead). The single-order path passes a one-element slice.
+/// window. The `filter` explicitly selects which orders are covered, so trades
+/// for one or many orders are fetched in a single query (eliminating the N+1
+/// query pattern and per-query connection overhead). The single-order path
+/// passes `These(&[hash])`.
 ///
-/// When `order_hashes` is empty the order-hash clause is removed entirely, so
-/// the query degenerates to "all trades for this chain/raindex" within the
-/// optional time window. Callers that want an empty result for an empty input
-/// should short-circuit before invoking this builder.
+/// The order-hash predicate rendered depends on `filter`:
+/// - [`OrderHashFilter::All`] => no order-hash predicate (every order).
+/// - [`OrderHashFilter::These`] non-empty => `AND order_hash IN (...)`.
+/// - [`OrderHashFilter::These`] empty => `AND 1=0` (match nothing): an empty
+///   `These` is *none*, never all. SQLite rejects `IN ()`, so the constant-false
+///   predicate stands in for it.
 pub fn build_fetch_order_trades_batch_stmt(
     raindex_id: &RaindexIdentifier,
-    order_hashes: &[B256],
+    filter: OrderHashFilter<'_>,
     start_timestamp: Option<u64>,
     end_timestamp: Option<u64>,
 ) -> Result<SqlStatement, SqlBuildError> {
     let mut stmt = SqlStatement::new(QUERY_TEMPLATE);
     stmt.push(SqlValue::from(raindex_id.chain_id));
     stmt.push(SqlValue::from(raindex_id.raindex_address));
-    stmt.bind_list_clause(
-        ORDER_HASH_CLAUSE,
-        ORDER_HASH_LIST_BODY,
-        order_hashes.iter().copied().map(SqlValue::from),
-    )?;
+    match filter {
+        OrderHashFilter::All => {
+            // No order-hash predicate: drop the marker, keep all orders.
+            stmt.replace(ORDER_HASH_CLAUSE, "")?;
+        }
+        OrderHashFilter::These([]) => {
+            // Empty `These` means none. `IN ()` is invalid in SQLite, so splice
+            // a constant-false predicate that yields zero rows.
+            stmt.replace(ORDER_HASH_CLAUSE, ORDER_HASH_MATCH_NONE_BODY)?;
+        }
+        OrderHashFilter::These(hashes) => {
+            stmt.bind_list_clause(
+                ORDER_HASH_CLAUSE,
+                ORDER_HASH_LIST_BODY,
+                hashes.iter().copied().map(SqlValue::from),
+            )?;
+        }
+    }
 
     // Optional time filters
     let start_param = if let Some(v) = start_timestamp {
@@ -112,7 +151,7 @@ mod tests {
         let hash_b = b256!("0x00000000000000000000000000000000000000000000000000000000deadface");
         let stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(137, Address::ZERO),
-            &[hash_a, hash_b],
+            OrderHashFilter::These(&[hash_a, hash_b]),
             Some(11),
             Some(22),
         )
@@ -144,7 +183,7 @@ mod tests {
         let hash = b256!("0x00000000000000000000000000000000000000000000000000000000deadbeef");
         let stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(1, Address::ZERO),
-            &[hash],
+            OrderHashFilter::These(&[hash]),
             None,
             None,
         )
@@ -170,7 +209,7 @@ mod tests {
         let hash = b256!("0x00000000000000000000000000000000000000000000000000000000deadface");
         let stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(137, Address::ZERO),
-            &[hash],
+            OrderHashFilter::These(&[hash]),
             Some(11),
             Some(22),
         )
@@ -193,22 +232,56 @@ mod tests {
     }
 
     #[test]
-    fn batch_empty_hashes_drops_order_hash_clause() {
+    fn all_emits_no_order_hash_predicate() {
+        // `All` is the only variant that drops the order-hash predicate: no IN
+        // list and no match-NONE constant. Every order is returned.
+        let stmt = build_fetch_order_trades_batch_stmt(
+            &RaindexIdentifier::new(1, Address::ZERO),
+            OrderHashFilter::All,
+            None,
+            None,
+        )
+        .unwrap();
+
+        // The marker is consumed and *no* order-hash predicate is rendered (the
+        // SELECT list still projects tws.order_hash, so only the predicate forms
+        // are asserted absent).
+        assert!(!stmt.sql.contains(ORDER_HASH_CLAUSE));
+        assert!(!stmt.sql.contains("tws.order_hash IN ("));
+        assert!(!stmt.sql.contains("tws.order_hash = "));
+        // Crucially, `All` does NOT emit the match-NONE predicate that empty
+        // `These` does — the two are opposites.
+        assert!(!stmt.sql.contains("1=0"));
+        // Only the two fixed params (chain id, raindex) remain.
+        assert_eq!(stmt.params.len(), 2);
+        assert_eq!(stmt.params[0], SqlValue::U64(1));
+        assert_eq!(stmt.params[1], SqlValue::Text(Address::ZERO.to_string()));
+    }
+
+    #[test]
+    fn these_empty_emits_match_none_predicate_not_dropped_clause() {
+        // An empty `These` means *none*: it emits the constant-false predicate
+        // `AND 1=0` so zero rows match. This is the deliberate opposite of `All`
+        // (which would drop the clause and return every order). It must NOT
+        // degenerate to "all".
         let stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(1, Address::ZERO),
-            &[],
+            OrderHashFilter::These(&[]),
             None,
             None,
         )
         .unwrap();
 
-        // With no hashes the order-hash WHERE predicate is removed entirely
-        // (the SELECT list still projects tws.order_hash, so only the predicate
-        // forms are asserted absent); only the two fixed params (chain id,
-        // raindex) remain.
+        // The match-NONE predicate is present...
+        assert!(stmt.sql.contains("1=0"));
+        // ...and the marker is consumed (not left unsubstituted).
         assert!(!stmt.sql.contains(ORDER_HASH_CLAUSE));
+        // No IN list / equality predicate and no bound hashes: empty These binds
+        // zero placeholders.
         assert!(!stmt.sql.contains("tws.order_hash IN ("));
         assert!(!stmt.sql.contains("tws.order_hash = "));
+        // Only the two fixed params (chain id, raindex) — the constant-false
+        // predicate binds nothing.
         assert_eq!(stmt.params.len(), 2);
         assert_eq!(stmt.params[0], SqlValue::U64(1));
         assert_eq!(stmt.params[1], SqlValue::Text(Address::ZERO.to_string()));

crates/common/src/raindex_client/local_db/query/fetch_order_trades.rs

@@ -1,5 +1,5 @@
 use crate::local_db::query::fetch_order_trades::{
-    build_fetch_order_trades_batch_stmt, LocalDbOrderTrade,
+    build_fetch_order_trades_batch_stmt, LocalDbOrderTrade, OrderHashFilter,
 };
 use crate::local_db::query::{LocalDbQueryError, LocalDbQueryExecutor};
 use crate::local_db::RaindexIdentifier;
@@ -40,12 +40,16 @@ pub async fn fetch_order_trades_batch<E: LocalDbQueryExecutor + ?Sized>(
     start_timestamp: Option<u64>,
     end_timestamp: Option<u64>,
 ) -> Result<Vec<LocalDbOrderTrade>, LocalDbQueryError> {
+    // Performance optimization, not a correctness guard: `These(&[])` already
+    // builds a valid match-NONE query that returns zero rows, so this is the
+    // same result the DB would give — we just skip the round-trip when we
+    // already know the answer is empty.
     if order_hashes.is_empty() {
         return Ok(Vec::new());
     }
     let stmt = build_fetch_order_trades_batch_stmt(
         raindex_id,
-        order_hashes,
+        OrderHashFilter::These(order_hashes),
         start_timestamp,
         end_timestamp,
     )?;
@@ -151,7 +155,7 @@ mod wasm_tests {
 
         let expected_stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(chain_id, raindex),
-            &[order_hash],
+            OrderHashFilter::These(&[order_hash]),
             start,
             end,
         )
@@ -192,7 +196,7 @@ mod wasm_tests {
 
         let expected_stmt = build_fetch_order_trades_batch_stmt(
             &RaindexIdentifier::new(chain_id, raindex),
-            &[hash_a, hash_b],
+            OrderHashFilter::These(&[hash_a, hash_b]),
             start,
             end,
         )