From abb08103dbe95b537088c85d2812245a4a590a85 Mon Sep 17 00:00:00 2001 From: Drew Stone Date: Wed, 10 Jan 2024 14:13:11 -0700 Subject: [PATCH] Update validation of profile updates --- Cargo.toml | 6 +-- pallets/roles/src/impls.rs | 95 ++++++++++++++++++++++++++++++++------ pallets/roles/src/lib.rs | 40 +++++++++++----- pallets/roles/src/tests.rs | 1 + 4 files changed, 112 insertions(+), 30 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 661d00a2d..2e0807802 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,7 @@ resolver = "2" substrate-wasm-builder = { git = "https://github.com/paritytech/polkadot-sdk", branch = "release-polkadot-v1.1.0", default-features = false } substrate-build-script-utils = "3.0.0" -hex-literal = '0.4.1' +hex-literal = "0.4.1" log = { version = "0.4.20", default-features = false } scale-info = { version = "2.9.0", default-features = false, features = ["derive"] } serde = { version = "1.0.101", default-features = false, features = ["derive"] } @@ -281,9 +281,9 @@ sc-transaction-pool-api = { git = "https://github.com/paritytech/polkadot-sdk", sc-consensus-manual-seal = { git = "https://github.com/paritytech/polkadot-sdk", branch = "release-polkadot-v1.1.0" } # Tangle dependencies -tangle-primitives = { path = 'primitives', default-features = false } +tangle-primitives = { path = "primitives", default-features = false } tangle-crypto-primitives = { path = "primitives/crypto", default-features = false } -pallet-transaction-pause = { path = 'pallets/transaction-pause', default-features = false } +pallet-transaction-pause = { path = "pallets/transaction-pause", default-features = false } # Light client dependencies pallet-eth2-light-client = { git = "https://github.com/webb-tools/pallet-eth2-light-client", default-features = false, tag = "v0.5.0" } diff --git a/pallets/roles/src/impls.rs b/pallets/roles/src/impls.rs index 1d4ea2e8c..a96ddb551 100644 --- a/pallets/roles/src/impls.rs +++ b/pallets/roles/src/impls.rs @@ -28,7 +28,7 @@ use sp_runtime::{ use sp_staking::offence::Offence; use tangle_primitives::{ - jobs::{traits::JobsHandler, ReportValidatorOffence}, + jobs::{traits::JobsHandler, JobId, ReportValidatorOffence}, roles::traits::RolesHandler, }; @@ -75,7 +75,10 @@ impl RolesHandler for Pallet { /// Functions for the pallet. impl Pallet { /// Validate updated profile for the given account. - /// This function will validate the updated profile for the given account. + /// This function will validate the updated profile for the given account by + /// checking if the account has any active jobs for the removed roles. If the + /// account has any active jobs for the removed roles, then it will return + /// the error `RoleCannotBeRemoved`. /// /// # Parameters /// - `account`: The account ID of the validator. @@ -85,10 +88,10 @@ impl Pallet { updated_profile: Profile, ) -> DispatchResult { let ledger = Self::ledger(&account).ok_or(Error::::NoProfileFound)?; + let active_jobs: Vec<(RoleType, JobId)> = T::JobsHandler::get_active_jobs(account.clone()); + // Check if the account has any active jobs for the removed roles. let removed_roles = ledger.profile.get_removed_roles(&updated_profile); if !removed_roles.is_empty() { - let active_jobs = T::JobsHandler::get_active_jobs(account.clone()); - // Check removed roles has any active jobs. for role in removed_roles { for job in active_jobs.clone() { let role_type = job.0; @@ -98,21 +101,83 @@ impl Pallet { } } }; - - let records = updated_profile.get_records(); - let min_restaking_bond = MinRestakingBond::::get(); - - for record in records { - if updated_profile.is_independent() { - // TODO: User cannot update profile to lower the restaking amount if there are any - // active services. - let record_restake = record.amount.unwrap_or_default(); - // Restaking amount of record should meet min restaking amount requirement. + // Changing a current independent profile to shared profile is not allowed if there are + // any active jobs for any role that is currently in the profile. The reason we don't + // allow this is because a user who requested the active job might have done so because + // they wanted independent risk for the security of their application. If the validator + // fails to perform the job of a different role, their stake for "this" role won't be + // affected. It won't fall below the minimum and any removal protocol will only be triggered + // on the role that failed to perform the job. If the validator is now shared, then the + // stake for all roles will be affected. + // + // *** Perhaps this is entirely unnecessary, and I am overthinking it. *** + if updated_profile.is_shared() && ledger.profile.is_independent() { + ensure!(active_jobs.len() == 0, Error::::HasRoleAssigned); + return Ok(()) + } + // Changing a current shared profile to an independent profile is allowed if there are + // active jobs as long as the stake allocated to the active roles is at least as much as + // the shared profile restaking amount. This is because the shared restaking profile for an + // active role is entirely allocated to that role (as it is shared between all selected + // roles). Thus, we allow the user to change to an independent profile as long as the + // restaking amount for the active roles is at least as much as the shared restaking amount. + if updated_profile.is_independent() && ledger.profile.is_shared() { + let roles_with_active_jobs: Vec = + active_jobs.iter().map(|job| job.0).fold(Vec::new(), |mut acc, role| { + if !acc.contains(&role) { + acc.push(role); + } + acc + }); + // For each role with an active job, ensure its stake is greater than or equal to the + // existing ledger's shared restaking amount. + for role in roles_with_active_jobs { + let updated_role_restaking_amount = updated_profile + .get_records() + .iter() + .find_map(|record| if record.role == role { record.amount } else { None }) + .unwrap_or_else(|| Zero::zero()); ensure!( - record_restake >= min_restaking_bond, + updated_role_restaking_amount >= ledger.profile.get_total_profile_restake(), Error::::InsufficientRestakingBond ); } + + return Ok(()) + } + // For each role with an active job, ensure its stake is greater than or equal to the + // existing ledger's restaking amount for that role. If it's a shared profile, then the + // restaking amount for that role is the entire shared restaking amount. + let min_restaking_bond = MinRestakingBond::::get(); + let roles_with_active_jobs: Vec = + active_jobs.iter().map(|job| job.0).fold(Vec::new(), |mut acc, role| { + if !acc.contains(&role) { + acc.push(role); + } + acc + }); + for record in updated_profile.clone().get_records() { + match updated_profile.clone() { + Profile::Independent(_) => + if roles_with_active_jobs.contains(&record.role) { + ensure!( + record.amount.unwrap_or_default() >= min_restaking_bond, + Error::::InsufficientRestakingBond + ); + ensure!( + record.amount.unwrap_or_default() >= ledger.restake_for(&record.role), + Error::::InsufficientRestakingBond + ); + }, + Profile::Shared(profile) => + if roles_with_active_jobs.contains(&record.role) { + ensure!( + record.amount.unwrap_or_default() >= + ledger.profile.get_total_profile_restake(), + Error::::InsufficientRestakingBond + ); + }, + } } Ok(()) } diff --git a/pallets/roles/src/lib.rs b/pallets/roles/src/lib.rs index 3a934b224..4a827b792 100644 --- a/pallets/roles/src/lib.rs +++ b/pallets/roles/src/lib.rs @@ -92,6 +92,14 @@ impl RoleStakingLedger { pub fn total_restake(&self) -> BalanceOf { self.total } + + /// Returns the amount of the stash's balance that is restaked for the given role. + /// If the role is not found, returns zero. + pub fn restake_for(&self, role: &RoleType) -> BalanceOf { + self.roles + .get(role) + .map_or_else(Zero::zero, |record| record.amount.unwrap_or_default()) + } } pub type CurrencyOf = ::Currency; @@ -374,28 +382,36 @@ pub mod pallet { Error::::NotValidator ); let mut ledger = Ledger::::get(&stash_account).ok_or(Error::::NoProfileFound)?; - - let total_profile_restake = updated_profile.get_total_profile_restake(); // Restaking amount of record should meet min restaking amount requirement. - let min_restaking_bond = MinRestakingBond::::get(); - ensure!( - total_profile_restake >= min_restaking_bond, - Error::::InsufficientRestakingBond - ); + match updated_profile.clone() { + Profile::Shared(profile) => { + ensure!( + profile.amount >= MinRestakingBond::::get(), + Error::::InsufficientRestakingBond + ); + }, + Profile::Independent(profile) => + for record in profile.records.iter() { + let record_restake = record.amount.unwrap_or_default(); + ensure!( + record_restake >= MinRestakingBond::::get(), + Error::::InsufficientRestakingBond + ); + }, + }; + // Total restaking amount should not exceed `max_restaking_amount`. let staking_ledger = pallet_staking::Ledger::::get(&stash_account).ok_or(Error::::NotValidator)?; - let max_restaking_bond = Self::calculate_max_restake_amount(staking_ledger.active); - // Total restaking amount should not exceed `max_restaking_amount`. ensure!( - total_profile_restake <= max_restaking_bond, + updated_profile.get_total_profile_restake() <= max_restaking_bond, Error::::ExceedsMaxRestakeValue ); Self::validate_updated_profile(stash_account.clone(), updated_profile.clone())?; ledger.profile = updated_profile.clone(); - ledger.total = total_profile_restake; + ledger.total = updated_profile.get_total_profile_restake().into(); let profile_roles: BoundedVec = BoundedVec::try_from(updated_profile.get_roles()) @@ -406,7 +422,7 @@ pub mod pallet { Self::deposit_event(Event::::ProfileUpdated { account: stash_account.clone(), - total_profile_restake, + total_profile_restake: updated_profile.get_total_profile_restake().into(), roles: updated_profile.get_roles(), }); diff --git a/pallets/roles/src/tests.rs b/pallets/roles/src/tests.rs index 27138d960..bda344c4a 100644 --- a/pallets/roles/src/tests.rs +++ b/pallets/roles/src/tests.rs @@ -175,6 +175,7 @@ fn test_create_profile_should_fail_if_min_required_restake_condition_is_not_met_ #[test] fn test_update_profile_from_independent_to_shared() { new_test_ext(vec![1, 2, 3, 4]).execute_with(|| { + println!("{:?}", pallet::MinRestakingBond::::get()); // Lets create independent profile. let profile = independent_profile(); assert_ok!(Roles::create_profile(RuntimeOrigin::signed(mock_pub_key(1)), profile.clone()));