(fast8) E2E tests for the fast-path settlement flow - #4863
Conversation
|
Claude finished @MartinquaXD's task in 3m 5s —— View job PR Review — E2E tests for the fast-path settlement flow
Nice, thorough coverage — the DB-level assertions on quote promotion, competition-table patching, and the compounded volume-fee capture are exactly the invariants worth pinning down, and the fallback test cleanly isolates the regular-auction path. Only substantive concern is timing-based flakiness in a couple of the wall-clock assertions on loaded CI runners — left two inline comments:
Both are non-blocking. Everything else (fee-ordering assumption, competition-auction proofs, the |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| let expected = model::time::now_in_epoch_seconds() + exclusivity.as_secs() as u32; | ||
| assert!( | ||
| valid_from.abs_diff(expected) <= 2, | ||
| "valid_from {valid_from} should be ~{expected} (now + exclusivity)" | ||
| ); |
There was a problem hiding this comment.
expected is sampled here — after create_order (an HTTP round-trip) and a DB acquire+query have elapsed — but valid_from was fixed server-side at order-creation time. The abs_diff <= 2 tolerance is therefore really "≤2s of wall time elapsed between order creation and this line". On a loaded CI runner that gap can plausibly exceed 2s, making this assert flaky.
Consider sampling now before create_order and asserting valid_from falls in [before+excl, after+excl], or widening the tolerance.
| // Held out: not settled early by the fast path. | ||
| assert_eq!( | ||
| services.get_order(&uid).await.unwrap().metadata.status, | ||
| OrderStatus::Open | ||
| ); | ||
| let valid_from = { | ||
| let mut db = services.db().acquire().await.unwrap(); | ||
| sqlx::query_scalar::<_, Option<i64>>("SELECT valid_from FROM orders WHERE uid = $1") | ||
| .bind(ByteArray(uid.0)) | ||
| .fetch_one(db.deref_mut()) | ||
| .await | ||
| .unwrap() | ||
| .expect("fast-path order has a valid_from") as u32 | ||
| }; | ||
| assert!( | ||
| valid_from > model::time::now_in_epoch_seconds(), | ||
| "valid_from {valid_from} should be in the future (order held out)" | ||
| ); |
There was a problem hiding this comment.
With a 5s exclusivity window, both of these assertions race against wall time. valid_from = creation_now + 5s; if create_order + this first get_order / DB read take longer than 5s (possible on a slow/loaded CI runner), the order can already have passed valid_from and been picked up by the regular auction — flipping status away from Open and failing valid_from > now. A larger window (as used in the other tests) would remove the race at the cost of a slightly longer test.
MartinquaXD
changed the title
1bc47b1 to
48f81b0
Compare
Adds three ignored E2E tests plus supporting registration: * `fast_path_quote_promotion` — quoting with `enableFastPath` writes synthetic competition rows, placing the order patches them to the real `order_uid` (both trait and onchain / ethflow paths). * `fast_path_settle` — the fast path settles the order out of competition within the exclusivity window, and never re-appears in a regular auction. * `fast_path_regular_auction_fallback` — when a fast-path order's exclusivity elapses without settlement, the regular auction picks it up and settles it. * `fast_path_volume_fees_captured` — configures a protocol volume fee and a partner volume fee, then asserts the `/trades` API reports both, with the correct policy factors and the buy amount reduced accordingly. Also removes the now obsolete `quote_fastpath_flags.rs` (previously asserted that placing a fast-path order was rejected). Signed-off-by: MartinquaXD <martin@cow.fi>
48f81b0 to
7c19175
Compare
| // 2) The transient `quotes` row must be tagged with the fast-path | ||
| // `auction_id`, and that auction id must be present across all | ||
| // competition tables written at quote time. | ||
| tracing::info!("Verifying competition tables written at quote time"); |
There was a problem hiding this comment.
All these assertions are quite "white box" (knowing how the implementation works) and not really the spirit of our other e2e tests. I think what we should test is that a fast path order gets settled "correctly" (right amounts, etc) before the next real auction is spawned. For this we should use the externally facing API functions that a user would use to look at their trades.
So I think having one fast_path test module with the three cases
- Regular order
- Ethflow order
- Fallback auction
is all we need here.
| // The order still requests the fast path, so the orderbook holds it out of | ||
| // the auction until `valid_from`. |
There was a problem hiding this comment.
This really tests the validFrom feature (which enables wait for CoW), but not the failover. For failover I'd expect we shut down the quote winning solver (so it doesn't settle) and expect the order to be settled by another solver in the first regular auction after the exclusivity period.
| fee.token, buy_token, | ||
| "fees on a sell order are taken from the buy token" | ||
| ); | ||
| assert!(!fee.amount.is_zero(), "fee amount should be positive"); |
There was a problem hiding this comment.
Here we should probably assert that the amounts are correct to avoid issues with the fee calculation logic.
2 participants
Description
Finally adds e2e test coverage of a few different things:
How to test
this is it