Paginated Trades API

[squadgazzz]

Summary

Implements offset/limit pagination for the /v1/trades endpoint, matching the existing pagination pattern used in /v1/user_orders.

Changes

• Add offset and limit query parameters to the trades API endpoint
• Update SQL queries in the database layer to support OFFSET/LIMIT
• Add validation for pagination parameters (limit must be between 1 and 1000)
• Default values: offset=0, limit=10

How to test

New tests, updated existing tests.

[Copilot]

Pull request overview

This PR implements offset/limit pagination for the /v1/trades endpoint to enable efficient retrieval of trade data with configurable result set sizes.

• Adds offset and limit query parameters with defaults (offset=0, limit=10) and validation (limit: 1-1000)
• Updates the database layer to support OFFSET/LIMIT in SQL queries across UNION branches
• Extends test coverage for pagination behavior and parameter validation

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
crates/orderbook/src/api/get_trades.rs	Adds pagination query parameters, validation logic, error handling, and comprehensive tests for offset/limit functionality
crates/orderbook/src/database/trades.rs	Updates TradeFilter struct with offset/limit fields and adds type conversion with error context
crates/database/src/trades.rs	Modifies SQL query to support pagination with LIMIT/OFFSET clauses in UNION subqueries and outer query, includes new pagination test
crates/orderbook/src/orderbook.rs	Updates existing trade query call to include required pagination parameters

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[fafk]

This query is super confusing.

Can we do

SELECT
    t.block_number,
    t.log_index,
    t.order_uid,
    t.buy_amount,
    t.sell_amount,
    t.sell_amount - t.fee_amount AS sell_amount_before_fees,
    COALESCE(o.owner, jit_o.owner) AS owner,
    COALESCE(o.buy_token, jit_o.buy_token) AS buy_token,
    COALESCE(o.sell_token, jit_o.sell_token) AS sell_token,
    settlement.tx_hash,
    settlement.auction_id
FROM trades t
LEFT JOIN LATERAL (
    SELECT tx_hash, auction_id
    FROM settlements s
    WHERE s.block_number = t.block_number
      AND s.log_index > t.log_index
    ORDER BY s.log_index ASC
    LIMIT 1
) AS settlement ON true
LEFT JOIN orders o
    ON o.uid = t.order_uid
LEFT JOIN onchain_placed_orders onchain_o
    ON onchain_o.uid = t.order_uid
LEFT JOIN jit_orders jit_o
    ON jit_o.uid = t.order_uid
WHERE
    (
        $1 IS NULL
        OR o.owner = $1
        OR onchain_o.sender = $1
        OR jit_o.owner = $1
    )
  AND ($2 IS NULL OR t.order_uid = $2)
ORDER BY t.block_number DESC, t.log_index DESC
LIMIT $3 OFFSET $4;

instead all the unions and the overfetching of $3 + $4?

[squadgazzz]

I'd experiment with this in a separate PR, since your version confuses me much more 😅

[m-sz]

I agree that it would be best to be able to see the query as a whole, in order to reason about its correctness easier.

[fafk]

I'd experiment with this in a separate PR, since your version confuses me much more 😅

I found 2 things confusing:

1. I had to write a test to output the final string to make things easier on me to review
2. The $3 + $4 threw me off (as it did copilot hehe), but an unusual pattern that you up to the offset and then the limit and then discard everything up to the offset in the outer layer 😅

The query I propose gets rid off the concatenations and just fetches everything at once so there is no need to "overfetch and discard". Anyway I think your SQL works, so I am approving and leaving it to your discretion if you wanna follow up with some refactoring. 🙌

[squadgazzz]

The $3 + $4 threw me off

We have the same approach in the get_user_orders query.

services/crates/database/src/order_history.rs

Lines 24 to 44 in 7585d2f

	const QUERY: &str = const_format::concatcp!(
	"(SELECT ", orders::SELECT,
	" FROM ", orders::FROM,
	" LEFT OUTER JOIN onchain_placed_orders onchain_o on onchain_o.uid = o.uid",
	" WHERE o.owner = $1",
	" ORDER BY creation_timestamp DESC LIMIT $2 + $3 ) ",
	" UNION ",
	" (SELECT ", orders::SELECT,
	" FROM ", orders::FROM,
	" LEFT OUTER JOIN onchain_placed_orders onchain_o on onchain_o.uid = o.uid",
	" WHERE onchain_o.sender = $1 ",
	" ORDER BY creation_timestamp DESC LIMIT $2 + $3 ) ",
	" UNION ",
	" (SELECT ", jit_orders::SELECT,
	" FROM ", jit_orders::FROM,
	" WHERE o.owner = $1 AND NOT EXISTS (SELECT 1 FROM orders ord WHERE o.uid = ord.uid)",
	" ORDER BY creation_timestamp DESC LIMIT $2 + $3 ) ",
	" ORDER BY creation_timestamp DESC ",
	" LIMIT $2 ",
	" OFFSET $3 ",
	);

[fafk]

I don't think it legitimizes the pattern. 😅

[m-sz]

Otherwise from how the query is structured, the change looks fine

[github-actions]

Reminder: Please consider backward compatibility when modifying the API specification.
If breaking changes are unavoidable, ensure:

• You explicitly pointed out breaking changes.
• You communicate the changes to affected teams (at least Frontend team and SAFE team).
• You provide proper versioning and migration mechanisms.

---

Caused by:

• crates/orderbook/openapi.yml

[MartinquaXD]

This is quite the breaking change, no?
Anyone using the existing endpoint will suddenly get a lot fewer orders. It should be pretty straight forward to introduce a v2 endpoint for the pagination while keeping the old one the same.
We could also immediately adjust the v1 description to mark it as deprecated.

[squadgazzz]

Ok, makes sense.