diff --git a/orderbook/openapi.yml b/orderbook/openapi.yml index 0608f584c..320db8e6b 100644 --- a/orderbook/openapi.yml +++ b/orderbook/openapi.yml @@ -156,27 +156,6 @@ paths: description: Invalid signature 404: description: Order was not found - /api/v1/tokens/{sellToken}/fee: - get: - description: | - The fee that is charged for placing an order. - The fee is described by a minimum fee - in order to cover the gas costs for onchain settling - and - a feeRatio charged to the users for using the service. - parameters: - - name: sellToken - in: path - required: true - schema: - $ref: "#/components/schemas/Address" - responses: - 200: - description: the fee - content: - application/json: - schema: - $ref: "#/components/schemas/LegacyFeeInformation" - 404: - description: sellToken non-existent /api/v1/trades: get: summary: Get existing Trades. @@ -329,29 +308,6 @@ components: required: - expirationDate - amount - LegacyFeeInformation: - description: | - Provides the information to calculate the fees. - type: object - properties: - expirationDate: - description: | - Expiration date of the offered fee. Order service might not accept - the fee after this expiration date. Encoded as ISO 8601 UTC. - type: string - example: "2020-12-03T18:35:18.814523Z" - minimalFee: - description: Absolute amount of minimal fee charged per order in specified sellToken - $ref: "#/components/schemas/TokenAmount" - feeRatio: - description: The fee ratio charged on a sellAmount. Denoted in basis points - example: 10 - type: number - format: int32 - required: - - expirationDate - - minimalFee - - feeRatio OrderType: description: Is this a buy order or sell order? type: string diff --git a/orderbook/src/api.rs b/orderbook/src/api.rs index 0bb533694..a40c372fb 100644 --- a/orderbook/src/api.rs +++ b/orderbook/src/api.rs @@ -33,7 +33,6 @@ pub fn handle_all_routes( ) -> impl Filter + Clone { let create_order = create_order::create_order(orderbook.clone()); let get_orders = get_orders::get_orders(orderbook.clone()); - let legacy_fee_info = get_fee_info::legacy_get_fee_info(fee_calculator.clone()); let fee_info = get_fee_info::get_fee_info(fee_calculator); let get_order = get_order_by_uid::get_order_by_uid(orderbook.clone()); let get_solvable_orders = get_solvable_orders::get_solvable_orders(orderbook.clone()); @@ -50,8 +49,6 @@ pub fn handle_all_routes( .unify() .or(fee_info.map(|reply| LabelledReply::new(reply, "fee_info"))) .unify() - .or(legacy_fee_info.map(|reply| LabelledReply::new(reply, "legacy_fee_info"))) - .unify() .or(get_order.map(|reply| LabelledReply::new(reply, "get_order"))) .unify() .or(get_solvable_orders.map(|reply| LabelledReply::new(reply, "get_solvable_orders"))) diff --git a/orderbook/src/api/get_fee_info.rs b/orderbook/src/api/get_fee_info.rs index 254607283..373a2bf18 100644 --- a/orderbook/src/api/get_fee_info.rs +++ b/orderbook/src/api/get_fee_info.rs @@ -4,7 +4,7 @@ use super::H160Wrapper; use anyhow::Result; use chrono::{DateTime, Utc}; use model::{order::OrderKind, u256_decimal}; -use primitive_types::{H160, U256}; +use primitive_types::U256; use serde::{Deserialize, Serialize}; use std::convert::Infallible; use std::sync::Arc; @@ -73,9 +73,9 @@ pub fn get_fee_info( fee_calculator .min_fee( query.sell_token.0, - Some(query.buy_token.0), - Some(query.amount), - Some(query.kind), + query.buy_token.0, + query.amount, + query.kind, ) .await, )) @@ -83,71 +83,12 @@ pub fn get_fee_info( }) } -// TODO remove legacy fee endpoint once frontend is updated - -#[derive(Debug, Serialize, Deserialize)] -#[serde(rename_all = "camelCase")] -struct LegacyFeeInfo { - pub expiration_date: DateTime, - #[serde(with = "u256_decimal")] - pub minimal_fee: U256, - pub fee_ratio: u32, -} - -pub fn legacy_get_fee_info_request() -> impl Filter + Clone { - warp::path!("tokens" / H160Wrapper / "fee") - .and(warp::get()) - .map(|token: H160Wrapper| token.0) -} - -pub fn legacy_get_fee_info_response( - result: Result<(U256, DateTime), MinFeeCalculationError>, -) -> impl Reply { - match result { - Ok((minimal_fee, expiration_date)) => { - let fee_info = LegacyFeeInfo { - expiration_date, - minimal_fee, - fee_ratio: 0u32, - }; - Ok(reply::with_status(reply::json(&fee_info), StatusCode::OK)) - } - Err(MinFeeCalculationError::NotFound) => Ok(reply::with_status( - super::error("NotFound", "Token was not found"), - StatusCode::NOT_FOUND, - )), - Err(MinFeeCalculationError::UnsupportedToken(token)) => Ok(reply::with_status( - super::error("UnsupportedToken", format!("Token address {:?}", token)), - StatusCode::BAD_REQUEST, - )), - Err(MinFeeCalculationError::Other(err)) => { - tracing::error!(?err, "get_fee error"); - Ok(reply::with_status( - super::internal_error(), - StatusCode::INTERNAL_SERVER_ERROR, - )) - } - } -} - -pub fn legacy_get_fee_info( - fee_calculator: Arc, -) -> impl Filter + Clone { - legacy_get_fee_info_request().and_then(move |token| { - let fee_calculator = fee_calculator.clone(); - async move { - Result::<_, Infallible>::Ok(legacy_get_fee_info_response( - fee_calculator.min_fee(token, None, None, None).await, - )) - } - }) -} - #[cfg(test)] mod tests { use super::*; use crate::api::response_body; use chrono::FixedOffset; + use primitive_types::H160; use warp::test::request; #[tokio::test] @@ -180,27 +121,4 @@ mod tests { assert_eq!(body.amount, U256::zero()); assert!(body.expiration_date.gt(&chrono::offset::Utc::now())) } - - #[tokio::test] - async fn legacy_get_fee_info_request_ok() { - let filter = legacy_get_fee_info_request(); - let token = String::from("0x0000000000000000000000000000000000000001"); - let path_string = format!("/tokens/{}/fee", token); - let request = request().path(&path_string).method("GET"); - let result = request.filter(&filter).await.unwrap(); - assert_eq!(result, H160::from_low_u64_be(1)); - } - - #[tokio::test] - async fn legacy_get_fee_info_response_() { - let response = - legacy_get_fee_info_response(Ok((U256::zero(), Utc::now() + FixedOffset::east(10)))) - .into_response(); - assert_eq!(response.status(), StatusCode::OK); - let body = response_body(response).await; - let body: LegacyFeeInfo = serde_json::from_slice(body.as_slice()).unwrap(); - assert_eq!(body.minimal_fee, U256::zero()); - assert_eq!(body.fee_ratio, 0); - assert!(body.expiration_date.gt(&chrono::offset::Utc::now())) - } } diff --git a/orderbook/src/database/fees.rs b/orderbook/src/database/fees.rs index e09543863..a0183a758 100644 --- a/orderbook/src/database/fees.rs +++ b/orderbook/src/database/fees.rs @@ -13,9 +13,9 @@ impl MinFeeStoring for Database { async fn save_fee_measurement( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, expiry: DateTime, min_fee: U256, ) -> Result<()> { @@ -23,9 +23,9 @@ impl MinFeeStoring for Database { "INSERT INTO min_fee_measurements (sell_token, buy_token, amount, order_kind, expiration_timestamp, min_fee) VALUES ($1, $2, $3, $4, $5, $6);"; sqlx::query(QUERY) .bind(sell_token.as_bytes()) - .bind(buy_token.as_ref().map(|t| t.as_bytes())) - .bind(amount.map(|a| u256_to_big_decimal(&a))) - .bind(kind.map(DbOrderKind::from)) + .bind(buy_token.as_bytes()) + .bind(u256_to_big_decimal(&amount)) + .bind(DbOrderKind::from(kind)) .bind(expiry) .bind(u256_to_big_decimal(&min_fee)) .execute(&self.pool) @@ -37,25 +37,25 @@ impl MinFeeStoring for Database { async fn get_min_fee( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, min_expiry: DateTime, ) -> Result> { const QUERY: &str = "\ SELECT MIN(min_fee) FROM min_fee_measurements \ WHERE sell_token = $1 \ - AND ($2 IS NULL OR buy_token = $2) \ - AND ($3 IS NULL OR amount = $3) \ - AND ($4 IS NULL OR order_kind = $4) \ + AND buy_token = $2 \ + AND amount = $3 \ + AND order_kind = $4 \ AND expiration_timestamp >= $5 "; let result: Option = sqlx::query_scalar(QUERY) .bind(sell_token.as_bytes()) - .bind(buy_token.as_ref().map(|t| t.as_bytes())) - .bind(amount.map(|a| u256_to_big_decimal(&a))) - .bind(kind.map(DbOrderKind::from)) + .bind(buy_token.as_bytes()) + .bind(u256_to_big_decimal(&amount)) + .bind(DbOrderKind::from(kind)) .bind(min_expiry) .fetch_one(&self.pool) .await @@ -100,14 +100,21 @@ mod tests { let token_b = H160::from_low_u64_be(2); // Save two measurements for token_a - db.save_fee_measurement(token_a, None, None, None, now, 100u32.into()) - .await - .unwrap(); db.save_fee_measurement( token_a, - None, - None, - None, + token_b, + 100.into(), + OrderKind::Sell, + now, + 100u32.into(), + ) + .await + .unwrap(); + db.save_fee_measurement( + token_a, + token_b, + 100.into(), + OrderKind::Sell, now + Duration::seconds(60), 200u32.into(), ) @@ -117,9 +124,9 @@ mod tests { // Save one measurement for token_b db.save_fee_measurement( token_b, - Some(token_a), - Some(100.into()), - Some(OrderKind::Buy), + token_a, + 100.into(), + OrderKind::Buy, now, 10u32.into(), ) @@ -128,61 +135,44 @@ mod tests { // Token A has readings valid until now and in 30s assert_eq!( - db.get_min_fee(token_a, None, None, None, now) + db.get_min_fee(token_a, token_b, 100.into(), OrderKind::Sell, now) .await .unwrap() .unwrap(), 100_u32.into() ); assert_eq!( - db.get_min_fee(token_a, None, None, None, now + Duration::seconds(30)) - .await - .unwrap() - .unwrap(), + db.get_min_fee( + token_a, + token_b, + 100.into(), + OrderKind::Sell, + now + Duration::seconds(30) + ) + .await + .unwrap() + .unwrap(), 200u32.into() ); // Token B only has readings valid until now assert_eq!( - db.get_min_fee(token_b, None, None, None, now) - .await - .unwrap() - .unwrap(), - 10u32.into() - ); - assert_eq!( - db.get_min_fee(token_b, None, None, None, now + Duration::seconds(30)) - .await - .unwrap(), - None - ); - - // Token B has readings for right filters - assert_eq!( - db.get_min_fee(token_b, Some(token_a), None, None, now) + db.get_min_fee(token_b, token_a, 100.into(), OrderKind::Buy, now) .await .unwrap() .unwrap(), 10u32.into() ); assert_eq!( - db.get_min_fee(token_b, None, Some(100.into()), None, now) - .await - .unwrap() - .unwrap(), - 10u32.into() - ); - assert_eq!( - db.get_min_fee(token_b, None, None, Some(OrderKind::Buy), now) - .await - .unwrap() - .unwrap(), - 10u32.into() - ); - assert_eq!( - db.get_min_fee(token_b, None, Some(U256::zero()), None, now) - .await - .unwrap(), + db.get_min_fee( + token_b, + token_a, + 100.into(), + OrderKind::Buy, + now + Duration::seconds(30) + ) + .await + .unwrap(), None ); @@ -190,7 +180,7 @@ mod tests { .await .unwrap(); assert_eq!( - db.get_min_fee(token_b, None, None, None, now) + db.get_min_fee(token_b, token_a, 100.into(), OrderKind::Buy, now) .await .unwrap(), None diff --git a/orderbook/src/fee.rs b/orderbook/src/fee.rs index b13523e84..205bab671 100644 --- a/orderbook/src/fee.rs +++ b/orderbook/src/fee.rs @@ -29,9 +29,9 @@ pub trait MinFeeStoring: Send + Sync { async fn save_fee_measurement( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, expiry: DateTime, min_fee: U256, ) -> Result<()>; @@ -41,15 +41,13 @@ pub trait MinFeeStoring: Send + Sync { async fn get_min_fee( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, min_expiry: DateTime, ) -> Result>; } -const GAS_PER_ORDER: f64 = 100_000.0; - // We use a longer validity internally for persistence to avoid writing a value to storage on every request // This way we can serve a previous estimate if the same token is queried again shortly after const STANDARD_VALIDITY_FOR_FEE_IN_SEC: i64 = 60; @@ -96,21 +94,15 @@ impl MinFeeCalculator { pub async fn min_fee( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, ) -> Result { if self.unsupported_tokens.contains(&sell_token) { return Err(MinFeeCalculationError::UnsupportedToken(sell_token)); } - - if buy_token - .map(|t| self.unsupported_tokens.contains(&t)) - .unwrap_or_default() - { - return Err(MinFeeCalculationError::UnsupportedToken( - buy_token.expect("Must exist"), - )); + if self.unsupported_tokens.contains(&buy_token) { + return Err(MinFeeCalculationError::UnsupportedToken(buy_token)); } let now = (self.now)(); @@ -150,28 +142,22 @@ impl MinFeeCalculator { async fn compute_min_fee( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, ) -> Result> { let gas_price = self.gas_estimator.estimate().await?; - let gas_amount = - if let (Some(buy_token), Some(amount), Some(kind)) = (buy_token, amount, kind) { - // We only apply the discount to the more sophisticated fee estimation, as the legacy one is already very favorable to the user in most cases - match self - .price_estimator - .estimate_gas(sell_token, buy_token, amount, kind) - .await - { - Ok(amount) => amount.to_f64_lossy() * self.discount_factor, - Err(err) => { - tracing::warn!("Failed to estimate gas amount: {}", err); - return Ok(None); - } - } - } else { - GAS_PER_ORDER - }; + let gas_amount = match self + .price_estimator + .estimate_gas(sell_token, buy_token, amount, kind) + .await + { + Ok(amount) => amount.to_f64_lossy() * self.discount_factor, + Err(err) => { + tracing::warn!("Failed to estimate gas amount: {}", err); + return Ok(None); + } + }; let fee_in_eth = gas_price * gas_amount; let token_price = match self .price_estimator @@ -194,17 +180,27 @@ impl MinFeeCalculator { } // Returns true if the fee satisfies a previous not yet expired estimate, or the fee is high enough given the current estimate. - pub async fn is_valid_fee(&self, sell_token: H160, fee: U256) -> bool { + pub async fn is_valid_fee( + &self, + sell_token: H160, + buy_token: H160, + amount: U256, + kind: OrderKind, + fee: U256, + ) -> bool { if let Ok(Some(past_fee)) = self .measurements - .get_min_fee(sell_token, None, None, None, (self.now)()) + .get_min_fee(sell_token, buy_token, amount, kind, (self.now)()) .await { if fee >= past_fee { return true; } } - if let Ok(Some(current_fee)) = self.compute_min_fee(sell_token, None, None, None).await { + if let Ok(Some(current_fee)) = self + .compute_min_fee(sell_token, buy_token, amount, kind) + .await + { return fee >= current_fee; } false @@ -212,9 +208,9 @@ impl MinFeeCalculator { } struct FeeMeasurement { - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, expiry: DateTime, min_fee: U256, } @@ -226,9 +222,9 @@ impl MinFeeStoring for InMemoryFeeStore { async fn save_fee_measurement( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, expiry: DateTime, min_fee: U256, ) -> Result<()> { @@ -250,21 +246,21 @@ impl MinFeeStoring for InMemoryFeeStore { async fn get_min_fee( &self, sell_token: H160, - buy_token: Option, - amount: Option, - kind: Option, + buy_token: H160, + amount: U256, + kind: OrderKind, min_expiry: DateTime, ) -> Result> { let mut guard = self.0.lock().expect("Thread holding Mutex panicked"); let measurements = guard.entry(sell_token).or_default(); measurements.retain(|measurement| { - if buy_token.is_some() && buy_token != measurement.buy_token { + if buy_token != measurement.buy_token { return false; } - if amount.is_some() && amount != measurement.amount { + if amount != measurement.amount { return false; } - if kind.is_some() && kind != measurement.kind { + if kind != measurement.kind { return false; } measurement.expiry >= min_expiry @@ -317,9 +313,10 @@ mod tests { let fee_estimator = MinFeeCalculator::new_for_test(gas_price_estimator, price_estimator, Box::new(now)); - let token = H160::from_low_u64_be(1); + let sell_token = H160::from_low_u64_be(1); + let buy_token = H160::from_low_u64_be(2); let (fee, expiry) = fee_estimator - .min_fee(token, None, None, None) + .min_fee(sell_token, buy_token, 100.into(), OrderKind::Sell) .await .unwrap(); @@ -328,11 +325,20 @@ mod tests { // fee is valid before expiry *time.lock().unwrap() = expiry - Duration::seconds(10); - assert!(fee_estimator.is_valid_fee(token, fee).await); + assert!( + fee_estimator + .is_valid_fee(sell_token, buy_token, 100.into(), OrderKind::Sell, fee) + .await + ); // fee is invalid for some uncached token let token = H160::from_low_u64_be(2); - assert_eq!(fee_estimator.is_valid_fee(token, fee).await, false); + assert_eq!( + fee_estimator + .is_valid_fee(token, buy_token, 100.into(), OrderKind::Sell, fee) + .await, + false + ); } #[tokio::test] @@ -348,20 +354,42 @@ mod tests { Box::new(Utc::now), ); - let token = H160::from_low_u64_be(1); + let sell_token = H160::from_low_u64_be(1); + let buy_token = H160::from_low_u64_be(2); let (fee, _) = fee_estimator - .min_fee(token, None, None, None) + .min_fee(sell_token, buy_token, 100.into(), OrderKind::Sell) .await .unwrap(); let lower_fee = fee - U256::one(); // slightly lower fee is not valid - assert_eq!(fee_estimator.is_valid_fee(token, lower_fee).await, false); + assert_eq!( + fee_estimator + .is_valid_fee( + sell_token, + buy_token, + 100.into(), + OrderKind::Sell, + lower_fee + ) + .await, + false + ); // Gas price reduces, and slightly lower fee is now valid *gas_price.lock().unwrap() /= 2.0; - assert!(fee_estimator.is_valid_fee(token, lower_fee).await); + assert!( + fee_estimator + .is_valid_fee( + sell_token, + buy_token, + 100.into(), + OrderKind::Sell, + lower_fee + ) + .await + ); } #[tokio::test] @@ -388,10 +416,9 @@ mod tests { fee_estimator .min_fee( unsupported_token, - Some(supported_token), - Some(100.into()), - Some(OrderKind::Sell) - ) + supported_token, + 100.into(), + OrderKind::Sell) .await, Err(MinFeeCalculationError::UnsupportedToken(t)) if t == unsupported_token )); @@ -401,9 +428,9 @@ mod tests { fee_estimator .min_fee( supported_token, - Some(unsupported_token), - Some(100.into()), - Some(OrderKind::Sell) + unsupported_token, + 100.into(), + OrderKind::Sell ) .await, Err(MinFeeCalculationError::UnsupportedToken(t)) if t == unsupported_token diff --git a/orderbook/src/orderbook.rs b/orderbook/src/orderbook.rs index 08061437b..4182d2a36 100644 --- a/orderbook/src/orderbook.rs +++ b/orderbook/src/orderbook.rs @@ -89,9 +89,19 @@ impl Orderbook { { return Ok(AddOrderResult::InsufficientValidTo); } + let amount = match order.kind { + model::order::OrderKind::Buy => order.buy_amount, + model::order::OrderKind::Sell => order.sell_amount, + }; if !self .fee_validator - .is_valid_fee(order.sell_token, order.fee_amount) + .is_valid_fee( + order.sell_token, + order.buy_token, + amount, + order.kind, + order.fee_amount, + ) .await { return Ok(AddOrderResult::InsufficientFee);