Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/configs/src/orderbook/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
17 changes: 17 additions & 0 deletions crates/configs/src/orderbook/order_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
}
}
}
Expand All @@ -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]
Expand All @@ -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));
Expand All @@ -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]
Expand All @@ -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();
Expand All @@ -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
);
}
}
61 changes: 33 additions & 28 deletions crates/orderbook/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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| {
Expand Down
114 changes: 109 additions & 5 deletions crates/shared/src/order_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
Loading