Fix Migration Test Definitions (#865)

The migration tests are defining `describe` blocks asyncronously within
an `it` block. AFAIU, this is a no-no and causes ugly test formatting.
This PR adjusts the test definitions so that they belong to the correct
parent `describe` block (one per migration version).

<details><summary>Test formatting <strong>before</strong>:</summary>

```
  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set

  execTransaction
    ✔ should be able to transfer ETH (48ms)

  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set

  execTransaction
    ✔ should be able to transfer ETH (48ms)

  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set

  execTransaction
    ✔ should be able to transfer ETH (45ms)

  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set

  execTransaction
    ✔ should be able to transfer ETH (49ms)

  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set

  execTransaction
    ✔ should be able to transfer ETH (52ms)

  addOwner
    ✔ should add owner and change threshold

  enableModule
    ✔ should enabled module and be able to use it

  multiSend
    ✔ execute multisend via delegatecall

  fallbackHandler
    ✔ should be correctly set
```

</details>

<details><summary>Test formatting <strong>after</strong>:</summary>

```
  Upgrade from Safe 1.1.1
    execTransaction
      ✔ should be able to transfer ETH (50ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set

  Upgrade from Safe 1.2.0
    execTransaction
      ✔ should be able to transfer ETH (45ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set

  Upgrade from Safe 1.3.0
    execTransaction
      ✔ should be able to transfer ETH (52ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set

  Upgrade from Safe 1.3.0 L2
    execTransaction
      ✔ should be able to transfer ETH (50ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set

  Upgrade from Safe 1.4.1
    execTransaction
      ✔ should be able to transfer ETH (52ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set

  Upgrade from Safe 1.4.1 L2
    execTransaction
      ✔ should be able to transfer ETH (51ms)
    addOwner
      ✔ should add owner and change threshold
    enableModule
      ✔ should enabled module and be able to use it
    multiSend
      ✔ execute multisend via delegatecall
    fallbackHandler
      ✔ should be correctly set
```

</details>

test/migration/UpgradeFromSafe111.spec.ts

@@ -19,7 +19,8 @@ describe("Upgrade from Safe 1.1.1", () => {
 
     // We migrate the Safe and run the verification tests
     const setupTests = deployments.createFixture(async ({ deployments }) => {
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
@@ -45,10 +46,9 @@ describe("Upgrade from Safe 1.1.1", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.1.1 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/UpgradeFromSafe120.spec.ts

@@ -22,7 +22,8 @@ describe("Upgrade from Safe 1.2.0", () => {
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         const singleton120 = (await (await user1.sendTransaction({ data: deploymentData.safe120 })).wait())?.contractAddress;
         if (!singleton120) throw new Error("Could not deploy Safe 1.2.0");
 
@@ -46,10 +47,9 @@ describe("Upgrade from Safe 1.2.0", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.2.0 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/UpgradeFromSafe130.spec.ts

@@ -20,7 +20,8 @@ describe("Upgrade from Safe 1.3.0", () => {
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         const singleton130 = (await (await user1.sendTransaction({ data: deploymentData.safe130.evm })).wait())?.contractAddress;
         if (!singleton130) throw new Error("Could not deploy Safe 1.3.0");
 
@@ -45,10 +46,9 @@ describe("Upgrade from Safe 1.3.0", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.3.0 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/UpgradeFromSafe130L2.spec.ts

@@ -20,7 +20,8 @@ describe("Upgrade from Safe 1.3.0 L2", () => {
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         const singleton130L2 = (await (await user1.sendTransaction({ data: deploymentData.safe130l2.evm })).wait())?.contractAddress;
         if (!singleton130L2) throw new Error("Could not deploy Safe 1.3.0 L2");
 
@@ -45,10 +46,9 @@ describe("Upgrade from Safe 1.3.0 L2", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.3.0 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/UpgradeFromSafe141.spec.ts

@@ -13,7 +13,8 @@ describe("Upgrade from Safe 1.4.1", () => {
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         const safeDeploymentData = hre.network.zksync ? deploymentData.safe141.zksync : deploymentData.safe141.evm;
         const safeContractFactory = new hre.ethers.ContractFactory(await getAbi("Safe"), safeDeploymentData, user1);
         const singleton141 = await (await safeContractFactory.deploy()).getAddress();
@@ -40,10 +41,9 @@ describe("Upgrade from Safe 1.4.1", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.4.1 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/UpgradeFromSafe141L2.spec.ts

@@ -13,7 +13,8 @@ describe("Upgrade from Safe 1.4.1 L2", () => {
         await deployments.fixture();
         const mock = await getMock();
         const mockAddress = await mock.getAddress();
-        const [user1] = await hre.ethers.getSigners();
+        const signers = await hre.ethers.getSigners();
+        const [user1] = signers;
         const safeDeploymentData = hre.network.zksync ? deploymentData.safe141l2.zksync : deploymentData.safe141l2.evm;
         const safeContractFactory = new hre.ethers.ContractFactory(await getAbi("Safe"), safeDeploymentData, user1);
         const singleton141L2 = await (await safeContractFactory.deploy()).getAddress();
@@ -40,10 +41,9 @@ describe("Upgrade from Safe 1.4.1 L2", () => {
             migratedSafe: safe,
             mock,
             multiSend: await getMultiSend(),
+            signers,
         };
     });
 
-    it("passes the Safe 1.4.1 tests", async () => {
-        await verificationTests(setupTests);
-    });
+    verificationTests(setupTests);
 });

test/migration/subTests.spec.ts

@@ -1,3 +1,4 @@
+import type { HardhatEthersSigner } from "@nomicfoundation/hardhat-ethers/signers";
 import { expect } from "chai";
 import hre, { ethers } from "hardhat";
 import { AddressOne } from "../../src/utils/constants";
@@ -9,14 +10,16 @@ interface TestSetup {
     migratedSafe: Safe;
     mock: MockContract;
     multiSend: MultiSend;
+    signers: HardhatEthersSigner[];
 }
 
-export const verificationTests = async (setupTests: () => Promise<TestSetup>) => {
-    const [user1, user2, user3] = await ethers.getSigners();
-
+export const verificationTests = (setupTests: () => Promise<TestSetup>) => {
     describe("execTransaction", () => {
         it("should be able to transfer ETH", async () => {
-            const { migratedSafe } = await setupTests();
+            const {
+                migratedSafe,
+                signers: [user1, user2],
+            } = await setupTests();
             const migrateSafeAddress = await migratedSafe.getAddress();
             await user1.sendTransaction({ to: migrateSafeAddress, value: ethers.parseEther("1") });
             const nonce = await migratedSafe.nonce();
@@ -34,7 +37,10 @@ export const verificationTests = async (setupTests: () => Promise<TestSetup>) =>
 
     describe("addOwner", () => {
         it("should add owner and change threshold", async () => {
-            const { migratedSafe } = await setupTests();
+            const {
+                migratedSafe,
+                signers: [user1, user2, user3],
+            } = await setupTests();
 
             await expect(executeContractCallWithSigners(migratedSafe, migratedSafe, "addOwnerWithThreshold", [user2.address, 2], [user1]))
                 .to.emit(migratedSafe, "AddedOwner")
@@ -62,7 +68,11 @@ export const verificationTests = async (setupTests: () => Promise<TestSetup>) =>
 
     describe("enableModule", () => {
         it("should enabled module and be able to use it", async () => {
-            const { migratedSafe, mock } = await setupTests();
+            const {
+                migratedSafe,
+                mock,
+                signers: [user1, user2],
+            } = await setupTests();
             const mockAddress = await mock.getAddress();
 
             await expect(executeContractCallWithSigners(migratedSafe, migratedSafe, "enableModule", [user2.address], [user1]))
@@ -82,7 +92,12 @@ export const verificationTests = async (setupTests: () => Promise<TestSetup>) =>
 
     describe("multiSend", () => {
         it("execute multisend via delegatecall", async () => {
-            const { migratedSafe, mock, multiSend } = await setupTests();
+            const {
+                migratedSafe,
+                mock,
+                multiSend,
+                signers: [user1, user2],
+            } = await setupTests();
             const migratedSafeAddress = await migratedSafe.getAddress();
             const mockAddress = await mock.getAddress();