fix: omnipool imbalance adjustment

pallets/omnipool/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pallet-omnipool"
-version = "1.4.1"
+version = "1.4.2"
 authors = ['GalacticCouncil']
 edition = "2021"
 license = "Apache-2.0"
@@ -31,7 +31,7 @@ orml-traits = { git = "https://github.com/open-web3-stack/open-runtime-module-li
 # Warehouse
 hydradx-traits = { git = "https://github.com/galacticcouncil/warehouse", rev = "b7cc6ff5efb641e4fd4da524459ce8111c32aef5", default-features = false }
 
-hydra-dx-math = { git = "https://github.com/galacticcouncil/HydraDX-math", rev = "7fc0204c401e79447eaae96e58087413fc6cb59c", default-features = false }
+hydra-dx-math = { git = "https://github.com/galacticcouncil/HydraDX-math", rev = "2d7f86ffae242fc855bc833dd22df2d8dc5d03df", default-features = false }
 
 # third party
 primitive-types = { version = "0.12.0", default-features = false }

pallets/omnipool/src/migration.rs

@@ -30,3 +30,32 @@ pub fn migrate_to_v1<T: Config, P: GetStorageVersion + PalletInfoAccess>() -> fr
 		T::DbWeight::get().reads(1)
 	}
 }
+
+/// Migrate the pallet storage to v2.
+pub fn migrate_to_v2<T: Config, P: GetStorageVersion + PalletInfoAccess>() -> frame_support::weights::Weight {
+	let on_chain_storage_version = <P as GetStorageVersion>::on_chain_storage_version();
+	log::info!(
+		target: "runtime::omnipool",
+		"Running migration storage v1 for omnipool with storage version {:?}",
+		on_chain_storage_version,
+	);
+
+	if on_chain_storage_version < 2 {
+		HubAssetImbalance::<T>::set(SimpleImbalance::default());
+		StorageVersion::new(2).put::<P>();
+		log::info!(
+			target: "runtime::omnipool",
+			"Running migration storage v1 for omnipool with storage version {:?} was complete",
+			on_chain_storage_version,
+		);
+		// calculate and return migration weights
+		T::DbWeight::get().reads_writes(1u64, 1u64)
+	} else {
+		log::warn!(
+			target: "runtime::omnipool",
+			"Attempted to apply migration to v1 but failed because storage version is {:?}",
+			on_chain_storage_version,
+		);
+		T::DbWeight::get().reads(1)
+	}
+}

pallets/omnipool/src/tests/imbalance.rs

