From 09570d3d719396add3ba529ccc5c0b41b62cdb93 Mon Sep 17 00:00:00 2001
From: salman01zp <pathansalman555@gmail.com>
Date: Tue, 7 Nov 2023 15:48:44 +0530
Subject: [PATCH] update doc, and review feedback

---
 pallets/roles/Cargo.toml     |  8 ++---
 pallets/roles/src/impls.rs   | 70 ++++++++++++++++++++++++++++++++----
 pallets/roles/src/lib.rs     | 51 ++++++++++++++++++--------
 pallets/roles/src/mock.rs    |  4 +--
 pallets/roles/src/tests.rs   |  4 +--
 pallets/roles/src/weights.rs |  8 ++---
 6 files changed, 112 insertions(+), 33 deletions(-)

diff --git a/pallets/roles/Cargo.toml b/pallets/roles/Cargo.toml
index 0d6b669d7..a7741be0c 100644
--- a/pallets/roles/Cargo.toml
+++ b/pallets/roles/Cargo.toml
@@ -46,9 +46,9 @@ std = [
 
 try-runtime = ["frame-support/try-runtime"]
 runtime-benchmarks = [
-	"frame-benchmarking/runtime-benchmarks",
-	"frame-support/runtime-benchmarks",
-	"frame-system/runtime-benchmarks",
-	"sp-runtime/runtime-benchmarks",
+  "frame-benchmarking/runtime-benchmarks",
+  "frame-support/runtime-benchmarks",
+  "frame-system/runtime-benchmarks",
+  "sp-runtime/runtime-benchmarks",
   "pallet-balances/runtime-benchmarks"
 ]
diff --git a/pallets/roles/src/impls.rs b/pallets/roles/src/impls.rs
index ef891b3d0..4637d972d 100644
--- a/pallets/roles/src/impls.rs
+++ b/pallets/roles/src/impls.rs
@@ -1,3 +1,19 @@
+// This file is part of Tangle.
+// Copyright (C) 2022-2023 Webb Technologies Inc.
+//
+// Tangle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Tangle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Tangle.  If not, see <http://www.gnu.org/licenses/>.
+
 use super::*;
 use frame_support::{pallet_prelude::DispatchResult, traits::WithdrawReasons};
 use sp_runtime::Saturating;
@@ -5,6 +21,14 @@ use tangle_primitives::{roles::RoleType, traits::roles::RolesHandler};
 
 /// Implements RolesHandler for the pallet.
 impl<T: Config> RolesHandler<T::AccountId> for Pallet<T> {
+	/// Validates if the given address has the given role.
+	///
+	/// # Parameters
+	/// - `address`: The account ID of the validator.
+	/// - `role`: The key representing the type of job.
+	///
+	/// # Returns
+	/// Returns `true` if the validator is permitted to work with this job type, otherwise `false`.
 	fn validate_role(address: T::AccountId, role: RoleType) -> bool {
 		let assigned_role = AccountRolesMapping::<T>::get(address);
 		match assigned_role {
@@ -17,6 +41,16 @@ impl<T: Config> RolesHandler<T::AccountId> for Pallet<T> {
 
 		false
 	}
+
+	/// Slash validator stake for the reported offence. The function should be a best effort
+	/// slashing, slash upto max possible by the offence type.
+	///
+	/// # Parameters
+	/// - `address`: The account ID of the validator.
+	/// - `offence`: The offence reported against the validator
+	///
+	/// # Returns
+	/// DispatchResult emitting `Slashed` event if validator is slashed
 	fn slash_validator(
 		address: T::AccountId,
 		_offence: tangle_primitives::jobs::ValidatorOffence,
@@ -30,13 +64,23 @@ impl<T: Config> RolesHandler<T::AccountId> for Pallet<T> {
 
 /// Functions for the pallet.
 impl<T: Config> Pallet<T> {
-	/// The total balance that can be slashed from a stash account as of right now.
+	/// Get the total amount of the balance that is locked for the given stash.
+	///
+	/// # Parameters
+	/// - `stash`: The stash account ID.
+	///
+	/// # Returns
+	/// The total amount of the balance that can be slashed.
 	pub fn slashable_balance_of(stash: &T::AccountId) -> BalanceOf<T> {
 		// Weight note: consider making the stake accessible through stash.
 		Self::ledger(&stash).map(|l| l.total_locked).unwrap_or_default()
 	}
 
-	/// Slash staker's balance by the given amount.
+	/// Slash the given amount from the stash account.
+	///
+	/// # Parameters
+	/// - `address`: The stash account ID.
+	/// - `slash_amount`: The amount to be slashed.
 	pub(crate) fn do_slash(
 		address: T::AccountId,
 		slash_amount: T::CurrencyBalance,
@@ -49,9 +93,14 @@ impl<T: Config> Pallet<T> {
 		Ok(())
 	}
 
-	/// Update the ledger for the staker.
+	/// Update the ledger for the given stash account.
+	///
+	/// # Parameters
+	/// - `staker`: The stash account ID.
+	/// - `ledger`: The new ledger.
 	///
-	/// This will also update the stash lock.
+	/// # Note
+	/// This function will set a lock on the stash account.
 	pub(crate) fn update_ledger(staker: &T::AccountId, ledger: &RoleStakingLedger<T>) {
 		T::Currency::set_lock(
 			ROLES_STAKING_ID,
@@ -61,19 +110,26 @@ impl<T: Config> Pallet<T> {
 		);
 		<Ledger<T>>::insert(staker, ledger);
 	}
-	/// Clear stash account information from pallet.
+
+	/// Kill the stash account and remove all related information.
 	pub(crate) fn kill_stash(stash: &T::AccountId) -> DispatchResult {
 		<Ledger<T>>::remove(&stash);
 		Ok(())
 	}
 
-	/// Unbond the full amount of the stash.
+	/// Unbond the stash account.
+	///
+	/// # Parameters
+	/// - `ledger`: The ledger of the stash account.
+	///
+	/// # Note
+	/// This function will remove the lock on the stash account.
 	pub(super) fn unbond(ledger: &RoleStakingLedger<T>) -> DispatchResult {
 		let stash = ledger.stash.clone();
 		if ledger.total_locked > T::Currency::minimum_balance() {
 			// Remove the lock.
 			T::Currency::remove_lock(ROLES_STAKING_ID, &stash);
-			// Kill the stash and related information
+			// Kill the stash and related information.
 			Self::kill_stash(&stash)?;
 		}
 		Ok(())
diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs
index c8ea5c4a2..f43e3618c 100644
--- a/pallets/roles/src/lib.rs
+++ b/pallets/roles/src/lib.rs
@@ -1,18 +1,18 @@
-// Copyright 2017-2020 Parity Technologies (UK) Ltd.
-// This file is part of Polkadot.
-
-// Substrate is free software: you can redistribute it and/or modify
+// This file is part of Tangle.
+// Copyright (C) 2022-2023 Webb Technologies Inc.
+//
+// Tangle is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
 // the Free Software Foundation, either version 3 of the License, or
 // (at your option) any later version.
-
-// Substrate is distributed in the hope that it will be useful,
+//
+// Tangle is distributed in the hope that it will be useful,
 // but WITHOUT ANY WARRANTY; without even the implied warranty of
 // MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 // GNU General Public License for more details.
-
+//
 // You should have received a copy of the GNU General Public License
-// along with Substrate.  If not, see <http://www.gnu.org/licenses/>.
+// along with Tangle.  If not, see <http://www.gnu.org/licenses/>.
 
 //! Pallet to process claims from Ethereum addresses.
 #![cfg_attr(not(feature = "std"), no_std)]
@@ -30,6 +30,8 @@ use scale_info::TypeInfo;
 use sp_runtime::{codec, traits::Zero};
 use sp_std::{convert::TryInto, prelude::*, vec};
 use tangle_primitives::{roles::RoleType, traits::roles::RolesHandler};
+#[cfg(feature = "runtime-benchmarks")]
+pub mod benchmarking;
 mod impls;
 #[cfg(test)]
 pub(crate) mod mock;
@@ -65,6 +67,7 @@ impl<T: Config> RoleStakingLedger<T> {
 		Self { stash, total_locked: Zero::zero() }
 	}
 
+	/// Returns `true` if the stash account has no funds at all.
 	pub fn is_empty(&self) -> bool {
 		self.total_locked.is_zero()
 	}
@@ -166,6 +169,17 @@ pub mod pallet {
 	#[pallet::getter(fn min_active_bond)]
 	pub(super) type MinActiveBond<T: Config> = StorageValue<_, BalanceOf<T>, ValueQuery>;
 
+	/// Assigns a role to the validator.
+	///
+	/// # Parameters
+	///
+	/// - `origin`: Origin of the transaction.
+	/// - `bond_value`: Amount of funds to bond.
+	/// - `role`: Role to assign to the validator.
+	///
+	/// This function will return error if
+	/// - Role is already assigned to the validator.
+	/// - Min active bond is not met.
 	#[pallet::call]
 	impl<T: Config> Pallet<T> {
 		#[pallet::weight({0})]
@@ -176,16 +190,16 @@ pub mod pallet {
 			role: RoleType,
 		) -> DispatchResult {
 			let stash_account = ensure_signed(origin)?;
-			// check if role is already assigned.
+			// Check if role is already assigned.
 			ensure!(
 				!AccountRolesMapping::<T>::contains_key(&stash_account),
 				Error::<T>::RoleAlreadyAssigned
 			);
-			// check if stash account is already paired.
+			// Check if stash account is already paired.
 			if <Ledger<T>>::contains_key(&stash_account) {
 				return Err(Error::<T>::AlreadyPaired.into())
 			}
-			// check if min active bond is met.
+			// Check if min active bond is met.
 			let min_active_bond = MinActiveBond::<T>::get();
 			if bond_value < min_active_bond.into() {
 				return Err(Error::<T>::InsufficientBond.into())
@@ -194,7 +208,7 @@ pub mod pallet {
 			let stash_balance = T::Currency::free_balance(&stash_account);
 			let value = bond_value.min(stash_balance);
 
-			// update ledger.
+			// Update ledger.
 			let item = RoleStakingLedger { stash: stash_account.clone(), total_locked: value };
 			Self::update_ledger(&stash_account, &item);
 
@@ -209,6 +223,15 @@ pub mod pallet {
 			Ok(())
 		}
 
+		/// Removes the role from the validator.
+		///
+		/// # Parameters
+		///
+		/// - `origin`: Origin of the transaction.
+		/// - `role`: Role to remove from the validator.
+		///
+		/// This function will return error if
+		/// - Role is not assigned to the validator.
 		#[pallet::weight({0})]
 		#[pallet::call_index(1)]
 		pub fn clear_role(origin: OriginFor<T>, role: RoleType) -> DispatchResult {
@@ -219,9 +242,9 @@ pub mod pallet {
 				Error::<T>::RoleNotAssigned
 			);
 			// TODO: Call jobs manager to remove the services.
-
 			// On successful removal of services, remove the role from the mapping.
-			// unbound locked funds.
+
+			// Unbound locked funds.
 			let ledger = Self::ledger(&stash_account).ok_or(Error::<T>::InvalidStashController)?;
 			Self::unbond(&ledger)?;
 			Self::deposit_event(Event::<T>::Unbonded {
diff --git a/pallets/roles/src/mock.rs b/pallets/roles/src/mock.rs
index 8ffc2db19..f4cc2c38c 100644
--- a/pallets/roles/src/mock.rs
+++ b/pallets/roles/src/mock.rs
@@ -1,5 +1,5 @@
-// This file is part of Webb.
-// Copyright (C) 2022 Webb Technologies Inc.
+// This file is part of Tangle.
+// Copyright (C) 2022-2023 Webb Technologies Inc.
 //
 // Tangle is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/pallets/roles/src/tests.rs b/pallets/roles/src/tests.rs
index 37a76483c..38abccadd 100644
--- a/pallets/roles/src/tests.rs
+++ b/pallets/roles/src/tests.rs
@@ -1,5 +1,5 @@
-// This file is part of Webb.
-// Copyright (C) 2022 Webb Technologies Inc.
+// This file is part of Tangle.
+// Copyright (C) 2022-2023 Webb Technologies Inc.
 //
 // Tangle is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
diff --git a/pallets/roles/src/weights.rs b/pallets/roles/src/weights.rs
index f59cdf037..bcb3f0e67 100644
--- a/pallets/roles/src/weights.rs
+++ b/pallets/roles/src/weights.rs
@@ -1,5 +1,5 @@
-// This file is part of Webb.
-// Copyright (C) 2022 Webb Technologies Inc.
+// This file is part of Tangle.
+// Copyright (C) 2022-2023 Webb Technologies Inc.
 //
 // Tangle is free software: you can redistribute it and/or modify
 // it under the terms of the GNU General Public License as published by
@@ -40,13 +40,13 @@
 use frame_support::{traits::Get, weights::{Weight, constants::RocksDbWeight}};
 use sp_std::marker::PhantomData;
 
-/// Weight functions needed for module_transaction_pause.
+/// Weight functions needed for roles pallet.
 pub trait WeightInfo {
 	fn assign_role() -> Weight;
 	fn clear_role() -> Weight;
 }
 
-/// Weights for module_transaction_pause using the Acala node and recommended hardware.
+/// Weights for roles pallet.
 pub struct TestWeightInfo<T>(PhantomData<T>);
 impl<T: frame_system::Config> WeightInfo for TestWeightInfo<T> {
 	fn assign_role() -> Weight {