Closes #246: Send along msg.sender to fallback handler (#270)

.solcover.js

@@ -1,6 +1,8 @@
 module.exports = {
     skipFiles: [
         'test/Token.sol',
+        'test/ERC20Token.sol',
+        'test/TestHandler.sol',
         'test/ERC1155Token.sol',
     ],
     mocha: {

contracts/base/FallbackManager.sol

@@ -47,7 +47,11 @@ contract FallbackManager is SelfAuthorized {
             // solium-disable-next-line security/no-inline-assembly
             assembly {
                 calldatacopy(0, 0, calldatasize())
-                let success := call(gas, handler, 0, 0, calldatasize(), 0, 0)
+                // The msg.sender address is shifted to the left by 12 bytes to remove the padding
+                // Then the address without padding is stored right after the calldata
+                mstore(calldatasize(), shl(96, caller()))
+                // Add 20 bytes for the address appended add the end
+                let success := call(gas, handler, 0, 0, add(calldatasize(), 20), 0, 0)
                 returndatacopy(0, 0, returndatasize())
                 if eq(success, 0) { revert(0, returndatasize()) }
                 return(0, returndatasize())

contracts/handler/HandlerContext.sol

@@ -0,0 +1,19 @@
+pragma solidity >=0.5.0 <0.6.0;
+
+/// @title Handler Context - allows to extract calling context
+/// @author Richard Meissner - <richard@gnosis.pm>
+/// @notice based on https://github.com/OpenZeppelin/openzeppelin-contracts/blob/f8cc8b844a9f92f63dc55aa581f7d643a1bc5ac1/contracts/metatx/ERC2771Context.sol
+contract HandlerContext {
+
+    // This function does not rely on a trusted forwarder. Use the returned value only to check information against the calling manager.
+    function _msgSender() internal pure returns (address sender) {
+        // The assembly code is more direct than the Solidity version using `abi.decode`.
+        assembly { sender := shr(96, calldataload(sub(calldatasize(), 20))) }
+    }
+
+    // Function do differentiate more clearly between msg.sender and the calling manager
+    function _manager() internal view returns (address) {
+        return msg.sender;
+    }
+
+}

contracts/test/TestHandler.sol

@@ -0,0 +1,7 @@
+pragma solidity >=0.5.0 <0.6.0;
+import "../handler/HandlerContext.sol";
+contract TestHandler is HandlerContext {
+	function dudududu() external returns (address sender, address manager) {
+		return (_msgSender(), _manager());
+	}
+}

test/core/GnosisSafe.FallbackManager.spec.ts

@@ -3,21 +3,35 @@ import hre, { deployments, waffle } from "hardhat";
 import { BigNumber } from "ethers";
 import "@nomiclabs/hardhat-ethers";
 import { AddressZero } from "@ethersproject/constants";
-import { defaultCallbackHandlerContract, defaultCallbackHandlerDeployment, getSafeTemplate } from "../utils/setup";
+import { defaultCallbackHandlerContract, defaultCallbackHandlerDeployment, deployContract, getMock, getSafeTemplate } from "../utils/setup";
 import { executeContractCallWithSigners } from "../utils/execution";
 
 describe("FallbackManager", async () => {
 
     const setupWithTemplate = deployments.createFixture(async ({ deployments }) => {
         await deployments.fixture();
-        return await getSafeTemplate()
+        const source = `
+        contract Mirror {
+            function lookAtMe() public returns (bytes memory) {
+                return msg.data;
+            }
+
+            function nowLookAtYou(address you, string memory howYouLikeThat) public returns (bytes memory) {
+                return msg.data;
+            }
+        }`
+        const mirror = await deployContract(user1, source);
+        return {
+            safe: await getSafeTemplate(),
+            mirror
+        }
     })
 
     const [user1, user2] = waffle.provider.getWallets();
 
     describe("setFallbackManager", async () => {
         it('is correctly set on deployment', async () => {
-            const safe = await setupWithTemplate()
+            const { safe } = await setupWithTemplate()
             const handler = await defaultCallbackHandlerDeployment()
 
             // Check fallback handler
@@ -36,7 +50,7 @@ describe("FallbackManager", async () => {
         })
 
         it('is correctly set', async () => {
-            const safe = await setupWithTemplate()
+            const { safe } = await setupWithTemplate()
             const handler = await defaultCallbackHandlerDeployment()
 
             // Setup Safe
@@ -57,7 +71,7 @@ describe("FallbackManager", async () => {
         })
 
         it('is called when set', async () => {
-            const safe = await setupWithTemplate()
+            const { safe } = await setupWithTemplate()
             const handler = await defaultCallbackHandlerDeployment()
             const safeHandler = (await defaultCallbackHandlerContract()).attach(safe.address)
             // Check that Safe is NOT setup
@@ -76,5 +90,49 @@ describe("FallbackManager", async () => {
                 await safeHandler.callStatic.onERC1155Received(AddressZero, AddressZero, 0, 0, "0x")
             ).to.be.eq("0xf23a6e61")
         })
+
+        it('sends along msg.sender on simple call', async () => {
+            const { safe, mirror } = await setupWithTemplate()
+            // Setup Safe
+            await safe.setup([user1.address, user2.address], 1, AddressZero, "0x", mirror.address, AddressZero, 0, AddressZero)
+
+            const tx = {
+                to: safe.address,
+                data: mirror.interface.encodeFunctionData("lookAtMe")
+            }
+            // Check that mock works as handler
+            const response = await user1.call(tx)
+            expect(response).to.be.eq(
+                "0x" +
+                "0000000000000000000000000000000000000000000000000000000000000020" +
+                "0000000000000000000000000000000000000000000000000000000000000018" +
+                "7f8dc53c" + user1.address.slice(2).toLowerCase() + "0000000000000000"
+            )
+        })
+
+        it('sends along msg.sender on more complex call', async () => {
+            const { safe, mirror } = await setupWithTemplate()
+            // Setup Safe
+            await safe.setup([user1.address, user2.address], 1, AddressZero, "0x", mirror.address, AddressZero, 0, AddressZero)
+
+            const tx = {
+                to: safe.address,
+                data: mirror.interface.encodeFunctionData("nowLookAtYou", [user2.address, "pink<>black"])
+            }
+            // Check that mock works as handler
+            const response = await user1.call(tx)
+            expect(response).to.be.eq(
+                "0x" +
+                "0000000000000000000000000000000000000000000000000000000000000020" +
+                "0000000000000000000000000000000000000000000000000000000000000098" +
+                // Function call
+                "b2a88d99" + 
+                "000000000000000000000000" + user2.address.slice(2).toLowerCase() +
+                "0000000000000000000000000000000000000000000000000000000000000040" + 
+                "000000000000000000000000000000000000000000000000000000000000000b" +
+                "70696e6b3c3e626c61636b000000000000000000000000000000000000000000" +
+                user1.address.slice(2).toLowerCase() + "0000000000000000"
+            )
+        })
     })
 })

test/handlers/HandlerContext.spec.ts

@@ -0,0 +1,45 @@
+import { expect } from "chai";
+import hre, { deployments, waffle } from "hardhat";
+import "@nomiclabs/hardhat-ethers";
+import { AddressZero } from "@ethersproject/constants";
+import { getSafeTemplate } from "../utils/setup";
+
+describe("HandlerContext", async () => {
+
+    const [user1, user2] = waffle.provider.getWallets();
+
+    const setup = deployments.createFixture(async ({ deployments }) => {
+        await deployments.fixture();
+        const TestHandler = await hre.ethers.getContractFactory("TestHandler");
+        const handler = await TestHandler.deploy();
+        return {
+            safe: await getSafeTemplate(),
+            handler
+        }
+    })
+
+    it('parses information correctly', async () => {
+        const { handler } = await setup();
+        const response = await user1.call({
+            to: handler.address,
+            data: handler.interface.encodeFunctionData("dudududu") + user2.address.slice(2)
+        })
+        expect(
+            handler.interface.decodeFunctionResult("dudududu", response)
+        ).to.be.deep.eq([user2.address, user1.address])
+    })
+
+    it('works with the Safe', async () => {
+        const { safe, handler } = await setup();
+        await safe.setup([user1.address, user2.address], 1, AddressZero, "0x", handler.address, AddressZero, 0, AddressZero)
+
+        const response = await user1.call({
+            to: safe.address,
+            data: handler.interface.encodeFunctionData("dudududu")
+        })
+
+        expect(
+            handler.interface.decodeFunctionResult("dudududu", response)
+        ).to.be.deep.eq([user1.address, safe.address])
+    })
+})