Dac customer trasnfer fix (#181)

pallets/ddc-customers/Cargo.toml

@@ -11,14 +11,14 @@ frame-support = { default-features = false, git = "https://github.com/paritytech
 frame-system = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 log = { version = "0.4.17", default-features = false }
 scale-info = { version = "2.1.1", default-features = false, features = ["derive"] }
+sp-io = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 sp-runtime = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 sp-std = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 
 [dev-dependencies]
 pallet-balances = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 pallet-timestamp = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 sp-core = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
-sp-io = { default-features = false, git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 sp-tracing = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 substrate-test-utils = { git = "https://github.com/paritytech/substrate.git", branch = "polkadot-v0.9.30" }
 

pallets/ddc-customers/src/lib.rs

@@ -15,15 +15,15 @@ use frame_support::{
 	traits::{Currency, DefensiveSaturating, ExistenceRequirement},
 	BoundedVec, PalletId,
 };
+pub use pallet::*;
 use scale_info::TypeInfo;
+use sp_io::hashing::blake2_128;
 use sp_runtime::{
-	traits::{AccountIdConversion, AtLeast32BitUnsigned, CheckedAdd, CheckedSub, Saturating, Zero},
+	traits::{AccountIdConversion, CheckedAdd, CheckedSub, Saturating, Zero},
 	DispatchError, RuntimeDebug, SaturatedConversion,
 };
 use sp_std::prelude::*;
 
-pub use pallet::*;
-
 /// The balance type of this pallet.
 pub type BalanceOf<T> =
 	<<T as Config>::Currency as Currency<<T as frame_system::Config>::AccountId>>::Balance;
@@ -36,10 +36,10 @@ parameter_types! {
 /// Just a Balance/BlockNumber tuple to encode when a chunk of funds will be unlocked.
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
 #[scale_info(skip_type_params(T))]
-pub struct UnlockChunk<Balance: HasCompact, T: Config> {
+pub struct UnlockChunk<T: Config> {
 	/// Amount of funds to be unlocked.
 	#[codec(compact)]
-	value: Balance,
+	value: BalanceOf<T>,
 	/// Block number at which point it'll be unlocked.
 	#[codec(compact)]
 	block: T::BlockNumber,
@@ -60,32 +60,27 @@ pub struct BucketsDetails<Balance: HasCompact> {
 
 #[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug, TypeInfo)]
 #[scale_info(skip_type_params(T))]
-pub struct AccountsLedger<AccountId, Balance: HasCompact, T: Config> {
+pub struct AccountsLedger<T: Config> {
 	/// The owner account whose balance is actually locked and can be used for CDN usage.
-	pub owner: AccountId,
+	pub owner: T::AccountId,
 	/// The total amount of the owner's balance that we are currently accounting for.
 	/// It's just `active` plus all the `unlocking` balances.
 	#[codec(compact)]
-	pub total: Balance,
+	pub total: BalanceOf<T>,
 	/// The total amount of the owner's balance that will be accessible for CDN payments in any
 	/// forthcoming rounds.
 	#[codec(compact)]
-	pub active: Balance,
+	pub active: BalanceOf<T>,
 	/// Any balance that is becoming free, which may eventually be transferred out of the owner
 	/// (assuming that the content owner has to pay for network usage). It is assumed that this
 	/// will be treated as a first in, first out queue where the new (higher value) eras get pushed
 	/// on the back.
-	pub unlocking: BoundedVec<UnlockChunk<Balance, T>, MaxUnlockingChunks>,
+	pub unlocking: BoundedVec<UnlockChunk<T>, MaxUnlockingChunks>,
 }
 
-impl<
-		AccountId,
-		Balance: HasCompact + Copy + Saturating + AtLeast32BitUnsigned + Zero,
-		T: Config,
-	> AccountsLedger<AccountId, Balance, T>
-{
+impl<T: Config> AccountsLedger<T> {
 	/// Initializes the default object using the given owner.
-	pub fn default_from(owner: AccountId) -> Self {
+	pub fn default_from(owner: T::AccountId) -> Self {
 		Self { owner, total: Zero::zero(), active: Zero::zero(), unlocking: Default::default() }
 	}
 
@@ -113,37 +108,6 @@ impl<
 
 		Self { owner: self.owner, total, active: self.active, unlocking }
 	}
-
-	/// Charge funds that were scheduled for unlocking.
-	///
-	/// Returns the updated ledger, and the amount actually charged.
-	fn charge_unlocking(mut self, value: Balance) -> Result<(Self, Balance), Error<T>> {
-		let mut unlocking_balance = Balance::zero();
-
-		while let Some(last) = self.unlocking.last_mut() {
-			let temp = unlocking_balance
-				.checked_add(&last.value)
-				.ok_or(Error::<T>::ArithmeticOverflow)?;
-			if temp <= value {
-				unlocking_balance = temp;
-				self.unlocking.pop();
-			} else {
-				let diff =
-					value.checked_sub(&unlocking_balance).ok_or(Error::<T>::ArithmeticUnderflow)?;
-
-				unlocking_balance =
-					unlocking_balance.checked_add(&diff).ok_or(Error::<T>::ArithmeticOverflow)?;
-				last.value =
-					last.value.checked_sub(&diff).ok_or(Error::<T>::ArithmeticUnderflow)?;
-			}
-
-			if unlocking_balance >= value {
-				break
-			}
-		}
-
-		Ok((self, unlocking_balance))
-	}
 }
 
 #[frame_support::pallet]
@@ -173,12 +137,7 @@ pub mod pallet {
 	/// Map from all (unlocked) "owner" accounts to the info regarding the staking.
 	#[pallet::storage]
 	#[pallet::getter(fn ledger)]
-	pub type Ledger<T: Config> = StorageMap<
-		_,
-		Blake2_128Concat,
-		T::AccountId,
-		AccountsLedger<T::AccountId, BalanceOf<T>, T>,
-	>;
+	pub type Ledger<T: Config> = StorageMap<_, Blake2_128Concat, T::AccountId, AccountsLedger<T>>;
 
 	#[pallet::type_value]
 	pub fn DefaultBucketCount<T: Config>() -> BucketId {
@@ -255,13 +214,7 @@ pub mod pallet {
 
 	#[pallet::genesis_build]
 	impl<T: Config> GenesisBuild<T> for GenesisConfig {
-		fn build(&self) {
-			let account_id = <Pallet<T>>::account_id();
-			let min = <T as pallet::Config>::Currency::minimum_balance();
-			if <T as pallet::Config>::Currency::free_balance(&account_id) < min {
-				let _ = <T as pallet::Config>::Currency::make_free_balance_be(&account_id, min);
-			}
-		}
+		fn build(&self) {}
 	}
 
 	#[pallet::call]
@@ -473,13 +426,11 @@ pub mod pallet {
 				let value =
 					old_total.checked_sub(&ledger.total).ok_or(Error::<T>::ArithmeticUnderflow)?;
 
-				let account_id = Self::account_id();
-
 				<T as pallet::Config>::Currency::transfer(
-					&account_id,
+					&Self::sub_account_id(&owner),
 					&owner,
 					value,
-					ExistenceRequirement::KeepAlive,
+					ExistenceRequirement::AllowDeath,
 				)?;
 				Self::deposit_event(Event::<T>::Withdrawn(owner, value));
 			}
@@ -492,18 +443,26 @@ pub mod pallet {
 		pub fn account_id() -> T::AccountId {
 			T::PalletId::get().into_account_truncating()
 		}
+
+		pub fn sub_account_id(account_id: &T::AccountId) -> T::AccountId {
+			let hash = blake2_128(&account_id.encode());
+
+			// hash is 28 bytes
+			T::PalletId::get().into_sub_account_truncating(hash)
+		}
+
 		/// Update the ledger for a owner.
 		///
 		/// This will also deposit the funds to pallet.
 		fn update_ledger_and_deposit(
 			owner: &T::AccountId,
-			ledger: &AccountsLedger<T::AccountId, BalanceOf<T>, T>,
+			ledger: &AccountsLedger<T>,
 		) -> DispatchResult {
 			<T as pallet::Config>::Currency::transfer(
 				owner,
-				&Self::account_id(),
+				&Self::sub_account_id(owner),
 				ledger.total,
-				ExistenceRequirement::KeepAlive,
+				ExistenceRequirement::AllowDeath,
 			)?;
 			<Ledger<T>>::insert(owner, ledger);
 
@@ -523,6 +482,42 @@ pub mod pallet {
 
 			Ok(())
 		}
+
+		/// Charge funds that were scheduled for unlocking.
+		///
+		/// Returns the updated ledger, and the amount actually charged.
+		fn charge_unlocking(
+			mut ledger: AccountsLedger<T>,
+			value: BalanceOf<T>,
+		) -> Result<(AccountsLedger<T>, BalanceOf<T>), Error<T>> {
+			let mut unlocking_balance = BalanceOf::<T>::zero();
+
+			while let Some(last) = ledger.unlocking.last_mut() {
+				let temp = unlocking_balance
+					.checked_add(&last.value)
+					.ok_or(Error::<T>::ArithmeticOverflow)?;
+				if temp <= value {
+					unlocking_balance = temp;
+					ledger.unlocking.pop();
+				} else {
+					let diff = value
+						.checked_sub(&unlocking_balance)
+						.ok_or(Error::<T>::ArithmeticUnderflow)?;
+
+					unlocking_balance = unlocking_balance
+						.checked_add(&diff)
+						.ok_or(Error::<T>::ArithmeticOverflow)?;
+					last.value =
+						last.value.checked_sub(&diff).ok_or(Error::<T>::ArithmeticUnderflow)?;
+				}
+
+				if unlocking_balance >= value {
+					break
+				}
+			}
+
+			Ok((ledger, unlocking_balance))
+		}
 	}
 
 	impl<T: Config> CustomerCharger<T> for Pallet<T> {
@@ -545,8 +540,6 @@ pub mod pallet {
 					.total
 					.checked_sub(&amount_to_deduct)
 					.ok_or(Error::<T>::ArithmeticUnderflow)?;
-
-				<Ledger<T>>::insert(&content_owner, &ledger);
 			} else {
 				let diff = amount_to_deduct
 					.checked_sub(&ledger.active)
@@ -559,19 +552,20 @@ pub mod pallet {
 					.ok_or(Error::<T>::ArithmeticUnderflow)?;
 				ledger.active = BalanceOf::<T>::zero();
 
-				let (ledger, charged) = ledger.charge_unlocking(diff)?;
+				let (_ledger, charged) = Self::charge_unlocking(ledger, diff)?;
+				ledger = _ledger;
 
 				actually_charged.checked_add(&charged).ok_or(Error::<T>::ArithmeticUnderflow)?;
-
-				<Ledger<T>>::insert(&content_owner, &ledger);
 			}
 
 			<T as pallet::Config>::Currency::transfer(
-				&Self::account_id(),
+				&Self::sub_account_id(&content_owner),
 				&billing_vault,
 				actually_charged,
 				ExistenceRequirement::AllowDeath,
 			)?;
+
+			<Ledger<T>>::insert(&content_owner, &ledger); // update state after successful transfer
 			Self::deposit_event(Event::<T>::Charged(content_owner, amount_to_deduct));
 
 			Ok(actually_charged.saturated_into::<u128>())

pallets/ddc-customers/src/mock.rs

@@ -19,7 +19,7 @@ use sp_runtime::{
 };
 
 /// The AccountId alias in this test module.
-pub(crate) type AccountId = u64;
+pub(crate) type AccountId = u128;
 pub(crate) type AccountIndex = u64;
 pub(crate) type BlockNumber = u64;
 pub(crate) type Balance = u128;

pallets/ddc-customers/src/tests.rs

@@ -134,8 +134,8 @@ fn charge_content_owner_works() {
 	ExtBuilder.build_and_execute(|| {
 		System::set_block_number(1);
 
-		let account_3 = 3;
-		let vault = 4;
+		let account_3: u128 = 3;
+		let vault: u128 = 4;
 		let deposit = 100_u128;
 
 		let balance_before_deposit = Balances::free_balance(account_3);
@@ -144,7 +144,7 @@ fn charge_content_owner_works() {
 		let balance_after_deposit = Balances::free_balance(account_3);
 		assert_eq!(balance_before_deposit - deposit, balance_after_deposit);
 
-		let pallet_balance = Balances::free_balance(DdcCustomers::account_id());
+		let pallet_balance = Balances::free_balance(DdcCustomers::sub_account_id(&account_3));
 		assert_eq!(deposit, pallet_balance);
 
 		// Check storage
@@ -172,7 +172,8 @@ fn charge_content_owner_works() {
 		let account_balance = Balances::free_balance(account_3);
 		assert_eq!(balance_after_deposit, account_balance);
 
-		let pallet_balance_after_charge = Balances::free_balance(DdcCustomers::account_id());
+		let pallet_balance_after_charge =
+			Balances::free_balance(DdcCustomers::sub_account_id(&account_3));
 		assert_eq!(pallet_balance - charged, pallet_balance_after_charge);
 
 		// Check storage
@@ -199,7 +200,7 @@ fn charge_content_owner_works() {
 			})
 		);
 
-		assert_eq!(0, Balances::free_balance(DdcCustomers::account_id()));
+		assert_eq!(0, Balances::free_balance(DdcCustomers::sub_account_id(&account_3)));
 		assert_eq!(charge_result, deposit - charge1);
 
 		assert_ok!(DdcCustomers::deposit_extra(RuntimeOrigin::signed(account_3), deposit));
@@ -213,7 +214,7 @@ fn charge_content_owner_works() {
 			})
 		);
 
-		assert_eq!(deposit, Balances::free_balance(DdcCustomers::account_id()));
+		assert_eq!(deposit, Balances::free_balance(DdcCustomers::sub_account_id(&account_3)));
 	})
 }
 
@@ -234,20 +235,15 @@ fn unlock_and_withdraw_deposit_works() {
 		assert_ok!(DdcCustomers::unlock_deposit(RuntimeOrigin::signed(account_1), 1_u128));
 		System::set_block_number(2);
 
-		let mut unlocking_chunks: BoundedVec<UnlockChunk<Balance, Test>, MaxUnlockingChunks> =
-			Default::default();
-		match unlocking_chunks.try_push(UnlockChunk { value: 1, block: 11 }) {
-			Ok(_) => (),
-			Err(_) => println!("No more chunks"),
-		};
+		let unlocking_chunks = vec![UnlockChunk { value: 1, block: 11 }];
 		// Check storage
 		assert_eq!(
 			DdcCustomers::ledger(&1),
 			Some(AccountsLedger {
 				owner: account_1,
 				total: 35_u128,
 				active: 34_u128,
-				unlocking: unlocking_chunks.clone(),
+				unlocking: BoundedVec::try_from(unlocking_chunks).unwrap(),
 			})
 		);
 

pallets/ddc-payouts/src/mock.rs

@@ -28,7 +28,7 @@ use sp_runtime::{
 use sp_std::prelude::*;
 
 /// The AccountId alias in this test module.
-pub type AccountId = u64;
+pub type AccountId = u128;
 pub(crate) type AccountIndex = u64;
 pub(crate) type BlockNumber = u64;
 pub(crate) type Balance = u128;
@@ -170,7 +170,7 @@ impl<T: frame_system::Config> PalletVisitor<T> for TestTreasuryVisitor {
 	}
 }
 
-fn create_account_id_from_u64<T: frame_system::Config>(id: u64) -> T::AccountId {
+fn create_account_id_from_u128<T: frame_system::Config>(id: u128) -> T::AccountId {
 	let bytes = id.to_ne_bytes();
 	T::AccountId::decode(&mut &bytes[..]).unwrap()
 }
@@ -184,9 +184,9 @@ impl<T: frame_system::Config> SortedListProvider<T::AccountId> for TestValidator
 	fn iter() -> Box<dyn Iterator<Item = T::AccountId>> {
 		Box::new(
 			vec![
-				create_account_id_from_u64::<T>(VALIDATOR1_ACCOUNT_ID),
-				create_account_id_from_u64::<T>(VALIDATOR2_ACCOUNT_ID),
-				create_account_id_from_u64::<T>(VALIDATOR3_ACCOUNT_ID),
+				create_account_id_from_u128::<T>(VALIDATOR1_ACCOUNT_ID),
+				create_account_id_from_u128::<T>(VALIDATOR2_ACCOUNT_ID),
+				create_account_id_from_u128::<T>(VALIDATOR3_ACCOUNT_ID),
 			]
 			.into_iter(),
 		)