From dd2fe4b6e549d63d0463bd89a1395829af5843d4 Mon Sep 17 00:00:00 2001 From: ilya Date: Wed, 17 Apr 2024 09:27:04 +0100 Subject: [PATCH] 0x liquidity order amount fix (#2616) --- .../driver/src/boundary/liquidity/zeroex.rs | 12 +- crates/driver/src/domain/liquidity/zeroex.rs | 16 +- crates/driver/src/infra/solver/dto/auction.rs | 4 +- crates/e2e/src/api/zeroex.rs | 3 +- crates/e2e/tests/e2e/liquidity.rs | 192 +++++++++++++++--- 5 files changed, 186 insertions(+), 41 deletions(-) diff --git a/crates/driver/src/boundary/liquidity/zeroex.rs b/crates/driver/src/boundary/liquidity/zeroex.rs index 79deed2008..9dc2cfbf81 100644 --- a/crates/driver/src/boundary/liquidity/zeroex.rs +++ b/crates/driver/src/boundary/liquidity/zeroex.rs @@ -2,7 +2,7 @@ use { crate::{ domain::liquidity::{ self, - zeroex::{self, Order, ZeroExSignature}, + zeroex::{self, Amounts, Order, ZeroExSignature}, }, infra::{self, Ethereum}, }, @@ -49,8 +49,10 @@ pub fn to_domain( sender: handler.order.sender, maker_token: handler.order.maker_token, taker_token: handler.order.taker_token, - maker_amount: handler.order.maker_amount, - taker_amount: handler.order.taker_amount, + amounts: Amounts { + maker: handler.order.maker_amount, + taker: handler.order.taker_amount, + }, taker_token_fee_amount: handler.order.taker_token_fee_amount, fee_recipient: handler.order.fee_recipient, pool: handler.order.pool, @@ -61,6 +63,10 @@ pub fn to_domain( let domain = zeroex::LimitOrder { order, + fillable: Amounts { + maker: limit_order.sell_amount.as_u128(), + taker: limit_order.buy_amount.as_u128(), + }, zeroex: handler.zeroex.clone(), }; diff --git a/crates/driver/src/domain/liquidity/zeroex.rs b/crates/driver/src/domain/liquidity/zeroex.rs index eada9e984f..3dd7e56f12 100644 --- a/crates/driver/src/domain/liquidity/zeroex.rs +++ b/crates/driver/src/domain/liquidity/zeroex.rs @@ -7,9 +7,18 @@ use { std::sync::Arc, }; +#[derive(Clone, Debug)] +pub struct Amounts { + pub maker: u128, + pub taker: u128, +} + #[derive(Clone, Debug)] pub struct LimitOrder { pub order: Order, + /// Scaled amounts according to how much of the partially fillable amounts + /// were already used in the order. + pub fillable: Amounts, pub zeroex: Arc, } @@ -20,8 +29,7 @@ pub struct Order { pub sender: H160, pub maker_token: H160, pub taker_token: H160, - pub maker_amount: u128, - pub taker_amount: u128, + pub amounts: Amounts, pub taker_token_fee_amount: u128, pub fee_recipient: H160, pub pool: H256, @@ -44,8 +52,8 @@ impl LimitOrder { ( self.order.maker_token, self.order.taker_token, - self.order.maker_amount, - self.order.taker_amount, + self.order.amounts.maker, + self.order.amounts.taker, self.order.taker_token_fee_amount, self.order.maker, self.order.taker, diff --git a/crates/driver/src/infra/solver/dto/auction.rs b/crates/driver/src/infra/solver/dto/auction.rs index 79525af94f..01963cb483 100644 --- a/crates/driver/src/infra/solver/dto/auction.rs +++ b/crates/driver/src/infra/solver/dto/auction.rs @@ -261,8 +261,8 @@ impl Auction { hash: Default::default(), maker_token: limit_order.order.maker_token, taker_token: limit_order.order.taker_token, - maker_amount: limit_order.order.maker_amount.into(), - taker_amount: limit_order.order.taker_amount.into(), + maker_amount: limit_order.fillable.maker.into(), + taker_amount: limit_order.fillable.taker.into(), taker_token_fee_amount: limit_order.order.taker_token_fee_amount.into(), }) } diff --git a/crates/e2e/src/api/zeroex.rs b/crates/e2e/src/api/zeroex.rs index c16615575b..3c900cb662 100644 --- a/crates/e2e/src/api/zeroex.rs +++ b/crates/e2e/src/api/zeroex.rs @@ -62,6 +62,7 @@ pub struct Eip712TypedZeroExOrder { pub taker_token: H160, pub maker_amount: u128, pub taker_amount: u128, + pub remaining_fillable_taker_amount: u128, pub taker_token_fee_amount: u128, pub maker: H160, pub taker: H160, @@ -87,7 +88,7 @@ impl Eip712TypedZeroExOrder { metadata: OrderMetadata { created_at: DateTime::::MIN_UTC, order_hash: self.hash_struct().to_vec(), - remaining_fillable_taker_amount: self.taker_amount, + remaining_fillable_taker_amount: self.remaining_fillable_taker_amount, }, order: Order { chain_id, diff --git a/crates/e2e/tests/e2e/liquidity.rs b/crates/e2e/tests/e2e/liquidity.rs index 9c7aa89830..465c161b77 100644 --- a/crates/e2e/tests/e2e/liquidity.rs +++ b/crates/e2e/tests/e2e/liquidity.rs @@ -1,6 +1,6 @@ use { chrono::{NaiveDateTime, Utc}, - contracts::{IZeroEx, ERC20}, + contracts::{i_zero_ex::Contract, IZeroEx, ERC20}, driver::domain::eth::H160, e2e::{ api::zeroex::{Eip712TypedZeroExOrder, ZeroExApi}, @@ -18,7 +18,14 @@ use { }, tx, }, - ethcontract::{prelude::U256, H256}, + ethcontract::{ + errors::MethodError, + prelude::U256, + transaction::TransactionResult, + Account, + Bytes, + H256, + }, ethrpc::Web3, hex_literal::hex, model::{ @@ -78,24 +85,31 @@ async fn zero_ex_liquidity(web3: Web3) { let usdt_whale = forked_node_api.impersonate(&USDT_WHALE).await.unwrap(); tx!( usdt_whale, - token_usdt.transfer(zeroex_maker.address(), amount * 2) + // With a lower amount 0x contract shows much lower fillable amount + token_usdt.transfer(zeroex_maker.address(), amount * 4) ); + // Required for the remaining fillable taker amount + tx!(usdc_whale, token_usdc.transfer(solver.address(), amount)); - // Approve GPv2 for trading tx!( trader.account(), token_usdc.approve(onchain.contracts().allowance, amount) ); tx!( zeroex_maker.account(), - token_usdt.approve(zeroex.address(), amount * 2) + // With a lower amount 0x contract shows much lower fillable amount + token_usdt.approve(zeroex.address(), amount * 4) + ); + tx!( + solver.account(), + token_usdc.approve(zeroex.address(), amount) ); let order = OrderCreation { sell_token: token_usdc.address(), sell_amount: amount, buy_token: token_usdt.address(), - buy_amount: amount - to_wei_with_exp(1, 8), + buy_amount: amount, valid_to: model::time::now_in_epoch_seconds() + 300, kind: OrderKind::Sell, ..Default::default() @@ -106,19 +120,15 @@ async fn zero_ex_liquidity(web3: Web3) { SecretKeyRef::from(&SecretKey::from_slice(trader.private_key()).unwrap()), ); - let zeroex_api_port = { - let chain_id = web3.eth().chain_id().await.unwrap().as_u64(); - let zeroex_liquidity_orders = create_zeroex_liquidity_orders( - order.clone(), - zeroex_maker, - zeroex.address(), - onchain.contracts().gp_settlement.address(), - chain_id, - onchain.contracts().weth.address(), - ); - - ZeroExApi::new(zeroex_liquidity_orders).run().await - }; + let chain_id = web3.eth().chain_id().await.unwrap().as_u64(); + let zeroex_liquidity_orders = create_zeroex_liquidity_orders( + order.clone(), + zeroex_maker.clone(), + zeroex.address(), + chain_id, + onchain.contracts().weth.address(), + ); + let zeroex_api_port = ZeroExApi::new(zeroex_liquidity_orders.to_vec()).run().await; // Place Orders let services = Services::new(onchain.contracts()).await; @@ -128,7 +138,7 @@ async fn zero_ex_liquidity(web3: Web3) { onchain.contracts(), vec![SolverEngine { name: "test_solver".into(), - account: solver, + account: solver.clone(), endpoint: solver_endpoint, }], colocation::LiquidityProvider::ZeroEx { @@ -184,26 +194,73 @@ async fn zero_ex_liquidity(web3: Web3) { }) .await .unwrap(); + + let zeroex_order_amounts = get_zeroex_order_amounts(&zeroex, &zeroex_liquidity_orders[0]) + .await + .unwrap(); + // [`relative-slippage`] config value is set to 0.1 + // crates/e2e/src/setup/colocation.rs:110 which is then applied to the + // original filled amount crates/solver/src/liquidity/slippage.rs:110 + let expected_filled_amount = amount.as_u128() + amount.as_u128() / 10u128; + assert_eq!(zeroex_order_amounts.filled, expected_filled_amount); + assert!(zeroex_order_amounts.fillable > 0u128); + assert_eq!( + zeroex_order_amounts.fillable, + amount.as_u128() * 2 - expected_filled_amount + ); + + // Fill the remaining part of the 0x order + let zeroex_order = Eip712TypedZeroExOrder { + maker_token: token_usdt.address(), + taker_token: token_usdc.address(), + maker_amount: zeroex_order_amounts.fillable, + taker_amount: zeroex_order_amounts.fillable, + // doesn't participate in the hash calculation + remaining_fillable_taker_amount: 0u128, + taker_token_fee_amount: 0, + maker: zeroex_maker.address(), + taker: Default::default(), + sender: Default::default(), + fee_recipient: zeroex.address(), + pool: H256::default(), + expiry: NaiveDateTime::MAX.timestamp() as u64, + salt: U256::from(Utc::now().timestamp()), + } + .to_order_record(chain_id, zeroex.address(), zeroex_maker); + fill_or_kill_zeroex_limit_order(&zeroex, &zeroex_order, solver.account().clone()) + .await + .unwrap(); + let zeroex_order_amounts = get_zeroex_order_amounts(&zeroex, &zeroex_order) + .await + .unwrap(); + assert_eq!( + zeroex_order_amounts.filled, + amount.as_u128() * 2 - expected_filled_amount + ); + assert_eq!(zeroex_order_amounts.fillable, 0u128); } fn create_zeroex_liquidity_orders( order_creation: OrderCreation, zeroex_maker: TestAccount, zeroex_addr: H160, - gpv2_addr: H160, chain_id: u64, weth_address: H160, -) -> Vec { +) -> [shared::zeroex_api::OrderRecord; 3] { let typed_order = Eip712TypedZeroExOrder { maker_token: order_creation.buy_token, taker_token: order_creation.sell_token, // fully covers execution costs maker_amount: order_creation.buy_amount.as_u128() * 3, taker_amount: order_creation.sell_amount.as_u128() * 2, + // makes 0x order partially filled, but the amount is higher than the cowswap order to + // make sure the 0x order is not overfilled in the end of the e2e test + remaining_fillable_taker_amount: order_creation.sell_amount.as_u128() * 3 / 2, taker_token_fee_amount: 0, maker: zeroex_maker.address(), - taker: gpv2_addr, - sender: gpv2_addr, + // Makes it possible for anyone to fill the order + taker: Default::default(), + sender: Default::default(), fee_recipient: zeroex_addr, pool: H256::default(), expiry: NaiveDateTime::MAX.timestamp() as u64, @@ -212,14 +269,15 @@ fn create_zeroex_liquidity_orders( let usdt_weth_order = Eip712TypedZeroExOrder { maker_token: weth_address, taker_token: order_creation.buy_token, - // the value comes from the `--amount-to-estimate-prices-with` config value to provide + // the value comes from the `--amount-to-estimate-prices-with` config to provide // sufficient liquidity maker_amount: 1_000_000_000_000_000_000u128, taker_amount: order_creation.sell_amount.as_u128(), + remaining_fillable_taker_amount: order_creation.sell_amount.as_u128(), taker_token_fee_amount: 0, maker: zeroex_maker.address(), - taker: gpv2_addr, - sender: gpv2_addr, + taker: Default::default(), + sender: Default::default(), fee_recipient: zeroex_addr, pool: H256::default(), expiry: NaiveDateTime::MAX.timestamp() as u64, @@ -228,14 +286,15 @@ fn create_zeroex_liquidity_orders( let usdc_weth_order = Eip712TypedZeroExOrder { maker_token: weth_address, taker_token: order_creation.sell_token, - // the value comes from the `--amount-to-estimate-prices-with` config value to provide + // the value comes from the `--amount-to-estimate-prices-with` config to provide // sufficient liquidity maker_amount: 1_000_000_000_000_000_000u128, taker_amount: order_creation.sell_amount.as_u128(), + remaining_fillable_taker_amount: order_creation.sell_amount.as_u128(), taker_token_fee_amount: 0, maker: zeroex_maker.address(), - taker: gpv2_addr, - sender: gpv2_addr, + taker: Default::default(), + sender: Default::default(), fee_recipient: zeroex_addr, pool: H256::default(), expiry: NaiveDateTime::MAX.timestamp() as u64, @@ -243,5 +302,76 @@ fn create_zeroex_liquidity_orders( }; [typed_order, usdt_weth_order, usdc_weth_order] .map(|order| order.to_order_record(chain_id, zeroex_addr, zeroex_maker.clone())) - .to_vec() +} + +#[derive(Debug)] +struct ZeroExOrderAmounts { + filled: u128, + fillable: u128, +} + +async fn get_zeroex_order_amounts( + zeroex: &Contract, + zeroex_order: &shared::zeroex_api::OrderRecord, +) -> Result { + zeroex + .get_limit_order_relevant_state( + ( + zeroex_order.order.maker_token, + zeroex_order.order.taker_token, + zeroex_order.order.maker_amount, + zeroex_order.order.taker_amount, + zeroex_order.order.taker_token_fee_amount, + zeroex_order.order.maker, + zeroex_order.order.taker, + zeroex_order.order.sender, + zeroex_order.order.fee_recipient, + Bytes(zeroex_order.order.pool.0), + zeroex_order.order.expiry, + zeroex_order.order.salt, + ), + ( + zeroex_order.order.signature.signature_type, + zeroex_order.order.signature.v, + Bytes(zeroex_order.order.signature.r.0), + Bytes(zeroex_order.order.signature.s.0), + ), + ) + .call() + .await + .map(|((_, _, filled), fillable, _)| ZeroExOrderAmounts { filled, fillable }) +} + +async fn fill_or_kill_zeroex_limit_order( + zeroex: &Contract, + zeroex_order: &shared::zeroex_api::OrderRecord, + from_account: Account, +) -> Result { + zeroex + .fill_or_kill_limit_order( + ( + zeroex_order.order.maker_token, + zeroex_order.order.taker_token, + zeroex_order.order.maker_amount, + zeroex_order.order.taker_amount, + zeroex_order.order.taker_token_fee_amount, + zeroex_order.order.maker, + zeroex_order.order.taker, + zeroex_order.order.sender, + zeroex_order.order.fee_recipient, + Bytes(zeroex_order.order.pool.0), + zeroex_order.order.expiry, + zeroex_order.order.salt, + ), + ( + zeroex_order.order.signature.signature_type, + zeroex_order.order.signature.v, + Bytes(zeroex_order.order.signature.r.0), + Bytes(zeroex_order.order.signature.s.0), + ), + zeroex_order.order.taker_amount, + ) + .from(from_account) + .send() + .await }