Migrate orderbook API from warp to axum #4080

jmg-duarte · 2026-01-22T17:21:07Z

Description

This PR migrates the orderbook API from Warp to Axum. The migration modernizes our web framework while maintaining API
compatibility, with improved code organization and reduced complexity (~200 lines removed).

Changes

Replace Warp filters with Axum Router and centralized AppState for dependency injection
Migrate all 18 API endpoints (v1 and v2) to Axum handler functions
Update dependencies: remove warp, add axum, tower, tower-http
Reorganize routes hierarchically with .nest(), alphabetically by prefix
Fix route conflicts using .merge() for multiple HTTP methods on same path
Fix bug in get_trades_v2 (use database_read instead of database_write)
Add comprehensive E2E test suite for HTTP behavior validation in malformed_requests.rs

Breaking Changes:

JSON deserialization errors now return 422 instead of 400 (Axum convention)

How to test

Existing tests + staging

I deployed these changes to base staging to ensure metrics were working, here's the graph

Screenshot 2026-01-23 at 12-42-46 Edit panel - GPv2 - Dashboards - Amazon Managed Grafana

Follow up:

Unmarked spots are running main, the wrong image was labeling endpoints with unknown because it was a fallback.
That code has been removed and replaced with the current metric labeling scheme.

That scheme can be improved (read, made less verbose and error prone while being more general) but we need to change the metrics label, which I didn't do to minimize changes.

gemini-code-assist

Code Review

This pull request migrates the orderbook API from Warp to Axum, modernizing the web framework and improving code organization. The changes include replacing Warp filters with Axum Router, migrating API endpoints, updating dependencies, and adding an E2E test suite. The review focuses on identifying critical and high severity issues, with actionable suggestions for improvement, while adhering to the repository's stricter code review style guide. One comment was modified to explicitly reference a relevant repository rule.

crates/orderbook/src/api.rs

crates/orderbook/src/api/get_trades_v2.rs

crates/orderbook/src/api.rs

crates/e2e/tests/e2e/replace_order.rs

fafk · 2026-01-26T11:13:32Z

crates/e2e/tests/e2e/malformed_requests.rs

            response.status(),
-            StatusCode::NOT_FOUND,
-            "Expected 404 for invalid OrderUid ({description}): {uid}"
+            StatusCode::BAD_REQUEST,


Strictly speaking axum using correct HTTP error codes makes this migration backwards incompatible. I'm not saying we should keep the wrong ones, but we might wanna ping the FE folks. 🤔

Pinged them, though I'm not expecting these error codes to be especially problematic

Should be ok
https://cowservices.slack.com/archives/C036G0J90BU/p1769427475645459

:chefskiss:

This is a public API. Not only FE uses it.

If needed I can go back, no worries. On the other hand, if clients are checking for a very specific error they, uh, shouldn't

The thing is that we have many integrations, and who knows how they use the API?

I'd rather create a separate issue for this. Notify everyone we can, and only then merge the breaking changes separately.

crates/observe/src/distributed_tracing/tracing_warp.rs

This commit migrates the orderbook HTTP API from the warp web framework to axum. The migration includes: - Replace warp filters with axum Router and handlers - Update HTTP status codes to match axum conventions (422 for JSON deserialization errors, proper 400/404 distinction) - Restructure API with centralized AppState shared across handlers - Implement axum-compatible middleware for metrics and tracing - Remove warp dependency and related tracing utilities - Update e2e tests to reflect new HTTP behavior Breaking changes: - Invalid JSON payloads now return 422 (Unprocessable Entity) instead of 400 - Path parameter validation now returns 400 for malformed values - Error response format remains compatible 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

