Benchmarks

bin/node/runtime/src/lib.rs

@@ -1496,6 +1496,7 @@ parameter_types! {
 	pub const ThawThrottle: (Perquintill, BlockNumber) = (Perquintill::from_percent(25), 5);
 	pub Target: Perquintill = Perquintill::zero();
 	pub const NisPalletId: PalletId = PalletId(*b"py/nis  ");
+	pub const NisReserveId: [u8; 8] = *b"py/nis  ";
 }
 
 impl pallet_nis::Config for Runtime {
@@ -1519,6 +1520,7 @@ impl pallet_nis::Config for Runtime {
 	type IntakePeriod = IntakePeriod;
 	type MaxIntakeWeight = MaxIntakeWeight;
 	type ThawThrottle = ThawThrottle;
+	type ReserveId = NisReserveId;
 }
 
 parameter_types! {

frame/nis/Cargo.toml

@@ -13,6 +13,7 @@ readme = "README.md"
 targets = ["x86_64-unknown-linux-gnu"]
 
 [dependencies]
+log = { version = "0.4.17", default-features = false, optional = true }
 codec = { package = "parity-scale-codec", version = "3.0.0", default-features = false, features = ["derive"] }
 scale-info = { version = "2.1.1", default-features = false, features = ["derive"] }
 frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" }
@@ -30,6 +31,7 @@ sp-io = { version = "7.0.0", path = "../../primitives/io" }
 [features]
 default = ["std"]
 std = [
+	"log/std",
 	"codec/std",
 	"frame-benchmarking?/std",
 	"frame-support/std",
@@ -41,6 +43,7 @@ std = [
 	"sp-std/std",
 ]
 runtime-benchmarks = [
+	"log",
 	"frame-benchmarking/runtime-benchmarks",
 	"frame-support/runtime-benchmarks",
 	"frame-system/runtime-benchmarks",

frame/nis/src/benchmarking.rs

@@ -21,7 +21,7 @@
 
 use super::*;
 use frame_benchmarking::{account, benchmarks, whitelisted_caller};
-use frame_support::traits::{Currency, EnsureOrigin, Get};
+use frame_support::traits::{nonfungible::Inspect, Currency, EnsureOrigin, Get};
 use frame_system::RawOrigin;
 use sp_arithmetic::Perquintill;
 use sp_runtime::{
@@ -106,6 +106,7 @@ benchmarks! {
 		T::Currency::make_free_balance_be(&caller, bid);
 		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), bid, 1)?;
 		Nis::<T>::process_queues(Perquintill::one(), 1, 1, &mut WeightCounter::unlimited());
+		Nis::<T>::communify(RawOrigin::Signed(caller.clone()).into(), 0)?;
 		let original = T::Currency::free_balance(&Nis::<T>::account_id());
 		T::Currency::make_free_balance_be(&Nis::<T>::account_id(), BalanceOf::<T>::min_value());
 	}: _<T::RuntimeOrigin>(origin)
@@ -116,7 +117,7 @@ benchmarks! {
 		assert!(missing <= Perquintill::one() / 100_000);
 	}
 
-	thaw {
+	thaw_private {
 		let caller: T::AccountId = whitelisted_caller();
 		T::Currency::make_free_balance_be(&caller, T::MinBid::get() * BalanceOf::<T>::from(3u32));
 		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
@@ -128,6 +129,42 @@ benchmarks! {
 		assert!(Receipts::<T>::get(0).is_none());
 	}
 
+	thaw_communal {
+		let caller: T::AccountId = whitelisted_caller();
+		T::Currency::make_free_balance_be(&caller, T::MinBid::get() * BalanceOf::<T>::from(3u32));
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::process_queues(Perquintill::one(), 1, 2, &mut WeightCounter::unlimited());
+		Receipts::<T>::mutate(0, |m_g| if let Some(ref mut g) = m_g { g.expiry = Zero::zero() });
+		Nis::<T>::communify(RawOrigin::Signed(caller.clone()).into(), 0)?;
+	}: _(RawOrigin::Signed(caller.clone()), 0)
+	verify {
+		assert!(Receipts::<T>::get(0).is_none());
+	}
+
+	privatize {
+		let caller: T::AccountId = whitelisted_caller();
+		T::Currency::make_free_balance_be(&caller, T::MinBid::get() * BalanceOf::<T>::from(3u32));
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::process_queues(Perquintill::one(), 1, 2, &mut WeightCounter::unlimited());
+		Nis::<T>::communify(RawOrigin::Signed(caller.clone()).into(), 0)?;
+	}: _(RawOrigin::Signed(caller.clone()), 0)
+	verify {
+		assert_eq!(Nis::<T>::owner(&0), Some(caller));
+	}
+
+	communify {
+		let caller: T::AccountId = whitelisted_caller();
+		T::Currency::make_free_balance_be(&caller, T::MinBid::get() * BalanceOf::<T>::from(3u32));
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::place_bid(RawOrigin::Signed(caller.clone()).into(), T::MinBid::get(), 1)?;
+		Nis::<T>::process_queues(Perquintill::one(), 1, 2, &mut WeightCounter::unlimited());
+	}: _(RawOrigin::Signed(caller.clone()), 0)
+	verify {
+		assert_eq!(Nis::<T>::owner(&0), None);
+	}
+
 	process_queues {
 		fill_queues::<T>()?;
 	}: {

frame/nis/src/lib.rs

@@ -660,7 +660,7 @@ pub mod pallet {
 		/// - `portion`: If `Some`, then only the given portion of the receipt should be thawed. If
 		///   `None`, then all of it should be.
 		#[pallet::call_index(3)]
-		#[pallet::weight(T::WeightInfo::thaw/*_private*/())]
+		#[pallet::weight(T::WeightInfo::thaw_private())]
 		pub fn thaw_private(
 			origin: OriginFor<T>,
 			#[pallet::compact] index: ReceiptIndex,
@@ -710,6 +710,15 @@ pub mod pallet {
 
 			let dropped = receipt.proportion.is_zero();
 
+			let reserved = T::Currency::reserved_balance_named(&T::ReserveId::get(), &who);
+			log::info!(
+				"{:?} {:?} {:?} {:?}",
+				amount,
+				on_hold,
+				reserved,
+				Self::issuance_with(&our_account, &summary)
+			);
+
 			if amount > on_hold {
 				T::Currency::unreserve_named(&T::ReserveId::get(), &who, on_hold);
 				let deficit = amount - on_hold;
@@ -719,18 +728,32 @@ pub mod pallet {
 				T::Currency::transfer(&our_account, &who, deficit, AllowDeath)
 					.map_err(|_| Error::<T>::Unfunded)?;
 			} else {
-				if dropped {
-					T::Currency::unreserve_named(&T::ReserveId::get(), &who, on_hold);
-					summary.receipts_on_hold -= on_hold;
-					let excess = on_hold - amount;
+				T::Currency::unreserve_named(&T::ReserveId::get(), &who, amount);
+				on_hold -= amount;
+				summary.receipts_on_hold -= amount;
+				if dropped && !on_hold.is_zero() {
+					// Reclaim any remainder:
 					// Transfer `excess` to the pot if we have now fully compensated for the
 					// receipt.
-					T::Currency::transfer(&who, &our_account, excess, AllowDeath)
-						.map_err(|_| Error::<T>::Unfunded)?;
-				} else {
-					T::Currency::unreserve_named(&T::ReserveId::get(), &who, amount);
-					on_hold -= amount;
-					summary.receipts_on_hold -= amount;
+					//
+					// This will legitimately fail if there is no pot account in existance.
+					// There's nothing we can do about this so we just swallow the error.
+					/*let _ = T::Currency::repatriate_reserved_named(
+						&T::ReserveId::get(),
+						&who,
+						&our_account,
+						excess,
+						BalanceStatus::Free,
+					).defensive();*/
+					T::Currency::unreserve_named(&T::ReserveId::get(), &who, on_hold);
+					// It could theoretically be locked, so really we should be using a more
+					// forceful variant. But the alternative `repatriate_reserved_named` will
+					// fail if the destination account doesn't exist. This should be fixed when
+					// we move to the `fungible::*` traits, which should include a force
+					// transfer function to transfer the reserved balance into free balance in
+					// the destination regardless of locks and create it if it doesn't exist.
+					let _ = T::Currency::transfer(&who, &Self::account_id(), on_hold, AllowDeath);
+					summary.receipts_on_hold -= on_hold;
 				}
 			}
 
@@ -756,15 +779,15 @@ pub mod pallet {
 		/// - `portion`: If `Some`, then only the given portion of the receipt should be thawed. If
 		///   `None`, then all of it should be.
 		#[pallet::call_index(4)]
-		#[pallet::weight(T::WeightInfo::thaw/*_communal*/())]
+		#[pallet::weight(T::WeightInfo::thaw_communal())]
 		pub fn thaw_communal(
 			origin: OriginFor<T>,
 			#[pallet::compact] index: ReceiptIndex,
 		) -> DispatchResult {
 			let who = ensure_signed(origin)?;
 
 			// Look for `index`
-			let mut receipt: ReceiptRecordOf<T> =
+			let receipt: ReceiptRecordOf<T> =
 				Receipts::<T>::get(index).ok_or(Error::<T>::UnknownReceipt)?;
 			// If found, check it is actually communal.
 			ensure!(receipt.owner.is_none(), Error::<T>::NotOwner);
@@ -773,47 +796,42 @@ pub mod pallet {
 
 			let mut summary: SummaryRecordOf<T> = Summary::<T>::get();
 
-			let proportion = receipt.proportion;
-
 			let (throttle, throttle_period) = T::ThawThrottle::get();
 			if now.saturating_sub(summary.last_period) >= throttle_period {
 				summary.thawed = Zero::zero();
 				summary.last_period = now;
 			}
-			summary.thawed.saturating_accrue(proportion);
+			summary.thawed.saturating_accrue(receipt.proportion);
 			ensure!(summary.thawed <= throttle, Error::<T>::Throttled);
 
-			T::Counterpart::burn_from(&who, T::CounterpartAmount::convert(proportion))?;
+			T::Counterpart::burn_from(&who, T::CounterpartAmount::convert(receipt.proportion))?;
 
 			// Multiply the proportion it is by the total issued.
 			let our_account = Self::account_id();
 			let effective_issuance = Self::issuance_with(&our_account, &summary).effective;
-			let amount = proportion * effective_issuance;
+			let amount = receipt.proportion * effective_issuance;
 
-			receipt.proportion.saturating_reduce(proportion);
-			summary.proportion_owed.saturating_reduce(proportion);
+			summary.proportion_owed.saturating_reduce(receipt.proportion);
 
-			let dropped = receipt.proportion.is_zero();
+			log::info!("{:?} {:?}", amount, Self::issuance_with(&our_account, &summary));
 
 			// Try to transfer amount owed from pot to receipt owner.
 			T::Currency::transfer(&our_account, &who, amount, AllowDeath)
 				.map_err(|_| Error::<T>::Unfunded)?;
 
-			if dropped {
-				Receipts::<T>::remove(index);
-			} else {
-				Receipts::<T>::insert(index, &receipt);
-			}
+			Receipts::<T>::remove(index);
 			Summary::<T>::put(&summary);
 
-			Self::deposit_event(Event::Thawed { index, who, amount, proportion, dropped });
+			let e =
+				Event::Thawed { index, who, amount, proportion: receipt.proportion, dropped: true };
+			Self::deposit_event(e);
 
 			Ok(())
 		}
 
 		/// Make a private receipt communal and create fungible counterparts for its owner.
 		#[pallet::call_index(5)]
-		#[pallet::weight(0/*T::WeightInfo::communify()*/)]
+		#[pallet::weight(T::WeightInfo::communify())]
 		pub fn communify(
 			origin: OriginFor<T>,
 			#[pallet::compact] index: ReceiptIndex,
@@ -852,7 +870,7 @@ pub mod pallet {
 
 		/// Make a private receipt communal and create fungible counterparts for its owner.
 		#[pallet::call_index(6)]
-		#[pallet::weight(0/*T::WeightInfo::privatize()*/)]
+		#[pallet::weight(T::WeightInfo::privatize())]
 		pub fn privatize(
 			origin: OriginFor<T>,
 			#[pallet::compact] index: ReceiptIndex,
@@ -896,7 +914,7 @@ pub mod pallet {
 	}
 
 	/// Issuance information returned by `issuance()`.
-	#[derive(RuntimeDebug)]
+	#[derive(Debug)]
 	pub struct IssuanceInfo<Balance> {
 		/// The balance held by this pallet instance together with the balances on hold across
 		/// all receipt-owning accounts.
@@ -906,8 +924,8 @@ pub mod pallet {
 		/// The effective total issuance, hypothetically if all outstanding receipts were thawed at
 		/// present.
 		pub effective: Balance,
-		/// The amount needed to be the pallet instance's account in case all outstanding receipts
-		/// were thawed at present.
+		/// The amount needed to be accessible to this pallet in case all outstanding receipts were
+		/// thawed at present. If it is more than `holdings`, then the pallet will need funding.
 		pub required: Balance,
 	}