Skip to content

Commit

Permalink
Allow solver fee to be None (#1966)
Browse files Browse the repository at this point in the history
# Description

The computation of surplus fees for partially fillable limit orders
happens delayed in the co-located world. This is because at the time of
solution ranking, the autopilot doesn't have access to the executed
amounts for each order (something that #1949 will hopefully address). We
therefore wait to populate the surplus fee until the settlement (and
thus the different order specific clearing prices which incorporate the
fee) has been observed on-chain.

The logic that then computes the fee loads and updates the existing
executions from the DB. However, it simply assumes there is no surplus
fee computation if a "solver fee" is set:


https://github.com/cowprotocol/services/blob/efa7c756ff9e58b8ae3258d5715f86e2ee04185f/crates/autopilot/src/decoded_settlement.rs#L336-L338

If we look at the code that is writing this value we see that currently
this fee is always set to 0! Therefore we also assume that the surplus
fee is zero and are not reporting it for limit orders (e.g. [this
one](https://explorer.cow.fi/orders/0xf61a074c198e0b8556eb07784a137f664023822ab05f3ff368883f1ed7e7eb5dffab14b181409170378471b13ff2bff5be012c64654f6e8a?tab=overview)).
The fix is to set a NULL instead of 0 solver fee when writing the
original order execution.

Alternatively, we could also fix the fee computation in
`decode_settlement` to always add a potential solver fee and the
discrepancy between uniform and order specific clearing prices (if any)
as those are effectively fees. This would be necessary if we ever
allowed both type of fees (fixed + surplus fees). I'm not sure if this
is necessary though.

# Changes

- [x] Make solver fee optional on save order_executions
- [x] Set it to None for orders where fee is set from surplus

## How to test
- [x] e2e test

## Related Issues

Fixes #1959
  • Loading branch information
fleupold authored Oct 16, 2023
1 parent e9dc9e6 commit dd1702b
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 18 deletions.
8 changes: 2 additions & 6 deletions crates/autopilot/src/database/competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,17 +82,13 @@ impl super::Postgres {
.context("solver_competition::save")?;

for order_execution in &competition.order_executions {
let solver_fee = order_execution
.executed_fee
.fee()
.copied()
.unwrap_or_default();
let solver_fee = order_execution.executed_fee.fee().map(u256_to_big_decimal);
database::order_execution::save(
&mut ex,
&ByteArray(order_execution.order_id.0),
competition.auction_id,
None,
&u256_to_big_decimal(&solver_fee),
solver_fee.as_ref(),
)
.await
.context("order_execution::save")?;
Expand Down
6 changes: 3 additions & 3 deletions crates/database/src/order_execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub async fn save(
order: &OrderUid,
auction: AuctionId,
surplus_fee: Option<&BigDecimal>,
solver_fee: &BigDecimal,
solver_fee: Option<&BigDecimal>,
) -> Result<(), sqlx::Error> {
const QUERY: &str = r#"
INSERT INTO order_execution (order_uid, auction_id, reward, surplus_fee, solver_fee)
Expand Down Expand Up @@ -60,7 +60,7 @@ mod tests {
let mut db = db.begin().await.unwrap();
crate::clear_DANGER_(&mut db).await.unwrap();

save(&mut db, &Default::default(), 0, None, &Default::default())
save(&mut db, &Default::default(), 0, None, Default::default())
.await
.unwrap();

Expand All @@ -69,7 +69,7 @@ mod tests {
&Default::default(),
1,
Some(&Default::default()),
&Default::default(),
Default::default(),
)
.await
.unwrap();
Expand Down
8 changes: 4 additions & 4 deletions crates/database/src/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1826,7 +1826,7 @@ mod tests {

let fee: BigDecimal = 1.into();
let solver_fee: BigDecimal = 2.into();
crate::order_execution::save(&mut db, &order_uid, 0, Some(&fee), &solver_fee)
crate::order_execution::save(&mut db, &order_uid, 0, Some(&fee), Some(&solver_fee))
.await
.unwrap();

Expand Down Expand Up @@ -1878,7 +1878,7 @@ mod tests {
&order_uid,
auction_id,
None,
&bigdecimal(463182886014406361088),
Some(&bigdecimal(463182886014406361088)),
)
.await
.unwrap();
Expand Down Expand Up @@ -1958,10 +1958,10 @@ mod tests {

insert_order(&mut db, &order).await.unwrap();

crate::order_execution::save(&mut db, &order.uid, 1, None, &bigdecimal(1))
crate::order_execution::save(&mut db, &order.uid, 1, None, Some(&bigdecimal(1)))
.await
.unwrap();
crate::order_execution::save(&mut db, &order.uid, 42, None, &bigdecimal(42))
crate::order_execution::save(&mut db, &order.uid, 42, None, Some(&bigdecimal(42)))
.await
.unwrap();

Expand Down
27 changes: 23 additions & 4 deletions crates/e2e/tests/e2e/colocation_partial_fill.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use {
e2e::{setup::*, tx, tx_value},
ethcontract::U256,
model::{
order::{OrderCreation, OrderKind},
order::{LimitOrderClass, OrderClass, OrderCreation, OrderKind},
signature::EcdsaSigningScheme,
},
secp256k1::SecretKey,
Expand Down Expand Up @@ -73,7 +73,7 @@ async fn test(web3: Web3) {
&onchain.contracts().domain_separator,
SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()),
);
services.create_order(&order).await.unwrap();
let uid = services.create_order(&order).await.unwrap();

tracing::info!("Waiting for trade.");
let trade_happened =
Expand All @@ -99,6 +99,25 @@ async fn test(web3: Web3) {
.contains(&buy_balance.as_u128())
);

// TODO: test that we have other important per-auction data that should have
// made its way into the DB.
onchain.mint_blocks_past_reorg_threshold().await;

let settlement_event_processed = || async {
let order = services.get_order(&uid).await.unwrap();
if let OrderClass::Limit(LimitOrderClass {
executed_surplus_fee,
}) = order.metadata.class
{
executed_surplus_fee > U256::zero()
} else {
panic!("order is not a limit order");
}
};
wait_for_condition(TIMEOUT, settlement_event_processed)
.await
.unwrap();

let tx_hash = services.get_trades(&uid).await.unwrap()[0].tx_hash.unwrap();
let competition = services.get_solver_competition(tx_hash).await.unwrap();
assert!(!competition.common.solutions.is_empty());
assert!(competition.common.auction.orders.contains(&uid));
}
2 changes: 1 addition & 1 deletion crates/orderbook/src/database/solver_competition.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ impl SolverCompetitionStoring for Postgres {
&ByteArray(order.0),
request.auction,
surplus_fee.as_ref(),
&u256_to_big_decimal(&execution.solver_fee),
Some(&u256_to_big_decimal(&execution.solver_fee)),
)
.await
.context("order_execution::save")?;
Expand Down

0 comments on commit dd1702b

Please sign in to comment.