From a8bd380582b2a26422a910cca83ba163b6e66960 Mon Sep 17 00:00:00 2001 From: josojo Date: Fri, 5 Aug 2022 09:41:40 +0200 Subject: [PATCH 1/8] Storing quotes longer, in case an on-chain order placement is intended --- Cargo.lock | 27 ++++- crates/database/src/quotes.rs | 74 ++++++++++++- crates/e2e/Cargo.toml | 1 + crates/e2e/tests/e2e/services.rs | 2 + crates/model/src/quote.rs | 17 ++- crates/model/src/signature.rs | 15 ++- crates/orderbook/openapi.yml | 12 ++- crates/orderbook/src/api/create_order.rs | 4 + crates/orderbook/src/api/post_quote.rs | 9 +- crates/orderbook/src/arguments.rs | 28 +++++ crates/orderbook/src/database/quotes.rs | 3 + crates/orderbook/src/main.rs | 2 + crates/orderbook/src/order_quoting.rs | 99 ++++++++++++++--- crates/orderbook/src/order_validation.rs | 101 +++++++++++++++--- ...otes_add_field_expiration_for_api_call.sql | 6 ++ 15 files changed, 351 insertions(+), 49 deletions(-) create mode 100644 database/sql/V027__quotes_add_field_expiration_for_api_call.sql diff --git a/Cargo.lock b/Cargo.lock index 238eb370da..e22aa67a6d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -341,6 +341,7 @@ dependencies = [ "num-integer", "num-traits", "serde", + "time 0.1.44", "winapi", ] @@ -792,6 +793,7 @@ version = "1.0.0" dependencies = [ "anyhow", "autopilot", + "chrono", "contracts", "criterion", "database", @@ -1247,7 +1249,7 @@ checksum = "4eb1a864a501629691edf6c15a593b7a51eebaa1e8468e9ddc623de7c9b58ec6" dependencies = [ "cfg-if", "libc", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", ] [[package]] @@ -1734,7 +1736,7 @@ checksum = "713d550d9b44d89174e066b7a6217ae06234c10cb47819a88290d2b353c31799" dependencies = [ "libc", "log", - "wasi", + "wasi 0.11.0+wasi-snapshot-preview1", "windows-sys", ] @@ -2788,7 +2790,7 @@ dependencies = [ "serde_with", "testlib", "thiserror", - "time", + "time 0.3.9", "tokio", "tokio-stream", "tracing", @@ -3125,6 +3127,17 @@ dependencies = [ "once_cell", ] +[[package]] +name = "time" +version = "0.1.44" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6db9e6914ab8b1ae1c260a4ae7a49b6c5611b40328a735b21862567685e73255" +dependencies = [ + "libc", + "wasi 0.10.0+wasi-snapshot-preview1", + "winapi", +] + [[package]] name = "time" version = "0.3.9" @@ -3330,7 +3343,7 @@ dependencies = [ "sharded-slab", "smallvec", "thread_local", - "time", + "time 0.3.9", "tracing", "tracing-core", "tracing-log", @@ -3491,6 +3504,12 @@ dependencies = [ "tracing", ] +[[package]] +name = "wasi" +version = "0.10.0+wasi-snapshot-preview1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a143597ca7c7793eff794def352d41792a93c481eb1042423ff7ff72ba2c31f" + [[package]] name = "wasi" version = "0.11.0+wasi-snapshot-preview1" diff --git a/crates/database/src/quotes.rs b/crates/database/src/quotes.rs index dd82c8262f..14d10bc705 100644 --- a/crates/database/src/quotes.rs +++ b/crates/database/src/quotes.rs @@ -1,4 +1,4 @@ -use crate::{orders::OrderKind, Address}; +use crate::{orders::{OrderKind}, Address}; use bigdecimal::BigDecimal; use sqlx::{ types::chrono::{DateTime, Utc}, @@ -7,6 +7,14 @@ use sqlx::{ pub type QuoteId = i64; +#[derive(Clone, Debug, PartialEq, sqlx::Type)] +#[sqlx(type_name = "OnchainSigningScheme")] +#[sqlx(rename_all = "lowercase")] +pub enum OnchainSigningScheme { + Eip1271, + PreSign, +} + /// One row in the `quotes` table. #[derive(Clone, Debug, PartialEq, sqlx::FromRow)] pub struct Quote { @@ -20,6 +28,7 @@ pub struct Quote { pub sell_token_price: f64, pub order_kind: OrderKind, pub expiration_timestamp: DateTime, + pub onchain_signing_scheme: Option, } /// Stores the quote and returns the id. The id of the quote parameter is not used. @@ -34,9 +43,10 @@ INSERT INTO quotes ( gas_price, sell_token_price, order_kind, - expiration_timestamp + expiration_timestamp, + onchain_signing_scheme, ) -VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9) +VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id "#; let (id,) = sqlx::query_as(QUERY) @@ -49,6 +59,7 @@ RETURNING id .bind(quote.sell_token_price) .bind(quote.order_kind) .bind(quote.expiration_timestamp) + .bind("e.onchain_signing_scheme) .fetch_one(ex) .await?; Ok(id) @@ -73,6 +84,7 @@ pub struct QuoteSearchParameters { pub buy_amount: BigDecimal, pub kind: OrderKind, pub expiration: DateTime, + pub onchain_signing_scheme: Option, } pub async fn find( @@ -91,7 +103,8 @@ WHERE (order_kind = 'buy' AND buy_amount = $5) ) AND order_kind = $6 AND - expiration_timestamp >= $7 + expiration_timestamp >= $7 AND + onchain_signing_scheme = $8 ORDER BY gas_amount * gas_price * sell_token_price ASC LIMIT 1 "#; @@ -103,6 +116,7 @@ LIMIT 1 .bind(¶ms.buy_amount) .bind(params.kind) .bind(params.expiration) + .bind(¶ms.onchain_signing_scheme) .fetch_optional(ex) .await } @@ -155,6 +169,7 @@ mod tests { sell_token_price: 7., order_kind: OrderKind::Sell, expiration_timestamp: now, + onchain_signing_scheme: None, }; let id = save(&mut db, "e).await.unwrap(); quote.id = id; @@ -181,11 +196,12 @@ mod tests { buy_token: ByteArray([3; 20]), sell_amount: 4.into(), buy_amount: 5.into(), - order_kind: OrderKind::Sell, gas_amount: 1., gas_price: 1., sell_token_price: 1., + order_kind: OrderKind::Sell, expiration_timestamp: now, + onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), }; let token_b = ByteArray([2; 20]); @@ -200,6 +216,7 @@ mod tests { gas_price: 1., sell_token_price: 1., expiration_timestamp: now, + onchain_signing_scheme: None, }; // Save two measurements for token_a @@ -247,6 +264,7 @@ mod tests { buy_amount: 1.into(), kind: quote_a.order_kind, expiration: now, + onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), }; assert_eq!( find(&mut db, &search_a).await.unwrap().unwrap(), @@ -306,6 +324,7 @@ mod tests { buy_amount: quote_b.buy_amount, kind: quote_b.order_kind, expiration: now, + onchain_signing_scheme: None, }; assert_eq!( find(&mut db, &search_b).await.unwrap().unwrap(), @@ -345,4 +364,49 @@ mod tests { assert_eq!(find(&mut db, &search_a).await.unwrap(), None); assert_eq!(find(&mut db, &search_b).await.unwrap(), None); } + + #[tokio::test] + #[ignore] + async fn postgres_save_and_find_quote_and_differentiates_by_signing_scheme() { + let mut db = PgConnection::connect("postgresql://").await.unwrap(); + let mut db = db.begin().await.unwrap(); + crate::clear_DANGER_(&mut db).await.unwrap(); + + let now = low_precision_now(); + let token_a = ByteArray([1; 20]); + let quote_a = Quote { + id: Default::default(), + sell_token: token_a, + buy_token: ByteArray([3; 20]), + sell_amount: 4.into(), + buy_amount: 5.into(), + gas_amount: 1., + gas_price: 1., + sell_token_price: 1., + order_kind: OrderKind::Sell, + expiration_timestamp: now, + onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), + }; + // Token A has readings valid until now and in 30s + let mut search_a = QuoteSearchParameters { + sell_token: quote_a.sell_token, + buy_token: quote_a.buy_token, + sell_amount_0: quote_a.sell_amount.clone(), + sell_amount_1: quote_a.sell_amount.clone(), + buy_amount: 1.into(), + kind: quote_a.order_kind, + expiration: now, + onchain_signing_scheme: quote_a.onchain_signing_scheme.clone(), + }; + + assert_eq!( + find(&mut db, &search_a).await.unwrap().unwrap(), + quote_a, + ); + search_a.onchain_signing_scheme = None; + assert_eq!( + find(&mut db, &search_a).await.unwrap(), + None, + ); + } } diff --git a/crates/e2e/Cargo.toml b/crates/e2e/Cargo.toml index 463ba10a0b..2ec84ff1ed 100644 --- a/crates/e2e/Cargo.toml +++ b/crates/e2e/Cargo.toml @@ -13,6 +13,7 @@ harness = false anyhow = "1.0" autopilot = { path = "../autopilot" } contracts = { path = "../contracts" } +chrono = "0.4" criterion = "0.3" database = { path = "../database" } ethcontract = { version = "0.17.0", default-features = false } diff --git a/crates/e2e/tests/e2e/services.rs b/crates/e2e/tests/e2e/services.rs index 365a71da3d..042fd64d4d 100644 --- a/crates/e2e/tests/e2e/services.rs +++ b/crates/e2e/tests/e2e/services.rs @@ -235,6 +235,8 @@ impl OrderbookServices { ..Default::default() }), api_db.clone(), + chrono::Duration::seconds(60i64), + chrono::Duration::seconds(60i64), )); let balance_fetcher = Arc::new(Web3BalanceFetcher::new( web3.clone(), diff --git a/crates/model/src/quote.rs b/crates/model/src/quote.rs index 6e58c4c8b0..8e5677e0fa 100644 --- a/crates/model/src/quote.rs +++ b/crates/model/src/quote.rs @@ -1,7 +1,6 @@ use crate::{ app_id::AppId, order::{BuyTokenDestination, OrderKind, SellTokenSource}, - signature::SigningScheme, time, u256_decimal, }; use chrono::{DateTime, Utc}; @@ -16,6 +15,20 @@ pub enum PriceQuality { Optimal, } +#[derive(Eq, PartialEq, Clone, Copy, Debug, Default, Deserialize, Serialize, Hash)] +#[serde(rename_all = "lowercase")] +pub enum QuoteSigningScheme { + #[default] + Eip712, + EthSign, + /// EIP1271 orders can be created via API-call or in the future via on-chain orders + Eip1271, + Eip1271ForOnchainOrder, + /// PreSign orders can be created via API-call or in the future via on-chain orders + PreSign, + PreSignForOnchainOrder, +} + /// The order parameters to quote a price and fee for. #[derive(Clone, Copy, Debug, Default, Deserialize, Serialize, PartialEq)] #[serde(rename_all = "camelCase")] @@ -38,7 +51,7 @@ pub struct OrderQuoteRequest { #[serde(default)] pub buy_token_balance: BuyTokenDestination, #[serde(default)] - pub signing_scheme: SigningScheme, + pub signing_scheme: QuoteSigningScheme, #[serde(default)] pub price_quality: PriceQuality, } diff --git a/crates/model/src/signature.rs b/crates/model/src/signature.rs index 59b4996782..4703532fac 100644 --- a/crates/model/src/signature.rs +++ b/crates/model/src/signature.rs @@ -1,4 +1,4 @@ -use crate::{bytes_hex, DomainSeparator}; +use crate::{bytes_hex, DomainSeparator, quote::QuoteSigningScheme}; use anyhow::{ensure, Context as _, Result}; use primitive_types::{H160, H256}; use serde::{de, Deserialize, Serialize}; @@ -21,6 +21,19 @@ pub enum SigningScheme { PreSign, } +impl From for SigningScheme { + fn from(scheme: QuoteSigningScheme) -> Self { + match scheme{ + QuoteSigningScheme::Eip712 => SigningScheme::Eip712, + QuoteSigningScheme::Eip1271 => SigningScheme::Eip1271, + QuoteSigningScheme::Eip1271ForOnchainOrder => SigningScheme::Eip1271, + QuoteSigningScheme::PreSign => SigningScheme::PreSign, + QuoteSigningScheme::PreSignForOnchainOrder => SigningScheme::PreSign, + QuoteSigningScheme::EthSign => SigningScheme::EthSign, + } + } +} + #[derive(Eq, PartialEq, Clone, Deserialize, Serialize, Hash)] #[serde(into = "JsonSignature", try_from = "JsonSignature")] pub enum Signature { diff --git a/crates/orderbook/openapi.yml b/crates/orderbook/openapi.yml index 6d00821672..37474878d7 100644 --- a/crates/orderbook/openapi.yml +++ b/crates/orderbook/openapi.yml @@ -542,7 +542,7 @@ components: Provides the information to calculate the fees. type: object properties: - expirationDate: + expiration: description: | Expiration date of the offered fee. Order service might not accept the fee after this expiration date. Encoded as ISO 8601 UTC. @@ -552,7 +552,7 @@ components: description: Absolute amount of fee charged per order in specified sellToken $ref: "#/components/schemas/TokenAmount" required: - - expirationDate + - expiration - amount OrderType: description: Is this a buy order or sell order? @@ -623,7 +623,7 @@ components: $ref: "#/components/schemas/BuyTokenDestination" default: "erc20" signingScheme: - $ref: "#/components/schemas/SigningScheme" + $ref: "#/components/schemas/QuoteSigningScheme" default: "eip712" required: - sellToken @@ -837,6 +837,10 @@ components: description: How was the order signed? type: string enum: [eip712, ethsign, presign] + QuoteSigningScheme: + description: How will the order be signed and is it intended for onchain order placement? + type: string + enum: [eip712, ethsign, presign, eip1271, eip1271asonchainorder, presignasonchainorder] EcdsaSigningScheme: description: How was the order signed? type: string @@ -1081,7 +1085,7 @@ components: $ref: "#/components/schemas/OrderParameters" from: $ref: "#/components/schemas/Address" - expirationDate: + expiration: description: | Expiration date of the offered fee. Order service might not accept the fee after this expiration date. Encoded as ISO 8601 UTC. diff --git a/crates/orderbook/src/api/create_order.rs b/crates/orderbook/src/api/create_order.rs index 68780a85b7..d01db6d9a3 100644 --- a/crates/orderbook/src/api/create_order.rs +++ b/crates/orderbook/src/api/create_order.rs @@ -159,6 +159,10 @@ impl IntoWarpReply for ValidationError { error("ZeroAmount", "Buy or sell amount is zero."), StatusCode::BAD_REQUEST, ), + Self::IncompatibleSigningScheme => with_status( + error("IncompatibleSigningScheme", "Signing scheme is not compatible with order placement method."), + StatusCode::BAD_REQUEST, + ), Self::Other(err) => with_status( internal_error(err.context("order_validation")), StatusCode::INTERNAL_SERVER_ERROR, diff --git a/crates/orderbook/src/api/post_quote.rs b/crates/orderbook/src/api/post_quote.rs index 78cfb9cf43..b660729887 100644 --- a/crates/orderbook/src/api/post_quote.rs +++ b/crates/orderbook/src/api/post_quote.rs @@ -63,9 +63,8 @@ mod tests { app_id::AppId, order::{BuyTokenDestination, SellTokenSource}, quote::{ - OrderQuote, OrderQuoteResponse, OrderQuoteSide, PriceQuality, SellAmount, Validity, - }, - signature::SigningScheme, + OrderQuote, OrderQuoteResponse, OrderQuoteSide, PriceQuality, SellAmount, Validity, QuoteSigningScheme, + } }; use serde_json::json; use shared::api::response_body; @@ -101,8 +100,8 @@ mod tests { partially_fillable: false, sell_token_balance: SellTokenSource::Erc20, buy_token_balance: BuyTokenDestination::Internal, - signing_scheme: SigningScheme::PreSign, - price_quality: PriceQuality::Optimal + signing_scheme: QuoteSigningScheme::PreSign, + price_quality: PriceQuality::Optimal, } ); } diff --git a/crates/orderbook/src/arguments.rs b/crates/orderbook/src/arguments.rs index e0f19b1a83..7ec4c42f69 100644 --- a/crates/orderbook/src/arguments.rs +++ b/crates/orderbook/src/arguments.rs @@ -40,6 +40,24 @@ pub struct Arguments { )] pub max_order_validity_period: Duration, + /// The time period an EIP1271-quote request is valid. + #[clap( + long, + env, + default_value = "600", + parse(try_from_str = shared::arguments::duration_from_seconds), + )] + pub eip1271_onchain_quote_validity_seconds: Duration, + + /// The time period an PRESIGN-quote request is valid. + #[clap( + long, + env, + default_value = "600", + parse(try_from_str = shared::arguments::duration_from_seconds), + )] + pub presign_onchain_quote_validity_seconds: Duration, + /// Don't use the trace_callMany api that only some nodes support to check whether a token /// should be denied. /// Note that if a node does not support the api we still use the less accurate call api. @@ -241,6 +259,16 @@ impl std::fmt::Display for Arguments { "max_order_validity_period: {:?}", self.max_order_validity_period )?; + writeln!( + f, + "eip1271_onchain_quote_validity_second: {:?}", + self.eip1271_onchain_quote_validity_seconds + )?; + writeln!( + f, + "presign_onchain_quote_validity_second: {:?}", + self.presign_onchain_quote_validity_seconds + )?; writeln!(f, "skip_trace_api: {}", self.skip_trace_api)?; writeln!( f, diff --git a/crates/orderbook/src/database/quotes.rs b/crates/orderbook/src/database/quotes.rs index 2afb6e4537..32e986e071 100644 --- a/crates/orderbook/src/database/quotes.rs +++ b/crates/orderbook/src/database/quotes.rs @@ -34,6 +34,7 @@ impl TryFrom for QuoteData { }, kind: order_kind_from(row.order_kind), expiration: row.expiration_timestamp, + onchain_signing_scheme: row.onchain_signing_scheme, }) } } @@ -58,6 +59,7 @@ impl QuoteStoring for Postgres { sell_token_price: data.fee_parameters.sell_token_price, order_kind: order_kind_into(data.kind), expiration_timestamp: data.expiration, + onchain_signing_scheme: data.onchain_signing_scheme, }; let id = database::quotes::save(&mut ex, &row).await?; Ok(Some(id)) @@ -93,6 +95,7 @@ impl QuoteStoring for Postgres { buy_amount: u256_to_big_decimal(¶ms.buy_amount), kind: order_kind_into(params.kind), expiration, + onchain_signing_scheme: params.onchain_signing_scheme, }; let quote = database::quotes::find(&mut ex, ¶ms) .await diff --git a/crates/orderbook/src/main.rs b/crates/orderbook/src/main.rs index 8626948c78..c83ffd1616 100644 --- a/crates/orderbook/src/main.rs +++ b/crates/orderbook/src/main.rs @@ -493,6 +493,8 @@ async fn main() { gas_price_estimator.clone(), fee_subsidy.clone(), storage, + chrono::Duration::from_std(args.eip1271_onchain_quote_validity_seconds).unwrap(), + chrono::Duration::from_std(args.presign_onchain_quote_validity_seconds).unwrap(), )) }; let optimal_quoter = create_quoter(price_estimator.clone(), database.clone()); diff --git a/crates/orderbook/src/order_quoting.rs b/crates/orderbook/src/order_quoting.rs index 50fc59b56a..f9348d5188 100644 --- a/crates/orderbook/src/order_quoting.rs +++ b/crates/orderbook/src/order_quoting.rs @@ -3,7 +3,8 @@ use crate::{ order_validation::{OrderValidating, PartialValidationError, PreOrderData}, }; use anyhow::Result; -use chrono::{DateTime, TimeZone as _, Utc}; +use chrono::{DateTime, TimeZone as _, Utc, Duration}; +use database::quotes::OnchainSigningScheme; use ethcontract::{H160, U256}; use futures::TryFutureExt as _; use gas_estimation::GasPriceEstimating; @@ -12,7 +13,7 @@ use model::{ order::OrderKind, quote::{ OrderQuote, OrderQuoteRequest, OrderQuoteResponse, OrderQuoteSide, PriceQuality, QuoteId, - SellAmount, + SellAmount, QuoteSigningScheme, }, }; use shared::price_estimation::{ @@ -111,6 +112,7 @@ pub struct QuoteParameters { pub side: OrderQuoteSide, pub from: H160, pub app_data: AppId, + pub signing_scheme: QuoteSigningScheme, } impl QuoteParameters { @@ -223,6 +225,11 @@ pub struct QuoteData { pub fee_parameters: FeeParameters, pub kind: OrderKind, pub expiration: DateTime, + /// Since different on-chain signing schemes have different expirations, + /// we need to store the onchain_signing_scheme to prevent missuse of quotes. + /// Since all off-chain orders have the same validity, no differentiation + /// needs to be stored and we can set the value to None + pub onchain_signing_scheme: Option, } impl Default for QuoteData { @@ -235,6 +242,7 @@ impl Default for QuoteData { fee_parameters: Default::default(), kind: Default::default(), expiration: Utc.timestamp(0, 0), + onchain_signing_scheme: None, } } } @@ -294,6 +302,7 @@ pub struct QuoteSearchParameters { pub kind: OrderKind, pub from: H160, pub app_data: AppId, + pub onchain_signing_scheme: Option, } impl QuoteSearchParameters { @@ -379,8 +388,8 @@ impl Now for DateTime { } } -/// How long a quote remains valid for. -const QUOTE_VALIDITY_SECONDS: i64 = 60; +/// Standard validity for a quote: Quotes are stored only as long as their validity is. +const STANDARD_QUOTE_VALIDITY_SECONDS: i64 = 60; /// An order quoter implementation that relies pub struct OrderQuoter { @@ -390,6 +399,8 @@ pub struct OrderQuoter { fee_subsidy: Arc, storage: Arc, now: Arc, + eip1271_onchain_quote_validity_seconds: Duration, + presign_onchain_quote_validity_seconds:Duration, } impl OrderQuoter { @@ -399,6 +410,8 @@ impl OrderQuoter { gas_estimator: Arc, fee_subsidy: Arc, storage: Arc, + eip1271_onchain_quote_validity_seconds: Duration, + presign_onchain_quote_validity_seconds: Duration, ) -> Self { Self { price_estimator, @@ -407,6 +420,8 @@ impl OrderQuoter { fee_subsidy, storage, now: Arc::new(Utc::now), + eip1271_onchain_quote_validity_seconds, + presign_onchain_quote_validity_seconds, } } @@ -414,7 +429,17 @@ impl OrderQuoter { &self, parameters: &QuoteParameters, ) -> Result { - let expiration = self.now.now() + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS); + let expiration = match parameters.signing_scheme{ + QuoteSigningScheme::Eip1271ForOnchainOrder => { + self.now.now() + + self.eip1271_onchain_quote_validity_seconds + } + QuoteSigningScheme::PreSignForOnchainOrder => { + self.now.now() + + self.presign_onchain_quote_validity_seconds + }, + _ => self.now.now() + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + }; let trade_query = parameters.to_price_query(); let (gas_estimate, trade_estimate, sell_token_price, _) = futures::try_join!( @@ -446,6 +471,12 @@ impl OrderQuoter { sell_token_price, }; + let onchain_signing_scheme = match parameters.signing_scheme { + QuoteSigningScheme::Eip1271ForOnchainOrder => Some(OnchainSigningScheme::Eip1271), + QuoteSigningScheme::PreSignForOnchainOrder => Some(OnchainSigningScheme::PreSign), + _ => None + }; + let quote = QuoteData { sell_token: parameters.sell_token, buy_token: parameters.buy_token, @@ -454,6 +485,7 @@ impl OrderQuoter { fee_parameters, kind: trade_query.kind, expiration, + onchain_signing_scheme, }; Ok(quote) @@ -584,7 +616,7 @@ impl From<&OrderQuoteRequest> for PreOrderData { partially_fillable: quote_request.partially_fillable, buy_token_balance: quote_request.buy_token_balance, sell_token_balance: quote_request.sell_token_balance, - signing_scheme: quote_request.signing_scheme, + signing_scheme: quote_request.signing_scheme.into(), is_liquidity_order: quote_request.partially_fillable, } } @@ -598,6 +630,7 @@ impl From<&OrderQuoteRequest> for QuoteParameters { side: request.side, from: request.from, app_data: request.app_data, + signing_scheme: request.signing_scheme, } } } @@ -654,6 +687,7 @@ mod tests { }, from: H160([3; 20]), app_data: AppId([4; 32]), + signing_scheme: QuoteSigningScheme::Eip712, }; let gas_price = GasPrice1559 { base_fee_per_gas: 1.5, @@ -713,7 +747,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Sell, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, })) .returning(|_| Ok(Some(1337))); @@ -724,6 +759,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -741,7 +778,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Sell, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, }, sell_amount: 70.into(), buy_amount: 29.into(), @@ -761,6 +799,7 @@ mod tests { }, from: H160([3; 20]), app_data: AppId([4; 32]), + signing_scheme: QuoteSigningScheme::Eip712, }; let gas_price = GasPrice1559 { base_fee_per_gas: 1.5, @@ -820,7 +859,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Sell, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, })) .returning(|_| Ok(Some(1337))); @@ -834,6 +874,8 @@ mod tests { }), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -851,7 +893,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Sell, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -871,6 +914,7 @@ mod tests { }, from: H160([3; 20]), app_data: AppId([4; 32]), + signing_scheme: QuoteSigningScheme::Eip712, }; let gas_price = GasPrice1559 { base_fee_per_gas: 1.5, @@ -930,7 +974,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Buy, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, })) .returning(|_| Ok(Some(1337))); @@ -945,6 +990,8 @@ mod tests { }), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -962,7 +1009,8 @@ mod tests { sell_token_price: 0.2, }, kind: OrderKind::Buy, - expiration: now + chrono::Duration::seconds(QUOTE_VALIDITY_SECONDS), + expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), + onchain_signing_scheme: None, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -981,6 +1029,7 @@ mod tests { }, from: H160([3; 20]), app_data: AppId([4; 32]), + signing_scheme: QuoteSigningScheme::Eip712, }; let gas_price = GasPrice1559 { base_fee_per_gas: 1., @@ -1023,6 +1072,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(MockQuoteStoring::new()), now: Arc::new(Utc::now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert!(matches!( @@ -1043,6 +1094,7 @@ mod tests { }, from: H160([3; 20]), app_data: AppId([4; 32]), + signing_scheme: QuoteSigningScheme::Eip712, }; let gas_price = GasPrice1559 { base_fee_per_gas: 1., @@ -1089,6 +1141,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(MockQuoteStoring::new()), now: Arc::new(Utc::now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert!(matches!( @@ -1124,6 +1178,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(Forget), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; let quote = quoter.calculate_quote(Default::default()).await.unwrap(); @@ -1143,6 +1199,7 @@ mod tests { kind: OrderKind::Sell, from: H160([3; 20]), app_data: AppId([4; 32]), + onchain_signing_scheme: None, }; let mut storage = MockQuoteStoring::new(); @@ -1159,6 +1216,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, })) }); @@ -1172,6 +1230,8 @@ mod tests { }), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -1190,6 +1250,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, }, sell_amount: 85.into(), // Allows for "out-of-price" buy amounts. This means that order @@ -1218,6 +1279,7 @@ mod tests { kind: OrderKind::Sell, from: H160([3; 20]), app_data: AppId([4; 32]), + onchain_signing_scheme: None, }; let mut storage = MockQuoteStoring::new(); @@ -1234,6 +1296,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, })) }); @@ -1244,6 +1307,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -1262,6 +1327,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -1282,6 +1348,7 @@ mod tests { kind: OrderKind::Buy, from: H160([3; 20]), app_data: AppId([4; 32]), + onchain_signing_scheme: None, }; let mut storage = MockQuoteStoring::new(); @@ -1303,6 +1370,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, }, ))) }); @@ -1314,6 +1382,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert_eq!( @@ -1332,6 +1402,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(10), + onchain_signing_scheme: None, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -1380,6 +1451,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(storage), now: Arc::new(now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert!(matches!( @@ -1408,6 +1481,8 @@ mod tests { fee_subsidy: Arc::new(Subsidy::default()), storage: Arc::new(storage), now: Arc::new(Utc::now), + eip1271_onchain_quote_validity_seconds: Duration::seconds(60i64), + presign_onchain_quote_validity_seconds: Duration::seconds(60i64), }; assert!(matches!( diff --git a/crates/orderbook/src/order_validation.rs b/crates/orderbook/src/order_validation.rs index cd9497bebf..13b5065dfc 100644 --- a/crates/orderbook/src/order_validation.rs +++ b/crates/orderbook/src/order_validation.rs @@ -4,13 +4,14 @@ use crate::order_quoting::{ }; use anyhow::anyhow; use contracts::WETH9; +use database::quotes::OnchainSigningScheme; use ethcontract::{H160, U256}; use model::{ order::{ BuyTokenDestination, Order, OrderCreation, OrderData, OrderKind, SellTokenSource, BUY_ETH_ADDRESS, }, - quote::{OrderQuoteSide, SellAmount}, + quote::{OrderQuoteSide, SellAmount, QuoteSigningScheme}, signature::{hashed_eip712_message, Signature, SigningScheme, VerificationError}, DomainSeparator, }; @@ -97,6 +98,7 @@ pub enum ValidationError { MissingFrom, WrongOwner(H160), ZeroAmount, + IncompatibleSigningScheme, Other(anyhow::Error), } @@ -358,7 +360,7 @@ impl OrderValidating for OrderValidator { .map_err(ValidationError::Partial)?; let quote = if !liquidity_owner { - Some(get_quote_and_check_fee(&*self.quoter, &order, owner).await?) + Some(get_quote_and_check_fee(&*self.quoter, &order, true, owner).await?) } else { // We don't try to get quotes for orders created by liqudity order // owners for two reasons: @@ -511,8 +513,10 @@ fn minimum_balance(order: &OrderData) -> Option { async fn get_quote_and_check_fee( quoter: &dyn OrderQuoting, order: &OrderCreation, + order_placement_via_api: bool, owner: H160, ) -> Result { + let onchain_signing_scheme= convert_signing_scheme_into_onchain_signing_scheme(order.signature.scheme(), order_placement_via_api)?; let parameters = QuoteSearchParameters { sell_token: order.data.sell_token, buy_token: order.data.buy_token, @@ -522,6 +526,7 @@ async fn get_quote_and_check_fee( kind: order.data.kind, from: owner, app_data: order.data.app_data, + onchain_signing_scheme, }; let quote = match quoter.find_quote(order.quote_id, parameters).await { @@ -532,6 +537,7 @@ async fn get_quote_and_check_fee( // We couldn't find a quote, and no ID was specified. Try computing a // fresh quote to use instead. Err(FindQuoteError::NotFound(_)) if order.quote_id.is_none() => { + let signing_scheme = convert_signing_scheme_into_quote_signing_scheme(order.signature.scheme(), order_placement_via_api)?; let parameters = QuoteParameters { sell_token: order.data.sell_token, buy_token: order.data.buy_token, @@ -547,6 +553,7 @@ async fn get_quote_and_check_fee( }, from: owner, app_data: order.data.app_data, + signing_scheme, }; let quote = quoter.calculate_quote(parameters).await?; @@ -572,16 +579,48 @@ fn is_order_outside_market_price(order: &OrderData, quote: &Quote) -> bool { order.sell_amount.full_mul(quote.buy_amount) < quote.sell_amount.full_mul(order.buy_amount) } +fn convert_signing_scheme_into_quote_signing_scheme(scheme: SigningScheme, order_placement_via_api: bool) -> Result { + match order_placement_via_api { + true => + match scheme{ + SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSign), + SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271), + SigningScheme::Eip712 => Ok(QuoteSigningScheme::Eip712), + SigningScheme::EthSign => Ok(QuoteSigningScheme::EthSign), + }, + false => + match scheme{ + SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSignForOnchainOrder), + SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271ForOnchainOrder), + SigningScheme::Eip712 => Err(ValidationError::IncompatibleSigningScheme), + SigningScheme::EthSign => Err(ValidationError::IncompatibleSigningScheme), + } + } +} + +fn convert_signing_scheme_into_onchain_signing_scheme(scheme: SigningScheme, order_placement_via_api: bool) -> Result,ValidationError> { + match order_placement_via_api { + true => Ok(None), + false => match scheme { + SigningScheme::Eip1271 => Ok(Some(OnchainSigningScheme::Eip1271)), + SigningScheme::PreSign => Ok(Some(OnchainSigningScheme::PreSign)), + _ => Err(ValidationError::IncompatibleSigningScheme), + } + } +} + #[cfg(test)] mod tests { use super::*; - use crate::order_quoting::MockOrderQuoting; + use crate::{ + order_quoting::{MockOrderQuoting}, + }; use anyhow::anyhow; use chrono::Utc; use ethcontract::web3::signing::SecretKeyRef; use maplit::hashset; use mockall::predicate::{always, eq}; - use model::{app_id::AppId, order::OrderBuilder, signature::EcdsaSigningScheme}; + use model::{app_id::AppId, order::OrderBuilder, signature::{EcdsaSigningScheme, EcdsaSignature}}; use secp256k1::ONE_KEY; use shared::{ account_balances::MockBalanceFetching, @@ -1381,6 +1420,7 @@ mod tests { kind: OrderKind::Buy, from: H160([0xf0; 20]), app_data: AppId([5; 32]), + onchain_signing_scheme: None, }), ) .returning(|_, _| { @@ -1390,7 +1430,7 @@ mod tests { }) }); - let quote = get_quote_and_check_fee(&order_quoter, &order, from) + let quote = get_quote_and_check_fee(&order_quoter, &order, true, from) .await .unwrap(); @@ -1436,6 +1476,7 @@ mod tests { }, from, app_data: AppId([5; 32]), + signing_scheme: QuoteSigningScheme::Eip712, })) .returning(|_| { Ok(Quote { @@ -1444,7 +1485,7 @@ mod tests { }) }); - let quote = get_quote_and_check_fee(&order_quoter, &order, from) + let quote = get_quote_and_check_fee(&order_quoter, &order, true, from) .await .unwrap(); @@ -1469,13 +1510,33 @@ mod tests { .expect_find_quote() .returning(|_, _| Err(FindQuoteError::NotFound(Some(0)))); - let err = get_quote_and_check_fee(&order_quoter, &order, Default::default()) + let err = get_quote_and_check_fee(&order_quoter, &order, true, Default::default()) .await .unwrap_err(); assert!(matches!(err, ValidationError::QuoteNotFound)); } + #[tokio::test] + async fn get_quote_errors_in_case_of_api_call_and_quote_requested_for_onchain_orders() { + let order = OrderCreation { + data: OrderData { + fee_amount: 1.into(), + ..Default::default() + }, + signature: Signature::Eip712(EcdsaSignature::default()), + ..Default::default() + }; + + let order_quoter = MockOrderQuoting::new(); + + let err = get_quote_and_check_fee(&order_quoter, &order, false, Default::default()) + .await + .unwrap_err(); + + assert!(matches!(err, ValidationError::IncompatibleSigningScheme)); + } + #[tokio::test] async fn get_quote_errors_on_insufficient_fees() { let order = OrderCreation { @@ -1494,7 +1555,7 @@ mod tests { }) }); - let err = get_quote_and_check_fee(&order_quoter, &order, Default::default()) + let err = get_quote_and_check_fee(&order_quoter, &order, true, Default::default()) .await .unwrap_err(); @@ -1510,10 +1571,14 @@ mod tests { .expect_find_quote() .returning(|_, _| Err($find_err)); - let err = - get_quote_and_check_fee(&order_quoter, &Default::default(), Default::default()) - .await - .unwrap_err(); + let err = get_quote_and_check_fee( + &order_quoter, + &Default::default(), + true, + Default::default(), + ) + .await + .unwrap_err(); assert!(matches!(err, $validation_err)); }}; @@ -1538,10 +1603,14 @@ mod tests { .expect_calculate_quote() .returning(|_| Err($calc_err)); - let err = - get_quote_and_check_fee(&order_quoter, &Default::default(), Default::default()) - .await - .unwrap_err(); + let err = get_quote_and_check_fee( + &order_quoter, + &Default::default(), + true, + Default::default(), + ) + .await + .unwrap_err(); assert!(matches!(err, $validation_err)); }}; diff --git a/database/sql/V027__quotes_add_field_expiration_for_api_call.sql b/database/sql/V027__quotes_add_field_expiration_for_api_call.sql new file mode 100644 index 0000000000..4926baf231 --- /dev/null +++ b/database/sql/V027__quotes_add_field_expiration_for_api_call.sql @@ -0,0 +1,6 @@ +-- Adding new column on table quotes to differentiate the two on-chain signing schemes +-- This is required as different signing schemes have different expirations +CREATE TYPE OnchainSigningScheme AS ENUM ('eip1271', 'presign'); + +ALTER TABLE quotes +ADD COLUMN onchain_signing_sheme OnchainSigningScheme; From a853a5ce4f65182c237e9de6a85ab3c59771001d Mon Sep 17 00:00:00 2001 From: josojo Date: Fri, 5 Aug 2022 10:03:23 +0200 Subject: [PATCH 2/8] formatting --- crates/database/src/quotes.rs | 14 +++----- crates/model/src/signature.rs | 4 +-- crates/orderbook/src/api/create_order.rs | 5 ++- crates/orderbook/src/api/post_quote.rs | 5 +-- crates/orderbook/src/order_quoting.rs | 20 +++++------ crates/orderbook/src/order_validation.rs | 42 +++++++++++++++--------- 6 files changed, 49 insertions(+), 41 deletions(-) diff --git a/crates/database/src/quotes.rs b/crates/database/src/quotes.rs index 14d10bc705..ea74d82992 100644 --- a/crates/database/src/quotes.rs +++ b/crates/database/src/quotes.rs @@ -1,4 +1,4 @@ -use crate::{orders::{OrderKind}, Address}; +use crate::{orders::OrderKind, Address}; use bigdecimal::BigDecimal; use sqlx::{ types::chrono::{DateTime, Utc}, @@ -398,15 +398,9 @@ mod tests { expiration: now, onchain_signing_scheme: quote_a.onchain_signing_scheme.clone(), }; - - assert_eq!( - find(&mut db, &search_a).await.unwrap().unwrap(), - quote_a, - ); + + assert_eq!(find(&mut db, &search_a).await.unwrap().unwrap(), quote_a,); search_a.onchain_signing_scheme = None; - assert_eq!( - find(&mut db, &search_a).await.unwrap(), - None, - ); + assert_eq!(find(&mut db, &search_a).await.unwrap(), None,); } } diff --git a/crates/model/src/signature.rs b/crates/model/src/signature.rs index 4703532fac..7a2a80e959 100644 --- a/crates/model/src/signature.rs +++ b/crates/model/src/signature.rs @@ -1,4 +1,4 @@ -use crate::{bytes_hex, DomainSeparator, quote::QuoteSigningScheme}; +use crate::{bytes_hex, quote::QuoteSigningScheme, DomainSeparator}; use anyhow::{ensure, Context as _, Result}; use primitive_types::{H160, H256}; use serde::{de, Deserialize, Serialize}; @@ -23,7 +23,7 @@ pub enum SigningScheme { impl From for SigningScheme { fn from(scheme: QuoteSigningScheme) -> Self { - match scheme{ + match scheme { QuoteSigningScheme::Eip712 => SigningScheme::Eip712, QuoteSigningScheme::Eip1271 => SigningScheme::Eip1271, QuoteSigningScheme::Eip1271ForOnchainOrder => SigningScheme::Eip1271, diff --git a/crates/orderbook/src/api/create_order.rs b/crates/orderbook/src/api/create_order.rs index d01db6d9a3..fde3a3d8f6 100644 --- a/crates/orderbook/src/api/create_order.rs +++ b/crates/orderbook/src/api/create_order.rs @@ -160,7 +160,10 @@ impl IntoWarpReply for ValidationError { StatusCode::BAD_REQUEST, ), Self::IncompatibleSigningScheme => with_status( - error("IncompatibleSigningScheme", "Signing scheme is not compatible with order placement method."), + error( + "IncompatibleSigningScheme", + "Signing scheme is not compatible with order placement method.", + ), StatusCode::BAD_REQUEST, ), Self::Other(err) => with_status( diff --git a/crates/orderbook/src/api/post_quote.rs b/crates/orderbook/src/api/post_quote.rs index b660729887..6bf181862d 100644 --- a/crates/orderbook/src/api/post_quote.rs +++ b/crates/orderbook/src/api/post_quote.rs @@ -63,8 +63,9 @@ mod tests { app_id::AppId, order::{BuyTokenDestination, SellTokenSource}, quote::{ - OrderQuote, OrderQuoteResponse, OrderQuoteSide, PriceQuality, SellAmount, Validity, QuoteSigningScheme, - } + OrderQuote, OrderQuoteResponse, OrderQuoteSide, PriceQuality, QuoteSigningScheme, + SellAmount, Validity, + }, }; use serde_json::json; use shared::api::response_body; diff --git a/crates/orderbook/src/order_quoting.rs b/crates/orderbook/src/order_quoting.rs index f9348d5188..9a0639365c 100644 --- a/crates/orderbook/src/order_quoting.rs +++ b/crates/orderbook/src/order_quoting.rs @@ -3,7 +3,7 @@ use crate::{ order_validation::{OrderValidating, PartialValidationError, PreOrderData}, }; use anyhow::Result; -use chrono::{DateTime, TimeZone as _, Utc, Duration}; +use chrono::{DateTime, Duration, TimeZone as _, Utc}; use database::quotes::OnchainSigningScheme; use ethcontract::{H160, U256}; use futures::TryFutureExt as _; @@ -13,7 +13,7 @@ use model::{ order::OrderKind, quote::{ OrderQuote, OrderQuoteRequest, OrderQuoteResponse, OrderQuoteSide, PriceQuality, QuoteId, - SellAmount, QuoteSigningScheme, + QuoteSigningScheme, SellAmount, }, }; use shared::price_estimation::{ @@ -227,7 +227,7 @@ pub struct QuoteData { pub expiration: DateTime, /// Since different on-chain signing schemes have different expirations, /// we need to store the onchain_signing_scheme to prevent missuse of quotes. - /// Since all off-chain orders have the same validity, no differentiation + /// Since all off-chain orders have the same validity, no differentiation /// needs to be stored and we can set the value to None pub onchain_signing_scheme: Option, } @@ -400,7 +400,7 @@ pub struct OrderQuoter { storage: Arc, now: Arc, eip1271_onchain_quote_validity_seconds: Duration, - presign_onchain_quote_validity_seconds:Duration, + presign_onchain_quote_validity_seconds: Duration, } impl OrderQuoter { @@ -429,15 +429,13 @@ impl OrderQuoter { &self, parameters: &QuoteParameters, ) -> Result { - let expiration = match parameters.signing_scheme{ + let expiration = match parameters.signing_scheme { QuoteSigningScheme::Eip1271ForOnchainOrder => { - self.now.now() - + self.eip1271_onchain_quote_validity_seconds + self.now.now() + self.eip1271_onchain_quote_validity_seconds } QuoteSigningScheme::PreSignForOnchainOrder => { - self.now.now() - + self.presign_onchain_quote_validity_seconds - }, + self.now.now() + self.presign_onchain_quote_validity_seconds + } _ => self.now.now() + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), }; @@ -474,7 +472,7 @@ impl OrderQuoter { let onchain_signing_scheme = match parameters.signing_scheme { QuoteSigningScheme::Eip1271ForOnchainOrder => Some(OnchainSigningScheme::Eip1271), QuoteSigningScheme::PreSignForOnchainOrder => Some(OnchainSigningScheme::PreSign), - _ => None + _ => None, }; let quote = QuoteData { diff --git a/crates/orderbook/src/order_validation.rs b/crates/orderbook/src/order_validation.rs index 13b5065dfc..b657d8c80d 100644 --- a/crates/orderbook/src/order_validation.rs +++ b/crates/orderbook/src/order_validation.rs @@ -11,7 +11,7 @@ use model::{ BuyTokenDestination, Order, OrderCreation, OrderData, OrderKind, SellTokenSource, BUY_ETH_ADDRESS, }, - quote::{OrderQuoteSide, SellAmount, QuoteSigningScheme}, + quote::{OrderQuoteSide, QuoteSigningScheme, SellAmount}, signature::{hashed_eip712_message, Signature, SigningScheme, VerificationError}, DomainSeparator, }; @@ -516,7 +516,10 @@ async fn get_quote_and_check_fee( order_placement_via_api: bool, owner: H160, ) -> Result { - let onchain_signing_scheme= convert_signing_scheme_into_onchain_signing_scheme(order.signature.scheme(), order_placement_via_api)?; + let onchain_signing_scheme = convert_signing_scheme_into_onchain_signing_scheme( + order.signature.scheme(), + order_placement_via_api, + )?; let parameters = QuoteSearchParameters { sell_token: order.data.sell_token, buy_token: order.data.buy_token, @@ -537,7 +540,10 @@ async fn get_quote_and_check_fee( // We couldn't find a quote, and no ID was specified. Try computing a // fresh quote to use instead. Err(FindQuoteError::NotFound(_)) if order.quote_id.is_none() => { - let signing_scheme = convert_signing_scheme_into_quote_signing_scheme(order.signature.scheme(), order_placement_via_api)?; + let signing_scheme = convert_signing_scheme_into_quote_signing_scheme( + order.signature.scheme(), + order_placement_via_api, + )?; let parameters = QuoteParameters { sell_token: order.data.sell_token, buy_token: order.data.buy_token, @@ -579,48 +585,54 @@ fn is_order_outside_market_price(order: &OrderData, quote: &Quote) -> bool { order.sell_amount.full_mul(quote.buy_amount) < quote.sell_amount.full_mul(order.buy_amount) } -fn convert_signing_scheme_into_quote_signing_scheme(scheme: SigningScheme, order_placement_via_api: bool) -> Result { +fn convert_signing_scheme_into_quote_signing_scheme( + scheme: SigningScheme, + order_placement_via_api: bool, +) -> Result { match order_placement_via_api { - true => - match scheme{ + true => match scheme { SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSign), SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271), SigningScheme::Eip712 => Ok(QuoteSigningScheme::Eip712), SigningScheme::EthSign => Ok(QuoteSigningScheme::EthSign), }, - false => - match scheme{ + false => match scheme { SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSignForOnchainOrder), SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271ForOnchainOrder), SigningScheme::Eip712 => Err(ValidationError::IncompatibleSigningScheme), SigningScheme::EthSign => Err(ValidationError::IncompatibleSigningScheme), - } + }, } } -fn convert_signing_scheme_into_onchain_signing_scheme(scheme: SigningScheme, order_placement_via_api: bool) -> Result,ValidationError> { +fn convert_signing_scheme_into_onchain_signing_scheme( + scheme: SigningScheme, + order_placement_via_api: bool, +) -> Result, ValidationError> { match order_placement_via_api { true => Ok(None), false => match scheme { SigningScheme::Eip1271 => Ok(Some(OnchainSigningScheme::Eip1271)), SigningScheme::PreSign => Ok(Some(OnchainSigningScheme::PreSign)), _ => Err(ValidationError::IncompatibleSigningScheme), - } + }, } } #[cfg(test)] mod tests { use super::*; - use crate::{ - order_quoting::{MockOrderQuoting}, - }; + use crate::order_quoting::MockOrderQuoting; use anyhow::anyhow; use chrono::Utc; use ethcontract::web3::signing::SecretKeyRef; use maplit::hashset; use mockall::predicate::{always, eq}; - use model::{app_id::AppId, order::OrderBuilder, signature::{EcdsaSigningScheme, EcdsaSignature}}; + use model::{ + app_id::AppId, + order::OrderBuilder, + signature::{EcdsaSignature, EcdsaSigningScheme}, + }; use secp256k1::ONE_KEY; use shared::{ account_balances::MockBalanceFetching, From 96aed46831aad6d72b7a2bc5f5922ce64171e2be Mon Sep 17 00:00:00 2001 From: josojo Date: Fri, 5 Aug 2022 12:48:29 +0200 Subject: [PATCH 3/8] fixing tests --- crates/database/src/quotes.rs | 60 ++++++++++--------- ..._quotes_adding_onchain_signing_scheme.sql} | 2 +- 2 files changed, 34 insertions(+), 28 deletions(-) rename database/sql/{V027__quotes_add_field_expiration_for_api_call.sql => V027__quotes_adding_onchain_signing_scheme.sql} (78%) diff --git a/crates/database/src/quotes.rs b/crates/database/src/quotes.rs index ea74d82992..501a845edd 100644 --- a/crates/database/src/quotes.rs +++ b/crates/database/src/quotes.rs @@ -44,7 +44,7 @@ INSERT INTO quotes ( sell_token_price, order_kind, expiration_timestamp, - onchain_signing_scheme, + onchain_signing_scheme ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id @@ -104,7 +104,7 @@ WHERE ) AND order_kind = $6 AND expiration_timestamp >= $7 AND - onchain_signing_scheme = $8 + onchain_signing_scheme IS NOT DISTINCT FROM $8 ORDER BY gas_amount * gas_price * sell_token_price ASC LIMIT 1 "#; @@ -196,12 +196,12 @@ mod tests { buy_token: ByteArray([3; 20]), sell_amount: 4.into(), buy_amount: 5.into(), + order_kind: OrderKind::Sell, gas_amount: 1., gas_price: 1., sell_token_price: 1., - order_kind: OrderKind::Sell, expiration_timestamp: now, - onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), + onchain_signing_scheme: None, }; let token_b = ByteArray([2; 20]); @@ -264,7 +264,7 @@ mod tests { buy_amount: 1.into(), kind: quote_a.order_kind, expiration: now, - onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), + onchain_signing_scheme: None, }; assert_eq!( find(&mut db, &search_a).await.unwrap().unwrap(), @@ -374,32 +374,38 @@ mod tests { let now = low_precision_now(); let token_a = ByteArray([1; 20]); - let quote_a = Quote { - id: Default::default(), - sell_token: token_a, - buy_token: ByteArray([3; 20]), - sell_amount: 4.into(), - buy_amount: 5.into(), - gas_amount: 1., - gas_price: 1., - sell_token_price: 1., - order_kind: OrderKind::Sell, - expiration_timestamp: now, - onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), - }; + let quote = + { + let mut quote = Quote { + id: Default::default(), + sell_token: token_a, + buy_token: ByteArray([3; 20]), + sell_amount: 4.into(), + buy_amount: 5.into(), + gas_amount: 1., + gas_price: 1., + sell_token_price: 1., + order_kind: OrderKind::Sell, + expiration_timestamp: now, + onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), + }; + let id = save(&mut db, "e).await.unwrap(); + quote.id = id; + quote + }; // Token A has readings valid until now and in 30s let mut search_a = QuoteSearchParameters { - sell_token: quote_a.sell_token, - buy_token: quote_a.buy_token, - sell_amount_0: quote_a.sell_amount.clone(), - sell_amount_1: quote_a.sell_amount.clone(), - buy_amount: 1.into(), - kind: quote_a.order_kind, - expiration: now, - onchain_signing_scheme: quote_a.onchain_signing_scheme.clone(), + sell_token: quote.sell_token, + buy_token: quote.buy_token, + sell_amount_0: quote.sell_amount.clone(), + sell_amount_1: quote.sell_amount.clone(), + buy_amount: quote.buy_amount.clone(), + kind: quote.order_kind, + expiration: quote.expiration_timestamp, + onchain_signing_scheme: quote.onchain_signing_scheme.clone(), }; - assert_eq!(find(&mut db, &search_a).await.unwrap().unwrap(), quote_a,); + assert_eq!(find(&mut db, &search_a).await.unwrap().unwrap(), quote,); search_a.onchain_signing_scheme = None; assert_eq!(find(&mut db, &search_a).await.unwrap(), None,); } diff --git a/database/sql/V027__quotes_add_field_expiration_for_api_call.sql b/database/sql/V027__quotes_adding_onchain_signing_scheme.sql similarity index 78% rename from database/sql/V027__quotes_add_field_expiration_for_api_call.sql rename to database/sql/V027__quotes_adding_onchain_signing_scheme.sql index 4926baf231..e6d69bc8a5 100644 --- a/database/sql/V027__quotes_add_field_expiration_for_api_call.sql +++ b/database/sql/V027__quotes_adding_onchain_signing_scheme.sql @@ -3,4 +3,4 @@ CREATE TYPE OnchainSigningScheme AS ENUM ('eip1271', 'presign'); ALTER TABLE quotes -ADD COLUMN onchain_signing_sheme OnchainSigningScheme; +ADD COLUMN onchain_signing_scheme OnchainSigningScheme DEFAULT NULL; From 5a06118a31e8afe18c9b08b330b62ec6c7fa7aad Mon Sep 17 00:00:00 2001 From: josojo Date: Fri, 5 Aug 2022 12:50:17 +0200 Subject: [PATCH 4/8] cargo fmt --- crates/database/src/quotes.rs | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/crates/database/src/quotes.rs b/crates/database/src/quotes.rs index 501a845edd..7701751756 100644 --- a/crates/database/src/quotes.rs +++ b/crates/database/src/quotes.rs @@ -374,25 +374,24 @@ mod tests { let now = low_precision_now(); let token_a = ByteArray([1; 20]); - let quote = - { - let mut quote = Quote { - id: Default::default(), - sell_token: token_a, - buy_token: ByteArray([3; 20]), - sell_amount: 4.into(), - buy_amount: 5.into(), - gas_amount: 1., - gas_price: 1., - sell_token_price: 1., - order_kind: OrderKind::Sell, - expiration_timestamp: now, - onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), - }; - let id = save(&mut db, "e).await.unwrap(); - quote.id = id; - quote + let quote = { + let mut quote = Quote { + id: Default::default(), + sell_token: token_a, + buy_token: ByteArray([3; 20]), + sell_amount: 4.into(), + buy_amount: 5.into(), + gas_amount: 1., + gas_price: 1., + sell_token_price: 1., + order_kind: OrderKind::Sell, + expiration_timestamp: now, + onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), }; + let id = save(&mut db, "e).await.unwrap(); + quote.id = id; + quote + }; // Token A has readings valid until now and in 30s let mut search_a = QuoteSearchParameters { sell_token: quote.sell_token, From 36f0a4ffb6ac3bf9db7fa4d41bb7c8b88a656e1f Mon Sep 17 00:00:00 2001 From: josojo Date: Sun, 7 Aug 2022 14:04:59 +0200 Subject: [PATCH 5/8] valentins suggestion --- crates/orderbook/src/order_validation.rs | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/crates/orderbook/src/order_validation.rs b/crates/orderbook/src/order_validation.rs index b657d8c80d..d29cb18aba 100644 --- a/crates/orderbook/src/order_validation.rs +++ b/crates/orderbook/src/order_validation.rs @@ -589,19 +589,15 @@ fn convert_signing_scheme_into_quote_signing_scheme( scheme: SigningScheme, order_placement_via_api: bool, ) -> Result { - match order_placement_via_api { - true => match scheme { - SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSign), - SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271), - SigningScheme::Eip712 => Ok(QuoteSigningScheme::Eip712), - SigningScheme::EthSign => Ok(QuoteSigningScheme::EthSign), - }, - false => match scheme { - SigningScheme::PreSign => Ok(QuoteSigningScheme::PreSignForOnchainOrder), - SigningScheme::Eip1271 => Ok(QuoteSigningScheme::Eip1271ForOnchainOrder), - SigningScheme::Eip712 => Err(ValidationError::IncompatibleSigningScheme), - SigningScheme::EthSign => Err(ValidationError::IncompatibleSigningScheme), - }, + match (order_placement_via_api, scheme) { + (true, SigningScheme::PreSign) => Ok(QuoteSigningScheme::PreSign), + (true, SigningScheme::Eip1271) => Ok(QuoteSigningScheme::Eip1271), + (true, SigningScheme::Eip712) => Ok(QuoteSigningScheme::Eip712), + (true, SigningScheme::EthSign) => Ok(QuoteSigningScheme::EthSign), + (false, SigningScheme::PreSign) => Ok(QuoteSigningScheme::PreSignForOnchainOrder), + (false, SigningScheme::Eip1271) => Ok(QuoteSigningScheme::Eip1271ForOnchainOrder), + (false, SigningScheme::Eip712) => Err(ValidationError::IncompatibleSigningScheme), + (false, SigningScheme::EthSign) => Err(ValidationError::IncompatibleSigningScheme), } } From 8fc2a86a94adf415be7af7e7b99c5b206a4edf51 Mon Sep 17 00:00:00 2001 From: josojo Date: Tue, 9 Aug 2022 10:52:01 +0200 Subject: [PATCH 6/8] Nick's nice suggestion for backwards compability --- crates/model/src/quote.rs | 182 ++++++++++++++++++++++- crates/model/src/signature.rs | 6 +- crates/orderbook/openapi.yml | 14 +- crates/orderbook/src/api/post_quote.rs | 4 +- crates/orderbook/src/order_quoting.rs | 20 ++- crates/orderbook/src/order_validation.rs | 10 +- 6 files changed, 205 insertions(+), 31 deletions(-) diff --git a/crates/model/src/quote.rs b/crates/model/src/quote.rs index 8e5677e0fa..658f2f814c 100644 --- a/crates/model/src/quote.rs +++ b/crates/model/src/quote.rs @@ -1,8 +1,10 @@ use crate::{ app_id::AppId, order::{BuyTokenDestination, OrderKind, SellTokenSource}, + signature::SigningScheme, time, u256_decimal, }; +use anyhow::bail; use chrono::{DateTime, Utc}; use primitive_types::{H160, U256}; use serde::{de, ser::SerializeStruct as _, Deserialize, Deserializer, Serialize, Serializer}; @@ -16,17 +18,48 @@ pub enum PriceQuality { } #[derive(Eq, PartialEq, Clone, Copy, Debug, Default, Deserialize, Serialize, Hash)] -#[serde(rename_all = "lowercase")] +#[serde( + rename_all = "lowercase", + tag = "signingScheme", + try_from = "QuoteSigningDeserializationData" +)] pub enum QuoteSigningScheme { #[default] Eip712, EthSign, - /// EIP1271 orders can be created via API-call or in the future via on-chain orders - Eip1271, - Eip1271ForOnchainOrder, - /// PreSign orders can be created via API-call or in the future via on-chain orders - PreSign, - PreSignForOnchainOrder, + Eip1271 { + #[serde(rename = "onchainOrder")] + onchain_order: bool, + }, + PreSign { + #[serde(rename = "onchainOrder")] + onchain_order: bool, + }, +} + +#[derive(Debug, Deserialize)] +#[serde(rename_all = "camelCase")] +struct QuoteSigningDeserializationData { + #[serde(default)] + signing_scheme: SigningScheme, + #[serde(default)] + onchain_order: bool, +} + +impl TryFrom for QuoteSigningScheme { + type Error = anyhow::Error; + + fn try_from(data: QuoteSigningDeserializationData) -> Result { + match (data.signing_scheme, data.onchain_order) { + (scheme, true) if scheme.is_ecdsa_scheme() => { + bail!("ECDSA-signed orders cannot be on-chain") + } + (SigningScheme::Eip712, _) => Ok(Self::Eip712), + (SigningScheme::EthSign, _) => Ok(Self::EthSign), + (SigningScheme::Eip1271, onchain_order) => Ok(Self::Eip1271 { onchain_order }), + (SigningScheme::PreSign, onchain_order) => Ok(Self::PreSign { onchain_order }), + } + } } /// The order parameters to quote a price and fee for. @@ -50,7 +83,7 @@ pub struct OrderQuoteRequest { pub sell_token_balance: SellTokenSource, #[serde(default)] pub buy_token_balance: BuyTokenDestination, - #[serde(default)] + #[serde(flatten)] pub signing_scheme: QuoteSigningScheme, #[serde(default)] pub price_quality: PriceQuality, @@ -226,4 +259,137 @@ mod tests { }) ); } + + #[test] + fn deserialize_quote_requests() { + let valid_json = [ + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "eip712", + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "ethsign", + "onchainOrder": false, + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "eip1271", + "onchainOrder": true, + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "eip1271", + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "presign", + "onchainOrder": true, + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "presign", + }), + ]; + let expected_standard_response = OrderQuoteRequest { + sell_token: H160::from_low_u64_be(1), + buy_token: H160::from_low_u64_be(2), + ..Default::default() + }; + let modify_signing_scheme = |signing_scheme: QuoteSigningScheme| { + let mut response = expected_standard_response; + response.signing_scheme = signing_scheme; + response + }; + let expected_quote_responses = vec![ + expected_standard_response, + expected_standard_response, + modify_signing_scheme(QuoteSigningScheme::EthSign), + modify_signing_scheme(QuoteSigningScheme::Eip1271 { + onchain_order: true, + }), + modify_signing_scheme(QuoteSigningScheme::Eip1271 { + onchain_order: false, + }), + modify_signing_scheme(QuoteSigningScheme::PreSign { + onchain_order: true, + }), + modify_signing_scheme(QuoteSigningScheme::PreSign { + onchain_order: false, + }), + ]; + for (i, json) in valid_json.iter().enumerate() { + assert_eq!( + serde_json::from_value::(json.clone()).unwrap(), + *expected_quote_responses.get(i).unwrap() + ); + } + let invalid_json = vec![ + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "onchainOrder": true, + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "eip712", + "onchainOrder": true, + }), + json!({ + "from": "0x0000000000000000000000000000000000000000", + "sellToken": "0x0000000000000000000000000000000000000001", + "buyToken": "0x0000000000000000000000000000000000000002", + "kind": "buy", + "buyAmountAfterFee": "1", + "signingScheme": "ethsign", + "onchainOrder": true, + }), + ]; + for json in invalid_json.iter() { + assert_eq!( + serde_json::from_value::(json.clone()) + .unwrap_err() + .to_string(), + String::from("ECDSA-signed orders cannot be on-chain") + ); + } + } } diff --git a/crates/model/src/signature.rs b/crates/model/src/signature.rs index 7a2a80e959..edf2d09ff4 100644 --- a/crates/model/src/signature.rs +++ b/crates/model/src/signature.rs @@ -25,10 +25,8 @@ impl From for SigningScheme { fn from(scheme: QuoteSigningScheme) -> Self { match scheme { QuoteSigningScheme::Eip712 => SigningScheme::Eip712, - QuoteSigningScheme::Eip1271 => SigningScheme::Eip1271, - QuoteSigningScheme::Eip1271ForOnchainOrder => SigningScheme::Eip1271, - QuoteSigningScheme::PreSign => SigningScheme::PreSign, - QuoteSigningScheme::PreSignForOnchainOrder => SigningScheme::PreSign, + QuoteSigningScheme::Eip1271 { .. } => SigningScheme::Eip1271, + QuoteSigningScheme::PreSign { .. } => SigningScheme::PreSign, QuoteSigningScheme::EthSign => SigningScheme::EthSign, } } diff --git a/crates/orderbook/openapi.yml b/crates/orderbook/openapi.yml index 37474878d7..3984c959b4 100644 --- a/crates/orderbook/openapi.yml +++ b/crates/orderbook/openapi.yml @@ -623,7 +623,7 @@ components: $ref: "#/components/schemas/BuyTokenDestination" default: "erc20" signingScheme: - $ref: "#/components/schemas/QuoteSigningScheme" + $ref: "#/components/schemas/SigningScheme" default: "eip712" required: - sellToken @@ -836,11 +836,7 @@ components: SigningScheme: description: How was the order signed? type: string - enum: [eip712, ethsign, presign] - QuoteSigningScheme: - description: How will the order be signed and is it intended for onchain order placement? - type: string - enum: [eip712, ethsign, presign, eip1271, eip1271asonchainorder, presignasonchainorder] + enum: [eip712, ethsign, presign, eip1271] EcdsaSigningScheme: description: How was the order signed? type: string @@ -1071,6 +1067,12 @@ components: priceQuality: $ref: "#/components/schemas/PriceQuality" default: "optimal" + signingScheme: + $ref: "#/components/schemas/SigningScheme" + default: "eip712" + onchainOrder: + description: "Flag to signal whether the order is intended for onchain order placement. Only valid for non ECDSA-signed orders" + default: false required: - sellToken - buyToken diff --git a/crates/orderbook/src/api/post_quote.rs b/crates/orderbook/src/api/post_quote.rs index 6bf181862d..6d40b5d11e 100644 --- a/crates/orderbook/src/api/post_quote.rs +++ b/crates/orderbook/src/api/post_quote.rs @@ -101,7 +101,9 @@ mod tests { partially_fillable: false, sell_token_balance: SellTokenSource::Erc20, buy_token_balance: BuyTokenDestination::Internal, - signing_scheme: QuoteSigningScheme::PreSign, + signing_scheme: QuoteSigningScheme::PreSign { + onchain_order: false + }, price_quality: PriceQuality::Optimal, } ); diff --git a/crates/orderbook/src/order_quoting.rs b/crates/orderbook/src/order_quoting.rs index 9a0639365c..0243a0ce9e 100644 --- a/crates/orderbook/src/order_quoting.rs +++ b/crates/orderbook/src/order_quoting.rs @@ -430,12 +430,12 @@ impl OrderQuoter { parameters: &QuoteParameters, ) -> Result { let expiration = match parameters.signing_scheme { - QuoteSigningScheme::Eip1271ForOnchainOrder => { - self.now.now() + self.eip1271_onchain_quote_validity_seconds - } - QuoteSigningScheme::PreSignForOnchainOrder => { - self.now.now() + self.presign_onchain_quote_validity_seconds - } + QuoteSigningScheme::Eip1271 { + onchain_order: true, + } => self.now.now() + self.eip1271_onchain_quote_validity_seconds, + QuoteSigningScheme::PreSign { + onchain_order: true, + } => self.now.now() + self.presign_onchain_quote_validity_seconds, _ => self.now.now() + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), }; @@ -470,8 +470,12 @@ impl OrderQuoter { }; let onchain_signing_scheme = match parameters.signing_scheme { - QuoteSigningScheme::Eip1271ForOnchainOrder => Some(OnchainSigningScheme::Eip1271), - QuoteSigningScheme::PreSignForOnchainOrder => Some(OnchainSigningScheme::PreSign), + QuoteSigningScheme::Eip1271 { + onchain_order: true, + } => Some(OnchainSigningScheme::Eip1271), + QuoteSigningScheme::PreSign { + onchain_order: true, + } => Some(OnchainSigningScheme::PreSign), _ => None, }; diff --git a/crates/orderbook/src/order_validation.rs b/crates/orderbook/src/order_validation.rs index d29cb18aba..d90e14837a 100644 --- a/crates/orderbook/src/order_validation.rs +++ b/crates/orderbook/src/order_validation.rs @@ -590,14 +590,16 @@ fn convert_signing_scheme_into_quote_signing_scheme( order_placement_via_api: bool, ) -> Result { match (order_placement_via_api, scheme) { - (true, SigningScheme::PreSign) => Ok(QuoteSigningScheme::PreSign), - (true, SigningScheme::Eip1271) => Ok(QuoteSigningScheme::Eip1271), (true, SigningScheme::Eip712) => Ok(QuoteSigningScheme::Eip712), (true, SigningScheme::EthSign) => Ok(QuoteSigningScheme::EthSign), - (false, SigningScheme::PreSign) => Ok(QuoteSigningScheme::PreSignForOnchainOrder), - (false, SigningScheme::Eip1271) => Ok(QuoteSigningScheme::Eip1271ForOnchainOrder), (false, SigningScheme::Eip712) => Err(ValidationError::IncompatibleSigningScheme), (false, SigningScheme::EthSign) => Err(ValidationError::IncompatibleSigningScheme), + (order_placement_via_api, SigningScheme::PreSign) => Ok(QuoteSigningScheme::PreSign { + onchain_order: !order_placement_via_api, + }), + (order_placement_via_api, SigningScheme::Eip1271) => Ok(QuoteSigningScheme::Eip1271 { + onchain_order: !order_placement_via_api, + }), } } From 673f75e77deb97ceb6dddfbe57824ccc53c297f6 Mon Sep 17 00:00:00 2001 From: josojo Date: Tue, 9 Aug 2022 18:22:37 +0200 Subject: [PATCH 7/8] Federico's suggestion --- crates/database/src/quotes.rs | 40 ++++++------- crates/orderbook/src/database/quotes.rs | 6 +- crates/orderbook/src/order_quoting.rs | 56 +++++++++---------- crates/orderbook/src/order_validation.rs | 22 ++++---- ...__quotes_adding_onchain_signing_scheme.sql | 6 -- 5 files changed, 61 insertions(+), 69 deletions(-) delete mode 100644 database/sql/V027__quotes_adding_onchain_signing_scheme.sql diff --git a/crates/database/src/quotes.rs b/crates/database/src/quotes.rs index 7701751756..9b17cb469f 100644 --- a/crates/database/src/quotes.rs +++ b/crates/database/src/quotes.rs @@ -7,12 +7,14 @@ use sqlx::{ pub type QuoteId = i64; -#[derive(Clone, Debug, PartialEq, sqlx::Type)] -#[sqlx(type_name = "OnchainSigningScheme")] +#[derive(Clone, Debug, Default, PartialEq, sqlx::Type)] +#[sqlx(type_name = "QuoteKind")] #[sqlx(rename_all = "lowercase")] -pub enum OnchainSigningScheme { - Eip1271, - PreSign, +pub enum QuoteKind { + #[default] + Standard, + Eip1271OnchainOrder, + PreSignOnchainOrder, } /// One row in the `quotes` table. @@ -28,7 +30,7 @@ pub struct Quote { pub sell_token_price: f64, pub order_kind: OrderKind, pub expiration_timestamp: DateTime, - pub onchain_signing_scheme: Option, + pub quote_kind: QuoteKind, } /// Stores the quote and returns the id. The id of the quote parameter is not used. @@ -44,7 +46,7 @@ INSERT INTO quotes ( sell_token_price, order_kind, expiration_timestamp, - onchain_signing_scheme + quote_kind ) VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9, $10) RETURNING id @@ -59,7 +61,7 @@ RETURNING id .bind(quote.sell_token_price) .bind(quote.order_kind) .bind(quote.expiration_timestamp) - .bind("e.onchain_signing_scheme) + .bind("e.quote_kind) .fetch_one(ex) .await?; Ok(id) @@ -84,7 +86,7 @@ pub struct QuoteSearchParameters { pub buy_amount: BigDecimal, pub kind: OrderKind, pub expiration: DateTime, - pub onchain_signing_scheme: Option, + pub quote_kind: QuoteKind, } pub async fn find( @@ -104,7 +106,7 @@ WHERE ) AND order_kind = $6 AND expiration_timestamp >= $7 AND - onchain_signing_scheme IS NOT DISTINCT FROM $8 + quote_kind = $8 ORDER BY gas_amount * gas_price * sell_token_price ASC LIMIT 1 "#; @@ -116,7 +118,7 @@ LIMIT 1 .bind(¶ms.buy_amount) .bind(params.kind) .bind(params.expiration) - .bind(¶ms.onchain_signing_scheme) + .bind(¶ms.quote_kind) .fetch_optional(ex) .await } @@ -169,7 +171,7 @@ mod tests { sell_token_price: 7., order_kind: OrderKind::Sell, expiration_timestamp: now, - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; let id = save(&mut db, "e).await.unwrap(); quote.id = id; @@ -201,7 +203,7 @@ mod tests { gas_price: 1., sell_token_price: 1., expiration_timestamp: now, - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; let token_b = ByteArray([2; 20]); @@ -216,7 +218,7 @@ mod tests { gas_price: 1., sell_token_price: 1., expiration_timestamp: now, - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; // Save two measurements for token_a @@ -264,7 +266,7 @@ mod tests { buy_amount: 1.into(), kind: quote_a.order_kind, expiration: now, - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; assert_eq!( find(&mut db, &search_a).await.unwrap().unwrap(), @@ -324,7 +326,7 @@ mod tests { buy_amount: quote_b.buy_amount, kind: quote_b.order_kind, expiration: now, - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; assert_eq!( find(&mut db, &search_b).await.unwrap().unwrap(), @@ -386,7 +388,7 @@ mod tests { sell_token_price: 1., order_kind: OrderKind::Sell, expiration_timestamp: now, - onchain_signing_scheme: Some(OnchainSigningScheme::Eip1271), + quote_kind: QuoteKind::Eip1271OnchainOrder, }; let id = save(&mut db, "e).await.unwrap(); quote.id = id; @@ -401,11 +403,11 @@ mod tests { buy_amount: quote.buy_amount.clone(), kind: quote.order_kind, expiration: quote.expiration_timestamp, - onchain_signing_scheme: quote.onchain_signing_scheme.clone(), + quote_kind: quote.quote_kind.clone(), }; assert_eq!(find(&mut db, &search_a).await.unwrap().unwrap(), quote,); - search_a.onchain_signing_scheme = None; + search_a.quote_kind = QuoteKind::Standard; assert_eq!(find(&mut db, &search_a).await.unwrap(), None,); } } diff --git a/crates/orderbook/src/database/quotes.rs b/crates/orderbook/src/database/quotes.rs index 32e986e071..6f9e5a3f60 100644 --- a/crates/orderbook/src/database/quotes.rs +++ b/crates/orderbook/src/database/quotes.rs @@ -34,7 +34,7 @@ impl TryFrom for QuoteData { }, kind: order_kind_from(row.order_kind), expiration: row.expiration_timestamp, - onchain_signing_scheme: row.onchain_signing_scheme, + quote_kind: row.quote_kind, }) } } @@ -59,7 +59,7 @@ impl QuoteStoring for Postgres { sell_token_price: data.fee_parameters.sell_token_price, order_kind: order_kind_into(data.kind), expiration_timestamp: data.expiration, - onchain_signing_scheme: data.onchain_signing_scheme, + quote_kind: data.quote_kind, }; let id = database::quotes::save(&mut ex, &row).await?; Ok(Some(id)) @@ -95,7 +95,7 @@ impl QuoteStoring for Postgres { buy_amount: u256_to_big_decimal(¶ms.buy_amount), kind: order_kind_into(params.kind), expiration, - onchain_signing_scheme: params.onchain_signing_scheme, + quote_kind: params.quote_kind, }; let quote = database::quotes::find(&mut ex, ¶ms) .await diff --git a/crates/orderbook/src/order_quoting.rs b/crates/orderbook/src/order_quoting.rs index 0243a0ce9e..2721afc31e 100644 --- a/crates/orderbook/src/order_quoting.rs +++ b/crates/orderbook/src/order_quoting.rs @@ -4,7 +4,7 @@ use crate::{ }; use anyhow::Result; use chrono::{DateTime, Duration, TimeZone as _, Utc}; -use database::quotes::OnchainSigningScheme; +use database::quotes::QuoteKind; use ethcontract::{H160, U256}; use futures::TryFutureExt as _; use gas_estimation::GasPriceEstimating; @@ -225,11 +225,9 @@ pub struct QuoteData { pub fee_parameters: FeeParameters, pub kind: OrderKind, pub expiration: DateTime, - /// Since different on-chain signing schemes have different expirations, - /// we need to store the onchain_signing_scheme to prevent missuse of quotes. - /// Since all off-chain orders have the same validity, no differentiation - /// needs to be stored and we can set the value to None - pub onchain_signing_scheme: Option, + /// Since different quote kinds have different expirations, + /// we need to store the quote kind to prevent missuse of quotes. + pub quote_kind: QuoteKind, } impl Default for QuoteData { @@ -242,7 +240,7 @@ impl Default for QuoteData { fee_parameters: Default::default(), kind: Default::default(), expiration: Utc.timestamp(0, 0), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, } } } @@ -302,7 +300,7 @@ pub struct QuoteSearchParameters { pub kind: OrderKind, pub from: H160, pub app_data: AppId, - pub onchain_signing_scheme: Option, + pub quote_kind: QuoteKind, } impl QuoteSearchParameters { @@ -388,7 +386,7 @@ impl Now for DateTime { } } -/// Standard validity for a quote: Quotes are stored only as long as their validity is. +/// Standard validity for a quote: Quotes are stored only as long as they are valid. const STANDARD_QUOTE_VALIDITY_SECONDS: i64 = 60; /// An order quoter implementation that relies @@ -469,14 +467,14 @@ impl OrderQuoter { sell_token_price, }; - let onchain_signing_scheme = match parameters.signing_scheme { + let quote_kind = match parameters.signing_scheme { QuoteSigningScheme::Eip1271 { onchain_order: true, - } => Some(OnchainSigningScheme::Eip1271), + } => QuoteKind::Eip1271OnchainOrder, QuoteSigningScheme::PreSign { onchain_order: true, - } => Some(OnchainSigningScheme::PreSign), - _ => None, + } => QuoteKind::PreSignOnchainOrder, + _ => QuoteKind::Standard, }; let quote = QuoteData { @@ -487,7 +485,7 @@ impl OrderQuoter { fee_parameters, kind: trade_query.kind, expiration, - onchain_signing_scheme, + quote_kind, }; Ok(quote) @@ -750,7 +748,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, })) .returning(|_| Ok(Some(1337))); @@ -781,7 +779,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 70.into(), buy_amount: 29.into(), @@ -862,7 +860,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, })) .returning(|_| Ok(Some(1337))); @@ -896,7 +894,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -977,7 +975,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, })) .returning(|_| Ok(Some(1337))); @@ -1012,7 +1010,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(STANDARD_QUOTE_VALIDITY_SECONDS), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -1201,7 +1199,7 @@ mod tests { kind: OrderKind::Sell, from: H160([3; 20]), app_data: AppId([4; 32]), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; let mut storage = MockQuoteStoring::new(); @@ -1218,7 +1216,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, })) }); @@ -1252,7 +1250,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 85.into(), // Allows for "out-of-price" buy amounts. This means that order @@ -1281,7 +1279,7 @@ mod tests { kind: OrderKind::Sell, from: H160([3; 20]), app_data: AppId([4; 32]), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; let mut storage = MockQuoteStoring::new(); @@ -1298,7 +1296,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, })) }); @@ -1329,7 +1327,7 @@ mod tests { }, kind: OrderKind::Sell, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 100.into(), buy_amount: 42.into(), @@ -1350,7 +1348,7 @@ mod tests { kind: OrderKind::Buy, from: H160([3; 20]), app_data: AppId([4; 32]), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }; let mut storage = MockQuoteStoring::new(); @@ -1372,7 +1370,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, ))) }); @@ -1404,7 +1402,7 @@ mod tests { }, kind: OrderKind::Buy, expiration: now + chrono::Duration::seconds(10), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }, sell_amount: 100.into(), buy_amount: 42.into(), diff --git a/crates/orderbook/src/order_validation.rs b/crates/orderbook/src/order_validation.rs index d90e14837a..a2874a4085 100644 --- a/crates/orderbook/src/order_validation.rs +++ b/crates/orderbook/src/order_validation.rs @@ -4,7 +4,7 @@ use crate::order_quoting::{ }; use anyhow::anyhow; use contracts::WETH9; -use database::quotes::OnchainSigningScheme; +use database::quotes::QuoteKind; use ethcontract::{H160, U256}; use model::{ order::{ @@ -516,10 +516,8 @@ async fn get_quote_and_check_fee( order_placement_via_api: bool, owner: H160, ) -> Result { - let onchain_signing_scheme = convert_signing_scheme_into_onchain_signing_scheme( - order.signature.scheme(), - order_placement_via_api, - )?; + let quote_kind = + convert_signing_scheme_into_quote_kind(order.signature.scheme(), order_placement_via_api)?; let parameters = QuoteSearchParameters { sell_token: order.data.sell_token, buy_token: order.data.buy_token, @@ -529,7 +527,7 @@ async fn get_quote_and_check_fee( kind: order.data.kind, from: owner, app_data: order.data.app_data, - onchain_signing_scheme, + quote_kind, }; let quote = match quoter.find_quote(order.quote_id, parameters).await { @@ -603,15 +601,15 @@ fn convert_signing_scheme_into_quote_signing_scheme( } } -fn convert_signing_scheme_into_onchain_signing_scheme( +fn convert_signing_scheme_into_quote_kind( scheme: SigningScheme, order_placement_via_api: bool, -) -> Result, ValidationError> { +) -> Result { match order_placement_via_api { - true => Ok(None), + true => Ok(QuoteKind::Standard), false => match scheme { - SigningScheme::Eip1271 => Ok(Some(OnchainSigningScheme::Eip1271)), - SigningScheme::PreSign => Ok(Some(OnchainSigningScheme::PreSign)), + SigningScheme::Eip1271 => Ok(QuoteKind::Eip1271OnchainOrder), + SigningScheme::PreSign => Ok(QuoteKind::PreSignOnchainOrder), _ => Err(ValidationError::IncompatibleSigningScheme), }, } @@ -1430,7 +1428,7 @@ mod tests { kind: OrderKind::Buy, from: H160([0xf0; 20]), app_data: AppId([5; 32]), - onchain_signing_scheme: None, + quote_kind: QuoteKind::Standard, }), ) .returning(|_, _| { diff --git a/database/sql/V027__quotes_adding_onchain_signing_scheme.sql b/database/sql/V027__quotes_adding_onchain_signing_scheme.sql deleted file mode 100644 index e6d69bc8a5..0000000000 --- a/database/sql/V027__quotes_adding_onchain_signing_scheme.sql +++ /dev/null @@ -1,6 +0,0 @@ --- Adding new column on table quotes to differentiate the two on-chain signing schemes --- This is required as different signing schemes have different expirations -CREATE TYPE OnchainSigningScheme AS ENUM ('eip1271', 'presign'); - -ALTER TABLE quotes -ADD COLUMN onchain_signing_scheme OnchainSigningScheme DEFAULT NULL; From 3a7e5ff8b88375d0b5eef09c4e708f127b1d6e42 Mon Sep 17 00:00:00 2001 From: josojo Date: Tue, 9 Aug 2022 18:22:51 +0200 Subject: [PATCH 8/8] Federico's suggestion --- database/sql/V027__quotes_adding_quote_kind.sql | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 database/sql/V027__quotes_adding_quote_kind.sql diff --git a/database/sql/V027__quotes_adding_quote_kind.sql b/database/sql/V027__quotes_adding_quote_kind.sql new file mode 100644 index 0000000000..f4d39f5ead --- /dev/null +++ b/database/sql/V027__quotes_adding_quote_kind.sql @@ -0,0 +1,6 @@ +-- Adding new column on table quotes to differentiate between quoting methods +-- This is required as different quotes have different expirations +CREATE TYPE QuoteKind AS ENUM ('standard', 'eip1271onchainorder', 'presignonchainorder'); + +ALTER TABLE quotes +ADD COLUMN quote_kind QuoteKind Not NULL DEFAULT 'standard';