From 60aa7fbe13e79c780de99882cf611470a6ecfce4 Mon Sep 17 00:00:00 2001
From: Michael Assaf <94772640+snowmead@users.noreply.github.com>
Date: Thu, 30 Jan 2025 11:25:01 -0500
Subject: [PATCH] fix: Skip BSP confirm file keys that have failed to update or
 create a payment stream (#341)

* skip file keys that have failed to update or create a payment stream

* add docs

* amend; docs
---
 pallets/file-system/src/utils.rs | 64 +++++++++++++++++++++-----------
 1 file changed, 42 insertions(+), 22 deletions(-)

diff --git a/pallets/file-system/src/utils.rs b/pallets/file-system/src/utils.rs
index c9b519af7..17832ea87 100644
--- a/pallets/file-system/src/utils.rs
+++ b/pallets/file-system/src/utils.rs
@@ -1380,6 +1380,48 @@ where
                 Error::<T>::InsufficientAvailableCapacity
             );
 
+            // All errors from the payment stream operations (create/update) are ignored, and the file key is added to the `skipped_file_keys` set instead of erroring out.
+            // This is done to avoid a malicious user, owner of one of the files from the batch of confirmations, being able to prevent the BSP from confirming any files by making itself insolvent so payment stream operations fail.
+            // This operation must be executed first, before updating any storage elements, to prevent potential cases
+            // where a storage element is updated but should not be.
+            match <T::PaymentStreams as PaymentStreamsInterface>::get_dynamic_rate_payment_stream_amount_provided(&bsp_id, &storage_request_metadata.owner) {
+				Some(previous_amount_provided) => {
+					// Update the payment stream.
+                    let new_amount_provided = &previous_amount_provided.checked_add(&storage_request_metadata.size).ok_or(ArithmeticError::Overflow)?;
+					if let Err(_) = <T::PaymentStreams as PaymentStreamsInterface>::update_dynamic_rate_payment_stream(
+						&bsp_id,
+						&storage_request_metadata.owner,
+						new_amount_provided,
+					) {
+                        // Skip file key if we could not successfully update the payment stream
+                        expect_or_err!(
+                            skipped_file_keys.try_insert(file_key),
+                            "Failed to push file key to skipped_file_keys",
+                            Error::<T>::TooManyStorageRequestResponses,
+                            result
+                        );
+                        continue;
+                    }
+				},
+				None => {
+					// Create the payment stream.
+					if let Err(_) = <T::PaymentStreams as PaymentStreamsInterface>::create_dynamic_rate_payment_stream(
+						&bsp_id,
+						&storage_request_metadata.owner,
+						&storage_request_metadata.size,
+					) {
+                        // Skip file key if we could not successfully create the payment stream
+                        expect_or_err!(
+                            skipped_file_keys.try_insert(file_key),
+                            "Failed to push file key to skipped_file_keys",
+                            Error::<T>::TooManyStorageRequestResponses,
+                            result
+                        );
+                        continue;
+                    }
+				}
+			}
+
             // Increment the number of BSPs confirmed.
             match storage_request_metadata
                 .bsps_confirmed
@@ -1419,28 +1461,6 @@ where
                 storage_request_metadata.size,
             )?;
 
-            // Check if a payment stream between the user and provider already exists.
-            // If it does not, create it. If it does, update it.
-            match <T::PaymentStreams as PaymentStreamsInterface>::get_dynamic_rate_payment_stream_amount_provided(&bsp_id, &storage_request_metadata.owner) {
-				Some(previous_amount_provided) => {
-					// Update the payment stream.
-                    let new_amount_provided = &previous_amount_provided.checked_add(&storage_request_metadata.size).ok_or(ArithmeticError::Overflow)?;
-					<T::PaymentStreams as PaymentStreamsInterface>::update_dynamic_rate_payment_stream(
-						&bsp_id,
-						&storage_request_metadata.owner,
-						new_amount_provided,
-					)?;
-				},
-				None => {
-					// Create the payment stream.
-					<T::PaymentStreams as PaymentStreamsInterface>::create_dynamic_rate_payment_stream(
-						&bsp_id,
-						&storage_request_metadata.owner,
-						&storage_request_metadata.size,
-					)?;
-				}
-			}
-
             // Get the file metadata to insert into the Provider's trie under the file key.
             let file_metadata = storage_request_metadata.clone().to_file_metadata();
             let encoded_trie_value = file_metadata.encode();