Fixed critical issues identified in PR review: - Fixed route conflicts by using .merge() for multiple HTTP methods on same paths - Changed get_trades_v2 to use database_read instead of database_write - Reorganized routes alphabetically by prefix using .nest() for better maintainability - Made METRIC_LABELS a module-level const for reusability - Removed convert_json_response helper (too simple to warrant abstraction) - Optimized Arc wrapping by using trait references directly - Applied per-method middleware before merging to ensure correct metric labels - Simplified test code by removing unnecessary verbose match expressions 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>

crates/orderbook/src/api/get_solver_competition_v2.rs

crates/orderbook/src/api/get_trades.rs

crates/orderbook/src/api/get_trades_v2.rs

crates/orderbook/src/api/get_trades.rs

crates/orderbook/src/api/post_quote.rs

crates/orderbook/src/api.rs

MartinquaXD · 2026-01-29T09:19:46Z

crates/orderbook/src/api.rs

+                axum::routing::get(get_user_orders::get_user_orders_handler)
+                    .layer(middleware::from_fn(with_labelled_metric("v1/get_user_orders"))),


Could we reduce the usage of with_labelled_metrics? Given that axums paths just use a simple string to define them this should be easier than before even.
I think a structure similar to what we had before would be nicer to read. Not sure if this possible though since these handlers all have different types so some boxing or so would probably be needed. 🤔

