Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove full_fee_amount and solver_fee #3223

Merged
merged 8 commits into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -611,11 +610,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 @@ -709,7 +703,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/V077__drop_full_fee_amount_solver_fee.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
ALTER TABLE orders
DROP COLUMN full_fee_amount;
Loading