From 7c821820f17667532021d223d1d27362bd2f494a Mon Sep 17 00:00:00 2001 From: Benjamin Smith Date: Wed, 13 Oct 2021 10:58:38 +0200 Subject: [PATCH] Fee&Quote Endpoint: Implemention with legacy logic. (#1155) * introducing legacy logic to the new fee and quote endpoint * testing calculate_quote * update the return type of legacy fee endpoint * use u32::MAX to validity for {Buy/Sell}Resuest -> OrderQuoteRequest so legacy endpoint doesn't fail order validation Co-authored-by: Nicholas Rodrigues Lordello --- e2e/tests/services.rs | 15 +- orderbook/openapi.yml | 28 +- orderbook/src/api.rs | 29 +- orderbook/src/api/create_order.rs | 2 +- orderbook/src/api/get_fee_and_quote.rs | 249 +++-------- orderbook/src/api/get_fee_info.rs | 6 +- orderbook/src/api/order_validation.rs | 34 +- orderbook/src/api/post_quote.rs | 577 +++++++++++++++++++++++-- orderbook/src/lib.rs | 9 +- orderbook/src/main.rs | 13 +- 10 files changed, 680 insertions(+), 282 deletions(-) diff --git a/e2e/tests/services.rs b/e2e/tests/services.rs index 400c51c12..374f613df 100644 --- a/e2e/tests/services.rs +++ b/e2e/tests/services.rs @@ -5,8 +5,9 @@ use ethcontract::{prelude::U256, H160}; use model::DomainSeparator; use orderbook::{ account_balances::Web3BalanceFetcher, api::order_validation::OrderValidator, - database::Postgres, event_updater::EventUpdater, fee::EthAwareMinFeeCalculator, - metrics::Metrics, orderbook::Orderbook, solvable_orders::SolvableOrdersCache, + api::post_quote::OrderQuoter, database::Postgres, event_updater::EventUpdater, + fee::EthAwareMinFeeCalculator, metrics::Metrics, orderbook::Orderbook, + solvable_orders::SolvableOrdersCache, }; use reqwest::Client; use shared::{ @@ -232,16 +233,20 @@ impl OrderbookServices { true, solvable_orders_cache.clone(), Duration::from_secs(600), - order_validator, + order_validator.clone(), )); let maintenance = ServiceMaintenance { maintainers: vec![db.clone(), event_updater], }; + let quoter = Arc::new(OrderQuoter::new( + fee_calculator, + price_estimator.clone(), + order_validator, + )); orderbook::serve_api( db.clone(), orderbook, - fee_calculator, - price_estimator.clone(), + quoter, API_HOST[7..].parse().expect("Couldn't parse API address"), pending(), ); diff --git a/orderbook/openapi.yml b/orderbook/openapi.yml index 91ff7abbe..0f0f999da 100644 --- a/orderbook/openapi.yml +++ b/orderbook/openapi.yml @@ -477,7 +477,7 @@ paths: content: application/json: schema: - $ref: "#/components/schemas/OrderQuote" + $ref: "#/components/schemas/OrderQuoteResponse" 400: description: Error quoting order. content: @@ -925,22 +925,28 @@ components: buyTokenBalance: $ref: "#/components/schemas/BuyTokenDestination" default: "erc20" + from: + $ref: "#/components/schemas/Address" required: - sellToken - buyToken - validTo - appData - partiallyFillable - OrderQuote: + - from + OrderQuoteResponse: description: | An order quoted by the back end that can be directly signed and submitted to the order creation backend. - allOf: - - $ref: "#/components/schemas/OrderParameters" - - type: object - properties: - from: - description: The expected address of the signer - $ref: "#/components/schemas/Address" - required: - - from + type: object + properties: + quote: + $ref: "#/components/schemas/OrderParameters" + from: + $ref: "#/components/schemas/Address" + 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: "1985-03-10T18:35:18.814523Z" diff --git a/orderbook/src/api.rs b/orderbook/src/api.rs index 1ef088f3d..cc7c82ef1 100644 --- a/orderbook/src/api.rs +++ b/orderbook/src/api.rs @@ -11,16 +11,14 @@ mod get_solvable_orders_v2; mod get_trades; mod get_user_orders; pub mod order_validation; -mod post_quote; +pub mod post_quote; use crate::{ - database::trades::TradeRetrieving, fee::EthAwareMinFeeCalculator, orderbook::Orderbook, + api::post_quote::OrderQuoter, database::trades::TradeRetrieving, orderbook::Orderbook, }; use anyhow::Error as anyhowError; -use serde::de::DeserializeOwned; -use serde::Serialize; -use shared::metrics::get_metric_storage_registry; -use shared::price_estimation::{PriceEstimating, PriceEstimationError}; +use serde::{de::DeserializeOwned, Serialize}; +use shared::{metrics::get_metric_storage_registry, price_estimation::PriceEstimationError}; use std::{convert::Infallible, sync::Arc}; use warp::{ hyper::StatusCode, @@ -31,26 +29,23 @@ use warp::{ pub fn handle_all_routes( database: Arc, orderbook: Arc, - fee_calculator: Arc, - price_estimator: Arc, + quoter: Arc, ) -> 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.clone()); + let legacy_fee_info = get_fee_info::legacy_get_fee_info(quoter.fee_calculator.clone()); + let fee_info = get_fee_info::get_fee_info(quoter.fee_calculator.clone()); 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()); let get_solvable_orders_v2 = get_solvable_orders_v2::get_solvable_orders(orderbook.clone()); let get_trades = get_trades::get_trades(database); let cancel_order = cancel_order::cancel_order(orderbook.clone()); - let get_amount_estimate = get_markets::get_amount_estimate(price_estimator.clone()); - let get_fee_and_quote_sell = - get_fee_and_quote::get_fee_and_quote_sell(fee_calculator.clone(), price_estimator.clone()); - let get_fee_and_quote_buy = - get_fee_and_quote::get_fee_and_quote_buy(fee_calculator, price_estimator.clone()); + let get_amount_estimate = get_markets::get_amount_estimate(quoter.price_estimator.clone()); + let get_fee_and_quote_sell = get_fee_and_quote::get_fee_and_quote_sell(quoter.clone()); + let get_fee_and_quote_buy = get_fee_and_quote::get_fee_and_quote_buy(quoter.clone()); let get_user_orders = get_user_orders::get_user_orders(orderbook.clone()); let get_orders_by_tx = get_orders_by_tx::get_orders_by_tx(orderbook); - let post_quote = post_quote::post_quote(); + let post_quote = post_quote::post_quote(quoter); let cors = warp::cors() .allow_any_origin() .allow_methods(vec!["GET", "POST", "DELETE", "OPTIONS", "PUT", "PATCH"]) @@ -132,7 +127,7 @@ fn internal_error() -> Json { } pub trait WarpReplyConverting { - fn to_warp_reply(&self) -> (Json, StatusCode); + fn into_warp_reply(self) -> (Json, StatusCode); } pub fn convert_get_orders_error_to_reply(err: anyhowError) -> WithStatus { diff --git a/orderbook/src/api/create_order.rs b/orderbook/src/api/create_order.rs index a73e785f0..09097e6bc 100644 --- a/orderbook/src/api/create_order.rs +++ b/orderbook/src/api/create_order.rs @@ -17,7 +17,7 @@ pub fn create_order_request( pub fn create_order_response(result: Result) -> impl Reply { let (body, status_code) = match result { Ok(AddOrderResult::Added(uid)) => (warp::reply::json(&uid), StatusCode::CREATED), - Ok(AddOrderResult::OrderValidation(err)) => err.to_warp_reply(), + Ok(AddOrderResult::OrderValidation(err)) => err.into_warp_reply(), Ok(AddOrderResult::UnsupportedSignature) => ( super::error("UnsupportedSignature", "signing scheme is not supported"), StatusCode::BAD_REQUEST, diff --git a/orderbook/src/api/get_fee_and_quote.rs b/orderbook/src/api/get_fee_and_quote.rs index c14c6baa4..228cc0f76 100644 --- a/orderbook/src/api/get_fee_and_quote.rs +++ b/orderbook/src/api/get_fee_and_quote.rs @@ -1,15 +1,15 @@ -use crate::{api::price_estimation_error_to_warp_reply, fee::MinFeeCalculating}; -use anyhow::{anyhow, Result}; +use crate::api::post_quote::{ + response, OrderQuoteRequest, OrderQuoteResponse, OrderQuoteSide, OrderQuoter, SellAmount, +}; +use anyhow::Result; use chrono::{DateTime, Utc}; use ethcontract::{H160, U256}; -use model::{order::OrderKind, u256_decimal}; +use model::u256_decimal; use serde::{Deserialize, Serialize}; -use shared::price_estimation::{self, PriceEstimating, PriceEstimationError}; -use std::convert::Infallible; -use std::sync::Arc; -use warp::{hyper::StatusCode, reply, Filter, Rejection, Reply}; +use std::{convert::Infallible, sync::Arc}; +use warp::{Filter, Rejection, Reply}; -#[derive(Serialize)] +#[derive(Debug, Serialize)] #[serde(rename_all = "camelCase")] struct Fee { #[serde(with = "u256_decimal")] @@ -27,6 +27,17 @@ struct SellQuery { sell_amount_before_fee: U256, } +impl From for OrderQuoteRequest { + fn from(query: SellQuery) -> Self { + let side = OrderQuoteSide::Sell { + sell_amount: SellAmount::BeforeFee { + value: query.sell_amount_before_fee, + }, + }; + OrderQuoteRequest::new(query.sell_token, query.buy_token, side) + } +} + #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct SellResponse { @@ -38,6 +49,18 @@ struct SellResponse { buy_amount_after_fee: U256, } +impl From for SellResponse { + fn from(response: OrderQuoteResponse) -> Self { + Self { + fee: Fee { + amount: response.quote.fee_amount, + expiration_date: response.expiration, + }, + buy_amount_after_fee: response.quote.buy_amount, + } + } +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] struct BuyQuery { @@ -48,6 +71,15 @@ struct BuyQuery { buy_amount_after_fee: U256, } +impl From for OrderQuoteRequest { + fn from(query: BuyQuery) -> Self { + let side = OrderQuoteSide::Buy { + buy_amount_after_fee: query.buy_amount_after_fee, + }; + OrderQuoteRequest::new(query.sell_token, query.buy_token, side) + } +} + #[derive(Serialize)] #[serde(rename_all = "camelCase")] struct BuyResponse { @@ -58,99 +90,16 @@ struct BuyResponse { sell_amount_before_fee: U256, } -#[derive(Debug)] -enum Error { - SellAmountDoesNotCoverFee, - PriceEstimate(PriceEstimationError), -} - -async fn calculate_sell( - fee_calculator: Arc, - price_estimator: Arc, - query: SellQuery, -) -> Result { - if query.sell_amount_before_fee.is_zero() { - return Err(Error::PriceEstimate(PriceEstimationError::ZeroAmount)); - } - - // TODO: would be nice to use true sell amount after the fee but that is more complicated. - let (fee, expiration_date) = fee_calculator - .compute_unsubsidized_min_fee( - query.sell_token, - Some(query.buy_token), - Some(query.sell_amount_before_fee), - Some(OrderKind::Sell), - None, - ) - .await - .map_err(Error::PriceEstimate)?; - let sell_amount_after_fee = query - .sell_amount_before_fee - .checked_sub(fee) - .ok_or(Error::SellAmountDoesNotCoverFee)? - .max(U256::one()); - - let estimate = price_estimator - .estimate(&price_estimation::Query { - sell_token: query.sell_token, - buy_token: query.buy_token, - in_amount: sell_amount_after_fee, - kind: OrderKind::Sell, - }) - .await - .map_err(Error::PriceEstimate)?; - - Ok(SellResponse { - fee: Fee { - expiration_date, - amount: fee, - }, - buy_amount_after_fee: estimate.out_amount, - }) -} - -async fn calculate_buy( - fee_calculator: Arc, - price_estimator: Arc, - query: BuyQuery, -) -> Result { - if query.buy_amount_after_fee.is_zero() { - return Err(Error::PriceEstimate(PriceEstimationError::ZeroAmount)); +impl From for BuyResponse { + fn from(response: OrderQuoteResponse) -> Self { + Self { + fee: Fee { + amount: response.quote.fee_amount, + expiration_date: response.expiration, + }, + sell_amount_before_fee: response.quote.sell_amount, + } } - - let (fee, expiration_date) = fee_calculator - .compute_unsubsidized_min_fee( - query.sell_token, - Some(query.buy_token), - Some(query.buy_amount_after_fee), - Some(OrderKind::Buy), - None, - ) - .await - .map_err(Error::PriceEstimate)?; - - let estimate = price_estimator - .estimate(&price_estimation::Query { - sell_token: query.sell_token, - buy_token: query.buy_token, - in_amount: query.buy_amount_after_fee, - kind: OrderKind::Buy, - }) - .await - .map_err(Error::PriceEstimate)?; - let sell_amount_before_fee = estimate.out_amount.checked_add(fee).ok_or_else(|| { - Error::PriceEstimate(PriceEstimationError::Other(anyhow!( - "overflow in sell_amount_before_fee" - ))) - })?; - - Ok(BuyResponse { - fee: Fee { - expiration_date, - amount: fee, - }, - sell_amount_before_fee, - }) } fn sell_request() -> impl Filter + Clone { @@ -165,48 +114,33 @@ fn buy_request() -> impl Filter + Clon .and(warp::query::()) } -fn response(result: Result) -> impl Reply { - match result { - Ok(response) => reply::with_status(reply::json(&response), StatusCode::OK), - Err(Error::SellAmountDoesNotCoverFee) => reply::with_status( - super::error( - "SellAmountDoesNotCoverFee", - "The sell amount for the sell order is lower than the fee.".to_string(), - ), - StatusCode::BAD_REQUEST, - ), - Err(Error::PriceEstimate(err)) => { - let (json, status_code) = price_estimation_error_to_warp_reply(err); - reply::with_status(json, status_code) - } - } -} - pub fn get_fee_and_quote_sell( - fee_calculator: Arc, - price_estimator: Arc, + quoter: Arc, ) -> impl Filter + Clone { - sell_request().and_then(move |query| { - let fee_calculator = fee_calculator.clone(); - let price_estimator = price_estimator.clone(); + sell_request().and_then(move |query: SellQuery| { + let quoter = quoter.clone(); async move { Result::<_, Infallible>::Ok(response( - calculate_sell(fee_calculator, price_estimator, query).await, + quoter + .calculate_quote(&query.into()) + .await + .map(SellResponse::from), )) } }) } pub fn get_fee_and_quote_buy( - fee_calculator: Arc, - price_estimator: Arc, + quoter: Arc, ) -> impl Filter + Clone { - buy_request().and_then(move |query| { - let fee_calculator = fee_calculator.clone(); - let price_estimator = price_estimator.clone(); + buy_request().and_then(move |query: BuyQuery| { + let quoter = quoter.clone(); async move { Result::<_, Infallible>::Ok(response( - calculate_buy(fee_calculator, price_estimator, query).await, + quoter + .calculate_quote(&query.into()) + .await + .map(BuyResponse::from), )) } }) @@ -215,67 +149,10 @@ pub fn get_fee_and_quote_buy( #[cfg(test)] mod tests { use super::*; - use crate::fee::MockMinFeeCalculating; use futures::FutureExt; use hex_literal::hex; - use shared::price_estimation::mocks::FakePriceEstimator; use warp::test::request; - #[test] - fn calculate_sell_() { - let mut fee_calculator = MockMinFeeCalculating::new(); - fee_calculator - .expect_compute_unsubsidized_min_fee() - .returning(|_, _, _, _, _| Ok((U256::from(3), Utc::now()))); - let price_estimator = FakePriceEstimator(price_estimation::Estimate { - out_amount: 14.into(), - gas: 1000.into(), - }); - let result = calculate_sell( - Arc::new(fee_calculator), - Arc::new(price_estimator), - SellQuery { - sell_token: H160::from_low_u64_ne(0), - buy_token: H160::from_low_u64_ne(1), - sell_amount_before_fee: 10.into(), - }, - ) - .now_or_never() - .unwrap() - .unwrap(); - assert_eq!(result.fee.amount, 3.into()); - // After the deducting the fee 10 - 3 = 7 units of sell token are being sold. - assert_eq!(result.buy_amount_after_fee, 14.into()); - } - - #[test] - fn calculate_buy_() { - let mut fee_calculator = MockMinFeeCalculating::new(); - fee_calculator - .expect_compute_unsubsidized_min_fee() - .returning(|_, _, _, _, _| Ok((U256::from(3), Utc::now()))); - let price_estimator = FakePriceEstimator(price_estimation::Estimate { - out_amount: 20.into(), - gas: 1000.into(), - }); - let result = calculate_buy( - Arc::new(fee_calculator), - Arc::new(price_estimator), - BuyQuery { - sell_token: H160::from_low_u64_ne(0), - buy_token: H160::from_low_u64_ne(1), - buy_amount_after_fee: 10.into(), - }, - ) - .now_or_never() - .unwrap() - .unwrap(); - assert_eq!(result.fee.amount, 3.into()); - // To buy 10 units of buy_token the fee in sell_token must be at least 3 and at least 20 - // units of sell_token must be sold. - assert_eq!(result.sell_amount_before_fee, 23.into()); - } - #[test] fn sell_query() { let path= "/feeAndQuote/sell?sellToken=0xdac17f958d2ee523a2206206994597c13d831ec7&buyToken=0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48&sellAmountBeforeFee=1000000"; diff --git a/orderbook/src/api/get_fee_info.rs b/orderbook/src/api/get_fee_info.rs index 9d191bb13..1425e3148 100644 --- a/orderbook/src/api/get_fee_info.rs +++ b/orderbook/src/api/get_fee_info.rs @@ -1,5 +1,5 @@ use crate::api::price_estimation_error_to_warp_reply; -use crate::fee::{EthAwareMinFeeCalculator, MinFeeCalculating}; +use crate::fee::MinFeeCalculating; use anyhow::Result; use chrono::{DateTime, Utc}; use model::{order::OrderKind, u256_decimal}; @@ -53,7 +53,7 @@ pub fn get_fee_info_response( } pub fn get_fee_info( - fee_calculator: Arc, + fee_calculator: Arc, ) -> impl Filter + Clone { get_fee_info_request().and_then(move |query: Query| { let fee_calculator = fee_calculator.clone(); @@ -108,7 +108,7 @@ pub fn legacy_get_fee_info_response( } pub fn legacy_get_fee_info( - fee_calculator: Arc, + fee_calculator: Arc, ) -> impl Filter + Clone { legacy_get_fee_info_request().and_then(move |token| { let fee_calculator = fee_calculator.clone(); diff --git a/orderbook/src/api/order_validation.rs b/orderbook/src/api/order_validation.rs index 760a7f65d..87bbe8063 100644 --- a/orderbook/src/api/order_validation.rs +++ b/orderbook/src/api/order_validation.rs @@ -9,7 +9,7 @@ use warp::{http::StatusCode, reply::Json}; #[cfg_attr(test, mockall::automock)] #[async_trait::async_trait] -pub trait OrderValidating { +pub trait OrderValidating: Send + Sync { /// Partial (aka Pre-) Validation is aimed at catching malformed order data during the /// fee & quote phase (i.e. before the order is signed). /// Thus, partial validation *doesn't* verify: @@ -22,7 +22,7 @@ pub trait OrderValidating { /// - the order validity is appropriate, /// - buy_token is not the same as sell_token, /// - buy and sell token destination and source are supported. - async fn partial_validate(&self, order: &PreOrderData) -> Result<(), PartialValidationError>; + async fn partial_validate(&self, order: PreOrderData) -> Result<(), PartialValidationError>; /// This is the full order validation performed at the time of order placement /// (i.e. once all the required fields on an Order are provided). Specifically, verifying that @@ -55,7 +55,7 @@ pub enum PartialValidationError { } impl WarpReplyConverting for PartialValidationError { - fn to_warp_reply(&self) -> (Json, StatusCode) { + fn into_warp_reply(self) -> (Json, StatusCode) { match self { Self::UnsupportedBuyTokenDestination(dest) => ( super::error("UnsupportedBuyTokenDestination", format!("Type {:?}", dest)), @@ -108,9 +108,9 @@ pub enum ValidationError { } impl WarpReplyConverting for ValidationError { - fn to_warp_reply(&self) -> (Json, StatusCode) { + fn into_warp_reply(self) -> (Json, StatusCode) { match self { - ValidationError::Partial(pre) => pre.to_warp_reply(), + ValidationError::Partial(pre) => pre.into_warp_reply(), Self::UnsupportedToken(token) => ( super::error("UnsupportedToken", format!("Token address {}", token)), StatusCode::BAD_REQUEST, @@ -162,7 +162,7 @@ pub struct OrderValidator { balance_fetcher: Arc, } -#[derive(Default)] +#[derive(Default, Debug, PartialEq)] pub struct PreOrderData { pub owner: H160, pub sell_token: H160, @@ -211,7 +211,7 @@ impl OrderValidator { #[async_trait::async_trait] impl OrderValidating for OrderValidator { - async fn partial_validate(&self, order: &PreOrderData) -> Result<(), PartialValidationError> { + async fn partial_validate(&self, order: PreOrderData) -> Result<(), PartialValidationError> { if self.banned_users.contains(&order.owner) { return Err(PartialValidationError::Forbidden); } @@ -233,7 +233,7 @@ impl OrderValidating for OrderValidator { { return Err(PartialValidationError::InsufficientValidTo); } - if has_same_buy_and_sell_token(order, &self.native_token) { + if has_same_buy_and_sell_token(&order, &self.native_token) { return Err(PartialValidationError::SameBuyAndSellToken); } if order.buy_token == BUY_ETH_ADDRESS { @@ -279,7 +279,7 @@ impl OrderValidating for OrderValidator { None => return Err(ValidationError::InvalidSignature), }; - self.partial_validate(&PreOrderData::from(order.clone())) + self.partial_validate(PreOrderData::from(order.clone())) .await .map_err(ValidationError::Partial)?; let order_creation = &order.order_creation; @@ -455,7 +455,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { owner: H160::from_low_u64_be(1), ..Default::default() }) @@ -468,7 +468,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { buy_token_balance: BuyTokenDestination::Internal, ..Default::default() }) @@ -481,7 +481,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { sell_token_balance: SellTokenSource::Internal, ..Default::default() }) @@ -494,7 +494,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { valid_to: 0, ..Default::default() }) @@ -507,7 +507,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { valid_to: legit_valid_to, buy_token: H160::from_low_u64_be(2), sell_token: H160::from_low_u64_be(2), @@ -522,7 +522,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { valid_to: legit_valid_to, buy_token: BUY_ETH_ADDRESS, ..Default::default() @@ -551,7 +551,7 @@ mod tests { format!( "{:?}", validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { valid_to: legit_valid_to, buy_token: BUY_ETH_ADDRESS, ..Default::default() @@ -576,7 +576,7 @@ mod tests { Arc::new(MockBalanceFetching::new()), ); assert!(validator - .partial_validate(&PreOrderData { + .partial_validate(PreOrderData { valid_to: shared::time::now_in_epoch_seconds() + min_order_validity_period.as_secs() as u32 + 2, diff --git a/orderbook/src/api/post_quote.rs b/orderbook/src/api/post_quote.rs index 4f16c9c02..8b9a3e5a4 100644 --- a/orderbook/src/api/post_quote.rs +++ b/orderbook/src/api/post_quote.rs @@ -1,5 +1,13 @@ -use crate::api; +use crate::{ + api::{ + self, + order_validation::{OrderValidating, PreOrderData, ValidationError}, + price_estimation_error_to_warp_reply, WarpReplyConverting, + }, + fee::MinFeeCalculating, +}; use anyhow::{anyhow, Result}; +use chrono::{DateTime, Utc}; use ethcontract::{H160, U256}; use model::{ app_id::AppId, @@ -7,13 +15,18 @@ use model::{ u256_decimal, }; use serde::{Deserialize, Serialize}; -use std::convert::Infallible; -use warp::{hyper::StatusCode, reply, Filter, Rejection, Reply}; +use shared::price_estimation::{self, PriceEstimating, PriceEstimationError}; +use std::{convert::Infallible, sync::Arc}; +use warp::{ + hyper::StatusCode, + reply::{self, Json}, + Filter, Rejection, Reply, +}; /// The order parameters to quote a price and fee for. -#[derive(Debug, Deserialize, PartialEq)] +#[derive(Clone, Copy, Debug, Default, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "camelCase")] -struct OrderQuoteRequest { +pub struct OrderQuoteRequest { from: H160, sell_token: H160, buy_token: H160, @@ -29,9 +42,24 @@ struct OrderQuoteRequest { buy_token_balance: BuyTokenDestination, } -#[derive(Debug, Deserialize, PartialEq)] +impl From<&OrderQuoteRequest> for PreOrderData { + fn from(quote_request: &OrderQuoteRequest) -> Self { + let owner = quote_request.from; + Self { + owner, + sell_token: quote_request.sell_token, + buy_token: quote_request.buy_token, + receiver: quote_request.receiver.unwrap_or(owner), + valid_to: quote_request.valid_to, + buy_token_balance: quote_request.buy_token_balance, + sell_token_balance: quote_request.sell_token_balance, + } + } +} + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq)] #[serde(tag = "kind", rename_all = "lowercase")] -enum OrderQuoteSide { +pub enum OrderQuoteSide { #[serde(rename_all = "camelCase")] Sell { #[serde(flatten)] @@ -44,9 +72,17 @@ enum OrderQuoteSide { }, } -#[derive(Debug, Deserialize, PartialEq)] +impl Default for OrderQuoteSide { + fn default() -> Self { + Self::Buy { + buy_amount_after_fee: U256::one(), + } + } +} + +#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq)] #[serde(untagged)] -enum SellAmount { +pub enum SellAmount { BeforeFee { #[serde(rename = "sellAmountBeforeFee", with = "u256_decimal")] value: U256, @@ -58,25 +94,245 @@ enum SellAmount { } /// The quoted order by the service. -#[derive(Serialize)] +#[derive(Debug, PartialEq, Serialize)] #[serde(rename_all = "camelCase")] -struct OrderQuote { - from: H160, - sell_token: H160, - buy_token: H160, - receiver: Option, +pub struct OrderQuote { + pub sell_token: H160, + pub buy_token: H160, + pub receiver: Option, #[serde(with = "u256_decimal")] - sell_amount: U256, + pub sell_amount: U256, #[serde(with = "u256_decimal")] - buy_amount: U256, - valid_to: u32, - app_data: AppId, + pub buy_amount: U256, + pub valid_to: u32, + pub app_data: AppId, #[serde(with = "u256_decimal")] + pub fee_amount: U256, + pub kind: OrderKind, + pub partially_fillable: bool, + pub sell_token_balance: SellTokenSource, + pub buy_token_balance: BuyTokenDestination, +} + +#[derive(Debug, PartialEq, Serialize)] +#[serde(rename_all = "camelCase")] +pub struct OrderQuoteResponse { + pub quote: OrderQuote, + pub from: H160, + pub expiration: DateTime, +} + +#[derive(Debug)] +pub enum FeeError { + SellAmountDoesNotCoverFee, + PriceEstimate(PriceEstimationError), +} + +impl WarpReplyConverting for FeeError { + fn into_warp_reply(self) -> (Json, StatusCode) { + match self { + FeeError::PriceEstimate(err) => price_estimation_error_to_warp_reply(err), + FeeError::SellAmountDoesNotCoverFee => ( + super::error( + "SellAmountDoesNotCoverFee", + "The sell amount for the sell order is lower than the fee.".to_string(), + ), + StatusCode::BAD_REQUEST, + ), + } + } +} + +#[derive(Debug)] +pub enum OrderQuoteError { + Fee(FeeError), + Order(ValidationError), +} + +impl OrderQuoteError { + pub fn convert_to_reply(self) -> (Json, StatusCode) { + match self { + OrderQuoteError::Fee(err) => err.into_warp_reply(), + OrderQuoteError::Order(err) => err.into_warp_reply(), + } + } +} + +#[derive(Debug, Serialize, PartialEq)] +struct FeeParameters { + buy_amount: U256, + sell_amount: U256, fee_amount: U256, + expiration: DateTime, kind: OrderKind, - partially_fillable: bool, - sell_token_balance: SellTokenSource, - buy_token_balance: BuyTokenDestination, +} + +#[derive(Clone)] +pub struct OrderQuoter { + pub fee_calculator: Arc, + pub price_estimator: Arc, + pub order_validator: Arc, +} + +impl OrderQuoter { + pub fn new( + fee_calculator: Arc, + price_estimator: Arc, + order_validator: Arc, + ) -> Self { + Self { + fee_calculator, + price_estimator, + order_validator, + } + } + + pub async fn calculate_quote( + &self, + quote_request: &OrderQuoteRequest, + ) -> Result { + tracing::debug!("Received quote request {:?}", quote_request); + self.order_validator + .partial_validate(quote_request.into()) + .await + .map_err(|err| OrderQuoteError::Order(ValidationError::Partial(err)))?; + let fee_parameters = self + .calculate_fee_parameters(quote_request) + .await + .map_err(OrderQuoteError::Fee)?; + Ok(OrderQuoteResponse { + quote: OrderQuote { + sell_token: quote_request.sell_token, + buy_token: quote_request.buy_token, + receiver: quote_request.receiver, + sell_amount: fee_parameters.sell_amount, + buy_amount: fee_parameters.buy_amount, + valid_to: quote_request.valid_to, + app_data: quote_request.app_data, + fee_amount: fee_parameters.fee_amount, + kind: fee_parameters.kind, + partially_fillable: quote_request.partially_fillable, + sell_token_balance: quote_request.sell_token_balance, + buy_token_balance: quote_request.buy_token_balance, + }, + from: quote_request.from, + expiration: fee_parameters.expiration, + }) + } + + async fn calculate_fee_parameters( + &self, + quote_request: &OrderQuoteRequest, + ) -> Result { + match quote_request.side { + OrderQuoteSide::Sell { + sell_amount: + SellAmount::BeforeFee { + value: sell_amount_before_fee, + }, + } => { + if sell_amount_before_fee.is_zero() { + Err(FeeError::PriceEstimate(PriceEstimationError::ZeroAmount)) + } else { + let (fee, expiration) = self + .fee_calculator + .compute_unsubsidized_min_fee( + quote_request.sell_token, + Some(quote_request.buy_token), + Some(sell_amount_before_fee), + Some(OrderKind::Sell), + Some(quote_request.app_data), + ) + .await + .map_err(FeeError::PriceEstimate)?; + let sell_amount_after_fee = sell_amount_before_fee + .checked_sub(fee) + .ok_or(FeeError::SellAmountDoesNotCoverFee)? + .max(U256::one()); + let estimate = self + .price_estimator + .estimate(&price_estimation::Query { + sell_token: quote_request.sell_token, + buy_token: quote_request.buy_token, + in_amount: sell_amount_after_fee, + kind: OrderKind::Sell, + }) + .await + .map_err(FeeError::PriceEstimate)?; + Ok(FeeParameters { + buy_amount: estimate.out_amount, + sell_amount: sell_amount_before_fee, + fee_amount: fee, + expiration, + kind: OrderKind::Sell, + }) + } + } + OrderQuoteSide::Sell { + sell_amount: SellAmount::AfterFee { .. }, + } => { + // TODO: Nice to have: true sell amount after the fee (more complicated). + Err(FeeError::PriceEstimate(PriceEstimationError::Other( + anyhow!("Currently unsupported route"), + ))) + } + OrderQuoteSide::Buy { + buy_amount_after_fee, + } => { + if buy_amount_after_fee.is_zero() { + Err(FeeError::PriceEstimate(PriceEstimationError::ZeroAmount)) + } else { + let (fee, expiration) = self + .fee_calculator + .compute_unsubsidized_min_fee( + quote_request.sell_token, + Some(quote_request.buy_token), + Some(buy_amount_after_fee), + Some(OrderKind::Buy), + Some(quote_request.app_data), + ) + .await + .map_err(FeeError::PriceEstimate)?; + let estimate = self + .price_estimator + .estimate(&price_estimation::Query { + sell_token: quote_request.sell_token, + buy_token: quote_request.buy_token, + in_amount: buy_amount_after_fee, + kind: OrderKind::Buy, + }) + .await + .map_err(FeeError::PriceEstimate)?; + let sell_amount_before_fee = + estimate.out_amount.checked_add(fee).ok_or_else(|| { + FeeError::PriceEstimate(PriceEstimationError::Other(anyhow!( + "overflow in sell_amount_before_fee" + ))) + })?; + Ok(FeeParameters { + buy_amount: buy_amount_after_fee, + sell_amount: sell_amount_before_fee, + fee_amount: fee, + expiration, + kind: OrderKind::Buy, + }) + } + } + } + } +} + +impl OrderQuoteRequest { + /// This method is used by the old, deprecated, fee endpoint to convert {Buy, Sell}Requests + pub fn new(sell_token: H160, buy_token: H160, side: OrderQuoteSide) -> Self { + Self { + sell_token, + buy_token, + side, + valid_to: u32::MAX, + ..Default::default() + } + } } fn post_quote_request() -> impl Filter + Clone { @@ -85,27 +341,43 @@ fn post_quote_request() -> impl Filter) -> impl Reply { +pub fn response(result: Result) -> impl Reply { match result { Ok(response) => reply::with_status(reply::json(&response), StatusCode::OK), - Err(err) => reply::with_status( - super::error("InternalServerError", err.to_string()), - StatusCode::INTERNAL_SERVER_ERROR, - ), + Err(err) => { + let (reply, status) = err.convert_to_reply(); + reply::with_status(reply, status) + } } } -pub fn post_quote() -> impl Filter + Clone { - post_quote_request().and_then(move |request| async move { - tracing::warn!("unimplemented request {:#?}", request); - Result::<_, Infallible>::Ok(post_order_response(Err(anyhow!("not yet implemented")))) +pub fn post_quote( + quoter: Arc, +) -> impl Filter + Clone { + post_quote_request().and_then(move |request: OrderQuoteRequest| { + let quoter = quoter.clone(); + async move { + let result = quoter.calculate_quote(&request).await; + if let Err(err) = &result { + tracing::error!(?err, ?request, "post_quote error"); + } + Result::<_, Infallible>::Ok(response(result)) + } }) } #[cfg(test)] mod tests { use super::*; + use crate::{ + api::{order_validation::MockOrderValidating, response_body}, + fee::MockMinFeeCalculating, + }; + use chrono::Utc; + use futures::FutureExt; use serde_json::json; + use shared::price_estimation::mocks::FakePriceEstimator; + use warp::test::request; #[test] fn deserializes_sell_after_fees_quote_request() { @@ -202,4 +474,247 @@ mod tests { } ); } + + #[tokio::test] + async fn post_quote_request_ok() { + let filter = post_quote_request(); + let request_payload = OrderQuoteRequest::default(); + let request = request() + .path("/quote") + .method("POST") + .header("content-type", "application/json") + .json(&request_payload); + let result = request.filter(&filter).await.unwrap(); + assert_eq!(result, request_payload); + } + + #[tokio::test] + async fn post_quote_request_err() { + let filter = post_quote_request(); + let request_payload = OrderQuoteRequest::default(); + // Path is wrong! + let request = request() + .path("/fee_quote") + .method("POST") + .header("content-type", "application/json") + .json(&request_payload); + assert!(request.filter(&filter).await.is_err()); + } + + #[tokio::test] + async fn post_quote_response_ok() { + let order_quote = OrderQuote { + sell_token: Default::default(), + buy_token: Default::default(), + receiver: None, + sell_amount: Default::default(), + buy_amount: Default::default(), + valid_to: 0, + app_data: Default::default(), + fee_amount: Default::default(), + kind: Default::default(), + partially_fillable: false, + sell_token_balance: Default::default(), + buy_token_balance: Default::default(), + }; + let response = response(Ok(&order_quote)).into_response(); + assert_eq!(response.status(), StatusCode::OK); + let body = response_body(response).await; + let body: serde_json::Value = serde_json::from_slice(body.as_slice()).unwrap(); + let expected = serde_json::to_value(order_quote).unwrap(); + assert_eq!(body, expected); + } + + #[tokio::test] + async fn post_quote_response_err() { + let response = response::(Err(OrderQuoteError::Order( + ValidationError::Other(anyhow!("Uh oh - error")), + ))) + .into_response(); + assert_eq!(response.status(), StatusCode::INTERNAL_SERVER_ERROR); + let body = response_body(response).await; + let body: serde_json::Value = serde_json::from_slice(body.as_slice()).unwrap(); + let expected_error = json!({"errorType": "InternalServerError", "description": ""}); + assert_eq!(body, expected_error); + // There are many other FeeAndQuoteErrors, but writing a test for each would follow the same pattern as this. + } + + #[test] + fn calculate_fee_sell_before_fees_quote_request() { + let mut fee_calculator = MockMinFeeCalculating::new(); + + let expiration = Utc::now(); + fee_calculator + .expect_compute_unsubsidized_min_fee() + .returning(move |_, _, _, _, _| Ok((U256::from(3), expiration))); + + let fee_calculator = Arc::new(fee_calculator); + let price_estimator = FakePriceEstimator(price_estimation::Estimate { + out_amount: 14.into(), + gas: 1000.into(), + }); + let sell_query = OrderQuoteRequest::new( + H160::from_low_u64_ne(0), + H160::from_low_u64_ne(1), + OrderQuoteSide::Sell { + sell_amount: SellAmount::BeforeFee { value: 10.into() }, + }, + ); + let quoter = Arc::new(OrderQuoter::new( + fee_calculator, + Arc::new(price_estimator), + Arc::new(MockOrderValidating::new()), + )); + let result = quoter + .calculate_fee_parameters(&sell_query) + .now_or_never() + .unwrap() + .unwrap(); + // After the deducting the fee 10 - 3 = 7 units of sell token are being sold. + assert_eq!( + result, + FeeParameters { + buy_amount: 14.into(), + sell_amount: 10.into(), + fee_amount: 3.into(), + expiration, + kind: OrderKind::Sell + } + ); + } + + #[test] + fn calculate_fee_sell_after_fees_quote_request() { + let mut fee_calculator = MockMinFeeCalculating::new(); + fee_calculator + .expect_compute_unsubsidized_min_fee() + .returning(|_, _, _, _, _| Ok((U256::from(3), Utc::now()))); + + let fee_calculator = Arc::new(fee_calculator); + let price_estimator = FakePriceEstimator(price_estimation::Estimate { + out_amount: 14.into(), + gas: 1000.into(), + }); + let sell_query = OrderQuoteRequest::new( + H160::from_low_u64_ne(0), + H160::from_low_u64_ne(1), + OrderQuoteSide::Sell { + sell_amount: SellAmount::AfterFee { value: 7.into() }, + }, + ); + + let quoter = Arc::new(OrderQuoter::new( + fee_calculator, + Arc::new(price_estimator), + Arc::new(MockOrderValidating::new()), + )); + let result = quoter + .calculate_fee_parameters(&sell_query) + .now_or_never() + .unwrap() + .unwrap_err(); + assert_eq!( + format!("{:?}", result), + "PriceEstimate(Other(Currently unsupported route))" + ); + } + + #[test] + fn calculate_fee_buy_quote_request() { + let mut fee_calculator = MockMinFeeCalculating::new(); + let expiration = Utc::now(); + fee_calculator + .expect_compute_unsubsidized_min_fee() + .returning(move |_, _, _, _, _| Ok((U256::from(3), expiration))); + + let fee_calculator = Arc::new(fee_calculator); + let price_estimator = FakePriceEstimator(price_estimation::Estimate { + out_amount: 20.into(), + gas: 1000.into(), + }); + let buy_query = OrderQuoteRequest::new( + H160::from_low_u64_ne(0), + H160::from_low_u64_ne(1), + OrderQuoteSide::Buy { + buy_amount_after_fee: 10.into(), + }, + ); + let quoter = Arc::new(OrderQuoter::new( + fee_calculator, + Arc::new(price_estimator), + Arc::new(MockOrderValidating::new()), + )); + let result = quoter + .calculate_fee_parameters(&buy_query) + .now_or_never() + .unwrap() + .unwrap(); + // To buy 10 units of buy_token the fee in sell_token must be at least 3 and at least 20 + // units of sell_token must be sold. + assert_eq!( + result, + FeeParameters { + buy_amount: 10.into(), + sell_amount: 23.into(), + fee_amount: 3.into(), + expiration, + kind: OrderKind::Buy + } + ); + } + + #[test] + fn pre_order_data_from_quote_request() { + let quote_request = OrderQuoteRequest::default(); + let result = PreOrderData::from("e_request); + let expected = PreOrderData::default(); + assert_eq!(result, expected); + } + + #[tokio::test] + async fn calculate_quote() { + let buy_request = OrderQuoteRequest { + sell_token: H160::from_low_u64_be(1), + buy_token: H160::from_low_u64_be(2), + side: OrderQuoteSide::Buy { + buy_amount_after_fee: 2.into(), + }, + ..Default::default() + }; + + let mut fee_calculator = MockMinFeeCalculating::new(); + fee_calculator + .expect_compute_unsubsidized_min_fee() + .returning(move |_, _, _, _, _| Ok((U256::from(3), Utc::now()))); + let price_estimator = FakePriceEstimator(price_estimation::Estimate { + out_amount: 14.into(), + gas: 1000.into(), + }); + let mut order_validator = MockOrderValidating::new(); + order_validator + .expect_partial_validate() + .returning(|_| Ok(())); + let quoter = Arc::new(OrderQuoter::new( + Arc::new(fee_calculator), + Arc::new(price_estimator), + Arc::new(order_validator), + )); + let result = quoter.calculate_quote(&buy_request).await.unwrap(); + + let expected = OrderQuote { + sell_token: H160::from_low_u64_be(1), + buy_token: H160::from_low_u64_be(2), + receiver: None, + sell_amount: 17.into(), // TODO - verify that this is indeed correct. + kind: OrderKind::Buy, + partially_fillable: false, + sell_token_balance: Default::default(), + buy_amount: 2.into(), + valid_to: 0, + app_data: Default::default(), + fee_amount: 3.into(), + buy_token_balance: Default::default(), + }; + assert_eq!(result.quote, expected); + } } diff --git a/orderbook/src/lib.rs b/orderbook/src/lib.rs index ec7bd7e19..5646070ce 100644 --- a/orderbook/src/lib.rs +++ b/orderbook/src/lib.rs @@ -8,26 +8,23 @@ pub mod metrics; pub mod orderbook; pub mod solvable_orders; -use crate::orderbook::Orderbook; +use crate::{api::post_quote::OrderQuoter, orderbook::Orderbook}; use anyhow::{anyhow, Context as _, Result}; use contracts::GPv2Settlement; use database::trades::TradeRetrieving; -use fee::EthAwareMinFeeCalculator; use futures::Future; use model::DomainSeparator; -use shared::price_estimation::PriceEstimating; use std::{net::SocketAddr, sync::Arc}; use tokio::{task, task::JoinHandle}; pub fn serve_api( database: Arc, orderbook: Arc, - fee_calculator: Arc, - price_estimator: Arc, + quoter: Arc, address: SocketAddr, shutdown_receiver: impl Future + Send + 'static, ) -> JoinHandle<()> { - let filter = api::handle_all_routes(database, orderbook, fee_calculator, price_estimator); + let filter = api::handle_all_routes(database, orderbook, quoter); tracing::info!(%address, "serving order book"); let (_, server) = warp::serve(filter).bind_with_graceful_shutdown(address, shutdown_receiver); task::spawn(server) diff --git a/orderbook/src/main.rs b/orderbook/src/main.rs index 4bb0fdbee..38fecf475 100644 --- a/orderbook/src/main.rs +++ b/orderbook/src/main.rs @@ -7,7 +7,7 @@ use model::{ }; use orderbook::{ account_balances::Web3BalanceFetcher, - api::order_validation::OrderValidator, + api::{order_validation::OrderValidator, post_quote::OrderQuoter}, database::{self, orders::OrderFilter, Postgres}, event_updater::EventUpdater, fee::EthAwareMinFeeCalculator, @@ -415,19 +415,22 @@ async fn main() { args.enable_presign_orders, solvable_orders_cache, args.solvable_orders_max_update_age, - order_validator, + order_validator.clone(), )); let service_maintainer = ServiceMaintenance { maintainers: vec![database.clone(), Arc::new(event_updater), pool_fetcher], }; check_database_connection(orderbook.as_ref()).await; - + let quoter = Arc::new(OrderQuoter::new( + fee_calculator, + price_estimator, + order_validator, + )); let (shutdown_sender, shutdown_receiver) = tokio::sync::oneshot::channel(); let serve_api = serve_api( database.clone(), orderbook.clone(), - fee_calculator, - price_estimator, + quoter, args.bind_address, async { let _ = shutdown_receiver.await;