@@ -0,0 +1,96 @@
+use super::*;
+use pretty_assertions::assert_eq;
+use sp_runtime::Permill;
+
+#[test]
+fn imbalance_should_update_correctly() {
+	ExtBuilder::default()
+		.with_endowed_accounts(vec![
+			(Omnipool::protocol_account(), DAI, 1000 * ONE),
+			(Omnipool::protocol_account(), HDX, NATIVE_AMOUNT),
+			(LP1, 100, 5000000000000000),
+			(LP1, 200, 5000000000000000),
+			(LP2, 100, 1000000000000000),
+			(LP3, 100, 1000000000000000),
+			(LP3, 1, 100000000000000),
+		])
+		.with_registered_asset(100)
+		.with_registered_asset(200)
+		.with_protocol_fee(Permill::from_percent(1))
+		.with_initial_pool(FixedU128::from_float(0.5), FixedU128::from(1))
+		.with_token(100, FixedU128::from_float(0.65), LP1, 2000 * ONE)
+		.with_token(200, FixedU128::from_float(0.65), LP1, 2000 * ONE)
+		.build()
+		.execute_with(|| {
+			assert_ok!(Omnipool::add_liquidity(Origin::signed(LP2), 100, 400000000000000));
+
+			assert_pool_state!(
+				13360000000000000,
+				26720000000000000,
+				SimpleImbalance {
+					value: 0,
+					negative: true
+				}
+			);
+
+			let old_imbalance = HubAssetImbalance::<Test>::get();
+			assert_ok!(Omnipool::sell(
+				Origin::signed(LP3),
+				1,
+				200,
+				50000000000000,
+				10000000000000
+			));
+
+			let updated_imbalance = HubAssetImbalance::<Test>::get();
+
+			// After lrna is sold to pool, imbalance should increase (more negative)
+			assert!(updated_imbalance.value > old_imbalance.value);
+
+			let q = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
+			let old_imbalance = HubAssetImbalance::<Test>::get();
+			assert_ok!(Omnipool::sell(Origin::signed(LP3), 200, 100, 1000000000000, 1,));
+			let updated_imbalance = HubAssetImbalance::<Test>::get();
+			let q_plus = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
+
+			// After non-lrna trade - sell, imbalance should decrease ( less negative )
+			assert!(updated_imbalance.value < old_imbalance.value);
+			assert_eq!(
+				q.checked_sub(old_imbalance.value).unwrap(),
+				q_plus.checked_sub(updated_imbalance.value).unwrap()
+			);
+
+			let position_id = <NextPositionId<Test>>::get();
+			let old_imbalance = HubAssetImbalance::<Test>::get();
+			assert_ok!(Omnipool::add_liquidity(Origin::signed(LP2), 100, 400000000000000));
+			let updated_imbalance = HubAssetImbalance::<Test>::get();
+
+			// After add additional liquidity , imbalance should increase ( more negative )
+			assert!(updated_imbalance.value > old_imbalance.value);
+
+			let position = Positions::<Test>::get(position_id).unwrap();
+			let old_imbalance = HubAssetImbalance::<Test>::get();
+			assert_ok!(Omnipool::remove_liquidity(
+				Origin::signed(LP2),
+				position_id,
+				position.shares
+			));
+			let updated_imbalance = HubAssetImbalance::<Test>::get();
+
+			// After remove additional liquidity , imbalance should decrease( less negative )
+			assert!(updated_imbalance.value < old_imbalance.value);
+
+			let q = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
+			let old_imbalance = HubAssetImbalance::<Test>::get();
+			assert_ok!(Omnipool::buy(Origin::signed(LP3), 200, 100, 1000000000000, u128::MAX,));
+			let updated_imbalance = HubAssetImbalance::<Test>::get();
+			let q_plus = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
+
+			// After non-lrna trade - buy, imbalance should decrease ( less negative )
+			assert!(updated_imbalance.value < old_imbalance.value);
+			assert_eq!(
+				q.checked_sub(old_imbalance.value).unwrap(),
+				q_plus.checked_sub(updated_imbalance.value).unwrap()
+			);
+		});
+}

pallets/omnipool/src/tests/invariants.rs

@@ -5,7 +5,7 @@ use primitive_types::U256;
 use proptest::prelude::*;
 
 pub const ONE: Balance = 1_000_000_000_000;
-pub const TOLERANCE: Balance = 1_000; // * 1_000 * 1_000;
+pub const TOLERANCE: Balance = 1_000_000_000;
 
 const BALANCE_RANGE: (Balance, Balance) = (100_000 * ONE, 10_000_000 * ONE);
 
@@ -21,6 +21,10 @@ fn price() -> impl Strategy<Value = FixedU128> {
 	(0.1f64..2f64).prop_map(FixedU128::from_float)
 }
 
+fn some_imbalance() -> impl Strategy<Value = SimpleImbalance<Balance>> {
+	(0..10000 * ONE).prop_map(|value| SimpleImbalance { value, negative: true })
+}
+
 fn assert_asset_invariant(
 	old_state: &AssetReserveState<Balance>,
 	new_state: &AssetReserveState<Balance>,
@@ -43,7 +47,10 @@ fn assert_asset_invariant(
 }
 fn fee() -> impl Strategy<Value = Permill> {
 	// Allow values between 0.001 and 0.1
-	(0u32..1u32, prop_oneof![Just(1000u32), Just(10000u32), Just(100_000u32)])
+	(
+		0u32..=1u32,
+		prop_oneof![Just(1000u32), Just(10000u32), Just(100_000u32)],
+	)
 		.prop_map(|(n, d)| Permill::from_rational(n, d))
 }
 
@@ -169,7 +176,8 @@ proptest! {
 		token_3 in pool_token(300),
 		token_4 in pool_token(400),
 		asset_fee in fee(),
-		protocol_fee in fee()
+		protocol_fee in fee(),
+		imbalance in some_imbalance(),
 	) {
 		let lp1: u64 = 100;
 		let lp2: u64 = 200;
@@ -192,7 +200,7 @@ proptest! {
 			.with_registered_asset(300)
 			.with_registered_asset(400)
 			.with_asset_fee(asset_fee)
-			.with_asset_fee(protocol_fee)
+			.with_protocol_fee(protocol_fee)
 			.with_initial_pool(
 				stable_price,
 				FixedU128::from(1),
@@ -203,6 +211,8 @@ proptest! {
 			.with_token(token_4.asset_id, token_4.price, lp4, token_4.amount)
 			.build()
 			.execute_with(|| {
+				HubAssetImbalance::<Test>::set(imbalance);
+
 				let old_state_200 = Omnipool::load_asset_state(200).unwrap();
 				let old_state_300 = Omnipool::load_asset_state(300).unwrap();
 				let old_state_hdx = Omnipool::load_asset_state(HDX).unwrap();
@@ -217,6 +227,12 @@ proptest! {
 
 				assert_ok!(Omnipool::sell(Origin::signed(seller), 200, 300, amount, Balance::zero()));
 
+				let updated_imbalance = HubAssetImbalance::<Test>::get();
+
+				assert!(updated_imbalance.value <= imbalance.value);
+
+				let imbalance_diff = imbalance.value - updated_imbalance.value;
+
 				let new_state_200 = Omnipool::load_asset_state(200).unwrap();
 				let new_state_300 = Omnipool::load_asset_state(300).unwrap();
 				let new_state_hdx = Omnipool::load_asset_state(HDX).unwrap();
@@ -231,22 +247,22 @@ proptest! {
 				// Total hub asset liquidity has not changed
 				let new_hub_liquidity = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
 
-				assert_eq!(old_hub_liquidity, new_hub_liquidity, "Total Hub liquidity has changed!");
+				assert_eq!(old_hub_liquidity, new_hub_liquidity + imbalance_diff, "Total Hub liquidity has changed!");
 
 				// total quantity of R_i remains unchanged
 				let new_asset_hub_liquidity = sum_asset_hub_liquidity();
 
-				assert_eq!(old_asset_hub_liquidity, new_asset_hub_liquidity, "Assets hub liquidity");
+				assert_eq!(old_asset_hub_liquidity, new_asset_hub_liquidity + imbalance_diff, "Assets hub liquidity");
 
 				let new_imbalance = <HubAssetImbalance<Test>>::get();
 
 				// No LRNA lost
 				let delta_q_200 = old_state_200.hub_reserve - new_state_200.hub_reserve;
 				let delta_q_300 = new_state_300.hub_reserve - old_state_300.hub_reserve;
 				let delta_q_hdx = new_state_hdx.hub_reserve - old_state_hdx.hub_reserve;
-				let delta_imbalance= new_imbalance.value - old_imbalance.value; // note: in current implementation: imbalance cannot be positive, let's simply and ignore the sign for now
+				let delta_imbalance = old_imbalance.value - new_imbalance.value;
 
-				let remaining = delta_q_300 - delta_q_200 - delta_q_hdx - delta_imbalance;
+				let remaining = delta_q_200 - delta_q_300 - delta_q_hdx - delta_imbalance;
 				assert_eq!(remaining, 0u128, "Some LRNA was lost along the way");
 			});
 	}
@@ -355,7 +371,8 @@ proptest! {
 		token_3 in pool_token(300),
 		token_4 in pool_token(400),
 		asset_fee in fee(),
-		protocol_fee in fee()
+		protocol_fee in fee(),
+		imbalance in some_imbalance(),
 	) {
 		let lp1: u64 = 100;
 		let lp2: u64 = 200;
@@ -378,7 +395,7 @@ proptest! {
 			.with_registered_asset(300)
 			.with_registered_asset(400)
 			.with_asset_fee(asset_fee)
-			.with_asset_fee(protocol_fee)
+			.with_protocol_fee(protocol_fee)
 			.with_initial_pool(
 				stable_price,
 				FixedU128::from(1),
@@ -389,6 +406,7 @@ proptest! {
 			.with_token(token_4.asset_id, token_4.price, lp4, token_4.amount)
 			.build()
 			.execute_with(|| {
+				HubAssetImbalance::<Test>::set(imbalance);
 				let old_state_200 = Omnipool::load_asset_state(200).unwrap();
 				let old_state_300 = Omnipool::load_asset_state(300).unwrap();
 				let old_state_hdx = Omnipool::load_asset_state(HDX).unwrap();
@@ -403,6 +421,9 @@ proptest! {
 
 				assert_ok!(Omnipool::buy(Origin::signed(buyer), 300, 200, amount, Balance::max_value()));
 
+				let updated_imbalance = HubAssetImbalance::<Test>::get();
+				assert!(updated_imbalance.value <= imbalance.value);
+
 				let new_state_200 = Omnipool::load_asset_state(200).unwrap();
 				let new_state_300 = Omnipool::load_asset_state(300).unwrap();
 				let new_state_hdx = Omnipool::load_asset_state(HDX).unwrap();
@@ -417,22 +438,24 @@ proptest! {
 				// Total hub asset liquidity has not changed
 				let new_hub_liquidity = Tokens::free_balance(LRNA, &Omnipool::protocol_account());
 
-				assert_eq!(old_hub_liquidity, new_hub_liquidity, "Total Hub liquidity has changed!");
+				let imbalance_diff = imbalance.value - updated_imbalance.value;
+
+				assert_eq!(old_hub_liquidity, new_hub_liquidity + imbalance_diff, "Total Hub liquidity has changed!");
 
 				// total quantity of R_i remains unchanged
 				let new_asset_hub_liquidity = sum_asset_hub_liquidity();
 
-				assert_eq!(old_asset_hub_liquidity, new_asset_hub_liquidity, "Assets hub liquidity");
+				assert_eq!(old_asset_hub_liquidity, new_asset_hub_liquidity + imbalance_diff, "Assets hub liquidity");
 
 				let new_imbalance = <HubAssetImbalance<Test>>::get();
 
 				// No LRNA lost
 				let delta_q_200 = old_state_200.hub_reserve - new_state_200.hub_reserve;
 				let delta_q_300 = new_state_300.hub_reserve - old_state_300.hub_reserve;
 				let delta_q_hdx = new_state_hdx.hub_reserve - old_state_hdx.hub_reserve;
-				let delta_imbalance= new_imbalance.value - old_imbalance.value; // note: in current implementation: imbalance cannot be positive, let's simply and ignore the sign for now
+				let delta_imbalance= old_imbalance.value - new_imbalance.value;
 
-				let remaining = delta_q_300 - delta_q_200 - delta_q_hdx - delta_imbalance;
+				let remaining = delta_q_200 - delta_q_300 - delta_q_hdx - delta_imbalance;
 				assert_eq!(remaining, 0u128, "Some LRNA was lost along the way");
 			});
 	}

pallets/omnipool/src/tests/mod.rs

@@ -10,6 +10,7 @@ mod invariants;
 mod remove_liquidity;
 mod sell;
 
+mod imbalance;
 mod init_pool;
 pub(crate) mod mock;
 mod positions;

runtime/hydradx/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "hydradx-runtime"
-version = "126.0.0"
+version = "127.0.0"
 authors = ["GalacticCouncil"]
 edition = "2021"
 license = "Apache 2.0"

runtime/hydradx/src/lib.rs

@@ -97,7 +97,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	spec_name: create_runtime_str!("hydradx"),
 	impl_name: create_runtime_str!("hydradx"),
 	authoring_version: 1,
-	spec_version: 126,
+	spec_version: 127,
 	impl_version: 0,
 	apis: RUNTIME_API_VERSIONS,
 	transaction_version: 1,

runtime/hydradx/src/migrations.rs

@@ -19,6 +19,10 @@ impl OnRuntimeUpgrade for OnRuntimeUpgradeMigration {
 		weight = weight.saturating_add(pallet_omnipool::migration::migrate_to_v1::<Runtime, Omnipool>());
 		frame_support::log::info!("Migrate Omnipool Pallet end");
 
+		frame_support::log::info!("Migrate Omnipool Pallet to v2 start");
+		weight = weight.saturating_add(pallet_omnipool::migration::migrate_to_v2::<Runtime, Omnipool>());
+		frame_support::log::info!("Migrate Omnipool Pallet to v2 end");
+
 		weight
 	}
 

runtime/testing-hydradx/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "testing-hydradx-runtime"
-version = "126.0.0"
+version = "127.0.0"
 authors = ["GalacticCouncil"]
 edition = "2021"
 license = "Apache 2.0"