From 1968374cd12821b46a5b78a549c2fd0f8b9be013 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Mon, 5 Feb 2024 10:49:03 +0100 Subject: [PATCH 1/4] Reject orders with unknown preimages --- crates/e2e/tests/e2e/app_data.rs | 9 +++------ crates/shared/src/order_validation.rs | 8 +++++--- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/crates/e2e/tests/e2e/app_data.rs b/crates/e2e/tests/e2e/app_data.rs index f2ba729a1f..e8e8515d50 100644 --- a/crates/e2e/tests/e2e/app_data.rs +++ b/crates/e2e/tests/e2e/app_data.rs @@ -60,21 +60,18 @@ async fn app_data(web3: Web3) { let services = Services::new(onchain.contracts()).await; services.start_protocol(solver).await; - // Temporarily custom hashes are still accepted. + // Unknown hashes are not accepted. let order0 = create_order(OrderCreationAppData::Hash { hash: AppDataHash([1; 32]), }); - let uid = services.create_order(&order0).await.unwrap(); - let order0_ = services.get_order(&uid).await.unwrap(); - assert_eq!(order0_.data.app_data, AppDataHash([1; 32])); - assert_eq!(order0_.metadata.full_app_data, None); - let err = services .get_app_data(AppDataHash([1; 32])) .await .unwrap_err(); assert_eq!(err.0, StatusCode::NOT_FOUND); + assert!(services.create_order(&order0).await.is_err()); + // hash matches let app_data = "{}"; let app_data_hash = AppDataHash(app_data_hash::hash_full_app_data(app_data.as_bytes())); diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 7d0ccfc23b..5428c00506 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1,7 +1,7 @@ use { crate::{ account_balances::{self, BalanceFetching, TransferSimulationError}, - app_data::{ProtocolAppData, ValidatedAppData}, + app_data::ValidatedAppData, bad_token::{BadTokenDetecting, TokenQuality}, code_fetching::CodeFetching, order_quoting::{ @@ -483,8 +483,10 @@ impl OrderValidating for OrderValidator { let protocol = if let Some(full) = full_app_data_override { validate(full)?.protocol } else { - tracing::warn!(hash = hex::encode(hash.0), "Unknown appData pre-image"); - ProtocolAppData::default() + return Err(AppDataValidationError::Invalid(anyhow!( + "Unknown pre-image for app data hash {:?}", + hash, + ))); }; ValidatedAppData { From e51b289bf4c8fab6dec6e056abf0513ef0ebbb91 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Mon, 5 Feb 2024 19:32:36 +0100 Subject: [PATCH 2/4] fix tests --- crates/e2e/tests/e2e/app_data_signer.rs | 15 --------------- crates/shared/src/order_validation.rs | 3 +++ 2 files changed, 3 insertions(+), 15 deletions(-) diff --git a/crates/e2e/tests/e2e/app_data_signer.rs b/crates/e2e/tests/e2e/app_data_signer.rs index eb88015864..6315614b20 100644 --- a/crates/e2e/tests/e2e/app_data_signer.rs +++ b/crates/e2e/tests/e2e/app_data_signer.rs @@ -68,21 +68,6 @@ async fn order_creation_checks_metadata_signer(web3: Web3) { let services = Services::new(onchain.contracts()).await; services.start_protocol(solver).await; - // Accepted: custom hashes that aren't found in the DB. - let custom_hash_app_data = AppDataHash([1; 32]); - let order0 = sign( - create_order(OrderCreationAppData::Hash { - hash: custom_hash_app_data, - }), - &trader, - ); - assert!(matches!( - services.get_app_data(custom_hash_app_data).await, - Err((StatusCode::NOT_FOUND, ..)) - )); - let uid = services.create_order(&order0).await.unwrap(); - assert!(matches!(services.get_order(&uid).await, Ok(..))); - // Rejected: app data with different signer. let full_app_data = full_app_data_with_signer(adversary.address()); let order1 = sign(create_order(full_app_data), &trader); diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 5428c00506..1809b2aaa0 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1935,6 +1935,9 @@ mod tests { buy_token: H160::from_low_u64_be(2), buy_amount: 1.into(), fee_amount: 1.into(), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; From 119a90991ba12aaa7c086f3cbcc059ab9762a3d5 Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Tue, 6 Feb 2024 08:50:30 +0100 Subject: [PATCH 3/4] fix tests --- crates/e2e/tests/e2e/app_data_signer.rs | 2 -- crates/shared/src/order_validation.rs | 34 ++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/crates/e2e/tests/e2e/app_data_signer.rs b/crates/e2e/tests/e2e/app_data_signer.rs index 6315614b20..7983eac22e 100644 --- a/crates/e2e/tests/e2e/app_data_signer.rs +++ b/crates/e2e/tests/e2e/app_data_signer.rs @@ -5,11 +5,9 @@ use { }, ethcontract::{prelude::U256, H160}, model::{ - app_data::AppDataHash, order::{OrderCreation, OrderCreationAppData, OrderKind}, signature::EcdsaSigningScheme, }, - reqwest::StatusCode, secp256k1::SecretKey, shared::ethrpc::Web3, web3::signing::SecretKeyRef, diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 1809b2aaa0..7f70efdbb2 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -1265,6 +1265,9 @@ mod tests { sell_amount: U256::from(1), fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; validator @@ -1390,7 +1393,9 @@ mod tests { let creation_ = OrderCreation { fee_amount: U256::zero(), partially_fillable: true, - app_data: OrderCreationAppData::default(), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..creation }; let (order, quote) = validator @@ -1453,6 +1458,9 @@ mod tests { buy_amount: U256::from(1), sell_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let res = validator @@ -1509,6 +1517,9 @@ mod tests { sell_amount: U256::from(0), fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1565,6 +1576,9 @@ mod tests { fee_amount: U256::from(1), kind: OrderKind::Sell, signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let (order, quote) = validator @@ -1619,6 +1633,9 @@ mod tests { fee_amount: U256::from(1), from: Some(Default::default()), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1667,6 +1684,9 @@ mod tests { sell_amount: U256::from(1), fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1718,6 +1738,9 @@ mod tests { sell_amount: U256::from(1), fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1773,6 +1796,9 @@ mod tests { sell_amount: U256::MAX, fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1822,6 +1848,9 @@ mod tests { sell_amount: U256::from(1), fee_amount: U256::from(1), signature: Signature::Eip712(EcdsaSignature::non_zero()), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let result = validator @@ -1877,6 +1906,9 @@ mod tests { fee_amount: U256::from(1), from: Some(H160([1; 20])), signature: Signature::Eip1271(vec![1, 2, 3]), + app_data: OrderCreationAppData::Full { + full: "{}".to_string(), + }, ..Default::default() }; let domain = DomainSeparator::default(); From ea438aa3919885770f5b12635ceaae8b184da55e Mon Sep 17 00:00:00 2001 From: Felix Leupold Date: Tue, 6 Feb 2024 10:20:52 +0100 Subject: [PATCH 4/4] fix tests --- crates/e2e/tests/e2e/order_cancellation.rs | 7 +++++-- crates/e2e/tests/e2e/smart_contract_orders.rs | 5 ++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/crates/e2e/tests/e2e/order_cancellation.rs b/crates/e2e/tests/e2e/order_cancellation.rs index ce48ee67af..1bffc41b69 100644 --- a/crates/e2e/tests/e2e/order_cancellation.rs +++ b/crates/e2e/tests/e2e/order_cancellation.rs @@ -3,12 +3,12 @@ use { e2e::{setup::*, tx}, ethcontract::prelude::U256, model::{ - app_data::AppDataHash, order::{ CancellationPayload, OrderCancellation, OrderCancellations, OrderCreation, + OrderCreationAppData, OrderStatus, OrderUid, SignedOrderCancellations, @@ -18,6 +18,7 @@ use { }, number::nonzero::U256 as NonZeroU256, secp256k1::SecretKey, + serde_json::json, shared::ethrpc::Web3, web3::signing::SecretKeyRef, }; @@ -82,7 +83,9 @@ async fn order_cancellation(web3: Web3) { value: NonZeroU256::try_from(to_wei(1)).unwrap(), }, }, - app_data: AppDataHash([salt; 32]).into(), + app_data: OrderCreationAppData::Full { + full: json!({"salt": salt}).to_string(), + }, ..Default::default() }; async move { diff --git a/crates/e2e/tests/e2e/smart_contract_orders.rs b/crates/e2e/tests/e2e/smart_contract_orders.rs index 5a8d769777..30d000d192 100644 --- a/crates/e2e/tests/e2e/smart_contract_orders.rs +++ b/crates/e2e/tests/e2e/smart_contract_orders.rs @@ -2,7 +2,6 @@ use { e2e::setup::{safe::Safe, *}, ethcontract::{Bytes, H160, U256}, model::{ - app_data::AppDataHash, order::{OrderCreation, OrderCreationAppData, OrderKind, OrderStatus, OrderUid}, signature::Signature, }, @@ -73,8 +72,8 @@ async fn smart_contract_orders(web3: Web3) { ..order_template.clone() }, OrderCreation { - app_data: OrderCreationAppData::Hash { - hash: AppDataHash([1; 32]), + app_data: OrderCreationAppData::Full { + full: "{\"salt\": \"second\"}".to_string(), }, from: Some(safe.address()), signature: Signature::PreSign,