From d65e80fecde663df2759426f60e1768ec8119c4c Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Wed, 24 Jul 2024 14:15:56 +0200 Subject: [PATCH 1/6] allow partial fill for small otc orders that can't fully close arbs --- pallets/otc-settlements/src/lib.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/pallets/otc-settlements/src/lib.rs b/pallets/otc-settlements/src/lib.rs index aed99cd45..a98fc3117 100644 --- a/pallets/otc-settlements/src/lib.rs +++ b/pallets/otc-settlements/src/lib.rs @@ -382,7 +382,10 @@ impl Pallet { .ok_or(ArithmeticError::Overflow)?; if price_diff > price_precision { ensure!(router_price_after <= otc_price, Error::::TradeAmountTooHigh); - ensure!(router_price_after >= otc_price, Error::::TradeAmountTooLow); + // partially fillable OTC can be fully filled if the arb is reduced + if amount != otc.amount_in { + ensure!(router_price_after >= otc_price, Error::::TradeAmountTooLow); + } } } From cb4b3687ed36e7e8bd8775f1a48ae4aa484bb424 Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Wed, 24 Jul 2024 14:16:32 +0200 Subject: [PATCH 2/6] add test for partial fill --- pallets/otc-settlements/src/tests.rs | 58 +++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/pallets/otc-settlements/src/tests.rs b/pallets/otc-settlements/src/tests.rs index 2365a88da..3c6d7f9b6 100644 --- a/pallets/otc-settlements/src/tests.rs +++ b/pallets/otc-settlements/src/tests.rs @@ -250,7 +250,7 @@ fn profit_should_be_transferred_to_treasury_when_nonzero_initial_pallet_balance( } #[test] -fn existing_arb_opportunity_should_trigger_trade() { +fn existing_arb_opportunity_should_trigger_trade_when_correct_amount_can_be_found() { let (mut ext, _) = ExtBuilder::default().build(); ext.execute_with(|| { assert_ok!(OTC::place_order( @@ -300,6 +300,62 @@ fn existing_arb_opportunity_should_trigger_trade() { }); } +#[test] +fn existing_arb_opportunity_should_trigger_trade_when_partially_fillable_otc_can_be_fully_filled() { + let (mut ext, _) = ExtBuilder::default().build(); + ext.execute_with(|| { + assert_ok!(OTC::place_order( + RuntimeOrigin::signed(ALICE), + HDX, // otc asset_in + DAI, // otc asset_out + 100 * ONE, + 205 * ONE, + true, + )); + + // get otc price + let otc_id = 0; + let otc = >::get(otc_id).unwrap(); + let otc_price = calculate_otc_price(&otc); + + // get trade price + let route = Router::get_route(AssetPair { + asset_in: otc.asset_out, + asset_out: otc.asset_in, + }); + let initial_router_price = Router::spot_price_with_fee(&route).unwrap(); + + // verify that there's an arb opportunity + assert!(otc_price > initial_router_price); + + let hdx_total_issuance = Currencies::total_issuance(HDX); + let dai_total_issuance = Currencies::total_issuance(DAI); + + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + >>::offchain_worker(System::block_number()); + + // verify that the arb is still there, but smaller than before + let final_router_price = Router::spot_price_with_fee(&route).unwrap(); + assert!(otc_price > final_router_price); + assert!(final_router_price > initial_router_price); + + assert_eq!(hdx_total_issuance, Currencies::total_issuance(HDX)); + assert_eq!(dai_total_issuance, Currencies::total_issuance(DAI)); + + // total issuance of tokens should not change + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + expect_last_events(vec![Event::Executed { + asset_id: HDX, + profit: 1_444_117_874_415, + } + .into()]); + }); +} + #[test] fn existing_arb_opportunity_should_trigger_trade_when_otc_is_not_partially_fillable() { let (mut ext, _) = ExtBuilder::default().build(); From cdb5fa22e28b9ef92e60167b1405091097eab8ab Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Wed, 24 Jul 2024 14:17:11 +0200 Subject: [PATCH 3/6] bump crate versions --- pallets/otc-settlements/Cargo.toml | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/pallets/otc-settlements/Cargo.toml b/pallets/otc-settlements/Cargo.toml index bfc59eae6..24e23f5d2 100644 --- a/pallets/otc-settlements/Cargo.toml +++ b/pallets/otc-settlements/Cargo.toml @@ -1,6 +1,6 @@ [package] name = 'pallet-otc-settlements' -version = '1.0.2' +version = '1.0.3' description = 'A pallet with offchain worker closing OTC arbs' authors = ['GalacticCouncil'] edition = '2021' diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index ec93adbf1..ca49513a1 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "251.0.0" +version = "252.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index be0f65d6b..8cd9d41c4 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -113,7 +113,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 251, + spec_version: 252, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1, From c84f1ee07e1491da6d0a9ae4a5a0b966e7a1e3c5 Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Wed, 24 Jul 2024 22:13:51 +0200 Subject: [PATCH 4/6] try to reduce arb when run out of iterations --- Cargo.lock | 4 +-- pallets/otc-settlements/README.md | 3 +- pallets/otc-settlements/src/lib.rs | 39 ++++++++++++++++++------- pallets/otc-settlements/src/mock.rs | 2 +- pallets/otc-settlements/src/tests.rs | 43 +++++++++++++++++++++++++--- 5 files changed, 73 insertions(+), 18 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2e52285cb..0f630c72f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4938,7 +4938,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "251.0.0" +version = "252.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system", @@ -8768,7 +8768,7 @@ dependencies = [ [[package]] name = "pallet-otc-settlements" -version = "1.0.2" +version = "1.0.3" dependencies = [ "frame-benchmarking", "frame-support", diff --git a/pallets/otc-settlements/README.md b/pallets/otc-settlements/README.md index 91b5364f9..080e2d109 100644 --- a/pallets/otc-settlements/README.md +++ b/pallets/otc-settlements/README.md @@ -9,7 +9,8 @@ by any user using signed origin. In the former case, the block producer doesn't ## Notes If the OTC order is partially fillable, the pallet tries to close the arbitrage opportunity by finding the amount that aligns the OTC and the Omnipool prices. Executing this trade needs to be profitable, but we are not trying to maximize -the profit. +the profit. If the pallet couldn't find the amount that closes the arb, the amount that reduces the size of the arb is +used. In the case of not partially fillable OTC orders, the pallet tries to maximize the profit. ## Dispatachable functions diff --git a/pallets/otc-settlements/src/lib.rs b/pallets/otc-settlements/src/lib.rs index a98fc3117..7604f9e6f 100644 --- a/pallets/otc-settlements/src/lib.rs +++ b/pallets/otc-settlements/src/lib.rs @@ -25,7 +25,7 @@ //! ## Notes //! If the OTC order is partially fillable, the pallet tries to close the arbitrage opportunity by finding the amount that //! aligns the OTC and the Omnipool prices. Executing this trade needs to be profitable, but we are not trying to maximize -//! the profit. +//! the profit. If the pallet couldn't find the amount that closes the arb, the amount that reduces the size of the arb is used. //! In the case of not partially fillable OTC orders, the pallet tries to maximize the profit. //! //! ## Dispatachable functions @@ -39,7 +39,7 @@ use frame_support::{ fungibles::{Inspect, Mutate}, tokens::{Fortitude, Precision, Preservation}, }, - PalletId, + transactional, PalletId, }; use frame_system::{ offchain::{SendTransactionTypes, SubmitTransaction}, @@ -221,7 +221,7 @@ pub mod pallet { /// /// Executes a trade between an OTC order and some route. /// If the OTC order is partially fillable, the extrinsic fails if the existing arbitrage - /// opportunity is not closed after the trade. + /// opportunity is not closed or reduced after the trade. /// If the OTC order is not partially fillable, fails if there is no profit after the trade. /// /// `Origin` calling this extrinsic is not paying or receiving anything. @@ -250,7 +250,9 @@ pub mod pallet { amount: Balance, route: Vec>>, ) -> DispatchResult { - Self::settle_otc(otc_id, amount, route) + // `is_execution` is set to `true`, so both full and partial closing of arbs is allowed. + // If set to `false`, an arb needs to be fully closed. + Self::settle_otc(otc_id, amount, route, true) } } } @@ -280,15 +282,23 @@ impl Pallet { /// /// Executes two trades: asset_a -> OTC -> asset_b, and asset_b -> Router -> asset_a. /// - /// If the OTC order is partially fillable, the extrinsic fails if the existing arbitrage + /// If the OTC order is partially fillable and `is_execution` is set to `false`, the extrinsic fails if the existing arbitrage /// opportunity is not closed after the trade and if there is no profit after the trade. + /// If the OTC order is partially fillable and `is_execution` is set to `true`, arbs are allowed to be partially closed. /// If the OTC order is not partially fillable, fails only if there is no profit after the trade. /// /// Parameters: /// - `otc_id`: ID of the OTC order with existing arbitrage opportunity. /// - `amount`: Amount necessary to close the arb. /// - `route`: The route we trade against. Required for the fee calculation. - pub fn settle_otc(otc_id: OrderId, amount: Balance, route: Vec>>) -> DispatchResult { + /// - `is_execution`: When enabled, test for the price precision is disabled. + #[transactional] + pub fn settle_otc( + otc_id: OrderId, + amount: Balance, + route: Vec>>, + is_execution: bool, + ) -> DispatchResult { log::debug!( target: "offchain_worker::settle_otc", "calling settle_otc(): otc_id: {:?} amount: {:?} route: {:?}", otc_id, amount, route); @@ -368,7 +378,7 @@ impl Pallet { // Compare OTC and Router price. // In the case of fully fillable orders, the resulting price is not important. - if otc.partially_fillable { + if !is_execution && otc.partially_fillable { let price_diff = { if otc_price > router_price_after { otc_price.saturating_sub(router_price_after) @@ -562,7 +572,7 @@ impl Pallet { log::debug!( target: "offchain_worker::settle_otcs::binary_search", "\nsell_amt: {:?}\nsell_amt_up: {:?}\nsell_amt_down: {:?}", sell_amt, sell_amt_up, sell_amt_down); - match Self::settle_otc_order(RawOrigin::None.into(), otc_id, sell_amt, route.to_vec()) { + match Self::settle_otc(otc_id, sell_amt, route.to_vec(), false) { Ok(_) => { log::debug!( target: "offchain_worker::settle_otcs", @@ -593,12 +603,21 @@ impl Pallet { } } } + // no more values to test if sell_amt_down == sell_amt_up { - return None; + break; } sell_amt = (sell_amt_up.checked_add(sell_amt_down)).and_then(|value| value.checked_div(2))?; } - None + // execute with the latest min value + if sell_amt_down != T::MinTradingLimit::get() { + match Self::settle_otc(otc_id, sell_amt_down, route.to_vec(), true) { + Ok(_) => Some(sell_amt_down), + Err(_) => None, + } + } else { + None + } } /// Calculates the price (asset_out/asset_in) after subtracting the OTC fee from the amount_out. diff --git a/pallets/otc-settlements/src/mock.rs b/pallets/otc-settlements/src/mock.rs index 50b7bc7c9..0fb3789c9 100644 --- a/pallets/otc-settlements/src/mock.rs +++ b/pallets/otc-settlements/src/mock.rs @@ -414,7 +414,7 @@ impl Default for ExtBuilder { endowed_accounts: vec![ (ALICE, HDX, 1_000_000_000_000 * ONE), (ALICE, LRNA, 1_000_000_000_000 * ONE), - (ALICE, DAI, 1_000_000_000_000 * ONE), + (ALICE, DAI, 1_000_000_000_000_000_000 * ONE), (ALICE, DOT, 1_000_000_000_000 * ONE), (ALICE, KSM, 1_000_000_000_000 * ONE), (ALICE, BTC, 1_000_000_000_000 * ONE), diff --git a/pallets/otc-settlements/src/tests.rs b/pallets/otc-settlements/src/tests.rs index 3c6d7f9b6..fde983164 100644 --- a/pallets/otc-settlements/src/tests.rs +++ b/pallets/otc-settlements/src/tests.rs @@ -618,7 +618,7 @@ fn trade_should_not_be_triggered_when_there_is_no_arb_opportunity() { } #[test] -fn trade_should_not_be_triggered_when_optimal_amount_not_found() { +fn trade_should_be_triggered_when_optimal_amount_not_found_but_arb_can_be_reduced() { let (mut ext, _) = ExtBuilder::default().build(); ext.execute_with(|| { assert_ok!(OTC::place_order( @@ -651,9 +651,7 @@ fn trade_should_not_be_triggered_when_optimal_amount_not_found() { assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); - assert_storage_noop!(>>::offchain_worker( - System::block_number() - )); + >>::offchain_worker(System::block_number()); assert_eq!(hdx_total_issuance, Currencies::total_issuance(HDX)); assert_eq!(dai_total_issuance, Currencies::total_issuance(DAI)); @@ -664,6 +662,43 @@ fn trade_should_not_be_triggered_when_optimal_amount_not_found() { }); } +#[test] +fn trade_should_not_be_triggered_when_amount_not_found() { + let (mut ext, _) = ExtBuilder::default().build(); + ext.execute_with(|| { + assert_ok!(OTC::place_order( + RuntimeOrigin::signed(ALICE), + HDX, // otc asset_in + DAI, // otc asset_out + 1_000 * ONE, + 800_000_000_000_000_001 * ONE, + true, + )); + + // get otc price + let otc_id = 0; + let otc = >::get(otc_id).unwrap(); + let otc_price = calculate_otc_price(&otc); + + // get trade price + let route = Router::get_route(AssetPair { + asset_in: otc.asset_out, + asset_out: otc.asset_in, + }); + let router_price = Router::spot_price_with_fee(&route).unwrap(); + + // verify that there's an arb opportunity + assert!(otc_price > router_price); + + assert!(Currencies::free_balance(HDX, &OtcSettlements::account_id()) == 0); + assert!(Currencies::free_balance(DAI, &OtcSettlements::account_id()) == 0); + + assert_storage_noop!(>>::offchain_worker( + System::block_number() + )); + }); +} + #[test] fn test_offchain_worker_unsigned_transaction_submission() { let (mut ext, pool_state) = ExtBuilder::default().build(); From f74a793323908b38b51bc70ee5b9a66f10650ad5 Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Thu, 25 Jul 2024 09:56:23 +0200 Subject: [PATCH 5/6] fix clippy --- pallets/otc-settlements/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pallets/otc-settlements/src/lib.rs b/pallets/otc-settlements/src/lib.rs index 7604f9e6f..faa9a3538 100644 --- a/pallets/otc-settlements/src/lib.rs +++ b/pallets/otc-settlements/src/lib.rs @@ -33,6 +33,9 @@ #![cfg_attr(not(feature = "std"), no_std)] +#[cfg(not(feature = "runtime-benchmarks"))] +use frame_system::RawOrigin; + use frame_support::{ pallet_prelude::*, traits::{ @@ -44,7 +47,6 @@ use frame_support::{ use frame_system::{ offchain::{SendTransactionTypes, SubmitTransaction}, pallet_prelude::{BlockNumberFor, OriginFor}, - RawOrigin, }; use hydradx_traits::router::{ AmmTradeWeights, AmountInAndOut, AssetPair, RouteProvider, RouteSpotPriceProvider, RouterT, Trade, From 93974101c922e6c50a241e958428e5aac4165a22 Mon Sep 17 00:00:00 2001 From: Roznovjak Date: Wed, 7 Aug 2024 23:41:12 +0200 Subject: [PATCH 6/6] bump runtime version --- Cargo.lock | 2 +- runtime/hydradx/Cargo.toml | 2 +- runtime/hydradx/src/lib.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1f46df7b7..2f30161a0 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4938,7 +4938,7 @@ dependencies = [ [[package]] name = "hydradx-runtime" -version = "254.0.0" +version = "255.0.0" dependencies = [ "cumulus-pallet-aura-ext", "cumulus-pallet-parachain-system", diff --git a/runtime/hydradx/Cargo.toml b/runtime/hydradx/Cargo.toml index 0b38f82bf..60e6e1458 100644 --- a/runtime/hydradx/Cargo.toml +++ b/runtime/hydradx/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "hydradx-runtime" -version = "254.0.0" +version = "255.0.0" authors = ["GalacticCouncil"] edition = "2021" license = "Apache 2.0" diff --git a/runtime/hydradx/src/lib.rs b/runtime/hydradx/src/lib.rs index f7d4e1b7e..e80d28be2 100644 --- a/runtime/hydradx/src/lib.rs +++ b/runtime/hydradx/src/lib.rs @@ -113,7 +113,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion { spec_name: create_runtime_str!("hydradx"), impl_name: create_runtime_str!("hydradx"), authoring_version: 1, - spec_version: 254, + spec_version: 255, impl_version: 0, apis: RUNTIME_API_VERSIONS, transaction_version: 1,