fix: debtor path in 'send_charging_customers_batch' call is covered

pallets/ddc-customers/src/lib.rs

@@ -18,7 +18,7 @@ use codec::{Decode, Encode, HasCompact};
 use ddc_primitives::{BucketId, ClusterId};
 use ddc_traits::{
 	cluster::{ClusterCreator, ClusterVisitor},
-	customer::{CustomerCharger, CustomerChargerError},
+	customer::{CustomerCharger, CustomerChargerError, CustomerDepositor, CustomerDepositorError},
 };
 use frame_support::{
 	parameter_types,
@@ -261,6 +261,8 @@ pub mod pallet {
 		ArithmeticOverflow,
 		// Arithmetic underflow
 		ArithmeticUnderflow,
+		// Transferring balance to pallet's vault has failed
+		TransferFailed,
 	}
 
 	#[pallet::genesis_config]
@@ -322,28 +324,8 @@ pub mod pallet {
 			#[pallet::compact] value: BalanceOf<T>,
 		) -> DispatchResult {
 			let owner = ensure_signed(origin)?;
-
-			if <Ledger<T>>::contains_key(&owner) {
-				Err(Error::<T>::AlreadyPaired)?
-			}
-
-			// Reject a deposit which is considered to be _dust_.
-			if value < <T as pallet::Config>::Currency::minimum_balance() {
-				Err(Error::<T>::InsufficientDeposit)?
-			}
-
-			frame_system::Pallet::<T>::inc_consumers(&owner).map_err(|_| Error::<T>::BadState)?;
-
-			let owner_balance = <T as pallet::Config>::Currency::free_balance(&owner);
-			let value = value.min(owner_balance);
-			let item = AccountsLedger {
-				owner: owner.clone(),
-				total: value,
-				active: value,
-				unlocking: Default::default(),
-			};
-			Self::update_ledger_and_deposit(&owner, &item)?;
-			Self::deposit_event(Event::<T>::Deposited(owner, value));
+			<Self as CustomerDepositor<T>>::deposit(owner, value.saturated_into())
+				.map_err(Into::<Error<T>>::into)?;
 			Ok(())
 		}
 
@@ -359,26 +341,8 @@ pub mod pallet {
 			#[pallet::compact] max_additional: BalanceOf<T>,
 		) -> DispatchResult {
 			let owner = ensure_signed(origin)?;
-
-			let mut ledger = Self::ledger(&owner).ok_or(Error::<T>::NotOwner)?;
-
-			let owner_balance = <T as pallet::Config>::Currency::free_balance(&owner);
-			let extra = owner_balance.min(max_additional);
-			ledger.total =
-				ledger.total.checked_add(&extra).ok_or(Error::<T>::ArithmeticOverflow)?;
-			ledger.active =
-				ledger.active.checked_add(&extra).ok_or(Error::<T>::ArithmeticOverflow)?;
-
-			// Last check: the new active amount of ledger must be more than ED.
-			ensure!(
-				ledger.active >= <T as pallet::Config>::Currency::minimum_balance(),
-				Error::<T>::InsufficientDeposit
-			);
-
-			Self::update_ledger_and_deposit(&owner, &ledger)?;
-
-			Self::deposit_event(Event::<T>::Deposited(owner, extra));
-
+			<Self as CustomerDepositor<T>>::deposit_extra(owner, max_additional.saturated_into())
+				.map_err(Into::<Error<T>>::into)?;
 			Ok(())
 		}
 
@@ -610,4 +574,78 @@ pub mod pallet {
 			Ok(actually_charged.saturated_into::<u128>())
 		}
 	}
