Fix offchain workers and add tests (#1409)

* fix offchain worker and add tests

* offchain worker tests wip

* finish cdp-engine test

* test max iterations

* remove dbg!

* add auction manager tests

* make None into DEFAULT_MAX_ITERATIONS

* add test for when max iterations is none

* key sets correctly for iterator

modules/auction-manager/src/lib.rs

@@ -352,7 +352,7 @@ impl<T: Config> Pallet<T> {
 		let max_iterations = StorageValueRef::persistent(OFFCHAIN_WORKER_MAX_ITERATIONS)
 			.get::<u32>()
 			.unwrap_or(Some(DEFAULT_MAX_ITERATIONS))
-			.ok_or(OffchainErr::OffchainStore)?;
+			.unwrap_or(DEFAULT_MAX_ITERATIONS);
 
 		log::debug!(
 			target: "auction-manager",
@@ -361,16 +361,16 @@ impl<T: Config> Pallet<T> {
 		);
 
 		// start iterations to cancel collateral auctions
-		let mut iterator = <CollateralAuctions<T>>::iter_from(start_key.ok_or(OffchainErr::OffchainStore)?);
+		let mut iterator = match start_key {
+			Some(key) => <CollateralAuctions<T>>::iter_from(key),
+			None => <CollateralAuctions<T>>::iter(),
+		};
+
 		let mut iteration_count = 0;
 		let mut finished = true;
 
 		#[allow(clippy::while_let_on_iterator)]
 		while let Some((collateral_auction_id, _)) = iterator.next() {
-			if iteration_count >= max_iterations {
-				finished = false;
-				break;
-			}
 			iteration_count += 1;
 
 			if let (Some(collateral_auction), Some((_, last_bid_price))) = (
@@ -380,10 +380,19 @@ impl<T: Config> Pallet<T> {
 				// if collateral auction has already been in reverse stage,
 				// should skip it.
 				if collateral_auction.in_reverse_stage(last_bid_price) {
+					if iteration_count == max_iterations {
+						finished = false;
+						break;
+					}
 					continue;
 				}
 			}
 			Self::submit_cancel_auction_tx(collateral_auction_id);
+
+			if iteration_count == max_iterations {
+				finished = false;
+				break;
+			}
 			guard.extend_lock().map_err(|_| OffchainErr::OffchainLock)?;
 		}
 

modules/auction-manager/src/mock.rs

@@ -287,4 +287,13 @@ impl ExtBuilder {
 
 		t.into()
 	}
+
+	pub fn lots_of_accounts() -> Self {
+		let mut balances = Vec::new();
+		for i in 0..1001 {
+			let account_id: AccountId = i;
+			balances.push((account_id, BTC, 1000));
+		}
+		Self { balances }
+	}
 }

modules/auction-manager/src/tests.rs

@@ -22,9 +22,20 @@
 
 use super::*;
 use frame_support::{assert_noop, assert_ok};
-use mock::{Event, *};
+use mock::{Call as MockCall, Event, *};
+use sp_core::offchain::{testing, DbExternalities, OffchainDbExt, OffchainWorkerExt, StorageKind, TransactionPoolExt};
+use sp_io::offchain;
 use sp_runtime::traits::One;
 
+fn run_to_block_offchain(n: u64) {
+	while System::block_number() < n {
+		System::set_block_number(System::block_number() + 1);
+		AuctionManagerModule::offchain_worker(System::block_number());
+		// this unlocks the concurrency storage lock so offchain_worker will fire next block
+		offchain::sleep_until(offchain::timestamp().add(Duration::from_millis(LOCK_DURATION + 200)));
+	}
+}
+
 #[test]
 fn get_auction_time_to_close_work() {
 	ExtBuilder::default().build().execute_with(|| {
@@ -471,3 +482,115 @@ fn cancel_collateral_auction_work() {
 		assert_eq!(bob_ref_count_1, bob_ref_count_0 - 1);
 	});
 }
+
+#[test]
+fn offchain_worker_cancels_auction_in_shutdown() {
+	let (offchain, _offchain_state) = testing::TestOffchainExt::new();
+	let (pool, pool_state) = testing::TestTransactionPoolExt::new();
+	let mut ext = ExtBuilder::default().build();
+	ext.register_extension(OffchainWorkerExt::new(offchain.clone()));
+	ext.register_extension(TransactionPoolExt::new(pool));
+	ext.register_extension(OffchainDbExt::new(offchain.clone()));
+
+	ext.execute_with(|| {
+		System::set_block_number(1);
+		assert_ok!(AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 10, 100));
+		assert!(AuctionManagerModule::collateral_auctions(0).is_some());
+		run_to_block_offchain(2);
+		// offchain worker does not have any tx because shutdown is false
+		assert!(!MockEmergencyShutdown::is_shutdown());
+		assert!(pool_state.write().transactions.pop().is_none());
+		mock_shutdown();
+		assert!(MockEmergencyShutdown::is_shutdown());
+
+		// now offchain worker will cancel auction as shutdown is true
+		run_to_block_offchain(3);
+		let tx = pool_state.write().transactions.pop().unwrap();
+		let tx = Extrinsic::decode(&mut &*tx).unwrap();
+		if let MockCall::AuctionManagerModule(crate::Call::cancel(auction_id)) = tx.call {
+			assert_ok!(AuctionManagerModule::cancel(Origin::none(), auction_id));
+		}
+
+		// auction is canceled
+		assert!(AuctionManagerModule::collateral_auctions(0).is_none());
+		assert!(pool_state.write().transactions.pop().is_none());
+	});
+}
+
+#[test]
+fn offchain_worker_max_iterations_check() {
+	let (mut offchain, _offchain_state) = testing::TestOffchainExt::new();
+	let (pool, pool_state) = testing::TestTransactionPoolExt::new();
+	let mut ext = ExtBuilder::default().build();
+	ext.register_extension(OffchainWorkerExt::new(offchain.clone()));
+	ext.register_extension(TransactionPoolExt::new(pool));
+	ext.register_extension(OffchainDbExt::new(offchain.clone()));
+
+	ext.execute_with(|| {
+		System::set_block_number(1);
+		// sets max iterations value to 1
+		offchain.local_storage_set(StorageKind::PERSISTENT, OFFCHAIN_WORKER_MAX_ITERATIONS, &1u32.encode());
+		assert_ok!(AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 10, 100));
+		assert_ok!(AuctionManagerModule::new_collateral_auction(&BOB, BTC, 10, 100));
+		assert!(AuctionManagerModule::collateral_auctions(1).is_some());
+		assert!(AuctionManagerModule::collateral_auctions(0).is_some());
+		mock_shutdown();
+		assert!(MockEmergencyShutdown::is_shutdown());
+
+		run_to_block_offchain(2);
+		// now offchain worker will cancel one auction but the other one will cancel next block
+		let tx = pool_state.write().transactions.pop().unwrap();
+		let tx = Extrinsic::decode(&mut &*tx).unwrap();
+		if let MockCall::AuctionManagerModule(crate::Call::cancel(auction_id)) = tx.call {
+			assert_ok!(AuctionManagerModule::cancel(Origin::none(), auction_id));
+		}
+		assert!(
+			AuctionManagerModule::collateral_auctions(1).is_some()
+				|| AuctionManagerModule::collateral_auctions(0).is_some()
+		);
+		// only one auction canceled so offchain tx pool is empty
+		assert!(pool_state.write().transactions.pop().is_none());
+
+		run_to_block_offchain(3);
+		// now offchain worker will cancel the next auction
+		let tx = pool_state.write().transactions.pop().unwrap();
+		let tx = Extrinsic::decode(&mut &*tx).unwrap();
+		if let MockCall::AuctionManagerModule(crate::Call::cancel(auction_id)) = tx.call {
+			assert_ok!(AuctionManagerModule::cancel(Origin::none(), auction_id));
+		}
+		assert!(AuctionManagerModule::collateral_auctions(1).is_none());
+		assert!(AuctionManagerModule::collateral_auctions(0).is_none());
+		assert!(pool_state.write().transactions.pop().is_none());
+	});
+}
+
+#[test]
+fn offchain_default_max_iterator_works() {
+	let (mut offchain, _offchain_state) = testing::TestOffchainExt::new();
+	let (pool, pool_state) = testing::TestTransactionPoolExt::new();
+	let mut ext = ExtBuilder::lots_of_accounts().build();
+	ext.register_extension(OffchainWorkerExt::new(offchain.clone()));
+	ext.register_extension(TransactionPoolExt::new(pool));
+	ext.register_extension(OffchainDbExt::new(offchain.clone()));
+
+	ext.execute_with(|| {
+		System::set_block_number(1);
+		// checks that max iterations is stored as none
+		assert!(offchain
+			.local_storage_get(StorageKind::PERSISTENT, OFFCHAIN_WORKER_MAX_ITERATIONS)
+			.is_none());
+
+		for i in 0..1001 {
+			let account_id: AccountId = i;
+			assert_ok!(AuctionManagerModule::new_collateral_auction(&account_id, BTC, 1, 10));
+		}
+
+		mock_shutdown();
+		run_to_block_offchain(2);
+		// should only run 1000 iterations stopping due to DEFAULT_MAX_ITERATION
+		assert_eq!(pool_state.write().transactions.len(), 1000);
+		run_to_block_offchain(3);
+		// next block iterator starts where it left off and adds the final account to tx pool
+		assert_eq!(pool_state.write().transactions.len(), 1001);
+	});
+}

modules/cdp-engine/src/lib.rs

@@ -649,24 +649,24 @@ impl<T: Config> Pallet<T> {
 		let max_iterations = StorageValueRef::persistent(OFFCHAIN_WORKER_MAX_ITERATIONS)
 			.get::<u32>()
 			.unwrap_or(Some(DEFAULT_MAX_ITERATIONS))
-			.ok_or(OffchainErr::OffchainStore)?;
+			.unwrap_or(DEFAULT_MAX_ITERATIONS);
 
 		let currency_id = collateral_currency_ids[collateral_position as usize];
 		let is_shutdown = T::EmergencyShutdown::is_shutdown();
-		let mut map_iterator =
-			<loans::Positions<T>>::iter_prefix_from(currency_id, start_key.clone().ok_or(OffchainErr::OffchainStore)?);
+
+		// If start key is Some(value) continue iterating from that point in storage otherwise start
+		// iterating from the beginning of <loans::Positons<T>>
+		let mut map_iterator = match start_key.clone() {
+			Some(key) => <loans::Positions<T>>::iter_prefix_from(currency_id, key),
+			None => <loans::Positions<T>>::iter_prefix(currency_id),
+		};
 
 		let mut finished = true;
 		let mut iteration_count = 0;
 		let iteration_start_time = sp_io::offchain::timestamp();
 
 		#[allow(clippy::while_let_on_iterator)]
 		while let Some((who, Position { collateral, debit })) = map_iterator.next() {
-			if iteration_count >= max_iterations {
-				finished = false;
-				break;
-			}
-
 			if !is_shutdown
 				&& matches!(
 					Self::check_cdp_status(currency_id, collateral, debit),
@@ -680,7 +680,10 @@ impl<T: Config> Pallet<T> {
 			}
 
 			iteration_count += 1;
-
+			if iteration_count == max_iterations {
+				finished = false;
+				break;
+			}
 			// extend offchain worker lock
 			guard.extend_lock().map_err(|_| OffchainErr::OffchainLock)?;
 		}

modules/cdp-engine/src/mock.rs

@@ -380,4 +380,13 @@ impl ExtBuilder {
 
 		t.into()
 	}
+
+	pub fn lots_of_accounts() -> Self {
+		let mut balances = Vec::new();
+		for i in 0..1001 {
+			let account_id: AccountId = i;
+			balances.push((account_id, BTC, 1000));
+		}
+		Self { balances }
+	}
 }