test: ✅ Fix race conditions in integration tests (#299)

* test: ✅ Fix race condition in `skipTo` waiting for proofs

* test: 🧑‍💻 Improve error log in `waitForTxInPool`

* fix: 🐛 Remove `break` from `for` loop counting pending proof submissions

* test: ✅ Fix race condition of many payment streams charged events in last block

* fix: ✅ Use correct event variable in test for expected value

* fix: ✅ Change `sleep`s for waiting and polling functions avoiding race conditions

* fix: ✅ Fix `waitForTxInPool`

* style: 🎨 Apply TS formatting

* fix: ✅ Increase timeout to 10s in `waitForBspStoredWithoutSealing` and `waitForMspResponseWithoutSealing`

* fix: ✅ Increase default timeout in `waitForTxInPool`

* fix: ✅ Clarity logic for waiting for proofs in `skipTo`

* fix: 🩹 Remove `only: true` from test

* test: ✅ Simplify `bsp-thresholds` test to avoid race conditions

* revert: ⏪ Rollback runtime change to speed up CI

* fix: ✅ Change name of parameter `watchForBspProofs` in `skipTo` to match `advanceToBlock`, making it pass down the parameter

* fix: ✅ Fix submit-proofs tests race conditions

* revert: ⏪ Rollback `only: true` flag

* fix: ✅ Fix race condition of sending transaction with BSP key when there's another in the pool

* fix: ✅ Replace `sleep` for `mspApi.wait.fileStorageComplete`

* fix: ✅ Wait for new signed up BSP to catch up to chain tip

test/scripts/generateBenchmarkProofs.ts

@@ -190,7 +190,7 @@ async function generateBenchmarkProofs() {
     fileKeys.push(fileMetadata.fileKey);
 
     await userApi.wait.bspVolunteer(1);
-    await bspApi.wait.bspFileStorageComplete(fileMetadata.fileKey);
+    await bspApi.wait.fileStorageComplete(fileMetadata.fileKey);
     await userApi.wait.bspStored(1);
   }
 

test/suites/integration/bsp/bsp-thresholds.test.ts

@@ -114,7 +114,7 @@ describeBspNet(
       );
 
       await userApi.sealBlock(
-        userApi.tx.sudo.sudo(userApi.tx.fileSystem.setGlobalParameters(null, 10))
+        userApi.tx.sudo.sudo(userApi.tx.fileSystem.setGlobalParameters(null, 500))
       );
 
       // Create a new BSP and onboard with no reputation
@@ -131,8 +131,8 @@ describeBspNet(
       await userApi.wait.bspCatchUpToChainTip(bspDownApi);
 
       const { fileKey } = await userApi.file.createBucketAndSendNewStorageRequest(
-        "res/smile.jpg",
-        "test/smile.jpg",
+        "res/whatsup.jpg",
+        "test/whatsup.jpg",
         "bucket-1"
       );
 
@@ -150,17 +150,27 @@ describeBspNet(
         )
       ).asOk.toNumber();
 
-      if ((await userApi.rpc.chain.getHeader()).number.toNumber() < lowReputationVolunteerTick) {
-        await userApi.block.skipTo(lowReputationVolunteerTick);
-      }
+      const currentBlockNumber = (await userApi.rpc.chain.getHeader()).number.toNumber();
+      assert(
+        currentBlockNumber === normalReputationVolunteerTick,
+        "The BSP with high reputation should be able to volunteer immediately"
+      );
+      assert(
+        currentBlockNumber < lowReputationVolunteerTick,
+        "The volunteer tick for the low reputation BSP should be in the future"
+      );
 
-      if (normalReputationVolunteerTick < lowReputationVolunteerTick) {
-        await userApi.wait.bspVolunteer();
-      } else {
-        await userApi.wait.bspVolunteer(2);
-      }
+      // Checking volunteering and confirming for the high reputation BSP
+      await userApi.wait.bspVolunteer(1);
+      await bspApi.wait.fileStorageComplete(fileKey);
+      await userApi.wait.bspStored(1);
+
+      // Checking volunteering and confirming for the low reputation BSP
+      await userApi.block.skipTo(lowReputationVolunteerTick);
+      await userApi.wait.bspVolunteer(1);
       const matchedEvents = await userApi.assert.eventMany("fileSystem", "AcceptedBspVolunteer"); // T1
 
+      // Check that it is in fact the BSP with low reputation that just volunteered
       const filtered = matchedEvents.filter(
         ({ event }) =>
           (userApi.events.fileSystem.AcceptedBspVolunteer.is(event) &&
@@ -224,49 +234,23 @@ describeBspNet(
         )
       ).asOk.toNumber();
 
-      if (bsp1VolunteerTick < bsp2VolunteerTick) {
-        // If the first BSP can volunteer first, wait for it to volunteer and confirm storing the file
-        if ((await userApi.rpc.chain.getHeader()).number.toNumber() < bsp1VolunteerTick) {
-          await userApi.block.skipTo(bsp1VolunteerTick);
-        }
-        await userApi.wait.bspVolunteer(1);
-        await bspApi.wait.bspFileStorageComplete(fileKey);
-        await userApi.wait.bspStored(1);
+      assert(bsp1VolunteerTick < bsp2VolunteerTick, "BSP one should be able to volunteer first");
+      const currentBlockNumber = (await userApi.rpc.chain.getHeader()).number.toNumber();
+      assert(
+        currentBlockNumber === bsp1VolunteerTick,
+        "BSP one should be able to volunteer immediately"
+      );
 
-        // Then wait for the second BSP to volunteer and confirm storing the file
-        if ((await userApi.rpc.chain.getHeader()).number.toNumber() < bsp2VolunteerTick) {
-          await userApi.block.skipTo(bsp2VolunteerTick);
-        }
-        await userApi.wait.bspVolunteer(1);
-        await bspTwoApi.wait.bspFileStorageComplete(fileKey);
-        await userApi.wait.bspStored(1);
-      } else if (bsp1VolunteerTick > bsp2VolunteerTick) {
-        // If the second BSP can volunteer first, wait for it to volunteer and confirm storing the file
-        if ((await userApi.rpc.chain.getHeader()).number.toNumber() < bsp2VolunteerTick) {
-          await userApi.block.skipTo(bsp2VolunteerTick);
-        }
-        await userApi.wait.bspVolunteer(1);
-        await bspTwoApi.wait.bspFileStorageComplete(fileKey);
-        await userApi.wait.bspStored(1);
+      await userApi.wait.bspVolunteer(1);
+      await bspApi.wait.fileStorageComplete(fileKey);
+      await userApi.wait.bspStored(1);
 
-        // Then wait for the first BSP to volunteer and confirm storing the file
-        if ((await userApi.rpc.chain.getHeader()).number.toNumber() < bsp1VolunteerTick) {
-          await userApi.block.skipTo(bsp1VolunteerTick);
-        }
-        await userApi.wait.bspVolunteer(1);
-        await bspApi.wait.bspFileStorageComplete(fileKey);
-        await userApi.wait.bspStored(1);
-      } else {
-        // If both BSPs can volunteer at the same time, advance to the tick where both can volunteer
-        if ((await userApi.rpc.chain.getHeader()).number.toNumber() < bsp1VolunteerTick) {
-          await userApi.block.skipTo(bsp1VolunteerTick);
-        }
-        // And wait for them to volunteer and confirm storing the file
-        await userApi.wait.bspVolunteer(2);
-        await bspApi.wait.bspFileStorageComplete(fileKey);
-        await bspTwoApi.wait.bspFileStorageComplete(fileKey);
-        await userApi.wait.bspStored(2);
-      }
+      // Then wait for the second BSP to volunteer and confirm storing the file
+      await userApi.block.skipTo(bsp2VolunteerTick);
+
+      await userApi.wait.bspVolunteer(1);
+      await bspTwoApi.wait.fileStorageComplete(fileKey);
+      await userApi.wait.bspStored(1);
 
       await bspTwoApi.disconnect();
       await userApi.docker.stopBspContainer("sh-bsp-two");
@@ -282,6 +266,7 @@ describeBspNet(
         bspStartingWeight: 800_000_000n
       });
       const bspThreeApi = await BspNetTestApi.create(`ws://127.0.0.1:${rpcPort}`);
+      await userApi.wait.bspCatchUpToChainTip(bspThreeApi);
 
       // Wait for it to catch up to the top of the chain
       await userApi.wait.bspCatchUpToChainTip(bspThreeApi);
@@ -319,11 +304,11 @@ describeBspNet(
       );
 
       // Advance to the tick where the new BSP can volunteer
-      if (
-        (await userApi.rpc.chain.getHeader()).number.toNumber() < highReputationBspVolunteerTick
-      ) {
-        await userApi.block.skipTo(highReputationBspVolunteerTick);
-      }
+      const currentBlockNumber = (await userApi.rpc.chain.getHeader()).number.toNumber();
+      assert(
+        currentBlockNumber === highReputationBspVolunteerTick,
+        "BSP with high reputation should be able to volunteer immediately"
+      );
 
       // Wait until the new BSP volunteers
       await userApi.wait.bspVolunteer(1);

test/suites/integration/bsp/debt-collection.test.ts

@@ -1,13 +1,6 @@
 import assert, { strictEqual } from "node:assert";
 import { after } from "node:test";
-import {
-  bob,
-  describeBspNet,
-  fetchEvent,
-  ShConsts,
-  sleep,
-  type EnrichedBspApi
-} from "../../../util";
+import { bob, describeBspNet, fetchEvent, ShConsts, type EnrichedBspApi } from "../../../util";
 import { BN } from "@polkadot/util";
 
 describeBspNet(
@@ -262,9 +255,9 @@ describeBspNet(
         3 // There are 3 running BSPs to fulfil the storage request
       );
       await userApi.wait.bspVolunteer(3);
-      await bspApi.wait.bspFileStorageComplete(cloudFileMetadata.fileKey);
-      await bspTwoApi.wait.bspFileStorageComplete(cloudFileMetadata.fileKey);
-      await bspThreeApi.wait.bspFileStorageComplete(cloudFileMetadata.fileKey);
+      await bspApi.wait.fileStorageComplete(cloudFileMetadata.fileKey);
+      await bspTwoApi.wait.fileStorageComplete(cloudFileMetadata.fileKey);
+      await bspThreeApi.wait.fileStorageComplete(cloudFileMetadata.fileKey);
       await userApi.wait.bspStored(3);
 
       const adolphusFileMetadata = await userApi.file.createBucketAndSendNewStorageRequest(
@@ -277,9 +270,9 @@ describeBspNet(
         3 // There are 3 running BSPs to fulfil the storage request
       );
       await userApi.wait.bspVolunteer(3);
-      await bspApi.wait.bspFileStorageComplete(adolphusFileMetadata.fileKey);
-      await bspTwoApi.wait.bspFileStorageComplete(adolphusFileMetadata.fileKey);
-      await bspThreeApi.wait.bspFileStorageComplete(adolphusFileMetadata.fileKey);
+      await bspApi.wait.fileStorageComplete(adolphusFileMetadata.fileKey);
+      await bspTwoApi.wait.fileStorageComplete(adolphusFileMetadata.fileKey);
+      await bspThreeApi.wait.fileStorageComplete(adolphusFileMetadata.fileKey);
       await userApi.wait.bspStored(3);
 
       // Check the payment stream info after adding the new files
@@ -493,7 +486,6 @@ describeBspNet(
 
       // Seal a block to allow BSPs to charge the payment stream
       await userApi.sealBlock();
-      await sleep(500);
 
       // Assert that event for the BSP charging its payment stream was emitted
       await userApi.assert.eventPresent("paymentStreams", "PaymentStreamCharged");

test/suites/integration/bsp/multiple-delete.test.ts

@@ -76,7 +76,7 @@ describeBspNet("Single BSP Volunteering", ({ before, createBspApi, it, createUse
     // Wait for the BSP to volunteer
     await userApi.wait.bspVolunteer(source.length);
     for (const fileKey of fileKeys) {
-      await bspApi.wait.bspFileStorageComplete(fileKey);
+      await bspApi.wait.fileStorageComplete(fileKey);
     }
 
     // Waiting for a confirmation of the first file to be stored
@@ -214,7 +214,7 @@ describeBspNet("Single BSP Volunteering", ({ before, createBspApi, it, createUse
       // Wait for the BSP to volunteer
       await userApi.wait.bspVolunteer(source.length);
       for (const fileKey of fileKeys) {
-        await bspApi.wait.bspFileStorageComplete(fileKey);
+        await bspApi.wait.fileStorageComplete(fileKey);
       }
 
       // Waiting for a confirmation of the first file to be stored

test/suites/integration/bsp/reorg-proof.test.ts

@@ -40,7 +40,7 @@ describeBspNet(
       await userApi.block.seal(); // To make sure we have a finalised head
       const nextChallengeTick = await getNextChallengeHeight(userApi);
       await userApi.block.skipTo(nextChallengeTick, {
-        waitForBspProofs: [ShConsts.DUMMY_BSP_ID, ShConsts.BSP_TWO_ID, ShConsts.BSP_THREE_ID],
+        watchForBspProofs: [ShConsts.DUMMY_BSP_ID, ShConsts.BSP_TWO_ID, ShConsts.BSP_THREE_ID],
         finalised: true
       });
 

test/suites/integration/bsp/single-volunteer.test.ts

@@ -226,7 +226,7 @@ describeBspNet("Single BSP multi-volunteers", ({ before, createBspApi, createUse
     // Wait for the BSP to receive and store all files
     for (let i = 0; i < source.length; i++) {
       const fileKey = fileKeys[i];
-      await bspApi.wait.bspFileStorageComplete(fileKey);
+      await bspApi.wait.fileStorageComplete(fileKey);
     }
 
     // The first file to be completed will immediately acquire the forest write lock

test/suites/integration/bsp/storage-capacity.test.ts

@@ -1,24 +1,22 @@
 import assert from "node:assert";
 import { bspKey, describeBspNet, type EnrichedBspApi, ferdie, sleep } from "../../../util";
 
-describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, createBspApi }) => {
+describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi }) => {
   let userApi: EnrichedBspApi;
-  let bspApi: EnrichedBspApi;
 
   before(async () => {
     userApi = await createUserApi();
-    bspApi = await createBspApi();
   });
 
   it("Unregistered accounts fail when changing capacities", async () => {
     const totalCapacityBefore = await userApi.query.providers.totalBspsCapacity();
     const bspCapacityBefore = await userApi.query.providers.backupStorageProviders(
-      bspApi.shConsts.DUMMY_BSP_ID
+      userApi.shConsts.DUMMY_BSP_ID
     );
     assert.ok(bspCapacityBefore.unwrap().capacity.eq(totalCapacityBefore));
 
     const { events, extSuccess } = await userApi.sealBlock(
-      userApi.tx.providers.changeCapacity(bspApi.shConsts.CAPACITY[1024]),
+      userApi.tx.providers.changeCapacity(userApi.shConsts.CAPACITY[1024]),
       ferdie
     );
     assert.strictEqual(extSuccess, false);
@@ -37,7 +35,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
 
     const totalCapacityAfter = await userApi.query.providers.totalBspsCapacity();
     const bspCapacityAfter = await userApi.query.providers.backupStorageProviders(
-      bspApi.shConsts.DUMMY_BSP_ID
+      userApi.shConsts.DUMMY_BSP_ID
     );
     assert.ok(bspCapacityAfter.unwrap().capacity.eq(totalCapacityBefore));
     assert.ok(totalCapacityAfter.eq(totalCapacityBefore));
@@ -50,7 +48,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
     );
 
     const capacityUsed = (
-      await userApi.query.providers.backupStorageProviders(bspApi.shConsts.DUMMY_BSP_ID)
+      await userApi.query.providers.backupStorageProviders(userApi.shConsts.DUMMY_BSP_ID)
     )
       .unwrap()
       .capacityUsed.toNumber();
@@ -59,6 +57,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
     const newCapacity = Math.max(minCapacity, capacityUsed + 1);
 
     // Set BSP's available capacity to 0 to force the BSP to increase its capacity before volunteering for the storage request.
+    await userApi.wait.waitForAvailabilityToSendTx(bspKey.address.toString());
     const { extSuccess } = await userApi.sealBlock(
       userApi.tx.providers.changeCapacity(newCapacity),
       bspKey
@@ -101,9 +100,9 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
 
     await userApi.sealBlock();
 
-    const updatedCapacity = BigInt(bspApi.shConsts.JUMP_CAPACITY_BSP + newCapacity);
+    const updatedCapacity = BigInt(userApi.shConsts.JUMP_CAPACITY_BSP + newCapacity);
     const bspCapacityAfter = await userApi.query.providers.backupStorageProviders(
-      bspApi.shConsts.DUMMY_BSP_ID
+      userApi.shConsts.DUMMY_BSP_ID
     );
     assert.strictEqual(bspCapacityAfter.unwrap().capacity.toBigInt(), updatedCapacity);
 
@@ -113,16 +112,17 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
 
   it("Total capacity updated when single BSP capacity updated", async () => {
     const newCapacity =
-      BigInt(Math.floor(Math.random() * 1000 * 1024 * 1024)) + bspApi.shConsts.CAPACITY_512;
+      BigInt(Math.floor(Math.random() * 1000 * 1024 * 1024)) + userApi.shConsts.CAPACITY_512;
 
     // Skip block height past threshold
     await userApi.block.skipToMinChangeTime();
 
+    await userApi.wait.waitForAvailabilityToSendTx(bspKey.address.toString());
     await userApi.sealBlock(userApi.tx.providers.changeCapacity(newCapacity), bspKey);
 
     const totalCapacityAfter = await userApi.query.providers.totalBspsCapacity();
     const bspCapacityAfter = await userApi.query.providers.backupStorageProviders(
-      bspApi.shConsts.DUMMY_BSP_ID
+      userApi.shConsts.DUMMY_BSP_ID
     );
     assert.strictEqual(bspCapacityAfter.unwrap().capacity.toBigInt(), newCapacity);
     assert.strictEqual(totalCapacityAfter.toBigInt(), newCapacity);
@@ -140,6 +140,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
     // Skip block height past threshold
     await userApi.block.skipToMinChangeTime();
 
+    await userApi.wait.waitForAvailabilityToSendTx(bspKey.address.toString());
     const { events, extSuccess } = await userApi.sealBlock(
       userApi.tx.providers.changeCapacity(2n),
       bspKey
@@ -160,7 +161,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
 
   it("Test BSP storage size increased twice in the same increasing period (check for race condition)", async () => {
     const capacityUsed = (
-      await userApi.query.providers.backupStorageProviders(bspApi.shConsts.DUMMY_BSP_ID)
+      await userApi.query.providers.backupStorageProviders(userApi.shConsts.DUMMY_BSP_ID)
     )
       .unwrap()
       .capacityUsed.toNumber();
@@ -169,6 +170,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
     const newCapacity = Math.max(minCapacity, capacityUsed + 1);
 
     // Set BSP's available capacity to 0 to force the BSP to increase its capacity before volunteering for the storage request.
+    await userApi.wait.waitForAvailabilityToSendTx(bspKey.address.toString());
     const { extSuccess } = await userApi.sealBlock(
       userApi.tx.providers.changeCapacity(newCapacity),
       bspKey
@@ -196,7 +198,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
     await sleep(500);
 
     // Assert BSP has sent a call to increase its capacity.
-    await bspApi.assert.extrinsicPresent({
+    await userApi.assert.extrinsicPresent({
       module: "providers",
       method: "changeCapacity",
       checkTxPool: true
@@ -209,7 +211,7 @@ describeBspNet("BSPNet: Validating max storage", ({ before, it, createUserApi, c
 
     const updatedCapacity = BigInt(userApi.shConsts.JUMP_CAPACITY_BSP + newCapacity);
     const bspCapacityAfter = await userApi.query.providers.backupStorageProviders(
-      bspApi.shConsts.DUMMY_BSP_ID
+      userApi.shConsts.DUMMY_BSP_ID
     );
     assert.strictEqual(bspCapacityAfter.unwrap().capacity.toBigInt(), updatedCapacity);
   });