Developed get_afloat_asset_id method in the functions module of the afloat pallet, and updated all clients using it. Added AfloatAssetNotSet error to the afloat pallet. Developed TestExternalitiesBuilder in the mock module of the afloat pallet to enable the configuration of the test setup, and updated all tests to use it. Developed test in afloat pallet to verify that an error is returned when calling an extrinsic and the afloat asset is not set. Developed set_xpub method of the bitcoin vaults pallet to ensure that the xpub is valid. Developed test in bitcoin vaults pallet to assert that an error is returned when an invalid xpub is set.

Author: sebastianmontero

pallets/afloat/src/functions.rs

@@ -17,12 +17,19 @@ use sp_runtime::{
 };
 
 impl<T: Config> Pallet<T> {
+	fn get_afloat_asset_id() -> Result<T::AssetId, Error<T>> {
+		let asset_id = AfloatAssetId::<T>::get();
+		match asset_id {
+			Some(id) => Ok(id),
+			None => Err(Error::<T>::AfloatAssetNotSet),
+		}
+	}
 	// ! Setup pallet
 	pub fn do_create_afloat_marketplace(
 		creator: T::AccountId,
 		admin: T::AccountId,
 	) -> DispatchResult {
-		let asset_id = AfloatAssetId::<T>::get().expect("AfloatAssetId should be set");
+		let asset_id = Self::get_afloat_asset_id()?;
 
 		let label: BoundedVec<u8, T::LabelMaxLen> =
 			BoundedVec::try_from(b"Afloat".to_vec()).map_err(|_| Error::<T>::LabelTooLong)?;
@@ -301,10 +308,10 @@ impl<T: Config> Pallet<T> {
 		amount: T::Balance,
 	) -> DispatchResult {
 		let authority = ensure_signed(origin.clone())?;
-		let asset_id = AfloatAssetId::<T>::get().expect("AfloatAssetId should be set");
+		let asset_id = Self::get_afloat_asset_id()?;
 		ensure!(UserInfo::<T>::contains_key(user_address.clone()), Error::<T>::UserNotFound);
 
-		let current_balance = Self::do_get_afloat_balance(user_address.clone());
+		let current_balance = Self::do_get_afloat_balance(user_address.clone())?;
 		if current_balance > amount {
 			let diff = current_balance - amount;
 			pallet_mapped_assets::Pallet::<T>::afloat_do_burn(
@@ -327,9 +334,9 @@ impl<T: Config> Pallet<T> {
 		Ok(())
 	}
 
-	pub fn do_get_afloat_balance(user_address: T::AccountId) -> T::Balance {
-		let asset_id = AfloatAssetId::<T>::get().expect("AfloatAssetId should be set");
-		pallet_mapped_assets::Pallet::<T>::balance(asset_id.into(), user_address)
+	pub fn do_get_afloat_balance(user_address: T::AccountId) -> Result<T::Balance, Error<T>> {
+		let asset_id = Self::get_afloat_asset_id()?;
+		Ok(pallet_mapped_assets::Pallet::<T>::balance(asset_id.into(), user_address))
 	}
 
 	pub fn do_create_sell_order(
@@ -468,7 +475,7 @@ impl<T: Config> Pallet<T> {
 		);
 		//ensure user has enough afloat balance
 		ensure!(
-			Self::do_get_afloat_balance(who.clone()) >=
+			Self::do_get_afloat_balance(who.clone())? >=
 				offer.price_per_credit * tax_credit_amount.into(),
 			Error::<T>::NotEnoughAfloatBalanceAvailable
 		);

pallets/afloat/src/lib.rs

@@ -148,6 +148,8 @@ pub mod pallet {
 		Underflow,
 		// Afloat marketplace label too long
 		LabelTooLong,
+		// Afloat asset has not been set
+		AfloatAssetNotSet,
 	}
 
 	#[pallet::storage]

pallets/afloat/src/mock.rs

@@ -239,6 +239,56 @@ impl pallet_mapped_assets::Config for Test {
 	type BenchmarkHelper = ();
 }
 
+pub struct TestExternalitiesBuilder {
+	test_externalities: sp_io::TestExternalities,
+}
+
+impl TestExternalitiesBuilder {
+	pub fn new() -> Self {
+		TestExternalitiesBuilder {
+			test_externalities: frame_system::GenesisConfig::<Test>::default()
+				.build_storage()
+				.unwrap()
+				.into(),
+		}
+	}
+
+	pub fn setup_balances(mut self) -> Self {
+		self.test_externalities.execute_with(|| {
+			Balances::make_free_balance_be(&1, 100);
+			Balances::make_free_balance_be(&2, 100);
+		});
+		self
+	}
+
+	pub fn initialize_all(self) -> Self {
+		let mut builder = self.setup_balances();
+		builder.test_externalities.execute_with(|| {
+			let args = InitialSetupArgs::All {
+				creator: 1,
+				admin: 2,
+				asset: CreateAsset::New { asset_id: 0, min_balance: 1 },
+			};
+			Afloat::initial_setup(RawOrigin::Root.into(), args)
+				.expect("Error configuring initial setup");
+		});
+		builder
+	}
+
+	pub fn initialize_roles(self) -> Self {
+		let mut builder = self.setup_balances();
+		builder.test_externalities.execute_with(|| {
+			let args = InitialSetupArgs::Roles { creator: 1, admin: 2 };
+			Afloat::initial_setup(RawOrigin::Root.into(), args).expect("Error configuring roles");
+		});
+		builder
+	}
+
+	pub fn build(self) -> sp_io::TestExternalities {
+		return self.test_externalities
+	}
+}
+
 // Build genesis storage according to the mock runtime.
 pub fn new_test_ext() -> sp_io::TestExternalities {
 	// TODO: get initial conf?

pallets/afloat/src/tests.rs

@@ -18,7 +18,7 @@ fn dummy_description() -> BoundedVec<u8, StringLimit> {
 
 #[test]
 fn sign_up_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		Balances::make_free_balance_be(&user, 100);
 		let args = SignUpArgs::BuyerOrSeller {
@@ -35,7 +35,7 @@ fn sign_up_works() {
 
 #[test]
 fn update_user_info_edit_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		Balances::make_free_balance_be(&user, 100);
 
@@ -66,7 +66,7 @@ fn update_user_info_edit_works() {
 
 #[test]
 fn update_other_user_info_by_not_admin_fails() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 
@@ -99,7 +99,7 @@ fn update_other_user_info_by_not_admin_fails() {
 
 #[test]
 fn update_other_user_info_by_admin_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let owner = new_account(1);
 		let admin = new_account(2);
 		let user = new_account(3);
@@ -137,7 +137,7 @@ fn update_other_user_info_by_admin_works() {
 
 #[test]
 fn update_user_info_delete_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		Balances::make_free_balance_be(&user, 100);
 
@@ -161,7 +161,7 @@ fn update_user_info_delete_works() {
 
 #[test]
 fn set_afloat_balance_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 
@@ -177,15 +177,15 @@ fn set_afloat_balance_works() {
 		assert_ok!(Afloat::sign_up(RawOrigin::Signed(user.clone()).into(), args.clone()));
 
 		assert_ok!(Afloat::set_afloat_balance(RawOrigin::Signed(1).into(), user.clone(), 10000));
-		assert_eq!(Afloat::do_get_afloat_balance(user.clone()), 10000);
+		assert_eq!(Afloat::do_get_afloat_balance(user.clone()).unwrap(), 10000);
 		assert_ok!(Afloat::set_afloat_balance(RawOrigin::Signed(1).into(), user.clone(), 1000));
-		assert_eq!(Afloat::do_get_afloat_balance(user.clone()), 1000);
+		assert_eq!(Afloat::do_get_afloat_balance(user.clone()).unwrap(), 1000);
 	});
 }
 
 #[test]
 fn set_balance_by_other_than_owner_fails() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 
@@ -211,9 +211,20 @@ fn set_balance_by_other_than_owner_fails() {
 	});
 }
 
+#[test]
+fn set_balance_without_initializing_afloat_asset_fails() {
+	TestExternalitiesBuilder::new().initialize_roles().build().execute_with(|| {
+		let other_user = new_account(4);
+		assert_noop!(
+			Afloat::set_afloat_balance(RawOrigin::Signed(1).into(), other_user.clone(), 10000),
+			Error::<Test>::AfloatAssetNotSet
+		);
+	});
+}
+
 #[test]
 fn create_tax_credit_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 
@@ -240,7 +251,7 @@ fn create_tax_credit_works() {
 
 #[test]
 fn create_sell_order_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 		let item_id = 0;
@@ -277,7 +288,7 @@ fn create_sell_order_works() {
 
 #[test]
 fn create_buy_order_works() {
-	new_test_ext().execute_with(|| {
+	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 		let user = new_account(3);
 		let other_user = new_account(4);
 		let item_id = 0;
@@ -317,7 +328,7 @@ fn create_buy_order_works() {
 
 // #[test]
 // fn take_buy_order_works() {
-// 	new_test_ext().execute_with(|| {
+// 	TestExternalitiesBuilder::new().initialize_all().build().execute_with(|| {
 // 		let user = new_account(3);
 // 		let other_user = new_account(4);
 // 		let item_id = 0;

pallets/bitcoin-vaults/src/lib.rs

@@ -184,6 +184,8 @@ pub mod pallet {
 		ProofNotFound,
 		/// The proposal/proof of reserve was already previously broadcasted
 		AlreadyBroadcasted,
+		/// The provided xpub is invalid
+		InvalidXpub,
 	}
 
 	/* --- Onchain storage section --- */
@@ -352,6 +354,7 @@ pub mod pallet {
 			// Check that the extrinsic was signed and get the signer.
 			let who = ensure_signed(origin.clone())?;
 			ensure!(xpub.len() > 0, Error::<T>::NoneValue);
+			ensure!(str::from_utf8(&xpub).is_ok(), Error::<T>::InvalidXpub);
 			ensure!(!<XpubsByOwner<T>>::contains_key(who.clone()), Error::<T>::UserAlreadyHasXpub);
 			let manual_hash = xpub.clone().using_encoded(blake2_256);
 			// Assert if the input xpub is free to take (or if the user owns it)

pallets/bitcoin-vaults/src/tests.rs

@@ -96,6 +96,19 @@ fn inserting_same_xpub_should_fail() {
 	});
 }
 
+#[test]
+fn inserting_invalid_xpub_should_fail() {
+	new_test_ext().execute_with(|| {
+		assert_noop!(
+			BitcoinVaults::set_xpub(
+				RuntimeOrigin::signed(test_pub(2)),
+				BoundedVec::try_from(vec![192, 175]).unwrap()
+			),
+			Error::<Test>::InvalidXpub
+		);
+	});
+}
+
 #[test]
 fn inserting_without_removing_xpub_should_fail() {
 	new_test_ext().execute_with(|| {