chore(permission0): allow empty recipients

Author: saiintbrisson

pallets/permission0/src/ext/stream_impl.rs

@@ -129,7 +129,7 @@ pub(crate) fn delegate_stream_permission_impl<T: Config>(
         Error::<T>::NotRegisteredAgent
     );
 
-    validate_stream_permission_recipients::<T>(&delegator, &recipients)?;
+    validate_stream_permission_recipients::<T>(&delegator, &revocation, &recipients)?;
 
     match &allocation {
         StreamAllocation::Streams(streams) => {
@@ -355,7 +355,11 @@ pub(crate) fn update_stream_permission<T: Config>(
             return Err(Error::<T>::NotAuthorizedToEdit.into());
         }
 
-        validate_stream_permission_recipients::<T>(&permission.delegator, &new_recipients)?;
+        validate_stream_permission_recipients::<T>(
+            &permission.delegator,
+            &permission.revocation,
+            &new_recipients,
+        )?;
 
         // Remove old indices for current recipients
         crate::permission::remove_permission_from_indices::<T>(
@@ -444,9 +448,12 @@ fn validate_stream_managers<T: Config>(
 
 fn validate_stream_permission_recipients<T: Config>(
     delegator: &T::AccountId,
+    revocation: &RevocationTerms<T>,
     recipients: &BoundedBTreeMap<T::AccountId, u16, T::MaxRecipientsPerPermission>,
 ) -> DispatchResult {
-    ensure!(!recipients.is_empty(), Error::<T>::NoRecipientsSpecified);
+    if !revocation.is_revokable() {
+        ensure!(!recipients.is_empty(), Error::<T>::NoRecipientsSpecified);
+    }
 
     for (recipient, weight) in recipients {
         ensure!(delegator != recipient, Error::<T>::InvalidRecipientWeight);

pallets/permission0/src/permission.rs

@@ -350,13 +350,7 @@ impl<T: Config> PermissionContract<T> {
     }
 
     pub fn is_updatable(&self) -> bool {
-        let current_block = frame_system::Pallet::<T>::block_number();
-
-        match &self.revocation {
-            RevocationTerms::RevocableByDelegator => true,
-            RevocationTerms::RevocableAfter(block) => &current_block > block,
-            _ => false,
-        }
+        self.revocation.is_revokable()
     }
 }
 
@@ -428,6 +422,18 @@ pub enum RevocationTerms<T: Config> {
 }
 
 impl<T: Config> RevocationTerms<T> {
+    /// Returns true if this revocation term is revocable by the delegator
+    /// or the current block is past the one defined.
+    pub fn is_revokable(&self) -> bool {
+        let current_block = frame_system::Pallet::<T>::block_number();
+
+        match &self {
+            RevocationTerms::RevocableByDelegator => true,
+            RevocationTerms::RevocableAfter(block) => &current_block > block,
+            _ => false,
+        }
+    }
+
     /// Checks if the child revocation terms are weaker than the parent.
     pub fn is_weaker(parent: &Self, child: &Self) -> bool {
         match (parent, child) {

pallets/permission0/tests/stream.rs

@@ -1395,22 +1395,64 @@ fn index_consistency_during_complex_recipient_updates() {
         assert!(!participants_2.contains(&permission_id)); // Removed
         assert!(participants_3.contains(&permission_id)); // Still there
         assert!(participants_4.contains(&permission_id)); // Added
+    });
+}
 
-        // Test complete removal of all recipients should delete permission
-        let empty_recipients = BoundedBTreeMap::new();
+#[test]
+fn cannot_create_empty_recipients_for_irrevocable_permissions() {
+    new_test_ext().execute_with(|| {
+        zero_min_burn();
+        let agent_0 = 0;
+        register_empty_agent(agent_0);
+
+        add_balance(agent_0, as_tors(10) + 1);
+
+        let stream_id = generate_root_stream_id(&agent_0);
+        let mut streams = BTreeMap::new();
+        streams.insert(stream_id, Percent::from_percent(50));
 
-        // This should fail as permissions need at least one recipient
         assert_err!(
-            pallet_permission0::Pallet::<Test>::update_stream_permission(
+            pallet_permission0::Pallet::<Test>::delegate_stream_permission(
                 get_origin(agent_0),
-                permission_id,
-                Some(empty_recipients),
+                Default::default(),
+                pallet_permission0::StreamAllocation::Streams(streams.clone().try_into().unwrap()),
+                pallet_permission0::DistributionControl::Manual,
+                pallet_permission0::PermissionDuration::Indefinite,
+                pallet_permission0::RevocationTerms::Irrevocable,
+                pallet_permission0::EnforcementAuthority::None,
                 None,
                 None,
+            ),
+            pallet_permission0::Error::<Test>::NoRecipientsSpecified
+        );
+
+        assert_err!(
+            pallet_permission0::Pallet::<Test>::delegate_stream_permission(
+                get_origin(agent_0),
+                Default::default(),
+                pallet_permission0::StreamAllocation::Streams(streams.clone().try_into().unwrap()),
+                pallet_permission0::DistributionControl::Manual,
+                pallet_permission0::PermissionDuration::Indefinite,
+                pallet_permission0::RevocationTerms::RevocableAfter(10),
+                pallet_permission0::EnforcementAuthority::None,
+                None,
                 None,
-                None
             ),
             pallet_permission0::Error::<Test>::NoRecipientsSpecified
         );
+
+        assert_ok!(
+            pallet_permission0::Pallet::<Test>::delegate_stream_permission(
+                get_origin(agent_0),
+                Default::default(),
+                pallet_permission0::StreamAllocation::Streams(streams.try_into().unwrap()),
+                pallet_permission0::DistributionControl::Manual,
+                pallet_permission0::PermissionDuration::Indefinite,
+                pallet_permission0::RevocationTerms::RevocableByDelegator,
+                pallet_permission0::EnforcementAuthority::None,
+                None,
+                None,
+            )
+        );
     });
 }