+
+	impl<T: Config> CustomerDepositor<T> for Pallet<T> {
+		fn deposit(owner: T::AccountId, amount: u128) -> Result<(), CustomerDepositorError> {
+			let value = amount.saturated_into::<BalanceOf<T>>();
+
+			if <Ledger<T>>::contains_key(&owner) {
+				Err(CustomerDepositorError::LedgerAlreadyExists)?
+			}
+
+			// Reject a deposit which is considered to be _dust_.
+			if value < <T as pallet::Config>::Currency::minimum_balance() {
+				Err(CustomerDepositorError::InsufficientDeposit)?
+			}
+
+			frame_system::Pallet::<T>::inc_consumers(&owner)
+				.map_err(|_| CustomerDepositorError::DepositFailed)?;
+
+			let owner_balance = <T as pallet::Config>::Currency::free_balance(&owner);
+			let value = value.min(owner_balance);
+			let item = AccountsLedger {
+				owner: owner.clone(),
+				total: value,
+				active: value,
+				unlocking: Default::default(),
+			};
+
+			Self::update_ledger_and_deposit(&owner, &item)
+				.map_err(|_| CustomerDepositorError::TransferFailed)?;
+			Self::deposit_event(Event::<T>::Deposited(owner, value));
+
+			Ok(())
+		}
+
+		fn deposit_extra(owner: T::AccountId, amount: u128) -> Result<(), CustomerDepositorError> {
+			let max_additional = amount.saturated_into::<BalanceOf<T>>();
+			let mut ledger = Self::ledger(&owner).ok_or(CustomerDepositorError::NotLedgerOwner)?;
+
+			let owner_balance = <T as pallet::Config>::Currency::free_balance(&owner);
+			let extra = owner_balance.min(max_additional);
+			ledger.total = ledger
+				.total
+				.checked_add(&extra)
+				.ok_or(CustomerDepositorError::ArithmeticOverflow)?;
+			ledger.active = ledger
+				.active
+				.checked_add(&extra)
+				.ok_or(CustomerDepositorError::ArithmeticOverflow)?;
+
+			// Last check: the new active amount of ledger must be more than ED.
+			ensure!(
+				ledger.active >= <T as pallet::Config>::Currency::minimum_balance(),
+				CustomerDepositorError::InsufficientDeposit
+			);
+
+			Self::update_ledger_and_deposit(&owner, &ledger)
+				.map_err(|_| CustomerDepositorError::TransferFailed)?;
+			Self::deposit_event(Event::<T>::Deposited(owner, extra));
+
+			Ok(())
+		}
+	}
+
+	impl<T> From<CustomerDepositorError> for Error<T> {
+		fn from(error: CustomerDepositorError) -> Self {
+			match error {
+				CustomerDepositorError::LedgerAlreadyExists => Error::<T>::AlreadyPaired,
+				CustomerDepositorError::InsufficientDeposit => Error::<T>::InsufficientDeposit,
+				CustomerDepositorError::DepositFailed => Error::<T>::BadState,
+				CustomerDepositorError::NotLedgerOwner => Error::<T>::NotOwner,
+				CustomerDepositorError::TransferFailed => Error::<T>::TransferFailed,
+				CustomerDepositorError::ArithmeticOverflow => Error::<T>::ArithmeticOverflow,
+			}
+		}
+	}
 }

pallets/ddc-customers/src/tests.rs

@@ -3,7 +3,6 @@
 use ddc_primitives::ClusterId;
 use frame_support::{assert_noop, assert_ok};
 use frame_system::Config;
-use pallet_balances::Error as BalancesError;
 
 use super::{mock::*, *};
 
