all tests are correct

runtime-modules/storage/src/lib.rs

@@ -2641,9 +2641,12 @@ impl<T: Trait> DataObjectStorage<T> for Module<T> {
         deletion_prize: Option<DynamicBagDeletionPrize<T>>,
     ) -> DispatchResult {
         // validate params and get storage & distribution buckets
-        let (storage_bucket_ids, distribution_bucket_ids) =
+        let bag_change =
             Self::validate_create_dynamic_bag_params(&dynamic_bag_id, &deletion_prize, &None)?;
 
+        let (storage_bucket_ids, distribution_bucket_ids) =
+            Self::pick_buckets_for_bag(dynamic_bag_id.clone(), &bag_change)?;
+
         //
         // == MUTATION SAFE ==
         //
@@ -2662,12 +2665,18 @@ impl<T: Trait> DataObjectStorage<T> for Module<T> {
         deletion_prize: Option<DynamicBagDeletionPrize<T>>,
         params: UploadParameters<T>,
     ) -> DispatchResult {
+        let bag_change = Self::validate_create_dynamic_bag_params(
+            &dynamic_bag_id,
+            &deletion_prize,
+            &Some(params),
+        )?;
+
         let (storage_bucket_ids, distribution_bucket_ids) =
-            Self::validate_create_dynamic_bag_params(
-                &dynamic_bag_id,
-                &deletion_prize,
-                &Some(params),
-            )?;
+            Self::pick_buckets_for_bag(dynamic_bag_id.clone(), &bag_change)?;
+
+        //
+        // == MUTATION SAFE ==
+        //
         Self::create_dynamic_bag_inner(
             &dynamic_bag_id,
             &deletion_prize,
@@ -2685,35 +2694,17 @@ impl<T: Trait> DataObjectStorage<T> for Module<T> {
     }
 
     fn can_create_dynamic_bag_with_objects_constraints(
-        bag_id: &DynamicBagId<T>,
+        dynamic_bag_id: &DynamicBagId<T>,
         deletion_prize: &Option<DynamicBagDeletionPrize<T>>,
         params: &UploadParameters<T>,
     ) -> DispatchResult {
-        Self::validate_create_dynamic_bag_params(bag_id, deletion_prize, &Some(params.clone()))
-            .map(|_| ())
-        // Self::can_create_dynamic_bag(bag_id, deletion_prize)?;
-        // Self::check_global_uploading_block()?;
-
-        // Self::ensure_objects_creation_list_validity(&params.object_creation_list)?;
-
-        // let bag_change = Self::construct_bag_change(&params.object_creation_list)?;
-
-        // Self::ensure_enough_balance_for_upload(
-        //     &bag_change,
-        //     &params.deletion_prize_source_account_id,
-        //     &params.expected_data_size_fee,
-        // )?;
-
-        // let (storage_bucket_ids, distribution_bucket_ids) =
-        //     Self::pick_buckets_for_bag(bag_id.clone(), Some(bag_change.voucher_update));
-
-        // // check if storage buckets have been found
-        // ensure!(
-        //     !storage_bucket_ids.is_empty(),
-        //     Error::<T>::StorageBucketIdCollectionsAreEmpty
-        // );
+        let bag_change = Self::validate_create_dynamic_bag_params(
+            dynamic_bag_id,
+            deletion_prize,
+            &Some(params.clone()),
+        )?;
 
-        // Ok((storage_bucket_ids, distribution_bucket_ids))
+        Self::pick_buckets_for_bag(dynamic_bag_id.clone(), &bag_change).map(|_| ())
     }
 
     fn ensure_bag_exists(bag_id: &BagId<T>) -> Result<Bag<T>, DispatchError> {
@@ -2826,13 +2817,14 @@ impl<T: Trait> Module<T> {
         dynamic_bag_id: &DynamicBagId<T>,
         deletion_prize: &Option<DynamicBagDeletionPrize<T>>,
         upload_params: &Option<UploadParameters<T>>,
-    ) -> Result<BucketPair<T>, DispatchError> {
+    ) -> Result<Option<BagUpdate<BalanceOf<T>>>, DispatchError> {
+        let bag_id: BagId<T> = dynamic_bag_id.clone().into();
         ensure!(
-            !<Bags<T>>::contains_key(dynamic_bag_id.clone().into()),
+            !<Bags<T>>::contains_key(bag_id),
             Error::<T>::DynamicBagExists
         );
 
-        let bag_change = upload_params.map_or(Ok(None), |params| {
+        let bag_change = upload_params.as_ref().map_or(Ok(None), |params| {
             // check params are valid
             ensure!(
                 params.expected_data_size_fee == Self::data_object_per_mega_byte_fee(),
@@ -2844,36 +2836,36 @@ impl<T: Trait> Module<T> {
             Self::ensure_objects_creation_list_validity(&params.object_creation_list)?;
 
             // compute bag change due to objects creation list
-            let bag_change = Self::construct_bag_change(&params.object_creation_list)?;
-
-            Ok(Some(bag_change))
+            Self::construct_bag_change(&params.object_creation_list).map(|res| Some(res))
         })?;
 
-        Self::ensure_sufficient_balance_for_dynamic_bag_creation(deletion_prize, &bag_change)?;
-
-        let (storage_bucket_ids, distribution_bucket_ids) =
-            bag_change.map_or(Ok((BTreeSet::new(), BTreeSet::new())), |bag_change| {
-                Self::pick_buckets_for_bag(dynamic_bag_id.clone(), Some(bag_change.voucher_update))
-            })?;
+        let total_upload_fee = deletion_prize
+            .as_ref()
+            .map_or(Zero::zero(), |del_prize| del_prize.prize)
+            .saturating_add(bag_change.as_ref().map_or(Zero::zero(), |bag_change| {
+                Self::compute_upload_fees(bag_change)
+            }));
+
+        Self::ensure_sufficient_balance_for_upload(
+            deletion_prize
+                .as_ref()
+                .map(|del_prize| del_prize.account_id.clone()),
+            total_upload_fee,
+        )?;
 
-        Ok((storage_bucket_ids, distribution_bucket_ids))
+        Ok(bag_change)
     }
 
-    fn ensure_sufficient_balance_for_dynamic_bag_creation(
-        deletion_prize: &Option<DynamicBagDeletionPrize<T>>,
-        bag_change: &Option<BagUpdate<T>>,
+    fn ensure_sufficient_balance_for_upload(
+        deletion_prize_source_account_id: Option<T::AccountId>,
+        required_balance: BalanceOf<T>,
     ) -> DispatchResult {
-        let usable_balance = deletion_prize.map_or(Zero::zero(), |del_prize| {
-            Balances::<T>::usable_balance(del_prize.account_id)
+        let usable_balance = deletion_prize_source_account_id.map_or(Zero::zero(), |account_id| {
+            Balances::<T>::usable_balance(account_id)
         });
 
-        let dyn_bag_prize = deletion_prize.map_or(Zero::zero(), |del_prize| del_prize.prize);
-
-        let upload_fees = bag_change.as_ref().map_or(Zero::zero(), |bag_change| {
-            Self::compute_upload_fees(bag_change)
-        });
         ensure!(
-            usable_balance >= dyn_bag_prize.saturating_add(upload_fees),
+            usable_balance >= required_balance,
             Error::<T>::InsufficientBalance
         );
         Ok(())
@@ -3277,12 +3269,9 @@ impl<T: Trait> Module<T> {
             Error::<T>::DataSizeFeeChanged
         );
 
-        Self::ensure_sufficient_balance_for_dynamic_bag_creation(
-            &Some(DynamicBagDeletionPrize::<T> {
-                prize: Zero::zero(),
-                account_id: params.deletion_prize_source_account_id.clone(),
-            }),
-            &Some(bag_change),
+        Self::ensure_sufficient_balance_for_upload(
+            Some(params.deletion_prize_source_account_id.clone()),
+            Self::compute_upload_fees(&bag_change),
         )?;
 
         Self::ensure_upload_bag_validity(&params.bag_id, &bag_change.voucher_update)?;
@@ -3466,19 +3455,23 @@ impl<T: Trait> Module<T> {
     // helper pick buckets for bag
     fn pick_buckets_for_bag(
         dynamic_bag_id: DynamicBagId<T>,
-        voucher_update: Option<VoucherUpdate>,
+        bag_change: &Option<BagUpdate<BalanceOf<T>>>,
     ) -> Result<BucketPair<T>, DispatchError> {
         let bag_type: DynamicBagType = dynamic_bag_id.into();
 
-        let storage_bucket_ids =
-            Self::pick_storage_buckets_for_dynamic_bag(bag_type, voucher_update);
+        let storage_bucket_ids = Self::pick_storage_buckets_for_dynamic_bag(
+            bag_type,
+            bag_change.map(|bag_change| bag_change.voucher_update.clone()),
+        );
 
         let distribution_bucket_ids = Self::pick_distribution_buckets_for_dynamic_bag(bag_type);
 
-        ensure!(
-            !storage_bucket_ids.is_empty(),
-            Error::<T>::StorageBucketIdCollectionsAreEmpty
-        );
+        if bag_change.is_some() {
+            ensure!(
+                !storage_bucket_ids.is_empty(),
+                Error::<T>::StorageBucketIdCollectionsAreEmpty
+            );
+        }
 
         Ok((storage_bucket_ids, distribution_bucket_ids))
     }

runtime-modules/storage/src/tests/fixtures.rs

@@ -1150,11 +1150,6 @@ impl CreateDynamicBagWithObjectsFixture {
     }
 
     pub fn call_and_assert(&self, expected_result: DispatchResult) {
-        let checker_result = Storage::can_create_dynamic_bag_with_objects_constraints(
-            &self.bag_id,
-            &self.deletion_prize,
-            &self.upload_parameters,
-        );
         let actual_result = Storage::create_dynamic_bag_with_objects_constraints(
             self.bag_id.clone(),
             self.deletion_prize.clone(),
@@ -1166,12 +1161,6 @@ impl CreateDynamicBagWithObjectsFixture {
         if actual_result.is_ok() {
             let bag_id: BagId<Test> = self.bag_id.clone().into();
             assert!(<crate::Bags<Test>>::contains_key(&bag_id));
-            assert!(checker_result.map_or(false, |pair| pair.0.iter().all(|id| {
-                Storage::ensure_storage_bucket_exists(id).map_or(false, |bucket| {
-                    bucket.voucher.objects_limit >= bucket.voucher.objects_used
-                        && bucket.voucher.size_limit >= bucket.voucher.size_used
-                })
-            })));
         }
     }
 }