Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reject orders with unknown preimages #2360

Merged
merged 5 commits into from
Feb 6, 2024
Merged

Reject orders with unknown preimages #2360

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1968374
Reject orders with unknown preimages
fleupold Feb 5, 2024
e51b289
fix tests
fleupold Feb 5, 2024
119a909
fix tests
fleupold Feb 6, 2024
ea438aa
fix tests
fleupold Feb 6, 2024
f698213
Merge branch 'main' into forbig_unknown_preimages
fleupold Feb 6, 2024
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
9 changes: 3 additions & 6 deletions crates/e2e/tests/e2e/app_data.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
17 changes: 0 additions & 17 deletions crates/e2e/tests/e2e/app_data_signer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -68,21 +66,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);
Expand Down
7 changes: 5 additions & 2 deletions crates/e2e/tests/e2e/order_cancellation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ use {
e2e::{setup::*, tx},
ethcontract::prelude::U256,
model::{
app_data::AppDataHash,
order::{
CancellationPayload,
OrderCancellation,
OrderCancellations,
OrderCreation,
OrderCreationAppData,
OrderStatus,
OrderUid,
SignedOrderCancellations,
Expand All @@ -18,6 +18,7 @@ use {
},
number::nonzero::U256 as NonZeroU256,
secp256k1::SecretKey,
serde_json::json,
shared::ethrpc::Web3,
web3::signing::SecretKeyRef,
};
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 2 additions & 3 deletions crates/e2e/tests/e2e/smart_contract_orders.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
Expand Down
45 changes: 41 additions & 4 deletions crates/shared/src/order_validation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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::{
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1263,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
Expand Down Expand Up @@ -1388,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
Expand Down Expand Up @@ -1451,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
Expand Down Expand Up @@ -1507,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
Expand Down Expand Up @@ -1563,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
Expand Down Expand Up @@ -1617,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
Expand Down Expand Up @@ -1665,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
Expand Down Expand Up @@ -1716,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
Expand Down Expand Up @@ -1771,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
Expand Down Expand Up @@ -1820,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
Expand Down Expand Up @@ -1875,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();
Expand Down Expand Up @@ -1933,6 +1967,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()
};

Expand Down
Loading