From e46c7769c38aced7bd36ca34b6f055a1d10fa7cd Mon Sep 17 00:00:00 2001 From: Richard Meissner Date: Tue, 23 Mar 2021 23:33:43 +0100 Subject: [PATCH] Closes #274: Revert on failure with safeTxGas 0 (#283) --- contracts/GnosisSafe.sol | 3 + docs/error_codes.md | 5 +- test/core/GnosisSafe.Execution.spec.ts | 41 ++++++++-- test/core/GnosisSafe.ModuleManager.spec.ts | 16 ++-- test/core/GnosisSafe.OwnerManager.spec.ts | 50 ++++++------ test/libraries/CreateCall.spec.ts | 91 +++++++++++----------- test/libraries/MultiSend.spec.ts | 4 +- test/utils/execution.ts | 10 +-- test/utils/multisend.ts | 4 +- 9 files changed, 130 insertions(+), 94 deletions(-) diff --git a/contracts/GnosisSafe.sol b/contracts/GnosisSafe.sol index 12445a6c3..0570757c6 100644 --- a/contracts/GnosisSafe.sol +++ b/contracts/GnosisSafe.sol @@ -170,6 +170,9 @@ contract GnosisSafe // We only substract 2500 (compared to the 3000 before) to ensure that the amount passed is still higher than safeTxGas success = execute(to, value, data, operation, gasPrice == 0 ? (gasleft() - 2500) : safeTxGas); gasUsed = gasUsed.sub(gasleft()); + // If no safeTxGas and no gasPrice was set (e.g. both are 0), then the internal tx is required to be successful + // This makes it possible to use `estimateGas` without issues, as it searches for the minimum gas where the tx doesn't revert + require(success || safeTxGas != 0 || gasPrice != 0, "GS013"); // We transfer the calculated tx costs to the tx.origin to avoid sending it to intermediate contracts that have made calls uint256 payment = 0; if (gasPrice > 0) { diff --git a/docs/error_codes.md b/docs/error_codes.md index e096d4f48..adacb2639 100644 --- a/docs/error_codes.md +++ b/docs/error_codes.md @@ -4,10 +4,11 @@ - `GS000`: `Could not finish initialization` - `GS001`: `Threshold needs to be defined` -### General gas related -- `GS010`: `Not enough gas to execute safe transaction` +### General gas/ execution related +- `GS010`: `Not enough gas to execute Safe transaction` - `GS011`: `Could not pay gas costs with ether` - `GS012`: `Could not pay gas costs with token` +- `GS013`: `Safe transaction failed when gasPrice and safeTxGas were 0` ### General signature validation related - `GS020`: `Signatures data too short` diff --git a/test/core/GnosisSafe.Execution.spec.ts b/test/core/GnosisSafe.Execution.spec.ts index 1c4d60178..2c2f017d2 100644 --- a/test/core/GnosisSafe.Execution.spec.ts +++ b/test/core/GnosisSafe.Execution.spec.ts @@ -67,13 +67,29 @@ describe("GnosisSafe", async () => { ).to.be.eq("0x" + "baddad".padEnd(64, "0")) }) - it('should emit event for failed call execution', async () => { + it('should emit event for failed call execution if safeTxGas > 0', async () => { const { safe, reverter } = await setupTests() await expect( - executeContractCallWithSigners(safe, reverter, "revert", [], [user1]) + executeContractCallWithSigners(safe, reverter, "revert", [], [user1], false, { safeTxGas: 1 }) + ).to.emit(safe, "ExecutionFailure") + }) + + it('should emit event for failed call execution if gasPrice > 0', async () => { + const { safe, reverter } = await setupTests() + // Fund refund + await user1.sendTransaction({ to: safe.address, value: 10000000 }) + await expect( + executeContractCallWithSigners(safe, reverter, "revert", [], [user1], false, { gasPrice: 1 }) ).to.emit(safe, "ExecutionFailure") }) + it('should revert for failed call execution if gasPrice == 0 and safeTxGas == 0', async () => { + const { safe, reverter } = await setupTests() + await expect( + executeContractCallWithSigners(safe, reverter, "revert", [], [user1]) + ).to.revertedWith("GS013") + }) + it('should emit event for successful delegatecall execution', async () => { const { safe, storageSetter } = await setupTests() await expect( @@ -89,14 +105,29 @@ describe("GnosisSafe", async () => { ).to.be.eq("0x" + "".padEnd(64, "0")) }) - it('should emit event for failed delegatecall execution', async () => { + it('should emit event for failed delegatecall execution if safeTxGas > 0', async () => { const { safe, reverter } = await setupTests() - const txHash = calculateSafeTransactionHash(safe, buildContractCall(reverter, "revert", [], await safe.nonce(), true), await chainId()) + const txHash = calculateSafeTransactionHash(safe, buildContractCall(reverter, "revert", [], await safe.nonce(), true, { safeTxGas: 1 }), await chainId()) await expect( - executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true) + executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true, { safeTxGas: 1 }) ).to.emit(safe, "ExecutionFailure").withArgs(txHash, 0) }) + it('should emit event for failed delegatecall execution if gasPrice > 0', async () => { + const { safe, reverter } = await setupTests() + await user1.sendTransaction({ to: safe.address, value: 10000000 }) + await expect( + executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true, { gasPrice: 1 }) + ).to.emit(safe, "ExecutionFailure") + }) + + it('should emit event for failed delegatecall execution if gasPrice == 0 and safeTxGas == 0', async () => { + const { safe, reverter } = await setupTests() + await expect( + executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true) + ).to.revertedWith("GS013") + }) + it('should revert on unknown operation', async () => { const { safe } = await setupTests() const tx = buildSafeTransaction({ to: safe.address, nonce: await safe.nonce(), operation: 2 }) diff --git a/test/core/GnosisSafe.ModuleManager.spec.ts b/test/core/GnosisSafe.ModuleManager.spec.ts index bfae5e231..d13a6aa05 100644 --- a/test/core/GnosisSafe.ModuleManager.spec.ts +++ b/test/core/GnosisSafe.ModuleManager.spec.ts @@ -30,14 +30,14 @@ describe("ModuleManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "enableModule", [AddressOne], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not set 0 Address', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "enableModule", [AddressZero], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not add module twice', async () => { @@ -47,7 +47,7 @@ describe("ModuleManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('emits event for new module', async () => { @@ -94,14 +94,14 @@ describe("ModuleManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, AddressOne], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not set 0 Address', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, AddressZero], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevModule, module pair provided - Invalid target', async () => { @@ -109,7 +109,7 @@ describe("ModuleManager", async () => { await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]) await expect( executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, user1.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevModule, module pair provided - Invalid sentinel', async () => { @@ -117,7 +117,7 @@ describe("ModuleManager", async () => { await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]) await expect( executeContractCallWithSigners(safe, safe, "disableModule", [AddressZero, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevModule, module pair provided - Invalid source', async () => { @@ -126,7 +126,7 @@ describe("ModuleManager", async () => { await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]) await expect( executeContractCallWithSigners(safe, safe, "disableModule", [user1.address, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('emits event for disabled module', async () => { diff --git a/test/core/GnosisSafe.OwnerManager.spec.ts b/test/core/GnosisSafe.OwnerManager.spec.ts index 6bce7c3c2..e95f30070 100644 --- a/test/core/GnosisSafe.OwnerManager.spec.ts +++ b/test/core/GnosisSafe.OwnerManager.spec.ts @@ -29,7 +29,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [safe.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not set sentinel', async () => { @@ -37,14 +37,14 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [AddressOne, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not set 0 Address', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [AddressZero, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not add owner twice', async () => { @@ -53,21 +53,21 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not add owner and change threshold to 0', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 0], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not add owner and change threshold to larger number than new owner count', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 3], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('emits event for new owner', async () => { @@ -107,7 +107,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, AddressOne, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not remove 0 Address', async () => { @@ -116,7 +116,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, AddressZero, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid target', async () => { @@ -124,7 +124,7 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, user1.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid sentinel', async () => { @@ -132,7 +132,7 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressZero, user2.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid source', async () => { @@ -140,7 +140,7 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user1.address, user2.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not remove owner and change threshold to larger number than new owner count', async () => { @@ -148,7 +148,7 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 2], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not remove owner and change threshold to 0', async () => { @@ -156,14 +156,14 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 0], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not remove owner only owner', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, user1.address, 1], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('emits event for removed owner and threshold if changed', async () => { @@ -200,7 +200,7 @@ describe("OwnerManager", async () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]) await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 2], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) }) @@ -210,12 +210,12 @@ describe("OwnerManager", async () => { await expect(safe.swapOwner(AddressOne, user1.address, user2.address)).to.be.revertedWith("GS031") }) - it('can not swap in Safe itseld', async () => { + it('can not swap in Safe itself', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, safe.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not swap in sentinel', async () => { @@ -223,7 +223,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, AddressOne], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not swap in 0 Address', async () => { @@ -231,7 +231,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, AddressZero], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not swap in existing owner', async () => { @@ -239,7 +239,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, user1.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not swap out sentinel', async () => { @@ -247,7 +247,7 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user1.address, AddressOne, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('can not swap out 0 address', async () => { @@ -255,28 +255,28 @@ describe("OwnerManager", async () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user3.address, AddressZero, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid target', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user3.address, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid sentinel', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressZero, user1.address, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Invalid prevOwner, owner pair provided - Invalid source', async () => { const { safe } = await setupTests() await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user2.address, user1.address, user2.address], [user1]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('emits event for replacing owner', async () => { diff --git a/test/libraries/CreateCall.spec.ts b/test/libraries/CreateCall.spec.ts index 51b0040a4..ebd6d6aff 100644 --- a/test/libraries/CreateCall.spec.ts +++ b/test/libraries/CreateCall.spec.ts @@ -5,81 +5,82 @@ import { compile, getCreateCall, getSafeWithOwners } from "../utils/setup"; import { buildContractCall, executeTx, safeApproveHash } from "../utils/execution"; import { parseEther } from "@ethersproject/units"; -describe("CreateCall", async () => { - - const CONTRACT_SOURCE = ` - contract Test { - address public creator; - constructor() payable { - creator = msg.sender; - } +const CONTRACT_SOURCE = ` +contract Test { + address public creator; + constructor() payable { + creator = msg.sender; + } + + function x() public pure returns (uint) { + return 21; + } +}` - function x() public pure returns (uint) { - return 21; - } - }` - const compiledTestContract = await compile(CONTRACT_SOURCE); +describe("CreateCall", async () => { const [user1] = waffle.provider.getWallets(); const setupTests = deployments.createFixture(async ({ deployments }) => { await deployments.fixture(); + const testContract = await compile(CONTRACT_SOURCE); return { safe: await getSafeWithOwners([user1.address]), - createCall: await getCreateCall() + createCall: await getCreateCall(), + testContract } }) describe("performCreate", async () => { it('should revert if called directly and no value is on the factory', async () => { - const { createCall } = await setupTests() + const { createCall, testContract } = await setupTests() await expect( - createCall.performCreate(1, compiledTestContract.data) + createCall.performCreate(1, testContract.data) ).to.be.revertedWith("Could not deploy contract") }) it('can call factory directly', async () => { - const { createCall } = await setupTests() + const { createCall, testContract } = await setupTests() const createCallNonce = await ethers.provider.getTransactionCount(createCall.address) const address = ethers.utils.getContractAddress({ from: createCall.address, nonce: createCallNonce }) await expect( - createCall.performCreate(0, compiledTestContract.data) + createCall.performCreate(0, testContract.data) ).to.emit(createCall, "ContractCreation").withArgs(address) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(createCall.address) }) it('should fail if Safe does not have value to send along', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() - const tx = await buildContractCall(createCall, "performCreate", [1, compiledTestContract.data], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate", [1, testContract.data], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('should successfully create contract and emit event', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() const safeEthereumNonce = await ethers.provider.getTransactionCount(safe.address) const address = ethers.utils.getContractAddress({ from: safe.address, nonce: safeEthereumNonce }) // We require this as 'emit' check the address of the event const safeCreateCall = createCall.attach(safe.address) - const tx = await buildContractCall(createCall, "performCreate", [0, compiledTestContract.data], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate", [0, testContract.data], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) ).to.emit(safe, "ExecutionSuccess").and.to.emit(safeCreateCall, "ContractCreation").withArgs(address) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(safe.address) }) it('should successfully create contract and send along ether', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() await user1.sendTransaction({ to: safe.address, value: parseEther("1") }) await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1")) @@ -88,14 +89,14 @@ describe("CreateCall", async () => { // We require this as 'emit' check the address of the event const safeCreateCall = createCall.attach(safe.address) - const tx = await buildContractCall(createCall, "performCreate", [parseEther("1"), compiledTestContract.data], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate", [parseEther("1"), testContract.data], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) ).to.emit(safe, "ExecutionSuccess").and.to.emit(safeCreateCall, "ContractCreation").withArgs(address) await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0")) await expect(await hre.ethers.provider.getBalance(address)).to.be.deep.eq(parseEther("1")) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(safe.address) }) }) @@ -105,66 +106,66 @@ describe("CreateCall", async () => { const salt = ethers.utils.keccak256(ethers.utils.toUtf8Bytes("createCall")) it('should revert if called directly and no value is on the factory', async () => { - const { createCall } = await setupTests() + const { createCall, testContract } = await setupTests() await expect( - createCall.performCreate2(1, compiledTestContract.data, salt) + createCall.performCreate2(1, testContract.data, salt) ).to.be.revertedWith("Could not deploy contract") }) it('can call factory directly', async () => { - const { createCall } = await setupTests() - const address = ethers.utils.getCreate2Address(createCall.address, salt, ethers.utils.keccak256(compiledTestContract.data)) + const { createCall, testContract } = await setupTests() + const address = ethers.utils.getCreate2Address(createCall.address, salt, ethers.utils.keccak256(testContract.data)) await expect( - createCall.performCreate2(0, compiledTestContract.data, salt) + createCall.performCreate2(0, testContract.data, salt) ).to.emit(createCall, "ContractCreation").withArgs(address) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(createCall.address) }) it('should fail if Safe does not have value to send along', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() - const tx = await buildContractCall(createCall, "performCreate2", [1, compiledTestContract.data, salt], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate2", [1, testContract.data, salt], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('should successfully create contract and emit event', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() - const address = ethers.utils.getCreate2Address(safe.address, salt, ethers.utils.keccak256(compiledTestContract.data)) + const address = ethers.utils.getCreate2Address(safe.address, salt, ethers.utils.keccak256(testContract.data)) // We require this as 'emit' check the address of the event const safeCreateCall = createCall.attach(safe.address) - const tx = await buildContractCall(createCall, "performCreate2", [0, compiledTestContract.data, salt], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate2", [0, testContract.data, salt], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) ).to.emit(safe, "ExecutionSuccess").and.to.emit(safeCreateCall, "ContractCreation").withArgs(address) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(safe.address) }) it('should successfully create contract and send along ether', async () => { - const { safe, createCall } = await setupTests() + const { safe, createCall, testContract } = await setupTests() await user1.sendTransaction({ to: safe.address, value: parseEther("1") }) await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1")) - const address = ethers.utils.getCreate2Address(safe.address, salt, ethers.utils.keccak256(compiledTestContract.data)) + const address = ethers.utils.getCreate2Address(safe.address, salt, ethers.utils.keccak256(testContract.data)) // We require this as 'emit' check the address of the event const safeCreateCall = createCall.attach(safe.address) - const tx = await buildContractCall(createCall, "performCreate2", [parseEther("1"), compiledTestContract.data, salt], await safe.nonce(), true) + const tx = await buildContractCall(createCall, "performCreate2", [parseEther("1"), testContract.data, salt], await safe.nonce(), true) await expect( executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)]) ).to.emit(safe, "ExecutionSuccess").and.to.emit(safeCreateCall, "ContractCreation").withArgs(address) await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0")) await expect(await hre.ethers.provider.getBalance(address)).to.be.deep.eq(parseEther("1")) - const newContract = new ethers.Contract(address, compiledTestContract.interface, user1) + const newContract = new ethers.Contract(address, testContract.interface, user1) expect(await newContract.creator()).to.be.eq(safe.address) }) }) diff --git a/test/libraries/MultiSend.spec.ts b/test/libraries/MultiSend.spec.ts index 748b2c386..36317b0b3 100644 --- a/test/libraries/MultiSend.spec.ts +++ b/test/libraries/MultiSend.spec.ts @@ -60,7 +60,7 @@ describe("MultiSend", async () => { const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce()) await expect( executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ]) - ).to.emit(safe, "ExecutionFailure") + ).to.revertedWith("GS013") }) it('Can execute empty multisend', async () => { @@ -99,7 +99,7 @@ describe("MultiSend", async () => { buildSafeTransaction({to: user2.address, value: parseEther("1"), nonce: 0}), buildSafeTransaction({to: user2.address, value: parseEther("1"), nonce: 0}), ] - const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce()) + const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce(), { safeTxGas: 1 }) await expect( executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ]) ).to.emit(safe, "ExecutionFailure") diff --git a/test/utils/execution.ts b/test/utils/execution.ts index fed64b531..14924e644 100644 --- a/test/utils/execution.ts +++ b/test/utils/execution.ts @@ -124,18 +124,18 @@ export const executeTx = async (safe: Contract, safeTx: SafeTransaction, signatu return safe.execTransaction(safeTx.to, safeTx.value, safeTx.data, safeTx.operation, safeTx.safeTxGas, safeTx.baseGas, safeTx.gasPrice, safeTx.gasToken, safeTx.refundReceiver, signatureBytes, overrides || {}) } -export const buildContractCall = (contract: Contract, method: string, params: any[], nonce: number, delegateCall?: boolean): SafeTransaction => { +export const buildContractCall = (contract: Contract, method: string, params: any[], nonce: number, delegateCall?: boolean, overrides?: Partial): SafeTransaction => { const data = contract.interface.encodeFunctionData(method, params) - return buildSafeTransaction({ + return buildSafeTransaction(Object.assign({ to: contract.address, data, operation: delegateCall ? 1 : 0, nonce - }) + }, overrides)) } -export const executeContractCallWithSigners = async (safe: Contract, contract: Contract, method: string, params: any[], signers: Wallet[], delegateCall?: boolean) => { - const tx = buildContractCall(contract, method, params, await safe.nonce(), delegateCall) +export const executeContractCallWithSigners = async (safe: Contract, contract: Contract, method: string, params: any[], signers: Wallet[], delegateCall?: boolean, overrides?: Partial) => { + const tx = buildContractCall(contract, method, params, await safe.nonce(), delegateCall, overrides) const sigs = await Promise.all(signers.map((signer) => safeSignTypedData(signer, safe, tx))) return executeTx(safe, tx, sigs) } diff --git a/test/utils/multisend.ts b/test/utils/multisend.ts index c9d2a3b21..32457a0fa 100644 --- a/test/utils/multisend.ts +++ b/test/utils/multisend.ts @@ -15,6 +15,6 @@ export const encodeMultiSend = (txs: MetaTransaction[]): string => { return "0x" + txs.map((tx) => encodeMetaTransaction(tx)).join("") } -export const buildMultiSendSafeTx = (multiSend: Contract, txs: MetaTransaction[], nonce: number): SafeTransaction => { - return buildContractCall(multiSend, "multiSend", [encodeMultiSend(txs)], nonce, true) +export const buildMultiSendSafeTx = (multiSend: Contract, txs: MetaTransaction[], nonce: number, overrides?: Partial): SafeTransaction => { + return buildContractCall(multiSend, "multiSend", [encodeMultiSend(txs)], nonce, true, overrides) } \ No newline at end of file