From 209c10609a3b384ceaca17611e8d8643e543e633 Mon Sep 17 00:00:00 2001 From: dodger213 Date: Thu, 8 Aug 2024 11:01:25 +0200 Subject: [PATCH] Unifying function to get Safe implementation (#808) Fixes #802 This PR unifies `getSafeWithOwners(...)` and `getSafeWithSingleton(...)` by editing the `getSafeTemplate(...)` to use an inside function which also takes the `singleton` as a parameter. --- benchmark/utils/setup.ts | 4 +- test/accessors/SimulateTxAccessor.spec.ts | 4 +- test/core/Safe.Execution.spec.ts | 4 +- test/core/Safe.GuardManager.spec.ts | 10 ++-- test/core/Safe.Incoming.spec.ts | 4 +- test/core/Safe.ModuleManager.spec.ts | 10 ++-- test/core/Safe.OwnerManager.spec.ts | 4 +- test/core/Safe.Signatures.spec.ts | 60 +++++++++---------- test/core/Safe.StorageAccessible.spec.ts | 4 +- test/factory/ProxyFactory.spec.ts | 4 +- test/guards/DebugTransactionGuard.spec.ts | 4 +- .../DelegateCallTransactionGuard.spec.ts | 4 +- test/guards/OnlyOwnersGuard.spec.ts | 4 +- .../guards/ReentrancyTransactionGuard.spec.ts | 4 +- .../CompatibilityFallbackHandler.spec.ts | 6 +- test/integration/Safe.0xExploit.spec.ts | 6 +- .../Safe.ReservedAddresses.spec.ts | 4 +- test/l2/Safe.Execution.spec.ts | 4 +- test/libraries/CreateCall.spec.ts | 4 +- test/libraries/Migration.spec.ts | 4 +- test/libraries/MultiSend.spec.ts | 4 +- test/libraries/MultiSendCallOnly.spec.ts | 4 +- test/libraries/SafeMigration.spec.ts | 6 +- test/libraries/SafeToL2Migration.spec.ts | 8 +-- test/libraries/SafeToL2Setup.spec.ts | 4 +- test/libraries/SignMessageLib.spec.ts | 4 +- test/utils/setup.ts | 37 ++++-------- 27 files changed, 104 insertions(+), 115 deletions(-) diff --git a/benchmark/utils/setup.ts b/benchmark/utils/setup.ts index 140e58b..ff863f5 100644 --- a/benchmark/utils/setup.ts +++ b/benchmark/utils/setup.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { BigNumberish } from "ethers"; -import { getTokenCallbackHandler, getSafeWithOwners } from "../../test/utils/setup"; +import { getTokenCallbackHandler, getSafe } from "../../test/utils/setup"; import { logGas, executeTx, @@ -24,7 +24,7 @@ const generateTarget = async (owners: number, threshold: number, guardAddress: s const fallbackHandler = await getTokenCallbackHandler(); const fallbackHandlerAddress = await fallbackHandler.getAddress(); const signers = (await ethers.getSigners()).slice(0, owners); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: signers.map((owner) => owner.address), threshold, fallbackHandler: fallbackHandlerAddress, diff --git a/test/accessors/SimulateTxAccessor.spec.ts b/test/accessors/SimulateTxAccessor.spec.ts index d2b9567..c06cdbd 100644 --- a/test/accessors/SimulateTxAccessor.spec.ts +++ b/test/accessors/SimulateTxAccessor.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { deployContract, getSimulateTxAccessor, getSafeWithOwners, getCompatFallbackHandler } from "../utils/setup"; +import { deployContract, getSimulateTxAccessor, getSafe, getCompatFallbackHandler } from "../utils/setup"; import { buildContractCall } from "../../src/utils/execution"; describe("SimulateTxAccessor", () => { @@ -20,7 +20,7 @@ describe("SimulateTxAccessor", () => { const interactor = await deployContract(user1, source); const handler = await getCompatFallbackHandler(); const handlerAddress = await handler.getAddress(); - const safe = await getSafeWithOwners({ owners: [user1.address], threshold: 1, fallbackHandler: handlerAddress }); + const safe = await getSafe({ owners: [user1.address], threshold: 1, fallbackHandler: handlerAddress }); const safeAddress = await safe.getAddress(); const simulator = await getCompatFallbackHandler(safeAddress); return { diff --git a/test/core/Safe.Execution.spec.ts b/test/core/Safe.Execution.spec.ts index 917f3ca..2890cf6 100644 --- a/test/core/Safe.Execution.spec.ts +++ b/test/core/Safe.Execution.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { deployContract, getSafeWithOwners } from "../utils/setup"; +import { deployContract, getSafe } from "../utils/setup"; import { safeApproveHash, buildSignatureBytes, @@ -42,7 +42,7 @@ describe("Safe", () => { }`; const reverter = await deployContract(user1, reverterSource); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), reverter, storageSetter, nativeTokenReceiver, diff --git a/test/core/Safe.GuardManager.spec.ts b/test/core/Safe.GuardManager.spec.ts index d485f34..f8521d6 100644 --- a/test/core/Safe.GuardManager.spec.ts +++ b/test/core/Safe.GuardManager.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getMock, getSafeWithOwners } from "../utils/setup"; +import { getMock, getSafe } from "../utils/setup"; import { buildContractCall, buildSafeTransaction, @@ -26,7 +26,7 @@ describe("GuardManager", () => { const guardContract = await hre.ethers.getContractAt("ITransactionGuard", AddressZero); const guardEip165Calldata = guardContract.interface.encodeFunctionData("supportsInterface", ["0xe6d7a83a"]); await validGuardMock.givenCalldataReturnBool(guardEip165Calldata, true); - const safe = await getSafeWithOwners({ owners: [user2.address] }); + const safe = await getSafe({ owners: [user2.address] }); await executeContractCallWithSigners(safe, safe, "setGuard", [validGuardMockAddress], [user2]); return { @@ -42,7 +42,7 @@ describe("GuardManager", () => { const { signers: [user1, user2], } = await setupWithTemplate(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await expect(executeContractCallWithSigners(safe, safe, "setGuard", [user2.address], [user1])).to.be.revertedWith("GS013"); }); @@ -53,7 +53,7 @@ describe("GuardManager", () => { signers: [user1], } = await setupWithTemplate(); const validGuardMockAddress = await validGuardMock.getAddress(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await expect(executeContractCallWithSigners(safe, safe, "setGuard", [validGuardMockAddress], [user1])) .to.emit(safe, "ChangedGuard") @@ -72,7 +72,7 @@ describe("GuardManager", () => { const invocationCountBefore = await validGuardMock.invocationCount(); const validGuardMockAddress = await validGuardMock.getAddress(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await executeContractCallWithSigners(safe, safe, "setGuard", [validGuardMockAddress], [user1]); diff --git a/test/core/Safe.Incoming.spec.ts b/test/core/Safe.Incoming.spec.ts index 9c0fc2b..c58a4c6 100644 --- a/test/core/Safe.Incoming.spec.ts +++ b/test/core/Safe.Incoming.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { deployContract, getSafeWithOwners } from "../utils/setup"; +import { deployContract, getSafe } from "../utils/setup"; describe("Safe", () => { const setupTests = deployments.createFixture(async ({ deployments }) => { @@ -21,7 +21,7 @@ describe("Safe", () => { const signers = await ethers.getSigners(); const [user1] = signers; return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), caller: await deployContract(user1, source), signers, }; diff --git a/test/core/Safe.ModuleManager.spec.ts b/test/core/Safe.ModuleManager.spec.ts index ccad530..797826d 100644 --- a/test/core/Safe.ModuleManager.spec.ts +++ b/test/core/Safe.ModuleManager.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithOwners, getMock } from "../utils/setup"; +import { getSafe, getMock } from "../utils/setup"; import { executeContractCallWithSigners } from "../../src/utils/execution"; import { AddressOne } from "../../src/utils/constants"; @@ -13,7 +13,7 @@ describe("ModuleManager", () => { const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const validModuleGuardMock = await getMock(); const moduleGuardContract = await hre.ethers.getContractAt("IModuleGuard", AddressZero); @@ -577,7 +577,7 @@ describe("ModuleManager", () => { const { signers: [user1, user2], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await expect(executeContractCallWithSigners(safe, safe, "setModuleGuard", [user2.address], [user1])).to.be.revertedWith( "GS013", @@ -590,7 +590,7 @@ describe("ModuleManager", () => { signers: [user1], } = await setupTests(); const validGuardMockAddress = await validModuleGuardMock.getAddress(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await expect(executeContractCallWithSigners(safe, safe, "setModuleGuard", [validGuardMockAddress], [user1])) .to.emit(safe, "ChangedModuleGuard") @@ -609,7 +609,7 @@ describe("ModuleManager", () => { const invocationCountBefore = await validModuleGuardMock.invocationCount(); const validModuleGuardMockAddress = await validModuleGuardMock.getAddress(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); await executeContractCallWithSigners(safe, safe, "setModuleGuard", [validModuleGuardMockAddress], [user1]); diff --git a/test/core/Safe.OwnerManager.spec.ts b/test/core/Safe.OwnerManager.spec.ts index 82ece88..b15a1f1 100644 --- a/test/core/Safe.OwnerManager.spec.ts +++ b/test/core/Safe.OwnerManager.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithOwners } from "../utils/setup"; +import { getSafe } from "../utils/setup"; import { executeContractCallWithSigners } from "../../src/utils/execution"; import { AddressOne } from "../../src/utils/constants"; @@ -11,7 +11,7 @@ describe("OwnerManager", () => { const signers = await ethers.getSigners(); const [user1] = signers; return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), signers, }; }); diff --git a/test/core/Safe.Signatures.spec.ts b/test/core/Safe.Signatures.spec.ts index 667214a..00167c1 100644 --- a/test/core/Safe.Signatures.spec.ts +++ b/test/core/Safe.Signatures.spec.ts @@ -3,7 +3,7 @@ import { calculateSafeMessageHash, signHash, buildContractSignature } from "./.. import { expect } from "chai"; import { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeTemplate, getSafeWithOwners } from "../utils/setup"; +import { getSafeTemplate, getSafe } from "../utils/setup"; import { safeSignTypedData, executeTx, @@ -24,7 +24,7 @@ describe("Safe", () => { const compatFallbackHandler = await getCompatFallbackHandler(); const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); return { safe, @@ -165,7 +165,7 @@ describe("Safe", () => { const { signers: [user1], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); await expect(executeTx(safe, tx, [await safeSignTypedData(user1, safeAddress, tx, 1)])).to.be.revertedWith("GS026"); @@ -243,7 +243,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); await expect(executeTx(safe, tx, [])).to.be.revertedWith("GS020"); @@ -253,7 +253,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); await expect( @@ -271,13 +271,13 @@ describe("Safe", () => { } = await setupTests(); const compatFallbackHandler = await getCompatFallbackHandler(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const signerSafe = await getSafeWithOwners({ + const signerSafe = await getSafe({ owners: [user5.address], threshold: 1, fallbackHandler: compatFallbackHandlerAddress, }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address, signerSafeAddress], }); const safeAddress = await safe.getAddress(); @@ -369,7 +369,7 @@ describe("Safe", () => { const { signers: [user1], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -403,7 +403,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -414,7 +414,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -432,13 +432,13 @@ describe("Safe", () => { } = await setupTests(); const compatFallbackHandler = await getCompatFallbackHandler(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const signerSafe = await getSafeWithOwners({ + const signerSafe = await getSafe({ owners: [user5.address], threshold: 1, fallbackHandler: compatFallbackHandlerAddress, }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address, signerSafeAddress], }); const safeAddress = await safe.getAddress(); @@ -526,7 +526,7 @@ describe("Safe", () => { const { signers: [user1], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHashData = preimageSafeTransactionHash(safeAddress, tx, await chainId()); @@ -554,7 +554,7 @@ describe("Safe", () => { compatFallbackHandler, signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address], threshold: 3, fallbackHandler: await compatFallbackHandler.getAddress(), @@ -571,7 +571,7 @@ describe("Safe", () => { compatFallbackHandler, signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address], threshold: 3, fallbackHandler: await compatFallbackHandler.getAddress(), @@ -594,13 +594,13 @@ describe("Safe", () => { signers: [user1, user2, user3, user4, user5], } = await setupTests(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const signerSafe = await getSafeWithOwners({ + const signerSafe = await getSafe({ owners: [user5.address], threshold: 1, fallbackHandler: compatFallbackHandlerAddress, }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address, signerSafeAddress], threshold: 5, fallbackHandler: compatFallbackHandlerAddress, @@ -725,7 +725,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -736,7 +736,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -756,13 +756,13 @@ describe("Safe", () => { } = await setupTests(); const compatFallbackHandler = await getCompatFallbackHandler(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const signerSafe = await getSafeWithOwners({ + const signerSafe = await getSafe({ owners: [user5.address], threshold: 1, fallbackHandler: compatFallbackHandlerAddress, }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address, signerSafeAddress], }); const safeAddress = await safe.getAddress(); @@ -797,7 +797,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3, user4], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address, user4.address] }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -810,7 +810,7 @@ describe("Safe", () => { const { signers: [user1, user2, user3, user4], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address, user2.address, user3.address, user4.address], threshold: 2 }); + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address], threshold: 2 }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -833,7 +833,7 @@ describe("Safe", () => { signers: [user1, user2], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const safeAddress = await safe.getAddress(); const tx = buildSafeTransaction({ to: safeAddress, nonce: await safe.nonce() }); const txHash = calculateSafeTransactionHash(safeAddress, tx, await chainId()); @@ -960,7 +960,7 @@ describe("Safe", () => { signers: [user1, user2, user3], } = await setupTests(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address], threshold: 3, fallbackHandler: compatFallbackHandlerAddress, @@ -978,7 +978,7 @@ describe("Safe", () => { signers: [user1, user2, user3], } = await setupTests(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address], threshold: 3, fallbackHandler: compatFallbackHandlerAddress, @@ -1001,13 +1001,13 @@ describe("Safe", () => { } = await setupTests(); const compatFallbackHandler = await getCompatFallbackHandler(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const signerSafe = await getSafeWithOwners({ + const signerSafe = await getSafe({ owners: [user5.address], threshold: 1, fallbackHandler: compatFallbackHandlerAddress, }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address, signerSafeAddress], threshold: 5, fallbackHandler: compatFallbackHandlerAddress, @@ -1046,7 +1046,7 @@ describe("Safe", () => { signers: [user1, user2, user3, user4], } = await setupTests(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address], threshold: 4, fallbackHandler: compatFallbackHandlerAddress, @@ -1066,7 +1066,7 @@ describe("Safe", () => { signers: [user1, user2, user3, user4], } = await setupTests(); const compatFallbackHandlerAddress = await compatFallbackHandler.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, user3.address, user4.address], threshold: 2, fallbackHandler: compatFallbackHandlerAddress, diff --git a/test/core/Safe.StorageAccessible.spec.ts b/test/core/Safe.StorageAccessible.spec.ts index 09a4fe5..7dd4b47 100644 --- a/test/core/Safe.StorageAccessible.spec.ts +++ b/test/core/Safe.StorageAccessible.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import { deployments, ethers } from "hardhat"; -import { getSafeSingleton, getSafeWithOwners } from "../utils/setup"; +import { getSafeSingleton, getSafe } from "../utils/setup"; import { killLibContract } from "../utils/contracts"; describe("StorageAccessible", () => { @@ -9,7 +9,7 @@ describe("StorageAccessible", () => { const [user1, user2] = await ethers.getSigners(); const killLib = await killLibContract(user1); return { - safe: await getSafeWithOwners({ owners: [user1.address, user2.address], threshold: 1 }), + safe: await getSafe({ owners: [user1.address, user2.address], threshold: 1 }), killLib, }; }); diff --git a/test/factory/ProxyFactory.spec.ts b/test/factory/ProxyFactory.spec.ts index 5f24ce2..e445263 100644 --- a/test/factory/ProxyFactory.spec.ts +++ b/test/factory/ProxyFactory.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { Contract } from "ethers"; -import { deployContract, getFactory, getMock, getSafeWithOwners, getSafeProxyRuntimeCode } from "../utils/setup"; +import { deployContract, getFactory, getMock, getSafe, getSafeProxyRuntimeCode } from "../utils/setup"; import { AddressZero } from "@ethersproject/constants"; import { calculateChainSpecificProxyAddress, calculateProxyAddress, calculateProxyAddressWithCallback } from "../../src/utils/proxies"; import { chainId } from "./../utils/encoding"; @@ -37,7 +37,7 @@ describe("ProxyFactory", () => { const [user1] = signers; const singleton = await deployContract(user1, SINGLETON_SOURCE); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), factory: await getFactory(), mock: await getMock(), singleton, diff --git a/test/guards/DebugTransactionGuard.spec.ts b/test/guards/DebugTransactionGuard.spec.ts index b68b35d..b4312dc 100644 --- a/test/guards/DebugTransactionGuard.spec.ts +++ b/test/guards/DebugTransactionGuard.spec.ts @@ -1,7 +1,7 @@ import { signHash } from "./../../src/utils/execution"; import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getMock, getSafeWithOwners } from "../utils/setup"; +import { getMock, getSafe } from "../utils/setup"; import { buildSafeTransaction, calculateSafeTransactionHash, executeContractCallWithSigners, executeTx } from "../../src/utils/execution"; import { chainId } from "../utils/encoding"; @@ -10,7 +10,7 @@ describe("DebugTransactionGuard", () => { await deployments.fixture(); const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const guardFactory = await hre.ethers.getContractFactory("DebugTransactionGuard"); const guard = await guardFactory.deploy(); const guardAddress = await guard.getAddress(); diff --git a/test/guards/DelegateCallTransactionGuard.spec.ts b/test/guards/DelegateCallTransactionGuard.spec.ts index f7b217b..125ef2c 100644 --- a/test/guards/DelegateCallTransactionGuard.spec.ts +++ b/test/guards/DelegateCallTransactionGuard.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithOwners } from "../utils/setup"; +import { getSafe } from "../utils/setup"; import { buildContractCall, executeContractCallWithSigners } from "../../src/utils/execution"; import { AddressOne } from "../../src/utils/constants"; @@ -10,7 +10,7 @@ describe("DelegateCallTransactionGuard", () => { await deployments.fixture(); const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const guardFactory = await hre.ethers.getContractFactory("DelegateCallTransactionGuard"); const guard = await guardFactory.deploy(AddressZero); const guardAddress = await guard.getAddress(); diff --git a/test/guards/OnlyOwnersGuard.spec.ts b/test/guards/OnlyOwnersGuard.spec.ts index 513bad4..88dbdcf 100644 --- a/test/guards/OnlyOwnersGuard.spec.ts +++ b/test/guards/OnlyOwnersGuard.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getMock, getSafeWithOwners } from "../utils/setup"; +import { getMock, getSafe } from "../utils/setup"; import { buildSafeTransaction, executeContractCallWithSigners, @@ -14,7 +14,7 @@ describe("OnlyOwnersGuard", () => { await deployments.fixture(); const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const guardFactory = await hre.ethers.getContractFactory("OnlyOwnersGuard"); const guard = await guardFactory.deploy(); const guardAddress = await guard.getAddress(); diff --git a/test/guards/ReentrancyTransactionGuard.spec.ts b/test/guards/ReentrancyTransactionGuard.spec.ts index 4c28e25..738dc77 100644 --- a/test/guards/ReentrancyTransactionGuard.spec.ts +++ b/test/guards/ReentrancyTransactionGuard.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getMock, getSafeWithOwners } from "../utils/setup"; +import { getMock, getSafe } from "../utils/setup"; import { buildSafeTransaction, buildSignatureBytes, @@ -15,7 +15,7 @@ describe("ReentrancyTransactionGuard", () => { await deployments.fixture(); const signers = await ethers.getSigners(); const [user1] = signers; - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const guardFactory = await hre.ethers.getContractFactory("ReentrancyTransactionGuard"); const guard = await guardFactory.deploy(); const guardAddress = await guard.getAddress(); diff --git a/test/handlers/CompatibilityFallbackHandler.spec.ts b/test/handlers/CompatibilityFallbackHandler.spec.ts index 5f93b21..b835409 100644 --- a/test/handlers/CompatibilityFallbackHandler.spec.ts +++ b/test/handlers/CompatibilityFallbackHandler.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getCompatFallbackHandler, getSafeWithOwners } from "../utils/setup"; +import { getCompatFallbackHandler, getSafe } from "../utils/setup"; import { buildSignatureBytes, executeContractCallWithSigners, @@ -21,9 +21,9 @@ describe("CompatibilityFallbackHandler", () => { const handlerAddress = await handler.getAddress(); const signers = await ethers.getSigners(); const [user1, user2] = signers; - const signerSafe = await getSafeWithOwners({ owners: [user1.address], threshold: 1, fallbackHandler: handlerAddress }); + const signerSafe = await getSafe({ owners: [user1.address], threshold: 1, fallbackHandler: handlerAddress }); const signerSafeAddress = await signerSafe.getAddress(); - const safe = await getSafeWithOwners({ + const safe = await getSafe({ owners: [user1.address, user2.address, signerSafeAddress], threshold: 2, fallbackHandler: handlerAddress, diff --git a/test/integration/Safe.0xExploit.spec.ts b/test/integration/Safe.0xExploit.spec.ts index bdc40a4..b8d5af0 100644 --- a/test/integration/Safe.0xExploit.spec.ts +++ b/test/integration/Safe.0xExploit.spec.ts @@ -2,7 +2,7 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; import { defaultAbiCoder } from "@ethersproject/abi"; -import { getSafeWithOwners, deployContract, getCompatFallbackHandler } from "../utils/setup"; +import { getSafe, deployContract, getCompatFallbackHandler } from "../utils/setup"; import { buildSignatureBytes, executeContractCallWithSigners, signHash } from "../../src/utils/execution"; describe("Safe", () => { @@ -12,7 +12,7 @@ describe("Safe", () => { const handlerAddress = await handler.getAddress(); const signers = await ethers.getSigners(); const [user1, user2] = signers; - const ownerSafe = await getSafeWithOwners({ + const ownerSafe = await getSafe({ owners: [user1.address, user2.address], threshold: 2, fallbackHandler: handlerAddress, @@ -20,7 +20,7 @@ describe("Safe", () => { const ownerSafeAddress = await ownerSafe.getAddress(); const messageHandler = await getCompatFallbackHandler(ownerSafeAddress); return { - safe: await getSafeWithOwners({ owners: [ownerSafeAddress, user1.address], threshold: 1 }), + safe: await getSafe({ owners: [ownerSafeAddress, user1.address], threshold: 1 }), ownerSafe, messageHandler, signers, diff --git a/test/integration/Safe.ReservedAddresses.spec.ts b/test/integration/Safe.ReservedAddresses.spec.ts index 65f2d48..d7940b1 100644 --- a/test/integration/Safe.ReservedAddresses.spec.ts +++ b/test/integration/Safe.ReservedAddresses.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getSafeWithOwners } from "../utils/setup"; +import { getSafe } from "../utils/setup"; import { AddressOne } from "../../src/utils/constants"; describe("Safe - Reserved Addresses", () => { @@ -8,7 +8,7 @@ describe("Safe - Reserved Addresses", () => { await deployments.fixture(); const [user1] = await ethers.getSigners(); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), }; }); diff --git a/test/l2/Safe.Execution.spec.ts b/test/l2/Safe.Execution.spec.ts index f8056ea..fbaf3a2 100644 --- a/test/l2/Safe.Execution.spec.ts +++ b/test/l2/Safe.Execution.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getMock, getSafeWithOwners } from "../utils/setup"; +import { getMock, getSafe } from "../utils/setup"; import { safeApproveHash, buildSafeTransaction, @@ -23,7 +23,7 @@ describe("SafeL2", () => { await deployments.fixture(); const mock = await getMock(); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), mock, signers, }; diff --git a/test/libraries/CreateCall.spec.ts b/test/libraries/CreateCall.spec.ts index d6354d1..3e48cbf 100644 --- a/test/libraries/CreateCall.spec.ts +++ b/test/libraries/CreateCall.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { compile, getCreateCall, getSafeWithOwners } from "../utils/setup"; +import { compile, getCreateCall, getSafe } from "../utils/setup"; import { buildContractCall, executeTx, safeApproveHash } from "../../src/utils/execution"; const CONTRACT_SOURCE = ` @@ -22,7 +22,7 @@ describe("CreateCall", () => { const signers = await ethers.getSigners(); const [user1] = signers; return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), createCall: await getCreateCall(), testContract, signers, diff --git a/test/libraries/Migration.spec.ts b/test/libraries/Migration.spec.ts index ffc3e52..ade0400 100644 --- a/test/libraries/Migration.spec.ts +++ b/test/libraries/Migration.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import { ethers, deployments } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithOwners, getSafeSingleton, migrationContract } from "../utils/setup"; +import { getSafe, getSafeSingleton, migrationContract } from "../utils/setup"; import deploymentData from "../json/safeDeployment.json"; import { executeContractCallWithSigners } from "../../src/utils/execution"; @@ -24,7 +24,7 @@ describe("Migration", () => { return { singleton: await getSafeSingleton(), singleton120, - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), migration, signers, }; diff --git a/test/libraries/MultiSend.spec.ts b/test/libraries/MultiSend.spec.ts index 64e1a43..c2890a7 100644 --- a/test/libraries/MultiSend.spec.ts +++ b/test/libraries/MultiSend.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { deployContract, getMock, getMultiSend, getSafeWithOwners, getDelegateCaller } from "../utils/setup"; +import { deployContract, getMock, getMultiSend, getSafe, getDelegateCaller } from "../utils/setup"; import { buildContractCall, buildSafeTransaction, @@ -30,7 +30,7 @@ describe("MultiSend", () => { const [user1] = signers; const storageSetter = await deployContract(user1, setterSource); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), multiSend: await getMultiSend(), mock: await getMock(), delegateCaller: await getDelegateCaller(), diff --git a/test/libraries/MultiSendCallOnly.spec.ts b/test/libraries/MultiSendCallOnly.spec.ts index 7629288..428215e 100644 --- a/test/libraries/MultiSendCallOnly.spec.ts +++ b/test/libraries/MultiSendCallOnly.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { deployContract, getMock, getMultiSendCallOnly, getSafeWithOwners, getDelegateCaller } from "../utils/setup"; +import { deployContract, getMock, getMultiSendCallOnly, getSafe, getDelegateCaller } from "../utils/setup"; import { buildContractCall, buildSafeTransaction, @@ -30,7 +30,7 @@ describe("MultiSendCallOnly", () => { const [user1] = signers; const storageSetter = await deployContract(user1, setterSource); return { - safe: await getSafeWithOwners({ owners: [user1.address] }), + safe: await getSafe({ owners: [user1.address] }), multiSend: await getMultiSendCallOnly(), mock: await getMock(), delegateCaller: await getDelegateCaller(), diff --git a/test/libraries/SafeMigration.spec.ts b/test/libraries/SafeMigration.spec.ts index 1973661..aaf3632 100644 --- a/test/libraries/SafeMigration.spec.ts +++ b/test/libraries/SafeMigration.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { ethers, deployments } from "hardhat"; import { - getSafeWithSingleton, + getSafe, getSafeSingletonAt, safeMigrationContract, getSafeSingletonContract, @@ -123,8 +123,8 @@ describe("SafeMigration Library", () => { return { signers, - safe: await getSafeWithSingleton(singleton, [user1.address]), - safeL2: await getSafeWithSingleton(singletonL2, [user1.address]), + safe: await getSafe({ singleton: singleton, owners: [user1.address] }), + safeL2: await getSafe({ singleton: singletonL2, owners: [user1.address] }), migration, }; }); diff --git a/test/libraries/SafeToL2Migration.spec.ts b/test/libraries/SafeToL2Migration.spec.ts index a7eae0f..173fabc 100644 --- a/test/libraries/SafeToL2Migration.spec.ts +++ b/test/libraries/SafeToL2Migration.spec.ts @@ -1,7 +1,7 @@ import { expect } from "chai"; import hre, { ethers, deployments } from "hardhat"; import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithSingleton, getSafeSingletonAt, getMock } from "../utils/setup"; +import { getSafe, getSafeSingletonAt, getMock } from "../utils/setup"; import deploymentData from "../json/safeDeployment.json"; import safeRuntimeBytecode from "../json/safeRuntimeBytecode.json"; import { @@ -83,9 +83,9 @@ describe("SafeToL2Migration library", () => { const safeToL2MigrationContract = await hre.ethers.getContractFactory("SafeToL2Migration"); const migration = await safeToL2MigrationContract.deploy(); return { - safe111: await getSafeWithSingleton(singleton111, [user1.address]), - safe130: await getSafeWithSingleton(singleton130, [user1.address]), - safe141: await getSafeWithSingleton(singleton141, [user1.address]), + safe111: await getSafe({ singleton: singleton111, owners: [user1.address] }), + safe130: await getSafe({ singleton: singleton130, owners: [user1.address] }), + safe141: await getSafe({ singleton: singleton141, owners: [user1.address] }), safeWith1967Proxy, migration, signers, diff --git a/test/libraries/SafeToL2Setup.spec.ts b/test/libraries/SafeToL2Setup.spec.ts index 89a0989..cd066d2 100644 --- a/test/libraries/SafeToL2Setup.spec.ts +++ b/test/libraries/SafeToL2Setup.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getFactory, getSafeL2SingletonContract, getSafeSingletonContract, getSafeWithOwners } from "../utils/setup"; +import { getFactory, getSafeL2SingletonContract, getSafeSingletonContract, getSafe } from "../utils/setup"; import { sameHexString } from "../utils/strings"; import { executeContractCallWithSigners } from "../../src"; import { EXPECTED_SAFE_STORAGE_LAYOUT, getContractStorageLayout } from "../utils/storage"; @@ -121,7 +121,7 @@ describe("SafeToL2Setup", () => { safeToL2SetupLib, signers: [user1], } = await setupTests(); - const safe = await getSafeWithOwners({ owners: [user1.address] }); + const safe = await getSafe({ owners: [user1.address] }); const safeToL2SetupLibAddress = await safeToL2SetupLib.getAddress(); await expect( diff --git a/test/libraries/SignMessageLib.spec.ts b/test/libraries/SignMessageLib.spec.ts index 460aea4..3695dbb 100644 --- a/test/libraries/SignMessageLib.spec.ts +++ b/test/libraries/SignMessageLib.spec.ts @@ -1,6 +1,6 @@ import { expect } from "chai"; import hre, { deployments, ethers } from "hardhat"; -import { getSafeWithOwners } from "../utils/setup"; +import { getSafe } from "../utils/setup"; import { executeContractCallWithSigners, calculateSafeMessageHash } from "../../src/utils/execution"; import { chainId } from "../utils/encoding"; @@ -11,7 +11,7 @@ describe("SignMessageLib", () => { const signers = await ethers.getSigners(); const [user1, user2] = signers; return { - safe: await getSafeWithOwners({ owners: [user1.address, user2.address] }), + safe: await getSafe({ owners: [user1.address, user2.address] }), lib, signers, }; diff --git a/test/utils/setup.ts b/test/utils/setup.ts index 29f6bc1..d400bf6 100644 --- a/test/utils/setup.ts +++ b/test/utils/setup.ts @@ -7,6 +7,10 @@ import { safeContractUnderTest } from "./config"; import { getRandomIntAsString } from "./numbers"; import { Safe, SafeL2, SafeMigration } from "../../typechain-types"; +type SafeSingleton = { + readonly singleton?: Safe | SafeL2; +}; + type SafeWithSetupConfig = { readonly owners: string[]; readonly threshold?: number; @@ -20,7 +24,7 @@ type LogGas = { readonly logGasUsage?: boolean; }; -type SafeCreationWithGasLog = SafeWithSetupConfig & LogGas; +type GetSafeParameters = SafeSingleton & SafeWithSetupConfig & LogGas; export const defaultTokenCallbackHandlerDeployment = async () => { return await deployments.get("TokenCallbackHandler"); @@ -68,14 +72,6 @@ export const getSafeL2SingletonContractFactory = async () => { return safeSingleton; }; -export const getSafeSingletonContractFactoryFromEnvVariable = async () => { - if (safeContractUnderTest() === "SafeL2") { - return await getSafeL2SingletonContractFactory(); - } - - return await getSafeSingletonContractFactory(); -}; - export const getSafeSingletonAt = async (address: string) => { const safe = await hre.ethers.getContractAt(safeContractUnderTest(), address); return safe as unknown as Safe | SafeL2; @@ -139,16 +135,20 @@ export const getMock = async () => { export const getSafeTemplate = async (saltNumber: string = getRandomIntAsString()) => { const singleton = await getSafeSingleton(); + return getSafeTemplateWithSingleton(singleton, saltNumber); +}; + +export const getSafeTemplateWithSingleton = async (singleton: Contract | Safe, saltNumber: string = getRandomIntAsString()) => { const singletonAddress = await singleton.getAddress(); const factory = await getFactory(); const template = await factory.createProxyWithNonce.staticCall(singletonAddress, "0x", saltNumber); await factory.createProxyWithNonce(singletonAddress, "0x", saltNumber).then((tx: any) => tx.wait()); - const Safe = await getSafeSingletonContractFactoryFromEnvVariable(); - return Safe.attach(template) as Safe | SafeL2; + return singleton.attach(template) as Safe | SafeL2; }; -export const getSafeWithOwners = async (safe: SafeCreationWithGasLog) => { +export const getSafe = async (safe: GetSafeParameters) => { const { + singleton = await getSafeSingleton(), owners, threshold = owners.length, to = AddressZero, @@ -158,7 +158,7 @@ export const getSafeWithOwners = async (safe: SafeCreationWithGasLog) => { saltNumber = getRandomIntAsString(), } = safe; - const template = await getSafeTemplate(saltNumber); + const template = await getSafeTemplateWithSingleton(singleton, saltNumber); await logGas( `Setup Safe with ${owners.length} owner(s)${fallbackHandler && fallbackHandler !== AddressZero ? " and fallback handler" : ""}`, template.setup(owners, threshold, to, data, fallbackHandler, AddressZero, 0, AddressZero), @@ -167,17 +167,6 @@ export const getSafeWithOwners = async (safe: SafeCreationWithGasLog) => { return template; }; -export const getSafeWithSingleton = async (singleton: Safe | SafeL2, owners: string[], saltNumber: string = getRandomIntAsString()) => { - const factory = await getFactory(); - const singletonAddress = await singleton.getAddress(); - const template = await factory.createProxyWithNonce.staticCall(singletonAddress, "0x", saltNumber); - await factory.createProxyWithNonce(singletonAddress, "0x", saltNumber).then((tx: any) => tx.wait()); - const safeProxy = singleton.attach(template) as Safe | SafeL2; - await safeProxy.setup(owners, owners.length, AddressZero, "0x", AddressZero, AddressZero, 0, AddressZero); - - return safeProxy; -}; - export const getTokenCallbackHandler = async (address?: string) => { const tokenCallbackHandler = await hre.ethers.getContractAt( "TokenCallbackHandler",