cleanup code a bit (Phala-Network#41)

* cleanup code a bit

* add test target

* fix tests

Makefile

@@ -10,12 +10,18 @@ build-wasm: githooks
 check: githooks
 	SKIP_WASM_BUILD= cargo check
 
+check-tests: githooks
+	SKIP_WASM_BUILD= cargo check --tests --all
+
 check-debug:
 	RUSTFLAGS="-Z external-macro-backtrace" BUILD_DUMMY_WASM_BINARY= cargo +nightly check
 
 check-dummy:
 	BUILD_DUMMY_WASM_BINARY= cargo check
 
+test: githooks
+	SKIP_WASM_BUILD= cargo test
+
 build: githooks
 	SKIP_WASM_BUILD= cargo build
 

modules/auction_manager/src/lib.rs

@@ -161,7 +161,7 @@ impl<T: Trait> AuctionHandler<T::AccountId, T::Balance, T::BlockNumber, AuctionI
 						.and_then(|n| n.checked_div(&new_bid.1))
 						.unwrap_or(auction_item.amount);
 
-					let deduct_amount = auction_item.amount.checked_sub(&new_amount).unwrap_or(0.into());
+					let deduct_amount = auction_item.amount.checked_sub(&new_amount).unwrap_or_default();
 
 					// ensure have sufficient collateral in auction module
 					if Self::total_collateral_in_auction(auction_item.currency_id) >= deduct_amount {
@@ -215,7 +215,7 @@ impl<T: Trait> AuctionManager<T::AccountId> for Module<T> {
 	type Balance = T::Balance;
 
 	fn new_collateral_auction(
-		who: T::AccountId,
+		who: &T::AccountId,
 		currency_id: Self::CurrencyId,
 		amount: Self::Balance,
 		target: Self::Balance,

modules/auction_manager/src/tests.rs

@@ -17,7 +17,7 @@ fn set_maximum_auction_size_work() {
 #[test]
 fn new_collateral_auction_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		AuctionManagerModule::new_collateral_auction(ALICE, BTC, 10, 100, 90);
+		AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 10, 100, 90);
 		assert_eq!(CdpTreasury::debit_pool(), 90);
 		assert_eq!(AuctionManagerModule::total_collateral_in_auction(BTC), 10);
 		assert_eq!(Auction::auctions_count(), 1);
@@ -27,7 +27,7 @@ fn new_collateral_auction_work() {
 #[test]
 fn on_new_bid_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		AuctionManagerModule::new_collateral_auction(ALICE, BTC, 10, 100, 90);
+		AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 10, 100, 90);
 		assert_eq!(CdpTreasury::debit_pool(), 90);
 		assert_eq!(AuctionManagerModule::total_collateral_in_auction(BTC), 10);
 		assert_eq!(CdpTreasury::surplus_pool(), 0);
@@ -43,7 +43,7 @@ fn on_new_bid_work() {
 #[test]
 fn bid_when_soft_cap_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		AuctionManagerModule::new_collateral_auction(ALICE, BTC, 10, 100, 90);
+		AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 10, 100, 90);
 		assert_eq!(
 			AuctionManagerModule::on_new_bid(10, 0, (BOB, 5), None).auction_end,
 			Some(Some(110))
@@ -63,7 +63,7 @@ fn bid_when_soft_cap_work() {
 #[test]
 fn reverse_collateral_auction_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		AuctionManagerModule::new_collateral_auction(ALICE, BTC, 100, 200, 90);
+		AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 100, 200, 90);
 		assert_eq!(AuctionManagerModule::total_collateral_in_auction(BTC), 100);
 		assert_eq!(Tokens::balance(BTC, &ALICE), 1000);
 		assert_eq!(Tokens::balance(AUSD, &BOB), 1000);
@@ -90,7 +90,7 @@ fn reverse_collateral_auction_work() {
 #[test]
 fn on_auction_ended_work() {
 	ExtBuilder::default().build().execute_with(|| {
-		AuctionManagerModule::new_collateral_auction(ALICE, BTC, 100, 200, 90);
+		AuctionManagerModule::new_collateral_auction(&ALICE, BTC, 100, 200, 90);
 		assert_eq!(AuctionManagerModule::total_collateral_in_auction(BTC), 100);
 		assert_eq!(Tokens::balance(BTC, &BOB), 1000);
 		assert_eq!(Tokens::balance(AUSD, &BOB), 1000);

modules/cdp_engine/src/debit_exchange_rate_convertor.rs

@@ -8,11 +8,11 @@ where
 	T: Trait,
 {
 	fn convert(a: (CurrencyIdOf<T>, DebitBalanceOf<T>)) -> BalanceOf<T> {
-		let balance = TryInto::<BalanceOf<T>>::try_into(TryInto::<u128>::try_into(a.1).unwrap_or(u128::max_value()))
-			.unwrap_or(BalanceOf::<T>::max_value());
-		<Module<T>>::debit_exchange_rate(a.0)
+		let (currency_id, balance) = a;
+		let balance: u128 = balance.unique_saturated_into();
+		let balance: BalanceOf<T> = balance.unique_saturated_into();
+		<Module<T>>::debit_exchange_rate(currency_id)
 			.unwrap_or(T::DefaulDebitExchangeRate::get())
-			.checked_mul_int(&balance)
-			.unwrap_or(BalanceOf::<T>::max_value())
+			.saturating_mul_int(&balance)
 	}
 }

modules/cdp_engine/src/lib.rs

@@ -4,7 +4,7 @@ use frame_support::{decl_error, decl_event, decl_module, decl_storage, ensure, t
 use orml_traits::{arithmetic::Signed, MultiCurrency, MultiCurrencyExtended, PriceProvider};
 use orml_utilities::FixedU128;
 use rstd::{convert::TryInto, marker, prelude::*, result};
-use sp_runtime::traits::{Bounded, CheckedAdd, CheckedSub, Convert};
+use sp_runtime::traits::{CheckedAdd, CheckedSub, Convert, Saturating, UniqueSaturatedInto, Zero};
 use support::{AuctionManager, CDPTreasury, ExchangeRate, Price, Rate, Ratio, RiskManager};
 use system::ensure_root;
 
@@ -131,18 +131,20 @@ decl_module! {
 			// handle all kinds of collateral type
 			for currency_id in T::CollateralCurrencyIds::get() {
 				let debit_exchange_rate = Self::debit_exchange_rate(currency_id).unwrap_or(T::DefaulDebitExchangeRate::get());
-				let stability_fee_rate = Self::stability_fee(currency_id).unwrap_or(Rate::from_parts(0)).checked_add(&global_stability_fee).unwrap_or(Rate::max_value());
+				let stability_fee_rate = Self::stability_fee(currency_id)
+					.unwrap_or_default()
+					.saturating_add(global_stability_fee);
 				let total_debits = <vaults::Module<T>>::total_debits(currency_id);
-				if stability_fee_rate > Rate::from_parts(0) && total_debits > 0.into() {
-					let debit_exchange_rate_increment = debit_exchange_rate.checked_mul(&stability_fee_rate).unwrap_or(ExchangeRate::max_value());
+				if !stability_fee_rate.is_zero() && !total_debits.is_zero() {
+					let debit_exchange_rate_increment = debit_exchange_rate.saturating_mul(stability_fee_rate);
 
 					// update exchange rate
-					let new_debit_exchange_rate = debit_exchange_rate.checked_add(&debit_exchange_rate_increment).unwrap_or(ExchangeRate::max_value());
+					let new_debit_exchange_rate = debit_exchange_rate.saturating_add(debit_exchange_rate_increment);
 					<DebitExchangeRate<T>>::insert(currency_id, new_debit_exchange_rate);
 
 					// issue stablecoin to surplus pool
 					let total_debit_value = DebitExchangeRateConvertor::<T>::convert((currency_id, total_debits));
-					let issued_stable_coin_balance = debit_exchange_rate_increment.checked_mul_int(&total_debit_value).unwrap_or(BalanceOf::<T>::max_value());
+					let issued_stable_coin_balance = debit_exchange_rate_increment.saturating_mul_int(&total_debit_value);
 					T::Treasury::on_surplus(issued_stable_coin_balance);
 				}
 			}
@@ -157,15 +159,8 @@ impl<T: Trait> Module<T> {
 		debit_balance: DebitBalanceOf<T>,
 		price: Price,
 	) -> Ratio {
-		let locked_collateral_value = TryInto::<u128>::try_into(
-			price
-				.checked_mul_int(&collateral_balance)
-				.unwrap_or(BalanceOf::<T>::max_value()),
-		)
-		.unwrap_or(u128::max_value());
-		let debit_value =
-			TryInto::<u128>::try_into(DebitExchangeRateConvertor::<T>::convert((currency_id, debit_balance)))
-				.unwrap_or(u128::max_value());
+		let locked_collateral_value = price.saturating_mul_int(&collateral_balance);
+		let debit_value = DebitExchangeRateConvertor::<T>::convert((currency_id, debit_balance));
 
 		Ratio::from_rational(locked_collateral_value, debit_value)
 	}
@@ -177,7 +172,7 @@ impl<T: Trait> Module<T> {
 	}
 
 	pub fn update_position(
-		who: T::AccountId,
+		who: &T::AccountId,
 		currency_id: CurrencyIdOf<T>,
 		collateral_adjustment: AmountOf<T>,
 		debit_adjustment: DebitAmountOf<T>,
@@ -195,18 +190,14 @@ impl<T: Trait> Module<T> {
 	// TODO: how to trigger cdp liquidation
 	pub fn liquidate_unsafe_cdp(who: T::AccountId, currency_id: CurrencyIdOf<T>) -> result::Result<(), Error> {
 		let debit_balance = <vaults::Module<T>>::debits(&who, currency_id);
-		let collateral_balance: BalanceOf<T> = <vaults::Module<T>>::collaterals(&who, currency_id);
+		let collateral_balance = <vaults::Module<T>>::collaterals(&who, currency_id);
 
 		// ensure the cdp is unsafe
-		let feed_price = <T as Trait>::PriceSource::get_price(T::GetStableCurrencyId::get(), currency_id)
-			.ok_or(Error::InvalidFeedPrice)?;
+		let feed_price =
+			T::PriceSource::get_price(T::GetStableCurrencyId::get(), currency_id).ok_or(Error::InvalidFeedPrice)?;
 		let collateral_ratio =
 			Self::calculate_collateral_ratio(currency_id, collateral_balance, debit_balance, feed_price);
-		let liquidation_ratio = if let Some(ratio) = Self::liquidation_ratio(currency_id) {
-			ratio
-		} else {
-			T::DefaultLiquidationRatio::get()
-		};
+		let liquidation_ratio = Self::liquidation_ratio(currency_id).unwrap_or_else(T::DefaultLiquidationRatio::get);
 		ensure!(collateral_ratio < liquidation_ratio, Error::CollateralRatioStillSafe);
 
 		// grab collaterals and debits from unsafe cdp
@@ -221,21 +212,9 @@ impl<T: Trait> Module<T> {
 		let bad_debt = DebitExchangeRateConvertor::<T>::convert((currency_id, debit_balance));
 		let mut target = bad_debt;
 		if let Some(penalty_ratio) = Self::liquidation_penalty(currency_id) {
-			target = target
-				.checked_add(
-					&penalty_ratio
-						.checked_mul_int(&target)
-						.unwrap_or(BalanceOf::<T>::max_value()),
-				)
-				.unwrap_or(BalanceOf::<T>::max_value());
+			target = target.saturating_add(penalty_ratio.saturating_mul_int(&target));
 		}
-		T::AuctionManagerHandler::new_collateral_auction(
-			who.clone(),
-			currency_id,
-			collateral_balance,
-			target,
-			bad_debt,
-		);
+		T::AuctionManagerHandler::new_collateral_auction(&who, currency_id, collateral_balance, target, bad_debt);
 		Self::deposit_event(RawEvent::LiquidateUnsafeCdp(
 			currency_id,
 			who,

modules/cdp_engine/src/mock.rs

@@ -140,7 +140,7 @@ impl AuctionManager<AccountId> for MockAuctionManager {
 
 	#[allow(unused_variables)]
 	fn new_collateral_auction(
-		who: AccountId,
+		who: &AccountId,
 		currency_id: Self::CurrencyId,
 		amount: Self::Balance,
 		target: Self::Balance,

modules/cdp_engine/src/tests.rs

@@ -177,23 +177,23 @@ fn update_position_work() {
 			Some(10000),
 		));
 		assert_noop!(
-			CdpEngineModule::update_position(ALICE, ACA, 100, 50),
+			CdpEngineModule::update_position(&ALICE, ACA, 100, 50),
 			Error::NotValidCurrencyId,
 		);
 		assert_eq!(Currencies::balance(BTC, &ALICE), 1000);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 0);
 		assert_eq!(VaultsModule::debits(ALICE, BTC), 0);
 		assert_eq!(VaultsModule::collaterals(ALICE, BTC), 0);
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 100, 50));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 100, 50));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 50);
 		assert_eq!(VaultsModule::debits(ALICE, BTC), 50);
 		assert_eq!(VaultsModule::collaterals(ALICE, BTC), 100);
 		assert_noop!(
-			CdpEngineModule::update_position(ALICE, BTC, 0, 20),
+			CdpEngineModule::update_position(&ALICE, BTC, 0, 20),
 			Error::UpdatePositionFailed,
 		);
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 0, -20));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 0, -20));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 30);
 		assert_eq!(VaultsModule::debits(ALICE, BTC), 30);
@@ -213,12 +213,12 @@ fn remain_debit_value_too_small_check() {
 			Some(Some(Ratio::from_rational(9, 5))),
 			Some(10000),
 		));
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 100, 50));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 100, 50));
 		assert_noop!(
-			CdpEngineModule::update_position(ALICE, BTC, 0, -49),
+			CdpEngineModule::update_position(&ALICE, BTC, 0, -49),
 			Error::UpdatePositionFailed,
 		);
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, -100, -50));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, -100, -50));
 	});
 }
 
