refac(permission0): general pre-release tweaks (#103)

This change serves more as a QoL update then everything.

The previous ID generation could allow for duplicate IDs in very
specific cases, and that is now handled. When the enforcement
authorities change, all ongoing enforcement votes for that permission
are wiped. We cover more cases of input validation.

Author: saiintbrisson

.vscode/settings.json

@@ -8,7 +8,7 @@
   "rust-analyzer.check.extraEnv": {
     "SKIP_WASM_BUILD": "1"
   },
-  "rust-analyzer.cargo.features": ["runtime-benchmarks"],
+  "rust-analyzer.cargo.features": ["runtime-benchmarks", "testnet"],
   "coverage-gutters.coverageFileNames": ["target/cov.xml"],
   // Spell checker
   "cSpell.words": [

Cargo.lock

@@ -52,6 +52,7 @@ num-traits = { version = "0.2.19", default-features = false, features = [
     "libm",
 ] }
 bitflags = { version = "2.9.1", default-features = false }
+rand = { version = "0.9.1", default-features = false }
 
 # Frontier / EVM
 fc-api = { git = "https://github.com/paritytech/frontier.git", rev = "b9b1c620c8b418bdeeadc79725f9cfa4703c0333" }

Cargo.toml

@@ -33,3 +33,4 @@ pallet-torus0.workspace = true
 pallet-emission0.workspace = true
 pallet-permission0.workspace = true
 pallet-governance-api.workspace = true
+rand = { workspace = true, features = ["thread_rng"] }

pallets/faucet/Cargo.toml

@@ -140,7 +140,7 @@ mod benchmarks {
 
         let params = proposal::GlobalParamsData {
             min_name_length: 2,
-            max_name_length: T::MaxAgentNameLengthConstraint::get() as u16 - 1,
+            max_name_length: (T::MaxAgentNameLengthConstraint::get() as u16).saturating_sub(1),
             max_allowed_agents: 1,
             min_weight_control_fee: 1,
             min_staking_fee: 1,

pallets/governance/src/benchmarking.rs

@@ -5,17 +5,28 @@ use polkadot_sdk::{
     frame_benchmarking::{account, v2::*},
     frame_system::RawOrigin,
     sp_runtime::Percent,
-    sp_std::vec,
 };
 
 use crate::*;
 
 #[benchmarks]
 mod benchmarks {
-    use polkadot_sdk::sp_std::collections::btree_map::BTreeMap;
+    use polkadot_sdk::{
+        sp_core::TryCollect, sp_std::collections::btree_map::BTreeMap, sp_std::vec,
+    };
 
     use super::*;
 
+    macro_rules! bounded_btree_map {
+        ($ ( $key:expr => $value:expr ),* $(,)?) => {
+            {
+                TryCollect::<$crate::BoundedBTreeMap<_, _, _>>::try_collect(
+                    vec![$(($key, $value)),*].into_iter()
+                ).unwrap()
+            }
+        };
+    }
+
     #[benchmark]
     fn grant_emission_permission() {
         let grantor: T::AccountId = account("Grantor", 0, 0);
@@ -35,7 +46,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
         let distribution = DistributionControl::Manual;
         let duration = PermissionDuration::Indefinite;
         let revocation = RevocationTerms::RevocableByGrantor;
@@ -76,7 +87,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
         let permission_id = ext::emission_impl::grant_emission_permission_impl::<T>(
             grantor.clone(),
             grantee,
@@ -116,7 +127,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
 
         let permission_id = ext::emission_impl::grant_emission_permission_impl::<T>(
             grantor.clone(),
@@ -159,7 +170,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
 
         let permission_id = ext::emission_impl::grant_emission_permission_impl::<T>(
             grantor.clone(),
@@ -201,7 +212,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
         let controllers = vec![controller.clone()].try_into().unwrap();
 
         let enforcement = EnforcementAuthority::ControlledBy {
@@ -250,7 +261,7 @@ mod benchmarks {
         let stream_id: StreamId = [0; 32].into();
         let streams = BTreeMap::from([(stream_id, Percent::from_percent(30))]);
         let allocation = EmissionAllocation::Streams(streams.try_into().unwrap());
-        let targets = vec![(target, 100)];
+        let targets = bounded_btree_map![target => 100];
 
         let permission_id = ext::emission_impl::grant_emission_permission_impl::<T>(
             grantor.clone(),
@@ -266,16 +277,15 @@ mod benchmarks {
         .expect("failed to grant permission");
 
         // Prepare new controllers
-        let controllers = vec![controller1, controller2];
+        let controllers = vec![controller1, controller2].try_into().unwrap();
         let required_votes = 1u32;
-
-        #[extrinsic_call]
-        set_enforcement_authority(
-            RawOrigin::Signed(grantor),
-            permission_id,
+        let enforcement = EnforcementAuthority::ControlledBy {
             controllers,
             required_votes,
-        )
+        };
+
+        #[extrinsic_call]
+        set_enforcement_authority(RawOrigin::Signed(grantor), permission_id, enforcement)
     }
 
     #[benchmark]

pallets/permission0/src/benchmarking.rs

@@ -134,7 +134,7 @@ pub fn grant_curator_permission_impl<T: Config>(
     }
 
     let scope = PermissionScope::Curator(CuratorScope { flags, cooldown });
-    let permission_id = generate_permission_id::<T>(&grantor, &grantee, &scope);
+    let permission_id = generate_permission_id::<T>(&grantor, &grantee, &scope)?;
 
     let contract = PermissionContract {
         grantor,

pallets/permission0/src/ext/curator_impl.rs

@@ -19,12 +19,11 @@ use polkadot_sdk::{
     },
     frame_system::{self, ensure_signed_or_root},
     polkadot_sdk_frame::prelude::{BlockNumberFor, OriginFor},
-    sp_core::Get,
+    sp_core::{Get, TryCollect},
     sp_runtime::{
         traits::{CheckedAdd, Saturating, Zero},
-        DispatchError, Percent, Vec,
+        BoundedBTreeMap, DispatchError, Percent, Vec,
     },
-    sp_std::collections::btree_map::BTreeMap,
 };
 
 use pallet_torus0_api::Torus0Api;
@@ -70,6 +69,11 @@ impl<T: Config>
         let revocation = super::translate_revocation_terms(revocation)?;
         let enforcement = super::translate_enforcement_authority(enforcement)?;
 
+        let targets = targets
+            .into_iter()
+            .try_collect()
+            .map_err(|_| crate::Error::<T>::TooManyTargets)?;
+
         grant_emission_permission_impl::<T>(
             grantor,
             grantee,
@@ -114,7 +118,7 @@ pub(crate) fn grant_emission_permission_impl<T: Config>(
     grantor: T::AccountId,
     grantee: T::AccountId,
     allocation: EmissionAllocation<T>,
-    targets: Vec<(T::AccountId, u16)>,
+    targets: BoundedBTreeMap<T::AccountId, u16, T::MaxTargetsPerPermission>,
     distribution: DistributionControl<T>,
     duration: PermissionDuration<T>,
     revocation: RevocationTerms<T>,
@@ -134,7 +138,8 @@ pub(crate) fn grant_emission_permission_impl<T: Config>(
     ensure!(grantor != grantee, Error::<T>::SelfPermissionNotAllowed);
     ensure!(!targets.is_empty(), Error::<T>::NoTargetsSpecified);
 
-    for (target, _) in &targets {
+    for (target, weight) in &targets {
+        ensure!(*weight > 0, Error::<T>::InvalidTargetWeight);
         ensure!(
             T::Torus::is_agent_registered(target),
             Error::<T>::NotRegisteredAgent
@@ -201,11 +206,6 @@ pub(crate) fn grant_emission_permission_impl<T: Config>(
         );
     }
 
-    let target_map: BTreeMap<_, _> = targets.into_iter().collect();
-    let targets = target_map
-        .try_into()
-        .map_err(|_| Error::<T>::TooManyTargets)?;
-
     let emission_scope = EmissionScope {
         allocation: allocation.clone(),
         distribution,
@@ -215,7 +215,7 @@ pub(crate) fn grant_emission_permission_impl<T: Config>(
 
     let scope = PermissionScope::Emission(emission_scope);
 
-    let permission_id = generate_permission_id::<T>(&grantor, &grantee, &scope);
+    let permission_id = generate_permission_id::<T>(&grantor, &grantee, &scope)?;
 
     let contract = PermissionContract {
         grantor: grantor.clone(),

pallets/permission0/src/ext/emission_impl.rs

@@ -221,12 +221,19 @@ pub mod pallet {
     pub enum Error<T> {
         /// The agent is not registered
         NotRegisteredAgent,
+        /// Permissions can only be created through extrinsics
+        PermissionCreationOutsideExtrinsic,
+        /// A permission with the same exact parameters was
+        /// already created in the current block
+        DuplicatePermissionInBlock,
         /// Permission not found
         PermissionNotFound,
         /// Self-permission is not allowed
         SelfPermissionNotAllowed,
         /// Invalid percentage (out of range)
         InvalidPercentage,
+        /// Invalid emission weight set to target
+        InvalidTargetWeight,
         /// No targets specified
         NoTargetsSpecified,
         /// Invalid threshold
@@ -296,7 +303,7 @@ pub mod pallet {
             origin: OriginFor<T>,
             grantee: T::AccountId,
             allocation: EmissionAllocation<T>,
-            targets: Vec<(T::AccountId, u16)>,
+            targets: BoundedBTreeMap<T::AccountId, u16, T::MaxTargetsPerPermission>,
             distribution: DistributionControl<T>,
             duration: PermissionDuration<T>,
             revocation: RevocationTerms<T>,
@@ -373,53 +380,18 @@ pub mod pallet {
         pub fn set_enforcement_authority(
             origin: OriginFor<T>,
             permission_id: PermissionId,
-            controllers: Vec<T::AccountId>,
-            required_votes: u32,
+            enforcement: EnforcementAuthority<T>,
         ) -> DispatchResult {
             let who = ensure_signed_or_root(origin)?;
 
-            let mut contract =
+            let contract =
                 Permissions::<T>::get(permission_id).ok_or(Error::<T>::PermissionNotFound)?;
 
-            // Only grantor or root can set enforcement authority
             if let Some(who) = &who {
                 ensure!(who == &contract.grantor, Error::<T>::NotPermissionGrantor);
             }
 
-            ensure!(
-                !controllers.is_empty(),
-                Error::<T>::InvalidNumberOfControllers
-            );
-            ensure!(required_votes > 0, Error::<T>::InvalidNumberOfControllers);
-            ensure!(
-                required_votes as usize <= controllers.len(),
-                Error::<T>::InvalidNumberOfControllers
-            );
-
-            let controllers = controllers
-                .try_into()
-                .map_err(|_| Error::<T>::TooManyControllers)?;
-
-            contract.enforcement = EnforcementAuthority::ControlledBy {
-                controllers,
-                required_votes,
-            };
-
-            Permissions::<T>::insert(permission_id, contract.clone());
-
-            if let EnforcementAuthority::ControlledBy {
-                controllers,
-                required_votes,
-            } = &contract.enforcement
-            {
-                <Pallet<T>>::deposit_event(Event::EnforcementAuthoritySet {
-                    permission_id,
-                    controllers_count: controllers.len() as u32,
-                    required_votes: *required_votes,
-                });
-            }
-
-            Ok(())
+            contract.update_enforcement(permission_id, enforcement)
         }
 
         /// Grant a permission for curator delegation
@@ -469,7 +441,7 @@ fn update_permission_indices<T: Config>(
     grantee: &T::AccountId,
     permission_id: PermissionId,
 ) -> Result<(), DispatchError> {
-    // Update (grantor, grantee) -> permission_id mapping
+    // Update (grantor, grantee) -> [permission_id] mapping
     PermissionsByParticipants::<T>::try_mutate(
         (grantor.clone(), grantee.clone()),
         |permissions| -> Result<(), DispatchError> {
@@ -491,7 +463,7 @@ fn update_permission_indices<T: Config>(
         },
     )?;
 
-    // Update grantor -> permission_id mapping
+    // Update grantor -> [permission_id] mapping
     PermissionsByGrantor::<T>::try_mutate(
         grantor.clone(),
         |permissions| -> Result<(), DispatchError> {
@@ -513,7 +485,7 @@ fn update_permission_indices<T: Config>(
         },
     )?;
 
-    // Update grantee -> permission_id mapping
+    // Update grantee -> [permission_id] mapping
     PermissionsByGrantee::<T>::try_mutate(
         grantee.clone(),
         |permissions| -> Result<(), DispatchError> {