Skip to content

Commit

Permalink
Remove full_fee_amount and solver_fee (#3223)
Browse files Browse the repository at this point in the history
# Description
As part of fee breakdown refactor, the time has come to remove
`full_fee_amount` and `solver_fee` from order API.

These fields were relevant more than 2 years ago when orders with signed
fee > 0 existed and when subsidies for that fee were calculated.

Will wait for confirmation from frontend that these fields are no longer
used anywhere, before merging.

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [ ] Removed fields from the API and all dependent code paths.

## How to test
Existing unit tests, e2e tests, postgresql tests.

<!--
## Related Issues

Fixes #
-->
  • Loading branch information
sunce86 authored Jan 10, 2025
1 parent d1972cd commit d9a7621
Show file tree
Hide file tree
Showing 12 changed files with 5 additions and 76 deletions.
3 changes: 0 additions & 3 deletions crates/autopilot/src/database/onchain_order_events/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -616,7 +616,6 @@ fn convert_onchain_order_placement(
settlement_contract: ByteArray(settlement_contract.0),
sell_token_balance: sell_token_source_into(order_data.sell_token_balance),
buy_token_balance: buy_token_destination_into(order_data.buy_token_balance),
full_fee_amount: u256_to_big_decimal(&order_data.fee_amount),
cancellation_timestamp: None,
class: match order_data.fee_amount.is_zero() {
true => OrderClass::Limit,
Expand Down Expand Up @@ -925,7 +924,6 @@ mod test {
settlement_contract: ByteArray(settlement_contract.0),
sell_token_balance: sell_token_source_into(expected_order_data.sell_token_balance),
buy_token_balance: buy_token_destination_into(expected_order_data.buy_token_balance),
full_fee_amount: u256_to_big_decimal(&expected_order_data.fee_amount),
cancellation_timestamp: None,
};
assert_eq!(onchain_order_placement, expected_onchain_order_placement);
Expand Down Expand Up @@ -1036,7 +1034,6 @@ mod test {
settlement_contract: ByteArray(settlement_contract.0),
sell_token_balance: sell_token_source_into(expected_order_data.sell_token_balance),
buy_token_balance: buy_token_destination_into(expected_order_data.buy_token_balance),
full_fee_amount: u256_to_big_decimal(&U256::zero()),
cancellation_timestamp: None,
};
assert_eq!(onchain_order_placement, expected_onchain_order_placement);
Expand Down
2 changes: 1 addition & 1 deletion crates/database/src/jit_orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use {

pub const SELECT: &str = r#"
o.uid, o.owner, o.creation_timestamp, o.sell_token, o.buy_token, o.sell_amount, o.buy_amount,
o.valid_to, o.app_data, o.fee_amount, o.fee_amount AS full_fee_amount, o.kind, o.partially_fillable, o.signature,
o.valid_to, o.app_data, o.fee_amount, o.kind, o.partially_fillable, o.signature,
o.receiver, o.signing_scheme, '\x9008d19f58aabd9ed0d60971565aa8510560ab41'::bytea AS settlement_contract, o.sell_token_balance, o.buy_token_balance,
'liquidity'::OrderClass AS class,
(SELECT COALESCE(SUM(t.buy_amount), 0) FROM trades t WHERE t.order_uid = o.uid) AS sum_buy,
Expand Down
13 changes: 2 additions & 11 deletions crates/database/src/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,6 @@ pub struct Order {
pub settlement_contract: Address,
pub sell_token_balance: SellTokenSource,
pub buy_token_balance: BuyTokenDestination,
pub full_fee_amount: BigDecimal,
pub cancellation_timestamp: Option<DateTime<Utc>>,
pub class: OrderClass,
}
Expand Down Expand Up @@ -140,11 +139,10 @@ INSERT INTO orders (
settlement_contract,
sell_token_balance,
buy_token_balance,
full_fee_amount,
cancellation_timestamp,
class
)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20)
"#;

pub async fn insert_order_and_ignore_conflicts(
Expand Down Expand Up @@ -183,7 +181,6 @@ async fn insert_order_execute_sqlx(
.bind(order.settlement_contract)
.bind(order.sell_token_balance)
.bind(order.buy_token_balance)
.bind(&order.full_fee_amount)
.bind(order.cancellation_timestamp)
.bind(order.class)
.execute(ex)
Expand Down Expand Up @@ -473,7 +470,6 @@ pub struct FullOrder {
pub valid_to: i64,
pub app_data: AppId,
pub fee_amount: BigDecimal,
pub full_fee_amount: BigDecimal,
pub kind: OrderKind,
pub class: OrderClass,
pub partially_fillable: bool,
Expand Down Expand Up @@ -547,7 +543,7 @@ impl FullOrder {
// that with the current amount of data this wouldn't be better.
pub const SELECT: &str = r#"
o.uid, o.owner, o.creation_timestamp, o.sell_token, o.buy_token, o.sell_amount, o.buy_amount,
o.valid_to, o.app_data, o.fee_amount, o.full_fee_amount, o.kind, o.partially_fillable, o.signature,
o.valid_to, o.app_data, o.fee_amount, o.kind, o.partially_fillable, o.signature,
o.receiver, o.signing_scheme, o.settlement_contract, o.sell_token_balance, o.buy_token_balance,
o.class,
(SELECT COALESCE(SUM(t.buy_amount), 0) FROM trades t WHERE t.order_uid = o.uid) AS sum_buy,
Expand Down Expand Up @@ -880,7 +876,6 @@ mod tests {
assert_eq!(order.valid_to, full_order.valid_to);
assert_eq!(order.app_data, full_order.app_data);
assert_eq!(order.fee_amount, full_order.fee_amount);
assert_eq!(order.full_fee_amount, full_order.full_fee_amount);
assert_eq!(order.kind, full_order.kind);
assert_eq!(order.class, full_order.class);
assert_eq!(order.partially_fillable, full_order.partially_fillable);
Expand Down Expand Up @@ -1364,10 +1359,6 @@ mod tests {
order_with_quote.full_order.fee_amount,
full_order.fee_amount
);
assert_eq!(
order_with_quote.full_order.full_fee_amount,
full_order.full_fee_amount
);
assert_eq!(order_with_quote.full_order.kind, full_order.kind);
assert_eq!(order_with_quote.full_order.class, full_order.class);
assert_eq!(
Expand Down
27 changes: 0 additions & 27 deletions crates/model/src/order.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,6 @@ pub struct OrderData {
/// as they should only ever be used to improve the price of a regular order
/// and should not be settled on their own.
/// This is 0 for limit orders as their fee gets taken from the surplus.
/// This is equal to `OrderMetadata::full_fee_amount` except for old orders
/// where the subsidy was applied (at the time when we used the subsidies).
#[serde_as(as = "HexOrDecimalU256")]
pub fee_amount: U256,
pub kind: OrderKind,
Expand Down Expand Up @@ -685,27 +683,6 @@ pub struct OrderMetadata {
#[serde(flatten)]
pub class: OrderClass,
pub settlement_contract: H160,
/// This is `fee_amount` for liquidity orders. See comment on `fee_amount`
/// for the reasoning.
/// For market/limit orders it's the gas used of the best trade
/// execution we could find while quoting converted to an equivalent
/// `sell_token` amount.
/// Does not take partial fill into account.
///
/// [TO BE DEPRECATED]
#[serde_as(as = "HexOrDecimalU256")]
pub full_fee_amount: U256,
/// The fee amount that should be used for objective value computations.
///
/// This is different than the actual signed fee in that it
/// does not have any subsidies applied and may be scaled by a constant
/// factor to make matching orders more valuable from an objective value
/// perspective.
/// Does not take partial fill into account.
///
/// [TO BE DEPRECATED]
#[serde_as(as = "HexOrDecimalU256")]
pub solver_fee: U256,
#[serde(default, skip_serializing_if = "Option::is_none")]
pub ethflow_data: Option<EthflowData>,
#[serde(default, skip_serializing_if = "Option::is_none")]
Expand Down Expand Up @@ -1028,8 +1005,6 @@ mod tests {
"executedSurplusFee": "1",
"executedFee": "1",
"executedFeeToken": "0x000000000000000000000000000000000000000a",
"fullFeeAmount": "115792089237316195423570985008687907853269984665640564039457584007913129639935",
"solverFee": "115792089237316195423570985008687907853269984665640564039457584007913129639935",
"kind": "buy",
"class": "limit",
"partiallyFillable": false,
Expand Down Expand Up @@ -1064,8 +1039,6 @@ mod tests {
invalidated: true,
status: OrderStatus::Open,
settlement_contract: H160::from_low_u64_be(2),
full_fee_amount: U256::MAX,
solver_fee: U256::MAX,
full_app_data: Some("123".to_string()),
..Default::default()
},
Expand Down
4 changes: 0 additions & 4 deletions crates/orderbook/openapi.yml
Original file line number Diff line number Diff line change
Expand Up @@ -912,10 +912,6 @@ components:
description: Order status.
allOf:
- $ref: "#/components/schemas/OrderStatus"
fullFeeAmount:
description: Amount that the signed fee would be without subsidies.
allOf:
- $ref: "#/components/schemas/TokenAmount"
isLiquidityOrder:
description: |-
Liquidity orders are functionally the same as normal smart contract
Expand Down
7 changes: 0 additions & 7 deletions crates/orderbook/src/database/orders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,6 @@ async fn insert_order(order: &Order, ex: &mut PgConnection) -> Result<(), Insert
settlement_contract: ByteArray(order.metadata.settlement_contract.0),
sell_token_balance: sell_token_source_into(order.data.sell_token_balance),
buy_token_balance: buy_token_destination_into(order.data.buy_token_balance),
full_fee_amount: u256_to_big_decimal(&order.metadata.full_fee_amount),
cancellation_timestamp: None,
};

Expand Down Expand Up @@ -632,11 +631,6 @@ fn full_order_into_model_order(order: FullOrder) -> Result<Order> {
is_liquidity_order: class == OrderClass::Liquidity,
class,
settlement_contract: H160(order.settlement_contract.0),
full_fee_amount: big_decimal_to_u256(&order.full_fee_amount)
.context("full_fee_amount is not U256")?,
// Initialize unscaled and scale later when required.
solver_fee: big_decimal_to_u256(&order.full_fee_amount)
.context("solver_fee is not U256")?,
ethflow_data,
onchain_user,
onchain_order_data,
Expand Down Expand Up @@ -730,7 +724,6 @@ mod tests {
valid_to: valid_to_timestamp.timestamp(),
app_data: ByteArray([0; 32]),
fee_amount: BigDecimal::default(),
full_fee_amount: BigDecimal::default(),
kind: DbOrderKind::Sell,
class: DbOrderClass::Liquidity,
partially_fillable: true,
Expand Down
13 changes: 0 additions & 13 deletions crates/shared/src/db_order_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,7 @@ pub fn full_order_into_model_order(order: database::orders::FullOrder) -> Result
sender: onchain_user,
placement_error: onchain_placement_error,
});
let full_fee_amount =
big_decimal_to_u256(&order.full_fee_amount).context("full_fee_amount is not U256")?;
let fee_amount = big_decimal_to_u256(&order.fee_amount).context("fee_amount is not U256")?;
let solver_fee = match &class {
// Liquidity orders should never have a fee unless the owner bribes the protocol by setting
// one themselves.
OrderClass::Liquidity => fee_amount,
// We can't use `surplus_fee` or `fee_amount` here because those values include subsidies.
// All else being equal a solver would then prefer including an unsubsidized order over a
// subsidized one which we don't want.
OrderClass::Limit | OrderClass::Market => full_fee_amount,
};

let metadata = OrderMetadata {
creation_date: order.creation_timestamp,
Expand Down Expand Up @@ -100,8 +89,6 @@ pub fn full_order_into_model_order(order: database::orders::FullOrder) -> Result
is_liquidity_order: class == OrderClass::Liquidity,
class,
settlement_contract: H160(order.settlement_contract.0),
full_fee_amount,
solver_fee,
ethflow_data,
onchain_user,
onchain_order_data,
Expand Down
1 change: 0 additions & 1 deletion crates/shared/src/order_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -713,7 +713,6 @@ impl OrderValidating for OrderValidator {
creation_date: chrono::offset::Utc::now(),
uid,
settlement_contract,
full_fee_amount: data.fee_amount,
class,
full_app_data: match order.app_data {
OrderCreationAppData::Both { full, .. }
Expand Down
7 changes: 0 additions & 7 deletions crates/shared/src/remaining_amounts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@ mod tests {
partially_fillable: false,
..Default::default()
},
metadata: OrderMetadata {
full_fee_amount: 42.into(),
..Default::default()
},
..Default::default()
}
.into();
Expand All @@ -220,7 +216,6 @@ mod tests {
},
metadata: OrderMetadata {
executed_sell_amount_before_fees: 0.into(),
full_fee_amount: 13.into(),
..Default::default()
},
..Default::default()
Expand All @@ -243,7 +238,6 @@ mod tests {
},
metadata: OrderMetadata {
executed_sell_amount_before_fees: 90.into(),
full_fee_amount: 200.into(),
..Default::default()
},
..Default::default()
Expand All @@ -265,7 +259,6 @@ mod tests {
},
metadata: OrderMetadata {
executed_buy_amount: 9_u32.into(),
full_fee_amount: 200.into(),
..Default::default()
},
..Default::default()
Expand Down
1 change: 0 additions & 1 deletion crates/solver/src/liquidity/order_converter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,6 @@ pub mod tests {
},
metadata: OrderMetadata {
executed_sell_amount_before_fees: 10.into(),
solver_fee: 60.into(),
..Default::default()
},
..Default::default()
Expand Down
1 change: 0 additions & 1 deletion database/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,6 @@ Column | Type | Nullable | Details
settlement\_contract | bytea | not null | address of the contract that should be used to settle this order
sell\_token\_balance | [enum](#selltokensource) | not null | defines how sell\_tokens need to be transferred into the settlement contract
buy\_token\_balance | [enum](#buytokendestination) | not null | defined how buy\_tokens need to be transferred back to the user
full\_fee\_amount | numeric | not null | estimated execution cost in sell\_token of this order
class | [enum](#orderclass) | not null | determines which special trade semantics will apply to the execution of this order


Expand Down
2 changes: 2 additions & 0 deletions database/sql/V078__drop_full_fee_amount.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE orders
DROP COLUMN full_fee_amount;

0 comments on commit d9a7621

Please sign in to comment.