Skip to content

Commit

Permalink
Always enable limit orders (#2308)
Browse files Browse the repository at this point in the history
# Description
Limit orders are now a critical part of the service. Being able to
disable the feature is no longer useful.

# Changes
Delete dead code

## How to test
CI, tests should still pass
  • Loading branch information
MartinquaXD authored Jan 22, 2024
1 parent 34fa87f commit 789fdd3
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 135 deletions.
1 change: 0 additions & 1 deletion crates/e2e/src/setup/services.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,6 @@ impl<'a> Services<'a> {
let args = [
"orderbook".to_string(),
"--enable-custom-interactions=true".to_string(),
"--allow-placing-partially-fillable-limit-orders=true".to_string(),
format!(
"--hooks-contract-address={:?}",
self.contracts.hooks.address()
Expand Down
20 changes: 0 additions & 20 deletions crates/orderbook/src/arguments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,6 @@ pub struct Arguments {
#[clap(long, env, default_value = "24")]
pub solvable_orders_max_update_age_blocks: u64,

/// Note that fill or kill liquidity limit orders are always allowed.
#[clap(long, env, action = clap::ArgAction::Set, default_value = "true")]
pub allow_placing_fill_or_kill_limit_orders: bool,

/// Note that partially fillable liquidity limit orders are always allowed.
#[clap(long, env, action = clap::ArgAction::Set, default_value = "true")]
pub allow_placing_partially_fillable_limit_orders: bool,

/// Max number of limit orders per user.
#[clap(long, env, default_value = "10")]
pub max_limit_orders_per_user: u64,
Expand Down Expand Up @@ -183,8 +175,6 @@ impl std::fmt::Display for Arguments {
solvable_orders_max_update_age_blocks,
native_price_estimators,
fast_price_estimation_results_required,
allow_placing_fill_or_kill_limit_orders,
allow_placing_partially_fillable_limit_orders,
max_limit_orders_per_user,
enable_custom_interactions,
ipfs_gateway,
Expand Down Expand Up @@ -244,16 +234,6 @@ impl std::fmt::Display for Arguments {
"fast_price_estimation_results_required: {}",
fast_price_estimation_results_required
)?;
writeln!(
f,
"allow_placing_fill_or_kill_limit_orders: {}",
allow_placing_fill_or_kill_limit_orders
)?;
writeln!(
f,
"allow_placing_partially_fillable_limit_orders: {}",
allow_placing_partially_fillable_limit_orders
)?;
writeln!(
f,
"max_limit_orders_per_user: {}",
Expand Down
2 changes: 0 additions & 2 deletions crates/orderbook/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,6 @@ pub async fn run(args: Arguments) {
Arc::new(CachedCodeFetcher::new(Arc::new(web3.clone()))),
app_data_validator.clone(),
)
.with_fill_or_kill_limit_orders(args.allow_placing_fill_or_kill_limit_orders)
.with_partially_fillable_limit_orders(args.allow_placing_partially_fillable_limit_orders)
.with_eth_smart_contract_payments(args.enable_eth_smart_contract_payments)
.with_custom_interactions(args.enable_custom_interactions)
.with_verified_quotes(args.price_estimation.trade_simulator.is_some()),
Expand Down
130 changes: 18 additions & 112 deletions crates/shared/src/order_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,8 +249,6 @@ pub struct OrderValidator {
quoter: Arc<dyn OrderQuoting>,
balance_fetcher: Arc<dyn BalanceFetching>,
signature_validator: Arc<dyn SignatureValidating>,
enable_fill_or_kill_limit_orders: bool,
enable_partially_fillable_limit_orders: bool,
limit_order_counter: Arc<dyn LimitOrderCounting>,
max_limit_orders_per_user: u64,
pub code_fetcher: Arc<dyn CodeFetching>,
Expand Down Expand Up @@ -343,8 +341,6 @@ impl OrderValidator {
quoter,
balance_fetcher,
signature_validator,
enable_fill_or_kill_limit_orders: false,
enable_partially_fillable_limit_orders: false,
limit_order_counter,
max_limit_orders_per_user,
code_fetcher,
Expand All @@ -355,16 +351,6 @@ impl OrderValidator {
}
}

pub fn with_fill_or_kill_limit_orders(mut self, enable: bool) -> Self {
self.enable_fill_or_kill_limit_orders = enable;
self
}

pub fn with_partially_fillable_limit_orders(mut self, enable: bool) -> Self {
self.enable_partially_fillable_limit_orders = enable;
self
}

pub fn with_eth_smart_contract_payments(mut self, enable: bool) -> Self {
self.enable_eth_smart_contract_payments = enable;
self
Expand Down Expand Up @@ -442,21 +428,8 @@ impl OrderValidating for OrderValidator {
return Err(PartialValidationError::Forbidden);
}

match order.class {
OrderClass::Market => {
if order.partially_fillable {
return Err(PartialValidationError::UnsupportedOrderType);
}
}
OrderClass::Limit => {
if order.partially_fillable && !self.enable_partially_fillable_limit_orders {
return Err(PartialValidationError::UnsupportedOrderType);
}
if !order.partially_fillable && !self.enable_fill_or_kill_limit_orders {
return Err(PartialValidationError::UnsupportedOrderType);
}
}
OrderClass::Liquidity => (),
if order.class == OrderClass::Market && order.partially_fillable {
return Err(PartialValidationError::UnsupportedOrderType);
}

if order.buy_token_balance != BuyTokenDestination::Erc20 {
Expand Down Expand Up @@ -1144,23 +1117,20 @@ mod tests {
OrderValidToError::Excessive,
))
));
{
let validator = validator.clone().with_fill_or_kill_limit_orders(true);
assert!(matches!(
validator
.partial_validate(PreOrderData {
valid_to: legit_valid_to
+ validity_configuration.max_limit.as_secs() as u32
+ 1,
class: OrderClass::Limit,
..Default::default()
})
.await,
Err(PartialValidationError::ValidTo(
OrderValidToError::Excessive,
))
));
}
assert!(matches!(
validator
.partial_validate(PreOrderData {
valid_to: legit_valid_to
+ validity_configuration.max_limit.as_secs() as u32
+ 1,
class: OrderClass::Limit,
..Default::default()
})
.await,
Err(PartialValidationError::ValidTo(
OrderValidToError::Excessive,
))
));
assert!(matches!(
validator
.partial_validate(PreOrderData {
Expand Down Expand Up @@ -1220,9 +1190,7 @@ mod tests {
0,
Arc::new(MockCodeFetching::new()),
Default::default(),
)
.with_fill_or_kill_limit_orders(true)
.with_partially_fillable_limit_orders(true);
);
let order = || PreOrderData {
valid_to: time::now_in_epoch_seconds()
+ validity_configuration.min.as_secs() as u32
Expand Down Expand Up @@ -1437,7 +1405,6 @@ mod tests {
fee_amount: U256::zero(),
..creation.clone()
};
let validator = validator.with_fill_or_kill_limit_orders(true);
let (order, quote) = validator
.validate_and_construct_order(creation_, &domain_separator, Default::default(), None)
.await
Expand All @@ -1451,7 +1418,6 @@ mod tests {
app_data: OrderCreationAppData::default(),
..creation
};
let validator = validator.with_partially_fillable_limit_orders(true);
let (order, quote) = validator
.validate_and_construct_order(creation_, &domain_separator, Default::default(), None)
.await
Expand Down Expand Up @@ -1503,8 +1469,7 @@ mod tests {
MAX_LIMIT_ORDERS_PER_USER,
Arc::new(MockCodeFetching::new()),
Default::default(),
)
.with_fill_or_kill_limit_orders(true);
);

let creation = OrderCreation {
valid_to: model::time::now_in_epoch_seconds() + 2,
Expand Down Expand Up @@ -1577,65 +1542,6 @@ mod tests {
assert!(matches!(result, Err(ValidationError::ZeroAmount)));
}

#[tokio::test]
async fn post_zero_fee_limit_orders_disabled() {
let mut order_quoter = MockOrderQuoting::new();
let mut bad_token_detector = MockBadTokenDetecting::new();
let mut balance_fetcher = MockBalanceFetching::new();
order_quoter.expect_find_quote().returning(|_, _| {
Ok(Quote {
fee_amount: U256::from(1),
..Default::default()
})
});
bad_token_detector
.expect_detect()
.returning(|_| Ok(TokenQuality::Good));
balance_fetcher
.expect_can_transfer()
.returning(|_, _| Ok(()));
let mut limit_order_counter = MockLimitOrderCounting::new();
limit_order_counter.expect_count().returning(|_| Ok(0u64));
let validator = OrderValidator::new(
dummy_contract!(WETH9, [0xef; 20]),
hashset!(),
hashset!(),
OrderValidPeriodConfiguration::any(),
false,
Arc::new(bad_token_detector),
dummy_contract!(HooksTrampoline, [0xcf; 20]),
Arc::new(order_quoter),
Arc::new(balance_fetcher),
Arc::new(MockSignatureValidating::new()),
Arc::new(limit_order_counter),
0,
Arc::new(MockCodeFetching::new()),
Default::default(),
);
let order = OrderCreation {
valid_to: time::now_in_epoch_seconds() + 2,
sell_token: H160::from_low_u64_be(1),
buy_token: H160::from_low_u64_be(2),
buy_amount: U256::from(1),
sell_amount: U256::from(1),
fee_amount: U256::zero(),
signature: Signature::Eip712(EcdsaSignature::non_zero()),
..Default::default()
};
let result = validator
.validate_and_construct_order(order, &Default::default(), Default::default(), None)
.await;
assert!(
matches!(
result,
Err(ValidationError::Partial(
PartialValidationError::UnsupportedOrderType
))
),
"{result:?}"
);
}

#[tokio::test]
async fn post_out_of_market_orders_when_limit_orders_disabled() {
let expected_buy_amount = U256::from(100);
Expand Down

0 comments on commit 789fdd3

Please sign in to comment.