From 583b96d8c6bc8870e4e49666ba14901bd32d1afb Mon Sep 17 00:00:00 2001
From: Richard Meissner <msc.meissner@gmail.com>
Date: Wed, 24 Mar 2021 11:32:56 +0100
Subject: [PATCH] Closes #227: Allow multisend when msg.value > 0; Add call
only multisend
---
contracts/libraries/MultiSend.sol | 3 +
contracts/libraries/MultiSendCallOnly.sol | 56 ++++++++
src/deploy/deploy_libraries.ts | 7 +
test/libraries/MultiSend.spec.ts | 17 +++
test/libraries/MultiSendCallOnly.spec.ts | 162 ++++++++++++++++++++++
test/utils/setup.ts | 6 +
6 files changed, 251 insertions(+)
create mode 100644 contracts/libraries/MultiSendCallOnly.sol
create mode 100644 test/libraries/MultiSendCallOnly.spec.ts
diff --git a/contracts/libraries/MultiSend.sol b/contracts/libraries/MultiSend.sol
index 8ccf73359..c47ac351c 100644
--- a/contracts/libraries/MultiSend.sol
+++ b/contracts/libraries/MultiSend.sol
@@ -25,8 +25,11 @@ contract MultiSend {
/// data length as a uint256 (=> 32 bytes),
/// data as bytes.
/// see abi.encodePacked for more information on packed encoding
+ /// @notice This method is payable as delegatecalls keep the msg.value from the previous call
+ /// If the calling method (e.g. execTransaction) received ETH this would revert otherwise
function multiSend(bytes memory transactions)
public
+ payable
{
require(guard != GUARD_VALUE, "MultiSend should only be called via delegatecall");
// solium-disable-next-line security/no-inline-assembly
diff --git a/contracts/libraries/MultiSendCallOnly.sol b/contracts/libraries/MultiSendCallOnly.sol
new file mode 100644
index 000000000..b11b32866
--- /dev/null
+++ b/contracts/libraries/MultiSendCallOnly.sol
@@ -0,0 +1,56 @@
+// SPDX-License-Identifier: LGPL-3.0-only
+pragma solidity >=0.7.0 <0.9.0;
+
+
+/// @title Multi Send Call Only - Allows to batch multiple transactions into one, but only calls
+/// @author Stefan George - <stefan@gnosis.io>
+/// @author Richard Meissner - <richard@gnosis.io>
+/// @notice The guard logic is not required here as this contract doesn't support nested delegate calls
+contract MultiSendCallOnly {
+
+ /// @dev Sends multiple transactions and reverts all if one fails.
+ /// @param transactions Encoded transactions. Each transaction is encoded as a packed bytes of
+ /// operation has to be uint8(0) in this version (=> 1 byte),
+ /// to as a address (=> 20 bytes),
+ /// value as a uint256 (=> 32 bytes),
+ /// data length as a uint256 (=> 32 bytes),
+ /// data as bytes.
+ /// see abi.encodePacked for more information on packed encoding
+ /// @notice The code is for most part the same as the normal MultiSend (to keep compatibility),
+ /// but reverts if a transaction tries to use a delegatecall.
+ /// @notice This method is payable as delegatecalls keep the msg.value from the previous call
+ /// If the calling method (e.g. execTransaction) received ETH this would revert otherwise
+ function multiSend(bytes memory transactions)
+ public
+ payable
+ {
+ // solium-disable-next-line security/no-inline-assembly
+ assembly {
+ let length := mload(transactions)
+ let i := 0x20
+ for { } lt(i, length) { } {
+ // First byte of the data is the operation.
+ // We shift by 248 bits (256 - 8 [operation byte]) it right since mload will always load 32 bytes (a word).
+ // This will also zero out unused data.
+ let operation := shr(0xf8, mload(add(transactions, i)))
+ // We offset the load address by 1 byte (operation byte)
+ // We shift it right by 96 bits (256 - 160 [20 address bytes]) to right-align the data and zero out unused data.
+ let to := shr(0x60, mload(add(transactions, add(i, 0x01))))
+ // We offset the load address by 21 byte (operation byte + 20 address bytes)
+ let value := mload(add(transactions, add(i, 0x15)))
+ // We offset the load address by 53 byte (operation byte + 20 address bytes + 32 value bytes)
+ let dataLength := mload(add(transactions, add(i, 0x35)))
+ // We offset the load address by 85 byte (operation byte + 20 address bytes + 32 value bytes + 32 data length bytes)
+ let data := add(transactions, add(i, 0x55))
+ let success := 0
+ switch operation
+ case 0 { success := call(gas(), to, value, data, dataLength, 0, 0) }
+ // This version does not allow delegatecalls
+ case 1 { revert(0, 0) }
+ if eq(success, 0) { revert(0, 0) }
+ // Next entry starts at 85 byte + data length
+ i := add(i, add(0x55, dataLength))
+ }
+ }
+ }
+}
diff --git a/src/deploy/deploy_libraries.ts b/src/deploy/deploy_libraries.ts
index 13b0a7b74..00523f81d 100644
--- a/src/deploy/deploy_libraries.ts
+++ b/src/deploy/deploy_libraries.ts
@@ -21,6 +21,13 @@ const deploy: DeployFunction = async function (
log: true,
deterministicDeployment: true,
});
+
+ await deploy("MultiSendCallOnly", {
+ from: deployer,
+ args: [],
+ log: true,
+ deterministicDeployment: true,
+ });
};
deploy.tags = ['libraries']
diff --git a/test/libraries/MultiSend.spec.ts b/test/libraries/MultiSend.spec.ts
index 36317b0b3..6d36068bf 100644
--- a/test/libraries/MultiSend.spec.ts
+++ b/test/libraries/MultiSend.spec.ts
@@ -108,6 +108,23 @@ describe("MultiSend", async () => {
await expect(await hre.ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance)
})
+ it('can be used when ETH is sent with execution', async () => {
+ const { safe, multiSend, storageSetter } = await setupTests()
+
+ const txs: MetaTransaction[] = [
+ buildContractCall(storageSetter, "setStorage", ["0xbaddad"], 0)
+ ]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0"))
+
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ], { value: parseEther("1") })
+ ).to.emit(safe, "ExecutionSuccess")
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+ })
+
it('can execute contract calls', async () => {
const { safe, multiSend, storageSetter } = await setupTests()
diff --git a/test/libraries/MultiSendCallOnly.spec.ts b/test/libraries/MultiSendCallOnly.spec.ts
new file mode 100644
index 000000000..6a09866b8
--- /dev/null
+++ b/test/libraries/MultiSendCallOnly.spec.ts
@@ -0,0 +1,162 @@
+import { expect } from "chai";
+import hre, { deployments, waffle } from "hardhat";
+import "@nomiclabs/hardhat-ethers";
+import { deployContract, getMock, getMultiSendCallOnly, getSafeWithOwners } from "../utils/setup";
+import { buildContractCall, buildSafeTransaction, executeTx, MetaTransaction, safeApproveHash } from "../utils/execution";
+import { buildMultiSendSafeTx } from "../utils/multisend";
+import { parseEther } from "@ethersproject/units";
+
+describe("MultiSendCallOnly", async () => {
+
+ const [user1, user2] = waffle.provider.getWallets();
+
+ const setupTests = deployments.createFixture(async ({ deployments }) => {
+ await deployments.fixture();
+ const setterSource = `
+ contract StorageSetter {
+ function setStorage(bytes3 data) public {
+ bytes32 slot = 0x4242424242424242424242424242424242424242424242424242424242424242;
+ // solium-disable-next-line security/no-inline-assembly
+ assembly {
+ sstore(slot, data)
+ }
+ }
+ }`
+ const storageSetter = await deployContract(user1, setterSource);
+ return {
+ safe: await getSafeWithOwners([user1.address]),
+ multiSend: await getMultiSendCallOnly(),
+ mock: await getMock(),
+ storageSetter
+ }
+ })
+
+ describe("multiSend", async () => {
+
+ it('Should fail when using invalid operation', async () => {
+ const { safe, multiSend } = await setupTests()
+
+ const txs = [buildSafeTransaction({to: user2.address, operation: 2, nonce: 0})]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.revertedWith("GS013")
+ })
+
+ it('Should fail when using delegatecall operation', async () => {
+ const { safe, multiSend } = await setupTests()
+
+ const txs = [buildSafeTransaction({to: user2.address, operation: 1, nonce: 0})]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.revertedWith("GS013")
+ })
+
+ it('Can execute empty multisend', async () => {
+ const { safe, multiSend } = await setupTests()
+
+ const txs: MetaTransaction[] = []
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.emit(safe, "ExecutionSuccess")
+ })
+
+ it('Can execute single ether transfer', async () => {
+ const { safe, multiSend } = await setupTests()
+ await user1.sendTransaction({to: safe.address, value: parseEther("1")})
+ const userBalance = await hre.ethers.provider.getBalance(user2.address)
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+
+ const txs: MetaTransaction[] = [buildSafeTransaction({to: user2.address, value: parseEther("1"), nonce: 0})]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.emit(safe, "ExecutionSuccess")
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0"))
+ await expect(await hre.ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance.add(parseEther("1")))
+ })
+
+ it('reverts all tx if any fails', async () => {
+ const { safe, multiSend } = await setupTests()
+ await user1.sendTransaction({to: safe.address, value: parseEther("1")})
+ const userBalance = await hre.ethers.provider.getBalance(user2.address)
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+
+ const txs: MetaTransaction[] = [
+ 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(), { safeTxGas: 1 })
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.emit(safe, "ExecutionFailure")
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+ await expect(await hre.ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance)
+ })
+
+ it('can be used when ETH is sent with execution', async () => {
+ const { safe, multiSend, storageSetter } = await setupTests()
+
+ const txs: MetaTransaction[] = [
+ buildContractCall(storageSetter, "setStorage", ["0xbaddad"], 0)
+ ]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0"))
+
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ], { value: parseEther("1") })
+ ).to.emit(safe, "ExecutionSuccess")
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+ })
+
+ it('can execute contract calls', async () => {
+ const { safe, multiSend, storageSetter } = await setupTests()
+
+ const txs: MetaTransaction[] = [
+ buildContractCall(storageSetter, "setStorage", ["0xbaddad"], 0)
+ ]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.emit(safe, "ExecutionSuccess")
+
+ await expect(
+ await hre.ethers.provider.getStorageAt(safe.address, "0x4242424242424242424242424242424242424242424242424242424242424242")
+ ).to.be.eq("0x" + "".padEnd(64, "0"))
+ await expect(
+ await hre.ethers.provider.getStorageAt(storageSetter.address, "0x4242424242424242424242424242424242424242424242424242424242424242")
+ ).to.be.eq("0x" + "baddad".padEnd(64, "0"))
+ })
+
+ it('can execute combinations', async () => {
+ const { safe, multiSend, storageSetter } = await setupTests()
+ await user1.sendTransaction({to: safe.address, value: parseEther("1")})
+ const userBalance = await hre.ethers.provider.getBalance(user2.address)
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("1"))
+
+ const txs: MetaTransaction[] = [
+ buildSafeTransaction({to: user2.address, value: parseEther("1"), nonce: 0}),
+ buildContractCall(storageSetter, "setStorage", ["0xbaddad"], 0)
+ ]
+ const safeTx = buildMultiSendSafeTx(multiSend, txs, await safe.nonce())
+ await expect(
+ executeTx(safe, safeTx, [ await safeApproveHash(user1, safe, safeTx, true) ])
+ ).to.emit(safe, "ExecutionSuccess")
+
+ await expect(await hre.ethers.provider.getBalance(safe.address)).to.be.deep.eq(parseEther("0"))
+ await expect(await hre.ethers.provider.getBalance(user2.address)).to.be.deep.eq(userBalance.add(parseEther("1")))
+ await expect(
+ await hre.ethers.provider.getStorageAt(safe.address, "0x4242424242424242424242424242424242424242424242424242424242424242")
+ ).to.be.eq("0x" + "".padEnd(64, "0"))
+ await expect(
+ await hre.ethers.provider.getStorageAt(storageSetter.address, "0x4242424242424242424242424242424242424242424242424242424242424242")
+ ).to.be.eq("0x" + "baddad".padEnd(64, "0"))
+ })
+ })
+})
\ No newline at end of file
diff --git a/test/utils/setup.ts b/test/utils/setup.ts
index 62c6c0ac9..15267460c 100644
--- a/test/utils/setup.ts
+++ b/test/utils/setup.ts
@@ -44,6 +44,12 @@ export const getMultiSend = async () => {
return MultiSend.attach(MultiSendDeployment.address);
}
+export const getMultiSendCallOnly = async () => {
+ const MultiSendDeployment = await deployments.get("MultiSendCallOnly");
+ const MultiSend = await hre.ethers.getContractFactory("MultiSendCallOnly");
+ return MultiSend.attach(MultiSendDeployment.address);
+}
+
export const getCreateCall = async () => {
const CreateCallDeployment = await deployments.get("CreateCall");
const CreateCall = await hre.ethers.getContractFactory("CreateCall");