chore: launch code review

README.md

@@ -94,6 +94,7 @@ Then open settings screen -> developer and paste
     "genesisHash": "Vec<u8>",
     "lastBlockHash": "Vec<u8>"
   },
+  "Currency": "AssetId",
   "CurrencyId": "AssetId",
   "CurrencyIdOf": "AssetId",
   "Intention": {
@@ -121,7 +122,6 @@ Then open settings screen -> developer and paste
   "TokenData": {
     "locked": "bool"
   },
-  "CID": "Vec<u8>",
   "ClassInfo": {
     "metadata": "Vec<u8>",
     "total_issuance": "TokenId",
@@ -144,7 +144,64 @@ Then open settings screen -> developer and paste
     "period_count": "u32",
     "per_period": "Compact<Balance>"
   },
-  "VestingScheduleOf": "VestingSchedule"
+  "VestingScheduleOf": "VestingSchedule",
+   "LBPAssetInfo": {
+    "id": "AssetId",
+    "amount": "Balance",
+    "initial_weight": "LBPWeight",
+    "final_weight": "LBPWeight"
+  },
+  "LBPWeight": "u128",
+  "WeightCurveType": {
+    "_enum": [
+      "Linear"
+    ]
+  },
+  "PoolId": "AccountId",
+  "BalanceOf": "Balance",
+  "AssetType": {
+    "_enum": [
+      "TOKEN"
+    ]
+  },
+  "Pool": {
+    "owner": "AccountId",
+    "start": "BlockNumber",
+    "end": "BlockNumber",
+    "assets": {
+      "asset_a": "AssetId",
+      "asset_b": "AssetId"
+    },
+    "initial_weights": {
+      "weight_a": "LBPWeight",
+      "weight_b": "LBPWeight"
+    },
+    "final_weights": {
+      "weight_a": "LBPWeight",
+      "weight_b": "LBPWeight"
+    },
+    "last_weight_update": "BlockNumber",
+    "last_weights": {
+      "weight_a": "LBPWeight",
+      "weight_b": "LBPWeight"
+    },
+    "weight_curve": "WeightCurveType",
+    "pausable": "bool",
+    "paused": "bool",
+    "fee": "Fee",
+    "fee_receiver": "AccountId"
+  },
+  "AssetType": {"_enum": ["Token"]},
+  "AssetNativeLocation":"MultiLocation",
+  "AssetDetails": {
+    "name": "Vec<u8>",
+    "asset_type": "AssetType",
+    "locked": "bool"
+  },
+  "AssetMetadata": {
+    "symbol":"Vec<u8>",
+    "decimals": "u8"
+  }
 }
 ```
 

pallets/asset-registry/src/lib.rs

@@ -107,7 +107,7 @@ pub mod pallet {
 	/// Details of an asset.
 	pub type Assets<T: Config> = StorageMap<
 		_,
-		Blake2_128Concat,
+		Twox64Concat,
 		T::AssetId,
 		AssetDetails<T::AssetId, BoundedVec<u8, T::StringLimit>>,
 		OptionQuery,
@@ -121,7 +121,7 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn asset_ids)]
 	/// Mapping between asset name and asset id.
-	pub type AssetIds<T: Config> = StorageMap<_, Twox64Concat, BoundedVec<u8, T::StringLimit>, T::AssetId, OptionQuery>;
+	pub type AssetIds<T: Config> = StorageMap<_, Blake2_128Concat, BoundedVec<u8, T::StringLimit>, T::AssetId, OptionQuery>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn locations)]
@@ -131,13 +131,13 @@ pub mod pallet {
 	#[pallet::storage]
 	#[pallet::getter(fn location_assets)]
 	/// Local asset for native location.
-	pub type LocationAssets<T: Config> = StorageMap<_, Twox64Concat, T::AssetNativeLocation, T::AssetId, OptionQuery>;
+	pub type LocationAssets<T: Config> = StorageMap<_, Blake2_128Concat, T::AssetNativeLocation, T::AssetId, OptionQuery>;
 
 	#[pallet::storage]
 	#[pallet::getter(fn asset_metadata)]
 	/// Metadata of an asset.
 	pub type AssetMetadataMap<T: Config> =
-		StorageMap<_, Blake2_128Concat, T::AssetId, AssetMetadata<BoundedVec<u8, T::StringLimit>>, OptionQuery>;
+		StorageMap<_, Twox64Concat, T::AssetId, AssetMetadata<BoundedVec<u8, T::StringLimit>>, OptionQuery>;
 
 	#[pallet::genesis_config]
 	pub struct GenesisConfig {
@@ -249,6 +249,8 @@ pub mod pallet {
 		/// Updates also mapping between name and asset id if provided name is different than currently registered.
 		///
 		/// Emits `Updated` event when successful.
+
+		// TODO: No tests
 		#[pallet::weight(<T as Config>::WeightInfo::update())]
 		#[transactional]
 		pub fn update(

pallets/asset-registry/src/tests.rs

@@ -24,6 +24,8 @@ use polkadot_xcm::v0::{Junction::*, MultiLocation::*};
 use primitives::AssetId;
 use sp_std::convert::TryInto;
 
+// TODO: Partial data tests
+// TODO: more unhappy tests -> empty strings, zeroes...
 #[test]
 fn register_asset_works() {
 	new_test_ext().execute_with(|| {
@@ -206,3 +208,86 @@ fn set_metadata_works() {
 			);
 		});
 }
+
+#[test]
+fn update_asset() {
+	new_test_ext().execute_with(|| {
+		let btc_asset_id: AssetId =
+			AssetRegistryPallet::get_or_create_asset(b"BTC".to_vec(), AssetType::Token).unwrap();
+
+		// set a new name and type for an existing asset
+		assert_ok!(AssetRegistryPallet::update(
+			Origin::root(),
+			btc_asset_id,
+			b"superBTC".to_vec(),
+			AssetType::Token
+		));
+
+		let new_btc_name: BoundedVec<u8, <Test as crate::Config>::StringLimit> =
+			b"superBTC".to_vec().try_into().unwrap();
+		assert_eq!(AssetRegistryPallet::asset_ids(new_btc_name).unwrap(), 1u32);
+
+		let usd_asset_id: AssetId =
+			AssetRegistryPallet::get_or_create_asset(b"USD".to_vec(), AssetType::Token).unwrap();
+
+		// cannot set existing name for an existing asset
+		assert_noop!(
+			(AssetRegistryPallet::update(Origin::root(), usd_asset_id, b"superBTC".to_vec(), AssetType::Token)),
+			Error::<Test>::AssetAlreadyRegistered
+		);
+
+		let usd_name: BoundedVec<u8, <Test as crate::Config>::StringLimit> = b"USD".to_vec().try_into().unwrap();
+
+		assert_eq!(AssetRegistryPallet::asset_ids(usd_name).unwrap(), 2u32);
+
+		let next_asset_id = AssetRegistryPallet::next_asset_id();
+
+		// cannot set a new name for a non-existent asset
+		assert_noop!(
+			(AssetRegistryPallet::update(Origin::root(), next_asset_id, b"VOID".to_vec(), AssetType::Token)),
+			Error::<Test>::AssetNotFound
+		);
+
+		let void_name: BoundedVec<u8, <Test as crate::Config>::StringLimit> = b"VOID".to_vec().try_into().unwrap();
+		assert_eq!(AssetRegistryPallet::asset_ids(void_name), None);
+
+		// corner case: change the name and also the type for an existing asset (token -> pool share)
+		assert_ok!(AssetRegistryPallet::update(
+			Origin::root(),
+			btc_asset_id,
+			b"BTCUSD".to_vec(),
+			AssetType::PoolShare(btc_asset_id, usd_asset_id)
+		));
+
+		let btcusd_name: BoundedVec<u8, <Test as crate::Config>::StringLimit> = b"BTCUSD".to_vec().try_into().unwrap();
+		assert_eq!(AssetRegistryPallet::asset_ids(btcusd_name.clone()).unwrap(), 1u32);
+		assert_eq!(
+			AssetRegistryPallet::assets(1u32).unwrap(),
+			AssetDetails {
+				name: btcusd_name,
+				asset_type: AssetType::PoolShare(btc_asset_id, usd_asset_id),
+				locked: false
+			}
+		);
+
+		// corner case: change the name and also the type for an existing asset (pool share -> token)
+		assert_ok!(AssetRegistryPallet::update(
+			Origin::root(),
+			btc_asset_id,
+			b"superBTC".to_vec(),
+			AssetType::Token
+		));
+
+		let superbtc_name: BoundedVec<u8, <Test as crate::Config>::StringLimit> =
+			b"superBTC".to_vec().try_into().unwrap();
+
+		assert_eq!(
+			AssetRegistryPallet::assets(1u32).unwrap(),
+			AssetDetails {
+				name: superbtc_name,
+				asset_type: AssetType::Token,
+				locked: false
+			}
+		);
+	});
+}

pallets/duster/src/benchmarking.rs

@@ -55,6 +55,24 @@ benchmarks! {
 		assert_eq!(T::MultiCurrency::free_balance(0u32.into(), &caller), reward);
 		assert_eq!(T::MultiCurrency::free_balance(1u32.into(), &crate::Pallet::<T>::dust_dest_account()), current_balance + dust_amount.try_into().ok().unwrap());
 	}
+
+	add_nondustable_account{
+		let caller = funded_account::<T>("caller", 0);
+		let nondustable_account = funded_account::<T>("dust", 0);
+	}: _(RawOrigin::Root, nondustable_account.clone())
+	verify {
+		assert!(crate::Pallet::<T>::blacklisted(&nondustable_account).is_some());
+	}
+
+	remove_nondustable_account{
+		let caller = funded_account::<T>("caller", 0);
+		let nondustable_account = funded_account::<T>("dust", 0);
+		let _ = crate::Pallet::<T>::add_nondustable_account(RawOrigin::Root.into(), nondustable_account.clone());
+
+	}: _(RawOrigin::Root, nondustable_account.clone())
+	verify {
+		assert!(crate::Pallet::<T>::blacklisted(&nondustable_account).is_none());
+	}
 }
 
 #[cfg(test)]
@@ -68,6 +86,8 @@ mod tests {
 	fn test_benchmarks() {
 		ExtBuilder::default().build().execute_with(|| {
 			assert_ok!(Pallet::<Test>::test_benchmark_dust_account());
+			assert_ok!(Pallet::<Test>::test_benchmark_add_nondustable_account());
+			assert_ok!(Pallet::<Test>::test_benchmark_remove_nondustable_account());
 		});
 	}
 }

pallets/duster/src/lib.rs

@@ -211,16 +211,6 @@ pub mod pallet {
 
 			Self::transfer_dust(&account, &Self::dust_dest_account(), currency_id, dust)?;
 
-			if currency_id == T::NativeCurrencyId::get() {
-				// When dusting native currency, in order to make sure that account is killed,
-				// we need to decrease providers count.
-				// It is due to the fact that the balances pallet has set its existential deposit to 0,
-				// therefore it will never do it.
-				//
-				// Not sure if we should fail here?
-				let _ = frame_system::Pallet::<T>::dec_providers(&account);
-			}
-
 			Self::deposit_event(Event::Dusted(account, dust));
 
 			// Ignore the result, it fails - no problem.
@@ -272,7 +262,7 @@ impl<T: Config> Pallet<T> {
 		(total < ed, total)
 	}
 
-	/// Send reward to account which ddid the dusting.
+	/// Send reward to account which did the dusting.
 	fn reward_duster(_duster: &T::AccountId, _currency_id: T::CurrencyId, _dust: T::Balance) -> DispatchResult {
 		let reserve_account = Self::reward_account();
 		let reward = T::Reward::get();

pallets/duster/src/mock.rs

@@ -1,7 +1,7 @@
 use crate as duster;
 
 use frame_support::parameter_types;
-use frame_support::traits::{Nothing, GenesisBuild, OnKilledAccount};
+use frame_support::traits::{GenesisBuild, Nothing, OnKilledAccount};
 
 use orml_currencies::BasicCurrencyAdapter;
 use orml_traits::parameter_type_with_key;
@@ -60,7 +60,7 @@ parameter_types! {
 	pub const SS58Prefix: u8 = 63;
 	pub const MaxLocks: u32 = 50;
 
-	pub const NativeExistentialDeposit: u128 = 0;
+	pub const NativeExistentialDeposit: u128 = 100;
 
 	pub NativeCurrencyId: AssetId = 0;
 	pub Reward: Balance = 10_000;

pallets/duster/src/tests.rs

@@ -51,20 +51,6 @@ fn dust_account_with_exact_dust_fails() {
 		});
 }
 
-#[test]
-fn dust_account_with_zero_fails() {
-	ExtBuilder::default()
-		.with_native_balance(*ALICE, 0)
-		.build()
-		.execute_with(|| {
-			assert_noop!(
-				Duster::dust_account(Origin::signed(*DUSTER), *ALICE, 0),
-				Error::<Test>::ZeroBalance
-			);
-			assert_eq!(Tokens::free_balance(1, &*TREASURY), 0);
-		});
-}
-
 #[test]
 fn dust_nonexisting_account_fails() {
 	ExtBuilder::default().build().execute_with(|| {
@@ -125,11 +111,11 @@ fn dust_account_native_works() {
 		}
 
 		expect_events(vec![
+			// system
+			frame_system::Event::KilledAccount(*ALICE).into(),
 			// dust transfer
 			pallet_balances::Event::Transfer(*ALICE, *TREASURY, 500).into(),
 			orml_currencies::Event::Transferred(currency_id, *ALICE, *TREASURY, 500).into(),
-			// system
-			frame_system::Event::KilledAccount(*ALICE).into(),
 			// duster
 			Event::Dusted(*ALICE, 500).into(),
 			//reward transfer