Skip to content

Remove the v1/trades endpoint - #5109

Merged
jmg-duarte merged 1 commit into
mainfrom
jmgd/remove-v1-trades
Oct 16, 2026
Merged

jmg-duarte merged 1 commit into
mainfrom
jmgd/remove-v1-trades

Conversation

@jmg-duarte

Copy link
Copy Markdown
Contributor

Description

Following the reflow from v1/trades traffic to v2/trades in https://github.com/cowprotocol/infrastructure/pull/6821, this PR fully removes v1/trades, the v1/trades reflow should also be removed following this.

Changes

  • Fully remove v1/trades endpoint

How to test

N/A

@jmg-duarte
jmg-duarte requested a review from a team as a code owner October 16, 2026 08:40
@jmg-duarte jmg-duarte changed the title Remove the v1 trades endpoint Remove the v1/trades endpoint Oct 16, 2026
@github-actions

Copy link
Copy Markdown

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:

`GET /api/v1/trades` is unpaginated and returns every fill for the
filter, which can saturate the database. `/api/v2/trades` is the
paginated replacement. Point the e2e helpers and the playground test
script at v2 and drop v1 from the OpenAPI spec.
@claude

claude Bot commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @jmg-duarte's task in 52s —— View job


PR Review: Remove the v1 trades endpoint

  • Read the diff and gather context
  • Verify callers migrated to /api/v2/trades behave correctly
  • Check for lingering references to the removed endpoint

Verdict: LGTM ✅ — clean, well-scoped removal. No changes requested.

I verified:

  • The deleted handler (get_trades.rs) and its route registration in api.rs are the only server-side wiring removed; the OpenAPI entry is also dropped. ✅
  • No lingering /api/v1/trades references remain anywhere (code, docs, playground, configs). ✅
  • The two remaining callers now point at /api/v2/trades:
    • crates/e2e/src/setup/services.rs get_trades() helper
    • playground/test_playground.sh

One thing I considered (not blocking): v2 differs from v1 in that it is paginated with a default limit of 10, whereas v1 was unpaginated. This is safe here because every get_trades() caller queries by a single orderUid, which realistically yields 1–few trades — comfortably within the default page size. The response shape stays a JSON array, so .first() / [0] / jq '.[0]' usages are unaffected. No change needed, just noting it for the record.
· jmgd/remove-v1-trades

@jmg-duarte
jmg-duarte force-pushed the jmgd/remove-v1-trades branch from 2e6da94 to 8b1a7c6 Compare October 16, 2026 08:41

@MartinquaXD MartinquaXD left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹

@jmg-duarte
jmg-duarte added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit c27a420 Oct 16, 2026
27 checks passed
@jmg-duarte
jmg-duarte deleted the jmgd/remove-v1-trades branch October 16, 2026 11:46
@github-actions github-actions Bot locked and limited conversation to collaborators Oct 16, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants