[fix] Handle expired buy orders

.github/workflows/ci.yml

@@ -87,7 +87,7 @@ jobs:
           target: wasm32-unknown-unknown
 
       - name: Unit tests
-        run: cargo test -p bitgreen-parachain --features runtime-benchmarks
+        run: cargo test -p bitgreen-parachain
 
   build-docker-image:
     # The type of runner that the job will run on

pallets/carbon-credits-pool/src/mock.rs

@@ -142,6 +142,7 @@ impl pallet_carbon_credits::Config for Test {
 	type ForceOrigin = frame_system::EnsureRoot<u64>;
 	type ItemId = u32;
 	type ProjectId = u32;
+	type MaxCoordinatesLength = ConstU32<8>;
 	type GroupId = u32;
 	type KYCProvider = KYCMembership;
 	type MarketplaceEscrow = MarketplaceEscrowAccount;

pallets/dex/Cargo.toml

@@ -15,6 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = [
 	"derive",
 ] }
+log = { version = "0.4.17", default-features = false }
 scale-info = { version = "2.1.1", default-features = false, features = ["derive"] }
 frame-benchmarking = { default-features = false, optional = true,  git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.33"  }
 frame-support = { default-features = false,  git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.33" }
@@ -47,7 +48,8 @@ std = [
 	"pallet-balances/std",
 	"primitives/std",
 	"orml-tokens/std",
-	"pallet-carbon-credits/std"
+	"pallet-carbon-credits/std",
+	"log/std"
 ]
 runtime-benchmarks = ["frame-benchmarking/runtime-benchmarks"]
 try-runtime = ["frame-support/try-runtime"]

pallets/dex/src/lib.rs

@@ -258,6 +258,63 @@ pub mod pallet {
 		ChainIdMismatch,
 		TxProofMismatch,
 		KYCAuthorisationFailed,
+		DuplicateValidation,
+	}
+
+	#[pallet::hooks]
+	impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
+		// Look for expired buy orders and remove from storage
+		fn on_idle(block: T::BlockNumber, remaining_weight: Weight) -> Weight {
+			let mut remaining_weight = remaining_weight;
+			for (key, buy_order) in BuyOrders::<T>::iter() {
+				remaining_weight = remaining_weight.saturating_sub(T::DbWeight::get().reads(1));
+				if buy_order.expiry_time < block {
+					// log the start of removal
+					log::info!(
+						target: "runtime::dex",
+						"INFO: Found expired buy order, going to remove buy_order_id: {}",
+						key
+					);
+					BuyOrders::<T>::take(key);
+					remaining_weight =
+						remaining_weight.saturating_sub(T::DbWeight::get().writes(1));
+					// add the credits to the sell order
+					let sell_order_updated = Orders::<T>::try_mutate(
+						buy_order.order_id,
+						|maybe_order| -> DispatchResult {
+							let order = maybe_order.as_mut().ok_or(Error::<T>::InvalidOrderId)?;
+							order.units = order
+								.units
+								.checked_add(&buy_order.units)
+								.ok_or(Error::<T>::OrderUnitsOverflow)?;
+							Ok(())
+						},
+					);
+
+					if sell_order_updated.is_err() {
+						log::warn!(
+							target: "runtime::dex",
+							"WARNING: Sell order units not credited back for buy_order_id: {}",
+							key
+						);
+					}
+
+					log::info!(
+						target: "runtime::dex",
+						"INFO: Removed Expired buy order with buy_order_id: {}",
+						key
+					);
+
+					// exit since we altered the map
+					break
+				}
+
+				if remaining_weight.all_lte(T::DbWeight::get().reads(1)) {
+					break
+				}
+			}
+			remaining_weight
+		}
 	}
 
 	#[pallet::call]
@@ -492,6 +549,11 @@ pub mod pallet {
 				if let Some(mut payment_info) = payment_info {
 					ensure!(payment_info.chain_id == chain_id, Error::<T>::ChainIdMismatch);
 					ensure!(payment_info.tx_proof == tx_proof, Error::<T>::TxProofMismatch);
+					ensure!(
+						!payment_info.validators.contains(&sender),
+						Error::<T>::DuplicateValidation
+					);
+
 					payment_info
 						.validators
 						.try_push(sender.clone())
@@ -516,8 +578,6 @@ pub mod pallet {
 							false,
 						)?;
 
-						// TODO : transfer native currency to user
-
 						Self::deposit_event(Event::BuyOrderCompleted { order_id });
 
 						// remove from storage if we reached the threshold and payment executed
@@ -530,7 +590,8 @@ pub mod pallet {
 				}
 				// else if paymentInfo is empty create it
 				else {
-					let mut validators: BoundedVec<T::AccountId, T::MaxValidators> = Default::default();
+					let mut validators: BoundedVec<T::AccountId, T::MaxValidators> =
+						Default::default();
 					validators
 						.try_push(sender.clone())
 						.map_err(|_| Error::<T>::TooManyValidatorAccounts)?;

pallets/dex/src/mock.rs

@@ -167,7 +167,7 @@ parameter_types! {
 	#[derive(Clone, scale_info::TypeInfo)]
 	pub const MaxTxHashLen : u32 = 100;
 	#[derive(Clone, scale_info::TypeInfo)]
-	pub const BuyOrderExpiryTime : u32 = 10;
+	pub const BuyOrderExpiryTime : u32 = 2;
 }
 
 impl pallet_dex::Config for Test {

pallets/dex/src/tests.rs

@@ -2,7 +2,9 @@
 // Copyright (C) 2022 BitGreen.
 // This code is licensed under MIT license (see LICENSE.txt for details)
 use crate::{mock::*, BuyOrders, Error, Event, Orders};
-use frame_support::{assert_noop, assert_ok, BoundedVec, PalletId};
+use frame_support::{
+	assert_noop, assert_ok, traits::OnIdle, weights::Weight, BoundedVec, PalletId,
+};
 use frame_system::RawOrigin;
 use sp_runtime::{traits::AccountIdConversion, Percent};
 
@@ -426,12 +428,23 @@ fn payment_is_processed_after_validator_threshold_reached() {
 			Event::BuyOrderPaymentValidated { order_id: 0, chain_id: 0u32, validator }.into()
 		);
 
+		//	same validator cannot validate again
+		assert_noop!(
+			Dex::validate_buy_order(
+				RuntimeOrigin::signed(validator),
+				buy_order_id,
+				0u32,
+				tx_proof.clone()
+			),
+			Error::<Test>::DuplicateValidation
+		);
+
 		//	next validator validates
 		assert_ok!(Dex::validate_buy_order(
 			RuntimeOrigin::signed(validator_two),
 			buy_order_id,
 			0u32,
-			tx_proof.clone()
+			tx_proof
 		));
 
 		// buy order storage should be cleared since payment is done
@@ -536,3 +549,57 @@ fn cannot_set_more_than_max_fee() {
 // 		assert_eq!(Tokens::free_balance(USDT, &dex_account), 775);
 // 	});
 // }
+
+#[test]
+fn buy_order_handle_expiry_should_work() {
+	new_test_ext().execute_with(|| {
+		let asset_id = 0;
+		let seller = 1;
+		let buyer = 4;
+		let validator = 10;
+
+		assert_ok!(Assets::force_create(RuntimeOrigin::root(), asset_id, 1, true, 1));
+		assert_ok!(Assets::mint(RuntimeOrigin::signed(seller), asset_id, 1, 100));
+		assert_eq!(Assets::balance(asset_id, seller), 100);
+
+		// should be able to create a sell order
+		assert_ok!(Dex::create_sell_order(RuntimeOrigin::signed(seller), asset_id, 5, 10));
+
+		add_validator_account(validator);
+
+		// use should be able to purchase
+		assert_ok!(Dex::create_buy_order(
+			RuntimeOrigin::signed(validator),
+			buyer,
+			0,
+			asset_id,
+			1,
+			11
+		));
+
+		// sell order storage should be updated correctly
+		let sell_order_storage = Orders::<Test>::get(0).unwrap();
+		assert_eq!(sell_order_storage.owner, seller);
+		assert_eq!(sell_order_storage.units, 4);
+		assert_eq!(sell_order_storage.price_per_unit, 10);
+		assert_eq!(sell_order_storage.asset_id, asset_id);
+
+		// buy order storage should be updated correctly
+		let buy_order_storage = BuyOrders::<Test>::get(0).unwrap();
+		assert_eq!(buy_order_storage.buyer, buyer);
+		assert_eq!(buy_order_storage.units, 1);
+		assert_eq!(buy_order_storage.expiry_time, 3);
+
+		Dex::on_idle(5, Weight::MAX);
+
+		// the order should be cleared
+		assert!(BuyOrders::<Test>::get(0).is_none());
+
+		// sell order storage should be restored correctly
+		let sell_order_storage = Orders::<Test>::get(0).unwrap();
+		assert_eq!(sell_order_storage.owner, seller);
+		assert_eq!(sell_order_storage.units, 5);
+		assert_eq!(sell_order_storage.price_per_unit, 10);
+		assert_eq!(sell_order_storage.asset_id, asset_id);
+	});
+}

pallets/kyc/src/lib.rs

@@ -276,8 +276,8 @@ pub mod pallet {
 				<Members<T, I>>::put(&members);
 
 				T::MembershipChanged::change_members_sorted(
-					&[new.clone()],
-					&[remove.clone()],
+					&[new],
+					&[remove],
 					&members[..],
 				);
 			}