let routes = [ ("v1/solver_competition/latest", axum::routing::get(handler) ]; routes.into_iter().map(|(path, handler)| { with_labelled_metric(path, handler) }) .collect()

There's a small issue with your approach — not all routes match their label 😭

IMO we should make them match the label and in that case, we get better, we should be able to apply a middleware wholesale because we can get the path with placeholder

We might be able to simplify further with the TraceLayer and MatchedPath too
https://docs.rs/axum/latest/axum/extract/struct.MatchedPath.html

In summary: if the team is ok with the rename (I am!) we can make this whole thing even simpler

We can also move with this as is, and rename later so we are sure nothing broke

MatchedPath sounds good. I'm fine with slightly renamed metric labels to make all of this simpler.

so we are sure nothing broke

I'm pretty sure those metrics are only used in the api latency dashboards which just shows all the labels that exist.

Done in 60a3e7a, not tested yet though

This is how it looks when the change "is happening"

crates/orderbook/src/api.rs

extrawurst · 2026-01-30T19:22:01Z

JSON deserialization errors now return 422 instead of 400 (Axum convention)

Is this in our API docs anywhere that we have to update aswell?

github-actions · 2026-02-02T11:52:20Z

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

squadgazzz · 2026-02-03T09:30:43Z

crates/orderbook/src/api/get_solver_competition.rs

-        let mut storage = MockSolverCompetitionStoring::new();
-        storage
-            .expect_load_competition()
-            .times(2)
-            .returning(|_| Ok(Default::default()));
-        storage


Why not continue using the mocked storage? The test now only validates the conversion of the result.

Before we were mocking just the single trait, now we'd need to mock the whole app state

I can create an extra layer that further extracts the trait component out of the state and that does the test instead

Probably makes sense if other e2e tests don't cover this already.

I've reviewed this and they are indeed tested in e2e, for example:

services/crates/e2e/src/setup/services.rs

Line 832 in 9a0f5fe

.get(format!("{API_HOST}/api/v1/solver_competition/{auction_id}"))

Regardless, I don't find the previous kind of tests still apply here.

For example, axum leverages DeserializeOwned to extract elements off of the paths, so what you should test first is the respective type's implementation of DeserializeOwned

About the errors, since trait is mocked, it simply returns what we want, and due to the handler itself being just a wrapper around the trait, we'd be just pretending to test it; if it wasn't a wrapper there would be more to it, not in this case.

So in the end, the thing that is actually useful to be tested, is the code that didn't change — the trait implementations

Ok, makes sense. Thanks for double-checking.

crates/orderbook/src/api/get_solver_competition.rs

crates/orderbook/src/api/version.rs

crates/orderbook/src/api.rs

squadgazzz · 2026-02-03T11:19:01Z

Deserialization error responses return plain text instead of JSON

Can we continue sending the same JSONs?

crates/e2e/tests/e2e/malformed_requests.rs

jmg-duarte · 2026-02-03T21:17:39Z

Can we continue sending the same JSONs?

This is still the case, I just asked Claude to write the description and didn't notice this.
Description has been updated, thanks!

MartinquaXD

Looks overall okay to me. Given how sensitive this change is I suggest to leave manually deploy it on mainnet staging and do some manual testing and only merge once that is okay. The reason is that if we just merge it now chances are subtle breaking changes will be overlooked and this gets rolled out to prod on Tuesday anyway.

crates/orderbook/src/api.rs

MartinquaXD · 2026-02-05T13:33:22Z

crates/orderbook/src/api.rs

-        .and(instrumented)
-        .recover(handle_rejection)
-        .with(cors)
-        .with(warp::log::log(log_prefix))


The functionality of this layer was dropped. This produced logs like:

2026-02-05T13:30:38.793Z INFO http_request{request_id=d756c2c9564e3e12aabd68cac8987945}: orderbook::api::request_summary: - "POST /api/v1/quote HTTP/1.1" 200 "-" "Mozilla/5.0 (X11; Linux i686; rv:142.0) Gecko/20100101 Firefox/142.0" 4.244271624s trace_id=5fda9db1c7c2c3dc96dc4eb82da62473

Where the now lost bit is: - "POST /api/v1/quote HTTP/1.1" 200 "-" "Mozilla/5.0 (X11; Linux i686; rv:142.0) Gecko/20100101 Firefox/142.0" 4.244271624s so a log like this would probably have to be emitted from the new metrics adapter thingy.

I've addressed this but not sure if it is what you're expecting. With the new logging it's not clear what would be the most useful message as we can push information to the fields that will be parsed as JSON

Here you can see the log line and respective JSON

Here the parsed fields

I'll be the first one to admit that log line is useless, but I'm not sure about duplicating parseable information in the log line either. I can put the timing in the log line but without the path, it's once again, not very helpful.

To make the timing useful, I can put the path, but then the method is also helpful to distinguish endpoints; if I add the method the status will also be helpful to distinguish successes from failures. So, in the end, the only thing I don't think is needed right in the log line is the user agent 😅

Having only request_summary as the message string and all the other information as values on the log line is fine. 👌

crates/orderbook/src/api.rs

squadgazzz · 2026-02-11T10:02:19Z

crates/orderbook/src/api.rs

+    pub error_type: Cow<'static, str>,
+    pub description: Cow<'static, str>,


Why is this change required? Can we continue using <'a>?

squadgazzz · 2026-02-11T10:04:36Z

crates/orderbook/src/api.rs

-        .requests_rejected
-        .with_label_values(&[response.status().as_str()])
-        .inc();
+    metrics.reset_requests_rejected();


Is it missing now?

Suggested change

metrics.reset_requests_rejected();

metrics.reset_requests_complete();

metrics.reset_requests_rejected();

squadgazzz · 2026-02-11T10:05:59Z

crates/model/src/order.rs

+        ensure!(
+            self.data.order_uids.len() < ORDER_UID_LIMIT,
+            "order uids are limited to {ORDER_UID_LIMIT}"
+        );


This duplicates the logic from here:
https://github.com/cowprotocol/services/pull/4080/changes#diff-e2c680b47c3828e14dc37fc519c878b8a59d477050d1e0bba26edcdef21e01fbR15-R20

jmg-duarte · 2026-02-11T10:11:09Z

I've dismissed the CodeQL as I've placed protections to ensure it doesn't happen for the current code

https://github.com/cowprotocol/services/pull/4080/changes#diff-e2c680b47c3828e14dc37fc519c878b8a59d477050d1e0bba26edcdef21e01fbR13-R20

https://github.com/cowprotocol/services/pull/4080/changes#diff-ba5bd3dd03f3c27aadd928f588dce2b18074b12c3090bcedb76d47f59dfb54c0R541-R544

jmg-duarte marked this pull request as ready for review January 23, 2026 15:08

jmg-duarte requested a review from a team as a code owner January 23, 2026 15:08

gemini-code-assist bot reviewed Jan 23, 2026

View reviewed changes

fafk reviewed Jan 26, 2026

View reviewed changes

crates/observe/src/distributed_tracing/tracing_warp.rs Show resolved Hide resolved

jmg-duarte and others added 7 commits January 26, 2026 11:25

fmt

1380e8d

testing layer change

801bb90

prom label fix

9504557

remove extra log

a4fe3a6

clippy

8f617f3

MartinquaXD reviewed Jan 29, 2026

View reviewed changes

jmg-duarte added 8 commits January 30, 2026 16:21

Merge branch 'main' into jmgd/axum/orderbook-2

ad90a95

cfg(test) on response_body

ed165a2

impl IntoResponse -> Response

8f2cccb

missed one endpoint

9d71def

flatten routes

43423a3

early returns

1f2486c

inline log

1b86466

response +db fix

8ebb11c

jmg-duarte added 2 commits January 30, 2026 22:05

move metrics

1e5857e

fixes

9af007d

jmg-duarte force-pushed the jmgd/axum/orderbook-2 branch from 45e0f9a to 9af007d Compare January 30, 2026 22:07

jmg-duarte added 2 commits January 30, 2026 22:10

fmt

b03fb86

update orderbook openapi

a7a6280

squadgazzz reviewed Feb 3, 2026

View reviewed changes

crates/e2e/tests/e2e/malformed_requests.rs Outdated Show resolved Hide resolved

jmg-duarte added 2 commits February 3, 2026 17:08

Merge branch 'main' into jmgd/axum/orderbook-2

3191b88

replace verbose path matching with uniform matchpath

60a3e7a

MartinquaXD approved these changes Feb 5, 2026

View reviewed changes

jmg-duarte added 11 commits February 9, 2026 14:20

Merge branch 'main' into jmgd/axum/orderbook-2

a3f1ed2

🐮

cf550dd

simplify version endpoint

0147031

Simplify response return types

ea59486

Remove unused From

d5ede20

negative numbers -> bad_request

3f1c59d

fix the request logging

3cf9a63

log instead of leveraging spans

9a0f5fe

De-Arc the QuoteHandler

29b7254

Bound cancellation batch size

f37a7fd

try again

d5d0a09

MartinquaXD approved these changes Feb 10, 2026

View reviewed changes

crates/orderbook/src/api.rs Outdated Show resolved Hide resolved

crates/orderbook/src/api.rs Outdated Show resolved Hide resolved

crates/orderbook/src/api.rs Outdated Show resolved Hide resolved

update logs

ec9c7c4

fafk approved these changes Feb 11, 2026

View reviewed changes

squadgazzz reviewed Feb 11, 2026

View reviewed changes

		axum::routing::get(get_user_orders::get_user_orders_handler)
		.layer(middleware::from_fn(with_labelled_metric("v1/get_user_orders"))),

		pub error_type: Cow<'static, str>,
		pub description: Cow<'static, str>,

	metrics.reset_requests_rejected();
	metrics.reset_requests_complete();
	metrics.reset_requests_rejected();

Migrate orderbook API from warp to axum #4080

Are you sure you want to change the base?

Migrate orderbook API from warp to axum #4080

Conversation

jmg-duarte commented Jan 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

How to test

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

extrawurst commented Jan 30, 2026

Uh oh!

github-actions bot commented Feb 2, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmg-duarte Feb 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

squadgazzz commented Feb 3, 2026

Uh oh!

Uh oh!

jmg-duarte commented Jan 22, 2026 •

edited

Loading

jmg-duarte Feb 10, 2026 •

edited

Loading