@@ -234,7 +234,7 @@ fn liquidate_unsafe_cdp_work() {
 			Some(Some(Ratio::from_rational(9, 5))),
 			Some(10000),
 		));
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 100, 50));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 100, 50));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 50);
 		assert_eq!(VaultsModule::debits(ALICE, BTC), 50);
@@ -294,7 +294,7 @@ fn on_finalize_work() {
 		CdpEngineModule::on_finalize(1);
 		assert_eq!(CdpEngineModule::debit_exchange_rate(BTC), None);
 		assert_eq!(CdpEngineModule::debit_exchange_rate(DOT), None);
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 100, 30));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 100, 30));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 30);
 		CdpEngineModule::on_finalize(2);
@@ -309,7 +309,7 @@ fn on_finalize_work() {
 			Some(ExchangeRate::from_rational(10201, 10000))
 		);
 		assert_eq!(CdpEngineModule::debit_exchange_rate(DOT), None);
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 0, -30));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 0, -30));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 0);
 		CdpEngineModule::on_finalize(4);

modules/dex/src/mock.rs

@@ -10,7 +10,6 @@ use super::*;
 
 mod dex {
 	pub use super::super::*;
-	use frame_support::impl_outer_event;
 }
 
 impl_outer_event! {
@@ -58,7 +57,6 @@ impl system::Trait for Runtime {
 	type AvailableBlockRatio = AvailableBlockRatio;
 	type Version = ();
 }
-pub type System = system::Module<Runtime>;
 
 impl orml_tokens::Trait for Runtime {
 	type Event = TestEvent;

modules/dex/src/tests.rs

@@ -4,7 +4,7 @@
 
 use super::*;
 use frame_support::{assert_noop, assert_ok};
-use mock::{DexModule, ExtBuilder, Origin, System, TestEvent, Tokens, ALICE, AUSD, BOB, BTC, CAROL, DOT};
+use mock::{DexModule, ExtBuilder, Origin, Tokens, ALICE, AUSD, BOB, BTC, CAROL, DOT};
 
 #[test]
 fn calculate_swap_target_amount_work() {

modules/honzon/src/lib.rs

@@ -76,7 +76,7 @@ decl_module! {
 		) {
 			let who = ensure_signed(origin).map_err(|_| Error::AccountUnSigned)?;
 
-			<cdp_engine::Module<T>>::update_position(who.clone(), currency_id, collateral, debit).map_err(|_| Error::UpdatePositionFailed)?;
+			<cdp_engine::Module<T>>::update_position(&who, currency_id, collateral, debit).map_err(|_| Error::UpdatePositionFailed)?;
 
 			Self::deposit_event(RawEvent::UpdateVault(who, currency_id, collateral, debit));
 		}

modules/honzon/src/mock.rs

@@ -137,7 +137,7 @@ impl AuctionManager<AccountId> for MockAuctionManager {
 
 	#[allow(unused_variables)]
 	fn new_collateral_auction(
-		who: AccountId,
+		who: &AccountId,
 		currency_id: Self::CurrencyId,
 		amount: Self::Balance,
 		target: Self::Balance,

modules/honzon/src/tests.rs

@@ -21,7 +21,7 @@ fn liquidate_unsafe_cdp_work() {
 			Some(Some(Ratio::from_rational(9, 5))),
 			Some(10000),
 		));
-		assert_ok!(CdpEngineModule::update_position(ALICE, BTC, 100, 50));
+		assert_ok!(CdpEngineModule::update_position(&ALICE, BTC, 100, 50));
 		assert_eq!(Currencies::balance(BTC, &ALICE), 900);
 		assert_eq!(Currencies::balance(AUSD, &ALICE), 50);
 		assert_eq!(VaultsModule::debits(ALICE, BTC), 50);

modules/support/src/lib.rs

@@ -25,7 +25,7 @@ pub trait AuctionManager<AccountId> {
 	type Balance;
 
 	fn new_collateral_auction(
-		who: AccountId,
+		who: &AccountId,
 		currency_id: Self::CurrencyId,
 		amount: Self::Balance,
 		target: Self::Balance,