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::::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 ::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(_) = ::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::::TooManyStorageRequestResponses, + result + ); + continue; + } + }, + None => { + // Create the payment stream. + if let Err(_) = ::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::::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 ::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)?; - ::update_dynamic_rate_payment_stream( - &bsp_id, - &storage_request_metadata.owner, - new_amount_provided, - )?; - }, - None => { - // Create the payment stream. - ::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(); diff --git a/test/suites/integration/bsp/multiple-delete.test.ts b/test/suites/integration/bsp/multiple-delete.test.ts index 68f11d963..dff30e677 100644 --- a/test/suites/integration/bsp/multiple-delete.test.ts +++ b/test/suites/integration/bsp/multiple-delete.test.ts @@ -138,8 +138,16 @@ describeBspNet("Single BSP Volunteering", ({ before, createBspApi, it, createUse // Wait enough blocks for the deletion to be allowed. const currentBlock = await userApi.rpc.chain.getBlock(); const currentBlockNumber = currentBlock.block.header.number.toNumber(); - const cooldown = - currentBlockNumber + userApi.consts.fileSystem.minWaitForStopStoring.toNumber(); + const minWaitForStopStoring = ( + await userApi.query.parameters.parameters({ + RuntimeConfig: { + MinWaitForStopStoring: null + } + }) + ) + .unwrap() + .asRuntimeConfig.asMinWaitForStopStoring.toNumber(); + const cooldown = currentBlockNumber + minWaitForStopStoring; await userApi.block.skipTo(cooldown); for (let i = 0; i < fileKeys.length; i++) { @@ -281,8 +289,16 @@ describeBspNet("Single BSP Volunteering", ({ before, createBspApi, it, createUse // Wait enough blocks for the deletion to be allowed. const currentBlock = await userApi.rpc.chain.getBlock(); const currentBlockNumber = currentBlock.block.header.number.toNumber(); - const cooldown = - currentBlockNumber + userApi.consts.fileSystem.minWaitForStopStoring.toNumber(); + const minWaitForStopStoring = ( + await userApi.query.parameters.parameters({ + RuntimeConfig: { + MinWaitForStopStoring: null + } + }) + ) + .unwrap() + .asRuntimeConfig.asMinWaitForStopStoring.toNumber(); + const cooldown = currentBlockNumber + minWaitForStopStoring; await userApi.block.skipTo(cooldown); // Batching the delete confirmation should fail because of the wrong inclusionForestProof for extrinsinc 2 and 3 diff --git a/test/suites/integration/bsp/reorg-proof.test.ts b/test/suites/integration/bsp/reorg-proof.test.ts index dfaecde42..c0fbda235 100644 --- a/test/suites/integration/bsp/reorg-proof.test.ts +++ b/test/suites/integration/bsp/reorg-proof.test.ts @@ -353,7 +353,15 @@ describeBspNet( // Wait the required time for the BSP to be able to confirm the deletion. const currentBlock = await userApi.rpc.chain.getBlock(); const currentBlockNumber = currentBlock.block.header.number.toNumber(); - const minWaitForStopStoring = userApi.consts.fileSystem.minWaitForStopStoring.toNumber(); + const minWaitForStopStoring = ( + await userApi.query.parameters.parameters({ + RuntimeConfig: { + MinWaitForStopStoring: null + } + }) + ) + .unwrap() + .asRuntimeConfig.asMinWaitForStopStoring.toNumber(); const blockToAdvanceTo = currentBlockNumber + minWaitForStopStoring; await userApi.block.skipTo(blockToAdvanceTo, { watchForBspProofs: [userApi.shConsts.DUMMY_BSP_ID] diff --git a/test/suites/integration/bsp/storage-delete.test.ts b/test/suites/integration/bsp/storage-delete.test.ts index 15ba4dbd7..71b3a660f 100644 --- a/test/suites/integration/bsp/storage-delete.test.ts +++ b/test/suites/integration/bsp/storage-delete.test.ts @@ -109,8 +109,16 @@ describeBspNet( // Wait for the right moment to confirm stop storing const currentBlock = await userApi.rpc.chain.getBlock(); const currentBlockNumber = currentBlock.block.header.number.toNumber(); - const cooldown = - currentBlockNumber + userApi.consts.fileSystem.minWaitForStopStoring.toNumber(); + const minWaitForStopStoring = ( + await userApi.query.parameters.parameters({ + RuntimeConfig: { + MinWaitForStopStoring: null + } + }) + ) + .unwrap() + .asRuntimeConfig.asMinWaitForStopStoring.toNumber(); + const cooldown = currentBlockNumber + minWaitForStopStoring; // New storage request does not get fulfilled and therefore gets cleaned up and we enqueue a checkpoint challenge remove mutation // Which then the bsp responds to and has the file key get removed from the forest diff --git a/test/suites/integration/bsp/submit-proofs.test.ts b/test/suites/integration/bsp/submit-proofs.test.ts index 5434c94f0..2da8dcf5b 100644 --- a/test/suites/integration/bsp/submit-proofs.test.ts +++ b/test/suites/integration/bsp/submit-proofs.test.ts @@ -214,8 +214,16 @@ describeBspNet( // Wait enough blocks for the deletion to be allowed. const currentBlock = await userApi.rpc.chain.getBlock(); const currentBlockNumber = currentBlock.block.header.number.toNumber(); - const cooldown = - currentBlockNumber + bspThreeApi.consts.fileSystem.minWaitForStopStoring.toNumber(); + const minWaitForStopStoring = ( + await userApi.query.parameters.parameters({ + RuntimeConfig: { + MinWaitForStopStoring: null + } + }) + ) + .unwrap() + .asRuntimeConfig.asMinWaitForStopStoring.toNumber(); + const cooldown = currentBlockNumber + minWaitForStopStoring; await userApi.block.skipTo(cooldown); await userApi.wait.waitForAvailabilityToSendTx(bspThreeKey.address.toString()); @@ -280,9 +288,19 @@ describeBspNet( oneBspfileMetadata = fileMetadata; }); - it("Only one BSP confirms it", async () => { + it("Only one BSP confirms it and the MSP accepts it", async () => { + // Wait for the MSP acceptance of the file to be in the TX pool + await userApi.assert.extrinsicPresent({ + module: "fileSystem", + method: "mspRespondStorageRequestsMultipleBuckets", + checkTxPool: true, + timeout: 5000 + }); + + // Then wait for the BSP volunteer to be in the TX pool and seal the block await userApi.wait.bspVolunteer(1); + // Finally, wait for the BSP to confirm storing the file and seal the block const address = userApi.createType("Address", NODE_INFOS.bsp.AddressId); await userApi.wait.bspStored(1, address); }); @@ -317,18 +335,6 @@ describeBspNet( checkTxPool: true }); - await userApi.assert.extrinsicPresent({ - module: "fileSystem", - method: "mspRespondStorageRequestsMultipleBuckets", - checkTxPool: true - }); - - await userApi.assert.extrinsicPresent({ - module: "fileSystem", - method: "mspRespondStorageRequestsMultipleBuckets", - checkTxPool: true - }); - // Seal block and check that the transaction was successful. await userApi.block.seal();