diff --git a/contracts/Safe.sol b/contracts/Safe.sol index e22fd3445..62d9c6e0e 100644 --- a/contracts/Safe.sol +++ b/contracts/Safe.sol @@ -175,7 +175,16 @@ contract Safe is 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 - if (!success && safeTxGas == 0 && gasPrice == 0) revertWithError("GS013"); + if (!success && safeTxGas == 0 && gasPrice == 0) { + /* solhint-disable no-inline-assembly */ + /// @solidity memory-safe-assembly + assembly { + let p := mload(0x40) + returndatacopy(p, 0, returndatasize()) + revert(p, returndatasize()) + } + /* solhint-enable no-inline-assembly */ + } // 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/test/core/Safe.Execution.spec.ts b/test/core/Safe.Execution.spec.ts index d8ccf2002..4a9e2e5d5 100644 --- a/test/core/Safe.Execution.spec.ts +++ b/test/core/Safe.Execution.spec.ts @@ -131,7 +131,7 @@ describe("Safe", () => { it("should revert for failed call execution if gasPrice == 0 and safeTxGas == 0", async () => { const { safe, reverter, signers } = await setupTests(); const [user1] = signers; - await expect(executeContractCallWithSigners(safe, reverter, "revert", [], [user1])).to.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, reverter, "revert", [], [user1])).to.revertedWith("Shit happens"); }); it("should emit event for successful delegatecall execution", async () => { @@ -184,7 +184,7 @@ describe("Safe", () => { it("should emit event for failed delegatecall execution if gasPrice == 0 and safeTxGas == 0", async () => { const { safe, reverter, signers } = await setupTests(); const [user1] = signers; - await expect(executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true)).to.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, reverter, "revert", [], [user1], true)).to.revertedWith("Shit happens"); }); it("should revert on unknown operation", async () => { diff --git a/test/core/Safe.FallbackManager.spec.ts b/test/core/Safe.FallbackManager.spec.ts index de31c0a71..e85ed56b3 100644 --- a/test/core/Safe.FallbackManager.spec.ts +++ b/test/core/Safe.FallbackManager.spec.ts @@ -174,10 +174,9 @@ describe("FallbackManager", () => { // Setup Safe await safe.setup([user1.address], 1, AddressZero, "0x", AddressZero, AddressZero, 0, AddressZero); - // The transaction execution function doesn't bubble up revert messages so we check for a generic transaction fail code GS013 await expect( executeContractCallWithSigners(safe, safe, "setFallbackHandler", [await safe.getAddress()], [user1]), - ).to.be.revertedWith("GS013"); + ).to.be.revertedWith("GS400"); }); }); }); diff --git a/test/core/Safe.GuardManager.spec.ts b/test/core/Safe.GuardManager.spec.ts index 9467aea76..5ed6a81d6 100644 --- a/test/core/Safe.GuardManager.spec.ts +++ b/test/core/Safe.GuardManager.spec.ts @@ -44,7 +44,7 @@ describe("GuardManager", () => { } = await setupWithTemplate(); const safe = await getSafe({ owners: [user1.address] }); - await expect(executeContractCallWithSigners(safe, safe, "setGuard", [user2.address], [user1])).to.be.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, safe, "setGuard", [user2.address], [user1])).to.be.reverted; }); it("emits an event when the guard is changed", async () => { diff --git a/test/core/Safe.ModuleManager.spec.ts b/test/core/Safe.ModuleManager.spec.ts index 797826dde..6f4841943 100644 --- a/test/core/Safe.ModuleManager.spec.ts +++ b/test/core/Safe.ModuleManager.spec.ts @@ -43,7 +43,7 @@ describe("ModuleManager", () => { signers: [user1], } = await setupTests(); - await expect(executeContractCallWithSigners(safe, safe, "enableModule", [AddressOne], [user1])).to.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, safe, "enableModule", [AddressOne], [user1])).to.revertedWith("GS101"); }); it("can not set 0 Address", async () => { @@ -51,7 +51,7 @@ describe("ModuleManager", () => { safe, signers: [user1], } = await setupTests(); - await expect(executeContractCallWithSigners(safe, safe, "enableModule", [AddressZero], [user1])).to.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, safe, "enableModule", [AddressZero], [user1])).to.revertedWith("GS101"); }); it("can not add module twice", async () => { @@ -62,7 +62,7 @@ describe("ModuleManager", () => { // Use module for execution to see error await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]); - await expect(executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1])).to.revertedWith("GS013"); + await expect(executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1])).to.revertedWith("GS102"); }); it("emits event for a new module", async () => { @@ -116,7 +116,7 @@ describe("ModuleManager", () => { } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, AddressOne], [user1])).to.revertedWith( - "GS013", + "GS101", ); }); @@ -126,7 +126,7 @@ describe("ModuleManager", () => { signers: [user1], } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, AddressZero], [user1])).to.revertedWith( - "GS013", + "GS101", ); }); @@ -137,7 +137,7 @@ describe("ModuleManager", () => { } = await setupTests(); await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]); await expect(executeContractCallWithSigners(safe, safe, "disableModule", [AddressOne, user1.address], [user1])).to.revertedWith( - "GS013", + "GS103", ); }); @@ -149,7 +149,7 @@ describe("ModuleManager", () => { await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]); await expect( executeContractCallWithSigners(safe, safe, "disableModule", [AddressZero, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS103"); }); it("Invalid prevModule, module pair provided - Invalid source", async () => { @@ -161,7 +161,7 @@ describe("ModuleManager", () => { await executeContractCallWithSigners(safe, safe, "enableModule", [user2.address], [user1]); await expect( executeContractCallWithSigners(safe, safe, "disableModule", [user1.address, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS103"); }); it("emits event for disabled module", async () => { @@ -579,9 +579,9 @@ describe("ModuleManager", () => { } = await setupTests(); const safe = await getSafe({ owners: [user1.address] }); - await expect(executeContractCallWithSigners(safe, safe, "setModuleGuard", [user2.address], [user1])).to.be.revertedWith( - "GS013", - ); + await expect( + executeContractCallWithSigners(safe, safe, "setModuleGuard", [user2.address], [user1]), + ).to.be.revertedWithoutReason(); }); it("emits an event when the module guard is changed", async () => { diff --git a/test/core/Safe.OwnerManager.spec.ts b/test/core/Safe.OwnerManager.spec.ts index b15a1f105..f6903df2a 100644 --- a/test/core/Safe.OwnerManager.spec.ts +++ b/test/core/Safe.OwnerManager.spec.ts @@ -33,7 +33,7 @@ describe("OwnerManager", () => { const safeAddress = await safe.getAddress(); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [safeAddress, 1], [user1])).to.revertedWith( - "GS013", + "GS203", ); }); @@ -44,7 +44,7 @@ describe("OwnerManager", () => { } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [AddressOne, 1], [user1])).to.revertedWith( - "GS013", + "GS203", ); }); @@ -54,7 +54,7 @@ describe("OwnerManager", () => { signers: [user1], } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [AddressZero, 1], [user1])).to.revertedWith( - "GS013", + "GS203", ); }); @@ -66,7 +66,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1])).to.revertedWith( - "GS013", + "GS204", ); }); @@ -76,7 +76,7 @@ describe("OwnerManager", () => { signers: [user1, user2], } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 0], [user1])).to.revertedWith( - "GS013", + "GS202", ); }); @@ -86,7 +86,7 @@ describe("OwnerManager", () => { signers: [user1, user2], } = await setupTests(); await expect(executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 3], [user1])).to.revertedWith( - "GS013", + "GS201", ); }); @@ -139,7 +139,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect(executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, AddressOne, 1], [user1])).to.revertedWith( - "GS013", + "GS203", ); }); @@ -151,7 +151,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect(executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, AddressZero, 1], [user1])).to.revertedWith( - "GS013", + "GS203", ); }); @@ -163,7 +163,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, user1.address, 1], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("Invalid prevOwner, owner pair provided - Invalid sentinel", async () => { @@ -174,7 +174,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressZero, user2.address, 1], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("Invalid prevOwner, owner pair provided - Invalid source", async () => { @@ -185,7 +185,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user1.address, user2.address, 1], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("can not remove owner and change threshold to larger number than new owner count", async () => { @@ -196,7 +196,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 2], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS201"); }); it("can not remove owner and change threshold to 0", async () => { @@ -207,7 +207,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 0], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS202"); }); it("can not remove owner only owner", async () => { @@ -217,7 +217,7 @@ describe("OwnerManager", () => { } = await setupTests(); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [AddressOne, user1.address, 1], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS201"); }); it("emits event for removed owner and threshold if changed", async () => { @@ -263,7 +263,7 @@ describe("OwnerManager", () => { await executeContractCallWithSigners(safe, safe, "addOwnerWithThreshold", [user2.address, 1], [user1]); await expect( executeContractCallWithSigners(safe, safe, "removeOwner", [user2.address, user1.address, 2], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS201"); }); }); @@ -285,7 +285,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, safeAddress], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS203"); }); it("can not swap in sentinel", async () => { @@ -296,7 +296,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, AddressOne], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS203"); }); it("can not swap in 0 Address", async () => { @@ -307,7 +307,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, AddressZero], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS203"); }); it("can not swap in existing owner", async () => { @@ -318,7 +318,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user1.address, user1.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS204"); }); it("can not swap out sentinel", async () => { @@ -329,7 +329,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user1.address, AddressOne, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS203"); }); it("can not swap out 0 address", async () => { @@ -340,7 +340,7 @@ describe("OwnerManager", () => { await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user3.address, AddressZero, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS203"); }); it("Invalid prevOwner, owner pair provided - Invalid target", async () => { @@ -350,7 +350,7 @@ describe("OwnerManager", () => { } = await setupTests(); await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressOne, user3.address, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("Invalid prevOwner, owner pair provided - Invalid sentinel", async () => { @@ -360,7 +360,7 @@ describe("OwnerManager", () => { } = await setupTests(); await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [AddressZero, user1.address, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("Invalid prevOwner, owner pair provided - Invalid source", async () => { @@ -370,7 +370,7 @@ describe("OwnerManager", () => { } = await setupTests(); await expect( executeContractCallWithSigners(safe, safe, "swapOwner", [user2.address, user1.address, user2.address], [user1]), - ).to.revertedWith("GS013"); + ).to.revertedWith("GS205"); }); it("emits event for replacing owner", async () => { diff --git a/test/guards/ReentrancyTransactionGuard.spec.ts b/test/guards/ReentrancyTransactionGuard.spec.ts index 53e3a48dd..0429fcc12 100644 --- a/test/guards/ReentrancyTransactionGuard.spec.ts +++ b/test/guards/ReentrancyTransactionGuard.spec.ts @@ -93,7 +93,7 @@ describe("ReentrancyTransactionGuard", () => { ], [user1], ), - ).to.be.revertedWith("GS013"); + ).to.be.revertedWith("Reentrancy detected"); expect(await mock.invocationCount()).to.be.eq(0); }); diff --git a/test/libraries/CreateCall.spec.ts b/test/libraries/CreateCall.spec.ts index b560ce02f..e8ed39ec6 100644 --- a/test/libraries/CreateCall.spec.ts +++ b/test/libraries/CreateCall.spec.ts @@ -69,7 +69,7 @@ describe("CreateCall", () => { } = await setupTests(); 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.revertedWith("GS013"); + await expect(executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)])).to.revertedWith("Could not deploy contract"); }); it("should successfully create contract and emit event", async () => { @@ -165,7 +165,7 @@ describe("CreateCall", () => { } = await setupTests(); 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.revertedWith("GS013"); + await expect(executeTx(safe, tx, [await safeApproveHash(user1, safe, tx, true)])).to.revertedWith("Could not deploy contract"); }); it("should successfully create contract and emit event", async () => { diff --git a/test/libraries/MultiSend.spec.ts b/test/libraries/MultiSend.spec.ts index 0cb3409cd..d744fccbf 100644 --- a/test/libraries/MultiSend.spec.ts +++ b/test/libraries/MultiSend.spec.ts @@ -78,9 +78,9 @@ describe("MultiSend", () => { const txs = [buildSafeTransaction({ to: user2.address, operation: 2, nonce: 0 })]; const safeTx = await buildMultiSendSafeTx(multiSend, txs, await safe.nonce()); - await expect(executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)])).to.revertedWith( - "GS013", - ); + await expect( + executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)]), + ).to.revertedWithoutReason(); }); it("Can execute empty multisend", async () => { diff --git a/test/libraries/MultiSendCallOnly.spec.ts b/test/libraries/MultiSendCallOnly.spec.ts index 783b18339..447ef9b51 100644 --- a/test/libraries/MultiSendCallOnly.spec.ts +++ b/test/libraries/MultiSendCallOnly.spec.ts @@ -49,9 +49,7 @@ describe("MultiSendCallOnly", () => { const txs = [buildSafeTransaction({ to: user2.address, operation: 2, nonce: 0 })]; const safeTx = await buildMultiSendSafeTx(multiSend, txs, await safe.nonce()); - await expect(executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)])).to.revertedWith( - "GS013", - ); + await expect(executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)])).to.reverted; }); it("Should fail when using delegatecall operation", async () => { @@ -63,9 +61,7 @@ describe("MultiSendCallOnly", () => { const txs = [buildSafeTransaction({ to: user2.address, operation: 1, nonce: 0 })]; const safeTx = await buildMultiSendSafeTx(multiSend, txs, await safe.nonce()); - await expect(executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)])).to.revertedWith( - "GS013", - ); + await expect(executeTx(safe.connect(user1), safeTx, [await safeApproveHash(user1, safe, safeTx, true)])).to.reverted; }); it("Can execute empty multisend", async () => { diff --git a/test/libraries/SafeToL2Setup.spec.ts b/test/libraries/SafeToL2Setup.spec.ts index aaaad232f..23d724ad1 100644 --- a/test/libraries/SafeToL2Setup.spec.ts +++ b/test/libraries/SafeToL2Setup.spec.ts @@ -126,7 +126,7 @@ describe("SafeToL2Setup", () => { await expect( executeContractCallWithSigners(safe, safeToL2SetupLib, "setupToL2", [safeToL2SetupLibAddress], [user1], true), - ).to.be.rejectedWith("GS013"); + ).to.be.rejectedWith("Safe must have not executed any tx"); }); it("changes the expected storage slot without touching the most important ones", async () => {