@@ -77,7 +76,7 @@ fn deposit_and_deposit_extra_works() {
 		// Deposit all tokens fails (should not kill account)
 		assert_noop!(
 			DdcCustomers::deposit(RuntimeOrigin::signed(account_1), 100_u128),
-			BalancesError::<Test, _>::KeepAlive
+			Error::<Test>::TransferFailed
 		);
 
 		// Deposited

pallets/ddc-payouts/src/benchmarking.rs

@@ -25,9 +25,15 @@ fn authorize_account<T: Config>(account: T::AccountId) {
 	AuthorisedCaller::<T>::put(account);
 }
 
-fn endow_account<T: Config>(account: &T::AccountId, amount: u128) {
+fn endow_customer<T: Config>(customer: &T::AccountId, amount: u128) {
 	let balance = amount.saturated_into::<BalanceOf<T>>();
-	let _ = T::Currency::make_free_balance_be(&account, balance);
+	let _ = T::Currency::make_free_balance_be(&customer, balance);
+	let _ = T::CustomerDepositor::deposit(
+		customer.clone(),
+		// we need to keep min existensial deposit
+		amount - T::Currency::minimum_balance().saturated_into::<u128>(),
+	)
+	.expect("Customer deposit failed");
 }
 
 fn create_cluster<T: Config>(
@@ -192,15 +198,24 @@ benchmarks! {
 		);
 
 		let batch_index: BatchIndex = 0;
-		let payers: Vec<(T::AccountId, CustomerUsage)> = (0..=b).map(|i| {
+		let payers: Vec<(T::AccountId, CustomerUsage)> = (0..b).map(|i| {
 			let customer = create_account::<T>("customer", i.into(), i.into());
-			endow_account::<T>(&customer, 1_000_000 * CERE);
+
+			if b % 2 == 0 {
+				// no customer debt path
+				endow_customer::<T>(&customer, 1_000_000 * CERE);
+			} else {
+				// customer debt path
+				endow_customer::<T>(&customer, 10 * CERE);
+			}
+
 			let customer_usage = CustomerUsage {
-				transferred_bytes: 1000,
-				stored_bytes: 500,
+				transferred_bytes: 200000000, // 200 mb
+				stored_bytes: 100000000, // 100 mb
 				number_of_gets: 100,
 				number_of_puts: 50,
 			};
+
 			(customer, customer_usage)
 		}).collect();
 
@@ -211,5 +226,4 @@ benchmarks! {
 		assert_eq!(billing_report.state, State::ChargingCustomers);
 		assert!(billing_report.charging_processed_batches.contains(&batch_index));
 	}
-
 }

pallets/ddc-payouts/src/lib.rs

@@ -28,7 +28,9 @@ mod tests;
 use ddc_primitives::{ClusterId, DdcEra};
 use ddc_traits::{
 	cluster::{ClusterCreator as ClusterCreatorType, ClusterVisitor as ClusterVisitorType},
-	customer::CustomerCharger as CustomerChargerType,
+	customer::{
+		CustomerCharger as CustomerChargerType, CustomerDepositor as CustomerDepositorType,
+	},
 	pallet::PalletVisitor as PalletVisitorType,
 };
 use frame_election_provider_support::SortedListProvider;
@@ -119,6 +121,7 @@ pub mod pallet {
 		type PalletId: Get<PalletId>;
 		type Currency: LockableCurrency<Self::AccountId, Moment = Self::BlockNumber>;
 		type CustomerCharger: CustomerChargerType<Self>;
+		type CustomerDepositor: CustomerDepositorType<Self>;
 		type TreasuryVisitor: PalletVisitorType<Self>;
 		type ClusterVisitor: ClusterVisitorType<Self>;
 		type ValidatorList: SortedListProvider<Self::AccountId>;

pallets/ddc-payouts/src/mock.rs

@@ -8,7 +8,7 @@ use ddc_primitives::{
 };
 use ddc_traits::{
 	cluster::{ClusterCreator, ClusterVisitor, ClusterVisitorError},
-	customer::{CustomerCharger, CustomerChargerError},
+	customer::{CustomerCharger, CustomerChargerError, CustomerDepositor, CustomerDepositorError},
 	pallet::PalletVisitor,
 };
 use frame_election_provider_support::SortedListProvider;
@@ -104,6 +104,7 @@ impl crate::pallet::Config for Test {
 	type PalletId = PayoutsPalletId;
 	type Currency = Balances;
 	type CustomerCharger = TestCustomerCharger;
+	type CustomerDepositor = TestCustomerDepositor;
 	type ClusterVisitor = TestClusterVisitor;
 	type TreasuryVisitor = TestTreasuryVisitor;
 	type ValidatorList = TestValidatorVisitor<Self>;
@@ -151,6 +152,16 @@ impl<T: Config> ClusterCreator<T, Balance> for TestClusterCreator {
 	}
 }
 
+pub struct TestCustomerDepositor;
+impl<T: Config> CustomerDepositor<T> for TestCustomerDepositor {
+	fn deposit(_customer: T::AccountId, _amount: u128) -> Result<(), CustomerDepositorError> {
+		Ok(())
+	}
+	fn deposit_extra(_customer: T::AccountId, _amount: u128) -> Result<(), CustomerDepositorError> {
+		Ok(())
+	}
+}
+
 pub const RESERVE_ACCOUNT_ID: AccountId = 999;
 pub const TREASURY_ACCOUNT_ID: AccountId = 888;
 pub const VALIDATOR1_ACCOUNT_ID: AccountId = 111;

pallets/ddc-payouts/src/weights.rs

@@ -44,31 +44,32 @@ impl<T: frame_system::Config> WeightInfo for SubstrateWeight<T> {
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	fn begin_billing_report() -> Weight {
-		Weight::from_ref_time(458_000_000 as u64)
+		Weight::from_ref_time(456_000_000 as u64)
 			.saturating_add(T::DbWeight::get().reads(2 as u64))
 			.saturating_add(T::DbWeight::get().writes(1 as u64))
 	}
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	fn begin_charging_customers() -> Weight {
-		Weight::from_ref_time(438_000_000 as u64)
+		Weight::from_ref_time(437_000_000 as u64)
 			.saturating_add(T::DbWeight::get().reads(2 as u64))
 			.saturating_add(T::DbWeight::get().writes(1 as u64))
 	}
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	// Storage: DdcClusters ClustersGovParams (r:1 w:0)
-	// Storage: DdcCustomers Ledger (r:2 w:0)
-	// Storage: DdcPayouts DebtorCustomers (r:2 w:2)
+	// Storage: DdcCustomers Ledger (r:1 w:1)
+	// Storage: System Account (r:2 w:2)
+	// Storage: DdcPayouts DebtorCustomers (r:1 w:1)
 	/// The range of component `b` is `[1, 1000]`.
 	fn send_charging_customers_batch(b: u32, ) -> Weight {
-		Weight::from_ref_time(875_000_000 as u64)
-			// Standard Error: 121_571
-			.saturating_add(Weight::from_ref_time(211_062_297 as u64).saturating_mul(b as u64))
-			.saturating_add(T::DbWeight::get().reads(5 as u64))
+		Weight::from_ref_time(1_171_000_000 as u64)
+			// Standard Error: 2_377_299
+			.saturating_add(Weight::from_ref_time(490_919_482 as u64).saturating_mul(b as u64))
+			.saturating_add(T::DbWeight::get().reads(7 as u64))
 			.saturating_add(T::DbWeight::get().reads((2 as u64).saturating_mul(b as u64)))
-			.saturating_add(T::DbWeight::get().writes(2 as u64))
-			.saturating_add(T::DbWeight::get().writes((1 as u64).saturating_mul(b as u64)))
+			.saturating_add(T::DbWeight::get().writes(5 as u64))
+			.saturating_add(T::DbWeight::get().writes((2 as u64).saturating_mul(b as u64)))
 	}
 }
 
@@ -82,30 +83,31 @@ impl WeightInfo for () {
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	fn begin_billing_report() -> Weight {
-		Weight::from_ref_time(458_000_000 as u64)
+		Weight::from_ref_time(456_000_000 as u64)
 			.saturating_add(RocksDbWeight::get().reads(2 as u64))
 			.saturating_add(RocksDbWeight::get().writes(1 as u64))
 	}
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	fn begin_charging_customers() -> Weight {
-		Weight::from_ref_time(438_000_000 as u64)
+		Weight::from_ref_time(437_000_000 as u64)
 			.saturating_add(RocksDbWeight::get().reads(2 as u64))
 			.saturating_add(RocksDbWeight::get().writes(1 as u64))
 	}
 	// Storage: DdcPayouts AuthorisedCaller (r:1 w:0)
 	// Storage: DdcPayouts ActiveBillingReports (r:1 w:1)
 	// Storage: DdcClusters ClustersGovParams (r:1 w:0)
-	// Storage: DdcCustomers Ledger (r:2 w:0)
-	// Storage: DdcPayouts DebtorCustomers (r:2 w:2)
+	// Storage: DdcCustomers Ledger (r:1 w:1)
+	// Storage: System Account (r:2 w:2)
+	// Storage: DdcPayouts DebtorCustomers (r:1 w:1)
 	/// The range of component `b` is `[1, 1000]`.
 	fn send_charging_customers_batch(b: u32, ) -> Weight {
-		Weight::from_ref_time(875_000_000 as u64)
-			// Standard Error: 121_571
-			.saturating_add(Weight::from_ref_time(211_062_297 as u64).saturating_mul(b as u64))
-			.saturating_add(RocksDbWeight::get().reads(5 as u64))
+		Weight::from_ref_time(1_171_000_000 as u64)
+			// Standard Error: 2_377_299
+			.saturating_add(Weight::from_ref_time(490_919_482 as u64).saturating_mul(b as u64))
+			.saturating_add(RocksDbWeight::get().reads(7 as u64))
 			.saturating_add(RocksDbWeight::get().reads((2 as u64).saturating_mul(b as u64)))
-			.saturating_add(RocksDbWeight::get().writes(2 as u64))
-			.saturating_add(RocksDbWeight::get().writes((1 as u64).saturating_mul(b as u64)))
+			.saturating_add(RocksDbWeight::get().writes(5 as u64))
+			.saturating_add(RocksDbWeight::get().writes((2 as u64).saturating_mul(b as u64)))
 	}
 }

runtime/cere-dev/src/lib.rs

@@ -1377,6 +1377,7 @@ impl pallet_ddc_payouts::Config for Runtime {
 	type PalletId = PayoutsPalletId;
 	type Currency = Balances;
 	type CustomerCharger = DdcCustomers;
+	type CustomerDepositor = DdcCustomers;
 	type ClusterVisitor = DdcClusters;
 	type TreasuryVisitor = TreasureWrapper;
 	type ValidatorList = pallet_staking::UseValidatorsMap<Self>;

traits/src/customer.rs

@@ -1,3 +1,5 @@
+use core::u128;
+
 pub trait CustomerCharger<T: frame_system::Config> {
 	// todo: WIP for decoupling payout and customers
 	fn charge_content_owner(
@@ -13,3 +15,18 @@ pub enum CustomerChargerError {
 	TransferFailed,
 	UnlockFailed,
 }
+
+pub trait CustomerDepositor<T: frame_system::Config> {
+	fn deposit(customer: T::AccountId, amount: u128) -> Result<(), CustomerDepositorError>;
+	fn deposit_extra(customer: T::AccountId, amount: u128) -> Result<(), CustomerDepositorError>;
+}
+
+#[derive(Debug)]
+pub enum CustomerDepositorError {
+	LedgerAlreadyExists,
+	InsufficientDeposit,
+	NotLedgerOwner,
+	DepositFailed,
+	TransferFailed,
+	ArithmeticOverflow,
+}