Merge pull request #2745 from ignazio-bovo/giza_deletion_bug

addressed bug when dealing with bags during deletion

runtime-modules/content/src/lib.rs

@@ -98,6 +98,9 @@ pub trait Trait:
 
     /// The maximum number of curators per group constraint
     type MaxNumberOfCuratorsPerGroup: Get<MaxNumber>;
+
+    /// The storage type used
+    type DataObjectStorage: storage::DataObjectStorage<Self>;
 }
 
 /// The owner of a channel, is the authorized "actor" that can update
@@ -747,28 +750,30 @@ decl_module! {
             let dyn_bag = DynamicBagIdType::<T::MemberId, T::ChannelId>::Channel(channel_id);
             let bag_id = storage::BagIdType::from(dyn_bag.clone());
 
-            // ensure that bag size provided is valid
-            ensure!(
-                storage::Bags::<T>::get(&bag_id).objects_number == num_objects_to_delete,
-                Error::<T>::InvalidBagSizeSpecified
-            );
-
-            // construct collection of assets to be removed
-            let assets_to_remove: BTreeSet<DataObjectId<T>> =
-                storage::DataObjectsById::<T>::iter_prefix(&bag_id).map(|x| x.0).collect();
-
-            // remove specified assets from storage
-            Self::remove_assets_from_storage(
-                &assets_to_remove,
-                &channel_id,
-                &channel.deletion_prize_source_account_id
-            )?;
+            // channel has a dynamic bag associated to it -> remove assets from storage
+            if let Ok(bag) = T::DataObjectStorage::ensure_bag_exists(&bag_id) {
+                // ensure that bag size provided is valid
+                ensure!(
+                    bag.objects_number == num_objects_to_delete,
+                    Error::<T>::InvalidBagSizeSpecified
+                );
 
-            // delete channel dynamic bag
-            Storage::<T>::delete_dynamic_bag(
-                channel.deletion_prize_source_account_id,
-                dyn_bag
-            )?;
+                // construct collection of assets to be removed
+                let assets_to_remove = T::DataObjectStorage::get_data_objects_id(&bag_id);
+
+                // remove specified assets from storage
+                Self::remove_assets_from_storage(
+                    &assets_to_remove,
+                    &channel_id,
+                    &channel.deletion_prize_source_account_id
+                )?;
+
+                // delete channel dynamic bag
+                Storage::<T>::delete_dynamic_bag(
+                    channel.deletion_prize_source_account_id,
+                    dyn_bag
+                )?;
+            }
 
             //
             // == MUTATION SAFE ==
@@ -1336,7 +1341,7 @@ impl<T: Trait> Module<T> {
         let dyn_bag = DynamicBagIdType::<T::MemberId, T::ChannelId>::Channel(*channel_id);
         let bag_id = BagIdType::from(dyn_bag.clone());
 
-        if !storage::Bags::<T>::contains_key(bag_id.clone()) {
+        if T::DataObjectStorage::ensure_bag_exists(&bag_id).is_err() {
             // create_dynamic_bag checks automatically satifsfied with None as second parameter
             Storage::<T>::create_dynamic_bag(dyn_bag, None).unwrap();
         }

runtime-modules/content/src/tests/channels.rs

@@ -68,6 +68,26 @@ fn successful_channel_deletion() {
             3u64,
             Ok(()),
         );
+
+        // create a channel with no assets:
+        let empty_channel_id = Content::next_channel_id();
+        create_channel_mock(
+            FIRST_MEMBER_ORIGIN,
+            ContentActor::Member(FIRST_MEMBER_ID),
+            ChannelCreationParametersRecord {
+                assets: None,
+                meta: None,
+                reward_account: None,
+            },
+            Ok(()),
+        );
+        delete_channel_mock(
+            FIRST_MEMBER_ORIGIN,
+            ContentActor::Member(FIRST_MEMBER_ID),
+            empty_channel_id,
+            43u64, // this param will be discarded if channel has no assets
+            Ok(()),
+        );
     })
 }
 

runtime-modules/content/src/tests/mock.rs

@@ -358,6 +358,9 @@ impl Trait for Test {
 
     /// The maximum number of curators per group constraint
     type MaxNumberOfCuratorsPerGroup = MaxNumberOfCuratorsPerGroup;
+
+    /// The data object used in storage
+    type DataObjectStorage = storage::Module<Self>;
 }
 
 pub type System = frame_system::Module<Test>;

runtime-modules/storage/src/lib.rs

@@ -211,6 +211,12 @@ pub trait DataObjectStorage<T: Trait> {
         bag_id: &DynamicBagId<T>,
         deletion_prize: &Option<DynamicBagDeletionPrize<T>>,
     ) -> DispatchResult;
+
+    /// Checks if a bag does exists and returns it. Static Always exists
+    fn ensure_bag_exists(bag_id: &BagId<T>) -> Result<Bag<T>, DispatchError>;
+
+    /// Get all objects id in a bag, without checking its existence
+    fn get_data_objects_id(bag_id: &BagId<T>) -> BTreeSet<T::DataObjectId>;
 }
 
 /// Storage trait.
@@ -2757,6 +2763,16 @@ impl<T: Trait> DataObjectStorage<T> for Module<T> {
     ) -> DispatchResult {
         Self::validate_create_dynamic_bag_params(bag_id, deletion_prize)
     }
+
+    fn ensure_bag_exists(bag_id: &BagId<T>) -> Result<Bag<T>, DispatchError> {
+        Self::ensure_bag_exists(bag_id)
+    }
+
+    fn get_data_objects_id(bag_id: &BagId<T>) -> BTreeSet<T::DataObjectId> {
+        DataObjectsById::<T>::iter_prefix(&bag_id)
+            .map(|x| x.0)
+            .collect()
+    }
 }
 
 impl<T: Trait> Module<T> {

runtime/src/lib.rs

@@ -442,6 +442,7 @@ impl content::Trait for Runtime {
     type SeriesId = SeriesId;
     type ChannelOwnershipTransferRequestId = ChannelOwnershipTransferRequestId;
     type MaxNumberOfCuratorsPerGroup = MaxNumberOfCuratorsPerGroup;
+    type DataObjectStorage = storage::Module<Runtime>;
 }
 
 impl hiring::Trait for Runtime {