Skip to content

Commit 62db491

Browse files
Persist streamed quotes (#4579)
# Description Streamed quotes currently don't carry a quote ID making it impossible to reuse them during order placement. This is problematic for a couple of reasons 1. It increases the amount of quote requests we are sending to solvers by a non trivial amount. 2. Re-quoting when the order is placed may mess with intended slippage, market order classification and other things that are needed for a reliable user and integrator experience. 3. Quotes without an ID can never receive a quote reward. If solvers find a way to segregate the flow, they would be better off selectively not responding to streamed quote requests, which are expected to become the standard way for users to make the decision whether they want to trade (therefore they need to be extra competitive) # Changes * Persist every quote candidate before it gets sent out in the stream * Update the documentation and test cases around this. ## How to test Existing tests ## Alternatives considered https://app.notion.com/p/cownation/RFC-Quote-Persistence-for-Streaming-Quotes-38f8da5f04ca81a98f0fc016a3a37e81 contains two alternative AI generated options: 1. Mint the id at completion, send it as a final event. 2. Reserve one id up front, store the best under it (recommended). The first option leads to having no quote id unless we wait for the full timeout. This defeats the purpose of being able to quickly place orders as soon as we have a quote that is "good enough". Especially with the high deadlines suggested in cowprotocol/docs#638 I think we would not end up getting the final event in a good amount of cases. In the second option, if the client terminates the connection prematurely there may be subjectivity around which quotes have been received (and therefore what the client saw as "best"). The main downside of the approach in this PR is that it adds a small amount of latency on the critical path (for the storage write), which in the grand scheme of current quote latency is likely irrelevant. It will also create more pressure on the DB, however since unused quotes are very short lived I don't expect this to be a concern in practice. It strikes me as the simplest approach therefore. --------- Co-authored-by: Martin Magnus <martin.beckmann@protonmail.com>
1 parent b1a7406 commit 62db491

5 files changed

Lines changed: 284 additions & 14 deletions

File tree

crates/database/src/quotes.rs

Lines changed: 191 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,12 @@ WHERE
123123
order_kind = $6 AND
124124
expiration_timestamp >= $7 AND
125125
quote_kind = $8
126-
ORDER BY gas_amount * gas_price * sell_token_price ASC
126+
-- Return the best quote for the user, mirroring the price-estimation
127+
-- competition: prefer verified quotes over unverified ones, then take the
128+
-- highest buy/sell exchange rate net of the sell-token-denominated fee.
129+
ORDER BY
130+
verified DESC,
131+
buy_amount / (sell_amount + gas_amount * gas_price / sell_token_price) DESC
127132
LIMIT 1
128133
"#;
129134
sqlx::query_as(QUERY)
@@ -395,6 +400,191 @@ mod tests {
395400
assert_eq!(find(&mut db, &search_b).await.unwrap(), None);
396401
}
397402

403+
#[tokio::test]
404+
#[ignore]
405+
async fn postgres_find_quote_picks_best_net_of_fee_rate_sell_order() {
406+
let mut db = PgConnection::connect("postgresql://").await.unwrap();
407+
let mut db = db.begin().await.unwrap();
408+
crate::clear_DANGER_(&mut db).await.unwrap();
409+
410+
let now = low_precision_now();
411+
// All candidates quote the same sell order (same sell amount); they only
412+
// differ in buy amount and fee.
413+
let base = Quote {
414+
id: Default::default(),
415+
sell_token: ByteArray([1; 20]),
416+
buy_token: ByteArray([2; 20]),
417+
sell_amount: 1000.into(),
418+
buy_amount: Default::default(),
419+
gas_amount: 0.,
420+
gas_price: 1.,
421+
sell_token_price: 0.1,
422+
order_kind: OrderKind::Sell,
423+
expiration_timestamp: now,
424+
quote_kind: QuoteKind::Standard,
425+
solver: ByteArray([1; 20]),
426+
verified: false,
427+
metadata: Default::default(),
428+
};
429+
430+
// Highest absolute buy amount, but an expensive fee.
431+
// net rate = 210 / (1000 + 100*1/0.1) = 210/2000 ≈ 0.105
432+
let mut high_buy_high_fee = Quote {
433+
buy_amount: 210.into(),
434+
gas_amount: 100.,
435+
solver: ByteArray([1; 20]),
436+
..base.clone()
437+
};
438+
high_buy_high_fee.id = save(&mut db, &high_buy_high_fee).await.unwrap();
439+
440+
// Lower absolute buy amount, but a negligible fee -> best net-of-fee rate.
441+
// net rate = 200 / (1000 + 1*1/0.1) = 200/1010 ≈ 0.198
442+
let mut best_rate = Quote {
443+
buy_amount: 200.into(),
444+
gas_amount: 1.,
445+
solver: ByteArray([2; 20]),
446+
..base.clone()
447+
};
448+
best_rate.id = save(&mut db, &best_rate).await.unwrap();
449+
450+
let search = QuoteSearchParameters {
451+
sell_token: base.sell_token,
452+
buy_token: base.buy_token,
453+
sell_amount_0: base.sell_amount.clone(),
454+
sell_amount_1: base.sell_amount.clone(),
455+
buy_amount: Default::default(),
456+
kind: OrderKind::Sell,
457+
expiration: now,
458+
quote_kind: QuoteKind::Standard,
459+
};
460+
assert_eq!(find(&mut db, &search).await.unwrap().unwrap(), best_rate);
461+
}
462+
463+
#[tokio::test]
464+
#[ignore]
465+
async fn postgres_find_quote_picks_best_net_of_fee_rate_buy_order() {
466+
let mut db = PgConnection::connect("postgresql://").await.unwrap();
467+
let mut db = db.begin().await.unwrap();
468+
crate::clear_DANGER_(&mut db).await.unwrap();
469+
470+
let now = low_precision_now();
471+
// All candidates quote the same buy order (same buy amount); they only
472+
// differ in sell amount and fee.
473+
let base = Quote {
474+
id: Default::default(),
475+
sell_token: ByteArray([1; 20]),
476+
buy_token: ByteArray([2; 20]),
477+
sell_amount: Default::default(),
478+
buy_amount: 100.into(),
479+
gas_amount: 0.,
480+
gas_price: 1.,
481+
sell_token_price: 0.1,
482+
order_kind: OrderKind::Buy,
483+
expiration_timestamp: now,
484+
quote_kind: QuoteKind::Standard,
485+
solver: ByteArray([1; 20]),
486+
verified: false,
487+
metadata: Default::default(),
488+
};
489+
490+
// Lowest absolute sell amount, but an expensive fee -> total spend 3000.
491+
// net rate = 100 / (1000 + 200*1/0.1) = 100/3000 ≈ 0.033
492+
let mut low_sell_high_fee = Quote {
493+
sell_amount: 1000.into(),
494+
gas_amount: 200.,
495+
solver: ByteArray([1; 20]),
496+
..base.clone()
497+
};
498+
low_sell_high_fee.id = save(&mut db, &low_sell_high_fee).await.unwrap();
499+
500+
// Higher absolute sell amount, but a negligible fee -> total spend 1110.
501+
// net rate = 100 / (1100 + 1*1/0.1) = 100/1110 ≈ 0.090
502+
let mut high_sell_low_fee = Quote {
503+
sell_amount: 1100.into(),
504+
gas_amount: 1.,
505+
solver: ByteArray([2; 20]),
506+
..base.clone()
507+
};
508+
high_sell_low_fee.id = save(&mut db, &high_sell_low_fee).await.unwrap();
509+
510+
let search = QuoteSearchParameters {
511+
sell_token: base.sell_token,
512+
buy_token: base.buy_token,
513+
sell_amount_0: Default::default(),
514+
sell_amount_1: Default::default(),
515+
buy_amount: base.buy_amount.clone(),
516+
kind: OrderKind::Buy,
517+
expiration: now,
518+
quote_kind: QuoteKind::Standard,
519+
};
520+
521+
assert_eq!(
522+
find(&mut db, &search).await.unwrap().unwrap(),
523+
high_sell_low_fee
524+
);
525+
}
526+
527+
#[tokio::test]
528+
#[ignore]
529+
async fn postgres_find_quote_prefers_verified_over_better_rate() {
530+
let mut db = PgConnection::connect("postgresql://").await.unwrap();
531+
let mut db = db.begin().await.unwrap();
532+
crate::clear_DANGER_(&mut db).await.unwrap();
533+
534+
let now = low_precision_now();
535+
// Two candidates for the same sell order, no fee; they differ only in
536+
// buy amount (rate) and verification status.
537+
let base = Quote {
538+
id: Default::default(),
539+
sell_token: ByteArray([1; 20]),
540+
buy_token: ByteArray([2; 20]),
541+
sell_amount: 1000.into(),
542+
buy_amount: Default::default(),
543+
gas_amount: 0.,
544+
gas_price: 1.,
545+
sell_token_price: 1.,
546+
order_kind: OrderKind::Sell,
547+
expiration_timestamp: now,
548+
quote_kind: QuoteKind::Standard,
549+
solver: ByteArray([1; 20]),
550+
verified: false,
551+
metadata: Default::default(),
552+
};
553+
554+
// Unverified but strictly better rate (more buy for the same sell).
555+
let mut unverified_better = Quote {
556+
buy_amount: 210.into(),
557+
verified: false,
558+
solver: ByteArray([1; 20]),
559+
..base.clone()
560+
};
561+
unverified_better.id = save(&mut db, &unverified_better).await.unwrap();
562+
563+
// Verified with a worse rate -> should still win
564+
let mut verified_worse = Quote {
565+
buy_amount: 200.into(),
566+
verified: true,
567+
solver: ByteArray([2; 20]),
568+
..base.clone()
569+
};
570+
verified_worse.id = save(&mut db, &verified_worse).await.unwrap();
571+
572+
let search = QuoteSearchParameters {
573+
sell_token: base.sell_token,
574+
buy_token: base.buy_token,
575+
sell_amount_0: base.sell_amount.clone(),
576+
sell_amount_1: base.sell_amount.clone(),
577+
buy_amount: Default::default(),
578+
kind: OrderKind::Sell,
579+
expiration: now,
580+
quote_kind: QuoteKind::Standard,
581+
};
582+
assert_eq!(
583+
find(&mut db, &search).await.unwrap().unwrap(),
584+
verified_worse
585+
);
586+
}
587+
398588
#[tokio::test]
399589
#[ignore]
400590
async fn postgres_save_and_find_quote_and_differentiates_by_signing_scheme() {

crates/e2e/tests/e2e/quoting.rs

Lines changed: 67 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ use {
2626
shared::web3::Web3,
2727
solvers_dto::solution::{SolverError, SolverErrorCode, SolverResponse},
2828
std::{
29+
ops::DerefMut,
2930
sync::Arc,
3031
time::{Duration, Instant},
3132
},
@@ -951,17 +952,37 @@ async fn volume_fee(web3: Web3) {
951952

952953
// Smoke test for the SSE streaming quote endpoint. Posts a quote request to
953954
// /api/v1/quote/stream and asserts that at least one SSE data line parses as
954-
// a valid OrderQuoteResponse with id == None.
955+
// a valid OrderQuoteResponse carrying a persisted quote id.
955956
async fn quote_stream_smoke(web3: Web3) {
956957
tracing::info!("Setting up chain state.");
957958
let mut onchain = OnchainComponents::deploy(web3).await;
958959

959960
let [solver] = onchain.make_solvers(10u64.eth()).await;
960-
let [trader] = onchain.make_accounts(2u64.eth()).await;
961+
let [trader] = onchain.make_accounts(10u64.eth()).await;
961962
let [token] = onchain
962963
.deploy_tokens_with_weth_uni_v2_pools(1_000u64.eth(), 1_000u64.eth())
963964
.await;
964965

966+
// Wrap and approve WETH so the trader can later place an order referencing
967+
// one of the streamed quotes.
968+
onchain
969+
.contracts()
970+
.weth
971+
.approve(onchain.contracts().allowance, 2u64.eth())
972+
.from(trader.address())
973+
.send_and_watch()
974+
.await
975+
.unwrap();
976+
onchain
977+
.contracts()
978+
.weth
979+
.deposit()
980+
.from(trader.address())
981+
.value(2u64.eth())
982+
.send_and_watch()
983+
.await
984+
.unwrap();
985+
965986
tracing::info!("Starting services.");
966987
let services = Services::new(&onchain).await;
967988
services.start_protocol(solver).await;
@@ -1026,8 +1047,8 @@ async fn quote_stream_smoke(web3: Web3) {
10261047
let buy_token = *token.address();
10271048
for response in &parsed {
10281049
assert!(
1029-
response.id.is_none(),
1030-
"streaming quotes should have id == null, got {:?}",
1050+
response.id.is_some(),
1051+
"streaming quotes should be persisted and carry an id, got {:?}",
10311052
response.id
10321053
);
10331054
assert_eq!(response.from, trader.address());
@@ -1038,4 +1059,46 @@ async fn quote_stream_smoke(web3: Web3) {
10381059
"streamed quote should have a non-zero buy amount, got {response:?}"
10391060
);
10401061
}
1062+
1063+
// Prove that placing an order with a streamed quote id reuses the persisted
1064+
// quote instead of re-quoting: the quote attached to the order must be the
1065+
// exact one we stored while streaming (same metadata).
1066+
let streamed = parsed
1067+
.iter()
1068+
.find(|response| response.id.is_some())
1069+
.expect("at least one streamed quote should carry an id");
1070+
let quote_id = streamed.id.unwrap();
1071+
let (streamed_metadata,) = crate::database::quote_metadata(services.db(), quote_id)
1072+
.await
1073+
.expect("streamed quote should be persisted");
1074+
1075+
tracing::info!("Placing order referencing a streamed quote id.");
1076+
let order = OrderCreation {
1077+
quote_id: Some(quote_id),
1078+
sell_token: weth,
1079+
sell_amount: 1u64.eth(),
1080+
buy_token,
1081+
buy_amount: streamed.quote.buy_amount,
1082+
valid_to: model::time::now_in_epoch_seconds() + 300,
1083+
kind: OrderKind::Sell,
1084+
..Default::default()
1085+
}
1086+
.sign(
1087+
EcdsaSigningScheme::Eip712,
1088+
&onchain.contracts().domain_separator,
1089+
&trader.signer,
1090+
);
1091+
let order_uid = services.create_order(&order).await.unwrap();
1092+
1093+
let order_quote = database::orders::read_quote(
1094+
services.db().acquire().await.unwrap().deref_mut(),
1095+
&database::byte_array::ByteArray(order_uid.0),
1096+
)
1097+
.await
1098+
.unwrap()
1099+
.expect("order should reference a stored quote");
1100+
assert_eq!(
1101+
streamed_metadata, order_quote.metadata,
1102+
"order should reuse the streamed quote, not trigger a re-quote"
1103+
);
10411104
}

crates/orderbook/openapi.yml

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -538,10 +538,9 @@ paths:
538538
endpoint opens a Server-Sent Events stream and emits one event per quote
539539
as solvers respond. Solvers without a usable quote emit no event, so you
540540
may receive fewer events than there are solvers. The stream closes when
541-
the quote timeout elapses or all solvers have responded. Each event's
542-
`id` is always `null`. Clients can use this to show progressive quote
543-
updates in real time.
544-
541+
the quote timeout elapses or all solvers have responded. Each emitted
542+
quote carries an `id` that can be referenced when placing an order.
543+
Clients can use this to show progressive quote updates in real time.
545544
546545
The `priceQuality` field of the request body is ignored: this endpoint
547546
always queries all solvers and attempts verification, emitting each

crates/orderbook/src/quoter.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ impl QuoteHandler {
189189
)
190190
.map_err(|err| OrderQuoteError::CalculateQuote(err.into()))
191191
.and_then(|adjusted| {
192-
build_order_quote_response(&request, &quote, &adjusted, None, valid_to)
192+
build_order_quote_response(&request, &quote, &adjusted, quote.id, valid_to)
193193
});
194194
match response {
195195
Ok(response) => {

crates/shared/src/order_quoting.rs

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -608,7 +608,9 @@ impl OrderQuoting for OrderQuoter {
608608
#[async_trait::async_trait]
609609
pub trait StreamingQuoting: Send + Sync {
610610
/// Fetches gas and native prices once, then yields one `Quote` per solver
611-
/// result as it arrives. Stores nothing.
611+
/// result as it arrives. Each yielded quote is persisted (as opposed to
612+
/// just the final quote in the one-shot quote endpoint) so it can be
613+
/// referenced by id during a later order placement.
612614
async fn calculate_quote_stream(
613615
&self,
614616
parameters: QuoteParameters,
@@ -664,6 +666,7 @@ impl StreamingQuoting for OrderQuoter {
664666
};
665667

666668
let additional_cost = parameters.additional_cost();
669+
let storage = self.storage.clone();
667670

668671
let stream = async_stream::stream! {
669672
let inner = estimator.estimate_stream(trade_query);
@@ -702,6 +705,14 @@ impl StreamingQuoting for OrderQuoter {
702705
}
703706
quote = quote.with_scaled_sell_amount(sell_amount);
704707
}
708+
// Persist the quote so it can be referenced by id when the
709+
// order is later placed, mirroring the one-shot endpoint. A
710+
// failed write must not turn an otherwise good quote into an
711+
// error event.
712+
match storage.save(quote.data.clone()).await {
713+
Ok(id) => quote.id = Some(id),
714+
Err(err) => tracing::error!(?err, "failed to persist streamed quote"),
715+
}
705716
yield Ok(quote);
706717
}
707718
};
@@ -1999,11 +2010,18 @@ mod tests {
19992010
gas_price: alloy::eips::eip1559::Eip1559Estimation,
20002011
now: chrono::DateTime<Utc>,
20012012
) -> OrderQuoter {
2013+
let next_id = std::sync::atomic::AtomicI64::new(1);
2014+
let mut storage = MockQuoteStoring::new();
2015+
storage
2016+
.expect_save()
2017+
.times(0..)
2018+
.returning(move |_| Ok(next_id.fetch_add(1, std::sync::atomic::Ordering::SeqCst)));
2019+
20022020
OrderQuoter {
20032021
price_estimator: Arc::new(MockPriceEstimating::new()),
20042022
native_price_estimator: Arc::new(native_price_estimator),
20052023
gas_estimator: Arc::new(FakeGasPriceEstimator::new(gas_price)),
2006-
storage: Arc::new(MockQuoteStoring::new()),
2024+
storage: Arc::new(storage),
20072025
now: Arc::new(now),
20082026
validity: Validity::default(),
20092027
default_quote_timeout: HEALTHY_PRICE_ESTIMATION_TIME,
@@ -2091,8 +2109,8 @@ mod tests {
20912109
let q2 = stream.next().await.expect("second quote").expect("ok");
20922110
assert!(stream.next().await.is_none());
20932111

2094-
assert_eq!(q1.id, None);
2095-
assert_eq!(q2.id, None);
2112+
assert_eq!(q1.id, Some(1));
2113+
assert_eq!(q2.id, Some(2));
20962114
assert_eq!(q1.data.quoted_buy_amount, AlloyU256::from(500));
20972115
assert_eq!(q2.data.quoted_buy_amount, AlloyU256::from(600));
20982116
}

0 commit comments

Comments
 (0)