diff --git a/hardhat.config.ts b/hardhat.config.ts index 6fc2ffae..4c523c8e 100644 --- a/hardhat.config.ts +++ b/hardhat.config.ts @@ -90,6 +90,7 @@ export default { }, networks: { hardhat: { + hardfork: "berlin", blockGasLimit: 12.5e6, }, mainnet: { diff --git a/src/contracts/libraries/GPv2Transfer.sol b/src/contracts/libraries/GPv2Transfer.sol index 360f4cc7..36c698f6 100644 --- a/src/contracts/libraries/GPv2Transfer.sol +++ b/src/contracts/libraries/GPv2Transfer.sol @@ -155,7 +155,26 @@ library GPv2Transfer { transfer.balance != GPv2Order.BALANCE_INTERNAL, "GPv2: unsupported internal ETH" ); - payable(transfer.account).transfer(transfer.amount); + + // NOTE: We transfer with a slightly higher stipend than the + // Solidity default of 2300 because of the recent Berlin hard + // fork. This is not strictly needed, as access lists would also + // allow sending Ether to SC wallets, but increasing the stipend + // seemed like an appropriate solution. The value of 5700 was + // chosen because: + // - it is small enough that no meaningful work can be done (no + // storage writing for example) + // - it is enough for a cold storage read (2100), cold delegate + // call (2600), and a little more for doing things (1000) + // - it is enough to transfer to a Gnosis Safe :) + // solhint-disable avoid-low-level-calls + (bool success, ) = + payable(transfer.account).call{ + value: transfer.amount, + gas: 5700 + }(hex""); + // solhint-enable avoid-low-level-calls + require(success, "GPv2: ether transfer failed"); } else if (transfer.balance == GPv2Order.BALANCE_ERC20) { transfer.token.safeTransfer(transfer.account, transfer.amount); } else { diff --git a/src/contracts/test/FallbackWriteStorage.sol b/src/contracts/test/FallbackWriteStorage.sol new file mode 100644 index 00000000..3ef2d1ba --- /dev/null +++ b/src/contracts/test/FallbackWriteStorage.sol @@ -0,0 +1,10 @@ +// SPDX-License-Identifier: LGPL-3.0-or-later +pragma solidity ^0.7.6; + +contract FallbackWriteStorage { + uint256 public receiveCount; + + receive() external payable { + receiveCount += 1; + } +} diff --git a/test/GPv2Transfer.test.ts b/test/GPv2Transfer.test.ts index 85d83a33..a9057360 100644 --- a/test/GPv2Transfer.test.ts +++ b/test/GPv2Transfer.test.ts @@ -1,13 +1,14 @@ import IERC20 from "@openzeppelin/contracts/build/contracts/IERC20.json"; import { expect } from "chai"; import { MockContract } from "ethereum-waffle"; -import { BigNumber, Contract } from "ethers"; +import { BigNumber, Contract, ContractFactory } from "ethers"; import { artifacts, ethers, waffle } from "hardhat"; import { BUY_ETH_ADDRESS } from "../src/ts"; import { UserBalanceOpKind } from "./balancer"; import { OrderBalanceId } from "./encoding"; +import { GnosisSafeManager } from "./safe"; describe("GPv2Transfer", () => { const [deployer, recipient, funder, ...traders] = @@ -17,6 +18,9 @@ describe("GPv2Transfer", () => { let vault: MockContract; let token: MockContract; + let NonPayable: ContractFactory; + let FallbackWriteStorage: ContractFactory; + beforeEach(async () => { const GPv2Transfer = await ethers.getContractFactory( "GPv2TransferTestInterface", @@ -26,6 +30,11 @@ describe("GPv2Transfer", () => { const IVault = await artifacts.readArtifact("IVault"); vault = await waffle.deployMockContract(deployer, IVault.abi); token = await waffle.deployMockContract(deployer, IERC20.abi); + + NonPayable = await ethers.getContractFactory("NonPayable"); + FallbackWriteStorage = await ethers.getContractFactory( + "FallbackWriteStorage", + ); }); const amount = ethers.utils.parseEther("0.1337"); @@ -508,6 +517,77 @@ describe("GPv2Transfer", () => { ); }); + it("should transfer Ether to a Smart Contract wallet", async () => { + await funder.sendTransaction({ + to: transfer.address, + value: amount, + }); + + const safeManager = await GnosisSafeManager.init(deployer); + const safe = await safeManager.newSafe([traders[0].address], 1); + + await transfer.transferToAccountsTest(vault.address, [ + { + account: safe.address, + token: BUY_ETH_ADDRESS, + amount, + balance: OrderBalanceId.ERC20, + }, + ]); + + expect(await ethers.provider.getBalance(safe.address)).to.deep.equal( + amount, + ); + }); + + it("should revert when transfering Ether to contract that reverts", async () => { + await funder.sendTransaction({ + to: transfer.address, + value: amount, + }); + + const smartWallet = await NonPayable.deploy(); + await expect( + transfer.transferToAccountsTest(vault.address, [ + { + account: smartWallet.address, + token: BUY_ETH_ADDRESS, + amount, + balance: OrderBalanceId.ERC20, + }, + ]), + ).to.be.reverted; + }); + + it("should revert when transfering Ether to contract writes storage", async () => { + await funder.sendTransaction({ + to: transfer.address, + value: amount, + }); + + const smartWallet = await FallbackWriteStorage.deploy(); + + await expect( + funder.sendTransaction({ + to: smartWallet.address, + value: 1, + }), + ).to.not.be.reverted; + expect(await smartWallet.receiveCount()).to.equal(1); + + await expect( + transfer.transferToAccountsTest(vault.address, [ + { + account: smartWallet.address, + token: BUY_ETH_ADDRESS, + amount, + balance: OrderBalanceId.ERC20, + }, + ]), + ).to.be.reverted; + expect(await smartWallet.receiveCount()).to.not.equal(2); + }); + it("should transfer many external and internal amounts to recipient", async () => { // NOTE: Make sure we have enough traders for our test :) expect(traders).to.have.length.above(5); diff --git a/test/e2e/contractOrdersWithGnosisSafe.test.ts b/test/e2e/contractOrdersWithGnosisSafe.test.ts index 9d5a6093..3d9ebc01 100644 --- a/test/e2e/contractOrdersWithGnosisSafe.test.ts +++ b/test/e2e/contractOrdersWithGnosisSafe.test.ts @@ -1,6 +1,3 @@ -import GnosisSafe from "@gnosis.pm/safe-contracts/build/artifacts/contracts/GnosisSafe.sol/GnosisSafe.json"; -import CompatibilityFallbackHandler from "@gnosis.pm/safe-contracts/build/artifacts/contracts/handler/CompatibilityFallbackHandler.sol/CompatibilityFallbackHandler.json"; -import GnosisSafeProxyFactory from "@gnosis.pm/safe-contracts/build/artifacts/contracts/proxies/GnosisSafeProxyFactory.sol/GnosisSafeProxyFactory.json"; import ERC20 from "@openzeppelin/contracts/build/contracts/ERC20PresetMinterPauser.json"; import { expect } from "chai"; import { BytesLike, Signer, Contract, Wallet } from "ethers"; @@ -15,6 +12,7 @@ import { domain, hashOrder, } from "../../src/ts"; +import { GnosisSafeManager } from "../safe"; import { deployTestContracts } from "./fixture"; @@ -23,57 +21,6 @@ interface SafeTransaction { data: BytesLike; } -class GnosisSafeManager { - constructor( - readonly deployer: Signer, - readonly masterCopy: Contract, - readonly signingFallback: Contract, - readonly proxyFactory: Contract, - ) {} - - static async init(deployer: Signer): Promise { - const masterCopy = await waffle.deployContract(deployer, GnosisSafe); - const proxyFactory = await waffle.deployContract( - deployer, - GnosisSafeProxyFactory, - ); - const signingFallback = await waffle.deployContract( - deployer, - CompatibilityFallbackHandler, - ); - return new GnosisSafeManager( - deployer, - masterCopy, - signingFallback, - proxyFactory, - ); - } - - async newSafe( - owners: string[], - threshold: number, - fallback = ethers.constants.AddressZero, - ): Promise { - const proxyAddress = await this.proxyFactory.callStatic.createProxy( - this.masterCopy.address, - "0x", - ); - await this.proxyFactory.createProxy(this.masterCopy.address, "0x"); - const safe = await ethers.getContractAt(GnosisSafe.abi, proxyAddress); - await safe.setup( - owners, - threshold, - ethers.constants.AddressZero, - "0x", - fallback, - ethers.constants.AddressZero, - 0, - ethers.constants.AddressZero, - ); - return safe; - } -} - async function gnosisSafeSign( message: BytesLike, signers: Signer[], diff --git a/test/safe.ts b/test/safe.ts new file mode 100644 index 00000000..05e018cb --- /dev/null +++ b/test/safe.ts @@ -0,0 +1,56 @@ +import GnosisSafe from "@gnosis.pm/safe-contracts/build/artifacts/contracts/GnosisSafe.sol/GnosisSafe.json"; +import CompatibilityFallbackHandler from "@gnosis.pm/safe-contracts/build/artifacts/contracts/handler/CompatibilityFallbackHandler.sol/CompatibilityFallbackHandler.json"; +import GnosisSafeProxyFactory from "@gnosis.pm/safe-contracts/build/artifacts/contracts/proxies/GnosisSafeProxyFactory.sol/GnosisSafeProxyFactory.json"; +import { Signer, Contract } from "ethers"; +import { ethers, waffle } from "hardhat"; + +export class GnosisSafeManager { + constructor( + readonly deployer: Signer, + readonly masterCopy: Contract, + readonly signingFallback: Contract, + readonly proxyFactory: Contract, + ) {} + + static async init(deployer: Signer): Promise { + const masterCopy = await waffle.deployContract(deployer, GnosisSafe); + const proxyFactory = await waffle.deployContract( + deployer, + GnosisSafeProxyFactory, + ); + const signingFallback = await waffle.deployContract( + deployer, + CompatibilityFallbackHandler, + ); + return new GnosisSafeManager( + deployer, + masterCopy, + signingFallback, + proxyFactory, + ); + } + + async newSafe( + owners: string[], + threshold: number, + fallback = ethers.constants.AddressZero, + ): Promise { + const proxyAddress = await this.proxyFactory.callStatic.createProxy( + this.masterCopy.address, + "0x", + ); + await this.proxyFactory.createProxy(this.masterCopy.address, "0x"); + const safe = await ethers.getContractAt(GnosisSafe.abi, proxyAddress); + await safe.setup( + owners, + threshold, + ethers.constants.AddressZero, + "0x", + fallback, + ethers.constants.AddressZero, + 0, + ethers.constants.AddressZero, + ); + return safe; + } +}