diff --git a/crates/configs/src/orderbook/mod.rs b/crates/configs/src/orderbook/mod.rs index 3f063ebfd6..91ef720c0d 100644 --- a/crates/configs/src/orderbook/mod.rs +++ b/crates/configs/src/orderbook/mod.rs @@ -354,6 +354,7 @@ mod tests { max_limit_orders_per_user: 5, max_gas_per_order: 6_000_000, same_tokens_policy: SameTokensPolicy::AllowSell, + skip_app_data_hash_verification: false, }, ipfs: Some(IpfsConfig { gateway: "https://gateway.pinata.cloud/ipfs/".parse().unwrap(), diff --git a/crates/configs/src/orderbook/order_validation.rs b/crates/configs/src/orderbook/order_validation.rs index f9244a8f0f..db746ea28c 100644 --- a/crates/configs/src/orderbook/order_validation.rs +++ b/crates/configs/src/orderbook/order_validation.rs @@ -75,6 +75,14 @@ pub struct OrderValidationConfig { /// Policy for orders where the buy and sell tokens are equal. #[serde(default)] pub same_tokens_policy: SameTokensPolicy, + + /// When enabled, orders that provide both the full app-data document and an + /// `appDataHash` are accepted even if the document does not hash to the + /// provided hash. The provided hash is still used as the order's app-data + /// hash (so the order signature keeps verifying); only the mismatch + /// rejection is skipped. + #[serde(default)] + pub skip_app_data_hash_verification: bool, } impl Default for OrderValidationConfig { @@ -86,6 +94,7 @@ impl Default for OrderValidationConfig { max_limit_orders_per_user: default_max_limit_orders_per_user(), max_gas_per_order: default_max_gas_per_order(), same_tokens_policy: Default::default(), + skip_app_data_hash_verification: false, } } } @@ -103,6 +112,7 @@ mod tests { config.max_limit_order_validity_period, Duration::from_secs(31_536_000) ); + assert!(!config.skip_app_data_hash_verification); } #[test] @@ -114,6 +124,7 @@ mod tests { max-limit-orders-per-user = 10 max-gas-per-order = 5000000 same-tokens-policy = "allow-sell" + skip-app-data-hash-verification = true "#; let config: OrderValidationConfig = toml::from_str(toml).unwrap(); assert_eq!(config.min_order_validity_period, Duration::from_secs(120)); @@ -125,6 +136,7 @@ mod tests { assert_eq!(config.max_limit_orders_per_user, 10); assert_eq!(config.max_gas_per_order, 5_000_000); assert_eq!(config.same_tokens_policy, SameTokensPolicy::AllowSell); + assert!(config.skip_app_data_hash_verification); } #[test] @@ -136,6 +148,7 @@ mod tests { max_limit_orders_per_user: 5, max_gas_per_order: 5_000_000, same_tokens_policy: SameTokensPolicy::AllowSell, + skip_app_data_hash_verification: true, }; let serialized = toml::to_string_pretty(&config).unwrap(); @@ -159,5 +172,9 @@ mod tests { ); assert_eq!(config.max_gas_per_order, deserialized.max_gas_per_order); assert_eq!(config.same_tokens_policy, deserialized.same_tokens_policy); + assert_eq!( + config.skip_app_data_hash_verification, + deserialized.skip_app_data_hash_verification + ); } } diff --git a/crates/orderbook/src/run.rs b/crates/orderbook/src/run.rs index b5afba27b4..26e4cff379 100644 --- a/crates/orderbook/src/run.rs +++ b/crates/orderbook/src/run.rs @@ -394,34 +394,39 @@ pub async fn run(config: Configuration) { } }); - let order_validator = Arc::new(OrderValidator::new( - native_token.clone(), - Arc::new(order_validation::banned::Users::new( - chainalysis_oracle, - config.banned_users.hermod.clone().map(|hermod| { - order_validation::banned::HermodConfig { - url: hermod.url, - hmac_key: hermod.hmac_key, - api_key: hermod.api_key, - } - }), - config.banned_users.addresses, - config.banned_users.max_cache_size.get().to_u64().unwrap(), - )), - validity_configuration, - config.eip1271_skip_creation_validation, - deny_listed_tokens.clone(), - hooks_contract, - optimal_quoter.clone(), - balance_fetcher, - signature_validator, - validator_simulator, - Arc::new(postgres_write.clone()), - config.order_validation.max_limit_orders_per_user, - app_data_validator.clone(), - config.order_validation.max_gas_per_order, - config.order_validation.same_tokens_policy, - )); + let order_validator = Arc::new( + OrderValidator::new( + native_token.clone(), + Arc::new(order_validation::banned::Users::new( + chainalysis_oracle, + config.banned_users.hermod.clone().map(|hermod| { + order_validation::banned::HermodConfig { + url: hermod.url, + hmac_key: hermod.hmac_key, + api_key: hermod.api_key, + } + }), + config.banned_users.addresses, + config.banned_users.max_cache_size.get().to_u64().unwrap(), + )), + validity_configuration, + config.eip1271_skip_creation_validation, + deny_listed_tokens.clone(), + hooks_contract, + optimal_quoter.clone(), + balance_fetcher, + signature_validator, + validator_simulator, + Arc::new(postgres_write.clone()), + config.order_validation.max_limit_orders_per_user, + app_data_validator.clone(), + config.order_validation.max_gas_per_order, + config.order_validation.same_tokens_policy, + ) + .with_skip_app_data_hash_verification( + config.order_validation.skip_app_data_hash_verification, + ), + ); let ipfs = config .ipfs .map(|ipfs| { diff --git a/crates/shared/src/order_validation.rs b/crates/shared/src/order_validation.rs index 4c0b1eb2a5..655e592d90 100644 --- a/crates/shared/src/order_validation.rs +++ b/crates/shared/src/order_validation.rs @@ -386,6 +386,9 @@ pub struct OrderValidator { app_data_validator: Validator, max_gas_per_order: u64, same_tokens_policy: SameTokensPolicy, + /// When set, orders providing both the full app-data and an `appDataHash` + /// are accepted even if the document does not hash to the provided hash. + skip_app_data_hash_verification: bool, } #[derive(Debug, Eq, PartialEq, Default)] @@ -473,9 +476,19 @@ impl OrderValidator { app_data_validator, max_gas_per_order, same_tokens_policy, + skip_app_data_hash_verification: false, } } + /// Configures whether orders that provide both the full app-data and an + /// `appDataHash` should be accepted even when the document does not hash to + /// the provided hash. The provided hash is still used as the order's + /// app-data hash so the order signature keeps verifying. + pub fn with_skip_app_data_hash_verification(mut self, skip: bool) -> Self { + self.skip_app_data_hash_verification = skip; + self + } + async fn check_max_limit_orders(&self, owner: Address) -> Result<(), ValidationError> { let num_limit_orders = self .limit_order_counter @@ -728,12 +741,28 @@ impl OrderValidating for OrderValidator { OrderCreationAppData::Both { full, expected } => { let validated = validate(full)?; if validated.hash != *expected { - return Err(AppDataValidationError::Mismatch { - provided: *expected, - actual: validated.hash, - }); + if !self.skip_app_data_hash_verification { + return Err(AppDataValidationError::Mismatch { + provided: *expected, + actual: validated.hash, + }); + } + tracing::warn!( + provided = ?expected, + actual = ?validated.hash, + "accepting app data with mismatched hash: hash verification disabled", + ); + // Keep the user-provided `expected` hash so the order + // signature (signed over `expected`) still verifies, while + // retaining the validated document for simulation/storage. + ValidatedAppData { + hash: *expected, + document: validated.document, + protocol: validated.protocol, + } + } else { + validated } - validated } OrderCreationAppData::Hash { hash } => { // Eventually we're not going to accept orders that set only a @@ -1337,6 +1366,81 @@ mod tests { assert!(!reparsed.interactions.pre.is_empty()); } + fn app_data_only_validator() -> OrderValidator { + let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().provider); + let mut limit_order_counter = MockLimitOrderCounting::new(); + limit_order_counter.expect_count().returning(|_| Ok(0u64)); + OrderValidator::new( + native_token, + Arc::new(order_validation::banned::Users::from_set(Default::default())), + OrderValidPeriodConfiguration::any(), + false, + DenyListedTokens::default(), + HooksTrampoline::Instance::new( + Address::repeat_byte(0xcf), + ProviderBuilder::new() + .connect_mocked_client(Asserter::new()) + .erased(), + ), + Arc::new(MockOrderQuoting::new()), + Arc::new(MockBalanceFetching::new()), + Arc::new(MockSignatureValidating::new()), + None, + Arc::new(limit_order_counter), + 0, + Default::default(), + u64::MAX, + SameTokensPolicy::Disallow, + ) + } + + #[test] + fn app_data_hash_mismatch_is_rejected_by_default() { + let validator = app_data_only_validator(); + let result = validator.validate_app_data( + &OrderCreationAppData::Both { + full: r#"{"version":"1.1.0","appCode":"test"}"#.to_string(), + expected: AppDataHash([0x11; 32]), + }, + &None, + ); + assert!(matches!( + result, + Err(AppDataValidationError::Mismatch { .. }) + )); + } + + #[test] + fn app_data_hash_mismatch_is_accepted_when_verification_disabled() { + let validator = app_data_only_validator().with_skip_app_data_hash_verification(true); + let full = r#"{"version":"1.1.0","appCode":"test"}"#.to_string(); + let expected = AppDataHash([0x11; 32]); + let app_data = validator + .validate_app_data( + &OrderCreationAppData::Both { + full: full.clone(), + expected, + }, + &None, + ) + .unwrap(); + // The user-provided hash is kept (so the signature keeps verifying)... + assert_eq!(app_data.inner.hash, expected); + // ...while the full document is retained for storage/simulation. + assert_eq!(app_data.inner.document, full); + } + + #[test] + fn app_data_matching_hash_still_accepted_when_verification_disabled() { + let validator = app_data_only_validator().with_skip_app_data_hash_verification(true); + let full = r#"{"version":"1.1.0","appCode":"test"}"#.to_string(); + let expected = OrderCreationAppData::Full { full: full.clone() }.hash(); + let app_data = validator + .validate_app_data(&OrderCreationAppData::Both { full, expected }, &None) + .unwrap(); + assert_eq!(app_data.inner.hash, expected); + } + #[tokio::test] async fn pre_validate_err() { let native_token = WETH9::Instance::new([0xef; 20].into(), ethrpc::mock::web3().provider);