meta-txs: optimize volatile sender using calldata

contracts/apps/AppStorage.sol

@@ -15,11 +15,9 @@ contract AppStorage {
     * Hardcoded constants to save gas
     * bytes32 internal constant KERNEL_POSITION = keccak256("aragonOS.appStorage.kernel");
     * bytes32 internal constant APP_ID_POSITION = keccak256("aragonOS.appStorage.appId");
-    * bytes32 internal constant VOLATILE_SENDER_POSITION = keccak256("aragonOS.appStorage.volatile.sender");
     */
     bytes32 internal constant KERNEL_POSITION = 0x4172f0f7d2289153072b0a6ca36959e0cbe2efc3afe50fc81636caa96338137b;
     bytes32 internal constant APP_ID_POSITION = 0xd625496217aa6a3453eecb9c3489dc5a53e6c67b444329ea2b2cbc9ff547639b;
-    bytes32 internal constant VOLATILE_SENDER_POSITION = 0xd6486d5aa3dac4242db35dd7559408452252cf8050988dbc66956eaa315379ce;
 
     function kernel() public view returns (IKernel) {
         return IKernel(KERNEL_POSITION.getStorageAddress());
@@ -29,19 +27,11 @@ contract AppStorage {
         return APP_ID_POSITION.getStorageBytes32();
     }
 
-    function volatileStorageSender() public view returns (address) {
-        return VOLATILE_SENDER_POSITION.getStorageAddress();
-    }
-
     function setKernel(IKernel _kernel) internal {
         KERNEL_POSITION.setStorageAddress(address(_kernel));
     }
 
     function setAppId(bytes32 _appId) internal {
         APP_ID_POSITION.setStorageBytes32(_appId);
     }
-
-    function setVolatileStorageSender(address _sender) internal {
-        VOLATILE_SENDER_POSITION.setStorageAddress(_sender);
-    }
 }

contracts/common/MemoryHelpers.sol

@@ -0,0 +1,49 @@
+pragma solidity ^0.4.24;
+
+
+library MemoryHelpers {
+
+    function append(bytes memory self, address addr) internal pure returns (bytes memory) {
+        // alloc required encoded data size
+        uint256 dataSize = self.length;
+        uint256 appendedDataSize = dataSize + 32;
+        bytes memory appendedData = new bytes(appendedDataSize);
+
+        // copy data
+        uint256 inputPointer;
+        uint256 outputPointer;
+        assembly {
+            inputPointer := add(self, 0x20)
+            outputPointer := add(appendedData, 0x20)
+        }
+        memcpy(outputPointer, inputPointer, dataSize);
+
+        // append address
+        assembly {
+            let signerPointer := add(add(appendedData, 0x20), dataSize)
+            mstore(signerPointer, addr)
+        }
+
+        return appendedData;
+    }
+
+    // From https://github.com/Arachnid/solidity-stringutils/blob/master/src/strings.sol
+    function memcpy(uint256 dest, uint256 src, uint256 len) private pure {
+        // Copy word-length chunks while possible
+        for(; len >= 32; len -= 32) {
+            assembly {
+                mstore(dest, mload(src))
+            }
+            dest += 32;
+            src += 32;
+        }
+
+        // Copy remaining bytes
+        uint256 mask = 256 ** (32 - len) - 1;
+        assembly {
+            let srcpart := and(mload(src), not(mask))
+            let destpart := and(mload(dest), mask)
+            mstore(dest, or(destpart, srcpart))
+        }
+    }
+}

contracts/kernel/IKernel.sol

@@ -5,6 +5,7 @@
 pragma solidity ^0.4.24;
 
 import "../acl/IACL.sol";
+import "../relayer/IRelayer.sol";
 import "../common/IVaultRecoverable.sol";
 
 
@@ -16,6 +17,7 @@ interface IKernelEvents {
 // This should be an interface, but interfaces can't inherit yet :(
 contract IKernel is IKernelEvents, IVaultRecoverable {
     function acl() public view returns (IACL);
+    function relayer() public view returns (IRelayer);
     function hasPermission(address who, address where, bytes32 what, bytes how) public view returns (bool);
 
     function setApp(bytes32 namespace, bytes32 appId, address app) public;

contracts/kernel/Kernel.sol

@@ -5,6 +5,7 @@ import "./KernelConstants.sol";
 import "./KernelStorage.sol";
 import "../acl/IACL.sol";
 import "../acl/ACLSyntaxSugar.sol";
+import "../relayer/IRelayer.sol";
 import "../common/ConversionHelpers.sol";
 import "../common/IsContract.sol";
 import "../common/Petrifiable.sol";
@@ -169,6 +170,7 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant
     function APP_ADDR_NAMESPACE() external pure returns (bytes32) { return KERNEL_APP_ADDR_NAMESPACE; }
     function KERNEL_APP_ID() external pure returns (bytes32) { return KERNEL_CORE_APP_ID; }
     function DEFAULT_ACL_APP_ID() external pure returns (bytes32) { return KERNEL_DEFAULT_ACL_APP_ID; }
+    function DEFAULT_RELAYER_APP_ID() external pure returns (bytes32) { return KERNEL_DEFAULT_RELAYER_APP_ID; }
     /* solium-enable function-order, mixedcase */
 
     /**
@@ -197,6 +199,14 @@ contract Kernel is IKernel, KernelStorage, KernelAppIds, KernelNamespaceConstant
         return IACL(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_ACL_APP_ID));
     }
 
+    /**
+    * @dev Get the installed Relayer app
+    * @return Relayer app
+    */
+    function relayer() public view returns (IRelayer) {
+        return IRelayer(getApp(KERNEL_APP_ADDR_NAMESPACE, KERNEL_DEFAULT_RELAYER_APP_ID));
+    }
+
     /**
     * @dev Function called by apps to check ACL on kernel or to check permission status
     * @param _who Sender of the original call

contracts/kernel/KernelConstants.sol

@@ -10,10 +10,12 @@ contract KernelAppIds {
     bytes32 internal constant KERNEL_CORE_APP_ID = apmNamehash("kernel");
     bytes32 internal constant KERNEL_DEFAULT_ACL_APP_ID = apmNamehash("acl");
     bytes32 internal constant KERNEL_DEFAULT_VAULT_APP_ID = apmNamehash("vault");
+    bytes32 internal constant KERNEL_DEFAULT_VAULT_APP_ID = apmNamehash("relayer");
     */
     bytes32 internal constant KERNEL_CORE_APP_ID = 0x3b4bf6bf3ad5000ecf0f989d5befde585c6860fea3e574a4fab4c49d1c177d9c;
     bytes32 internal constant KERNEL_DEFAULT_ACL_APP_ID = 0xe3262375f45a6e2026b7e7b18c2b807434f2508fe1a2a3dfb493c7df8f4aad6a;
     bytes32 internal constant KERNEL_DEFAULT_VAULT_APP_ID = 0x7e852e0fcfce6551c13800f1e7476f982525c2b5277ba14b24339c68416336d1;
+    bytes32 internal constant KERNEL_DEFAULT_RELAYER_APP_ID = 0x7641595d1a2007abf0fe95c31d0b7a822954acbf6fb0cbe3bd1161d9dec9e1d3;
 }
 
 

contracts/relayer/IRelayer.sol

@@ -0,0 +1,6 @@
+pragma solidity ^0.4.24;
+
+
+contract IRelayer {
+    function relay(address from, address to, uint256 nonce, bytes calldata, bytes signature) external;
+}

contracts/relayer/RelayedAragonApp.sol

@@ -1,34 +1,25 @@
 pragma solidity ^0.4.24;
 
-
+import "./IRelayer.sol";
 import "../apps/AragonApp.sol";
 
 
-interface IRelayedAragonApp {
-    function exec(address from, bytes calldata) external;
-}
+contract RelayedAragonApp is AragonApp {
 
-contract RelayedAragonApp is IRelayedAragonApp, AragonApp {
-    bytes32 public constant RELAYER_ROLE = keccak256("RELAYER_ROLE");
+    function sender() internal view returns (address) {
+        address relayer = address(_relayer());
+        if (msg.sender != relayer) return msg.sender;
 
-    function exec(address from, bytes calldata) external auth(RELAYER_ROLE) {
-        setVolatileStorageSender(from);
-        bool success = address(this).call(calldata);
-        if (!success) revertForwardingError();
-        setVolatileStorageSender(address(0));
+        address signer = _decodeSigner();
+        return signer != address(0) ? signer : relayer;
     }
 
-    function sender() internal view returns (address) {
-        if (msg.sender != address(this)) return msg.sender;
-        address volatileSender = volatileStorageSender();
-        return volatileSender != address(0) ? volatileSender : address(this);
+    function _decodeSigner() internal returns (address signer) {
+        bytes memory calldata = msg.data;
+        assembly { signer := mload(add(calldata, calldatasize)) }
     }
 
-    function revertForwardingError() private {
-        assembly {
-            let ptr := mload(0x40)
-            returndatacopy(ptr, 0, returndatasize)
-            revert(ptr, returndatasize)
-        }
+    function _relayer() internal returns (IRelayer) {
+        return kernel().relayer();
     }
 }

contracts/relayer/Relayer.sol

@@ -1,13 +1,16 @@
 pragma solidity ^0.4.24;
 
+import "./IRelayer.sol";
 import "./RelayedAragonApp.sol";
 import "../lib/sig/ECDSA.sol";
 import "../apps/AragonApp.sol";
+import "../common/MemoryHelpers.sol";
 import "../common/DepositableStorage.sol";
 
 
-contract Relayer is AragonApp, DepositableStorage {
+contract Relayer is IRelayer, AragonApp, DepositableStorage {
     using ECDSA for bytes32;
+    using MemoryHelpers for bytes;
 
     bytes32 public constant ALLOW_OFF_CHAIN_SERVICE_ROLE = keccak256("ALLOW_OFF_CHAIN_SERVICE_ROLE");
     bytes32 public constant DISALLOW_OFF_CHAIN_SERVICE_ROLE = keccak256("DISALLOW_OFF_CHAIN_SERVICE_ROLE");
@@ -21,7 +24,7 @@ contract Relayer is AragonApp, DepositableStorage {
 
     event ServiceAllowed(address indexed service);
     event ServiceDisallowed(address indexed service);
-    event TransactionRelayed(address indexed from, address indexed to, uint256 nonce, bytes calldata);
+    event TransactionRelayed(address from, address to, uint256 nonce, bytes calldata);
 
     mapping (address => bool) internal allowedServices;
     mapping (address => uint256) internal lastUsedNonce;
@@ -48,8 +51,9 @@ contract Relayer is AragonApp, DepositableStorage {
         assertValidTransaction(from, nonce, calldata, signature);
 
         lastUsedNonce[from] = nonce;
-        IRelayedAragonApp(to).exec(from, calldata);
+        relayCall(from, to, calldata);
         emit TransactionRelayed(from, to, nonce, calldata);
+        forwardReturnedData();
     }
 
     function allowService(address service) external authP(ALLOW_OFF_CHAIN_SERVICE_ROLE, arr(service)) {
@@ -84,4 +88,24 @@ contract Relayer is AragonApp, DepositableStorage {
     function messageHash(bytes calldata, uint256 nonce) internal pure returns (bytes32) {
         return keccak256(abi.encodePacked(keccak256(calldata), nonce));
     }
+
+    function relayCall(address from, address to, bytes calldata) private {
+        bytes memory encodedSignerCalldata = calldata.append(from);
+        assembly {
+            let success := call(gas, to, 0, add(encodedSignerCalldata, 0x20), mload(encodedSignerCalldata), 0, 0)
+            switch success case 0 {
+                let ptr := mload(0x40)
+                returndatacopy(ptr, 0, returndatasize)
+                revert(ptr, returndatasize)
+            }
+        }
+    }
+
+    function forwardReturnedData() private {
+        assembly {
+            let ptr := mload(0x40)
+            returndatacopy(ptr, 0, returndatasize)
+            return(ptr, returndatasize)
+        }
+    }
 }

contracts/test/mocks/common/KeccakConstants.sol

@@ -22,6 +22,7 @@ contract KeccakConstants {
     bytes32 public constant KERNEL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("kernel")));
     bytes32 public constant DEFAULT_ACL_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("acl")));
     bytes32 public constant DEFAULT_VAULT_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("vault")));
+    bytes32 public constant DEFAULT_RELAYER_APP_ID = keccak256(abi.encodePacked(APM_NODE, keccak256("relayer")));
 
     // ACL
     bytes32 public constant CREATE_PERMISSIONS_ROLE = keccak256(abi.encodePacked("CREATE_PERMISSIONS_ROLE"));

contracts/test/mocks/kernel/KernelConstantsMock.sol

@@ -12,4 +12,5 @@ contract KernelConstantsMock is Kernel {
     function getKernelAppId() external pure returns (bytes32) { return KERNEL_CORE_APP_ID; }
     function getDefaultACLAppId() external pure returns (bytes32) { return KERNEL_DEFAULT_ACL_APP_ID; }
     function getDefaultVaultAppId() external pure returns (bytes32) { return KERNEL_DEFAULT_VAULT_APP_ID; }
+    function getDefaultRelayerAppId() external pure returns (bytes32) { return KERNEL_DEFAULT_RELAYER_APP_ID; }
 }

test/contracts/common/keccak_constants.js

@@ -27,6 +27,7 @@ contract('Constants', () => {
     assert.equal(await kernelConstants.getKernelAppId(), await keccakConstants.KERNEL_APP_ID(), "kernel app id doesn't match")
     assert.equal(await kernelConstants.getDefaultACLAppId(), await keccakConstants.DEFAULT_ACL_APP_ID(), "default ACL id doesn't match")
     assert.equal(await kernelConstants.getDefaultVaultAppId(), await keccakConstants.DEFAULT_VAULT_APP_ID(), "default vault id doesn't match")
+    assert.equal(await kernelConstants.getDefaultRelayerAppId(), await keccakConstants.DEFAULT_RELAYER_APP_ID(), "default relayer id doesn't match")
     assert.equal(await kernelConstants.getKernelCoreNamespace(), await keccakConstants.KERNEL_CORE_NAMESPACE(), "core namespace doesn't match")
     assert.equal(await kernelConstants.getKernelAppBasesNamespace(), await keccakConstants.KERNEL_APP_BASES_NAMESPACE(), "base namespace doesn't match")
     assert.equal(await kernelConstants.getKernelAppAddrNamespace(), await keccakConstants.KERNEL_APP_ADDR_NAMESPACE(), "app namespace doesn't match")

test/contracts/relayer/relayer.js

@@ -1,4 +1,5 @@
 const { sha3, soliditySha3 } = require('web3-utils')
+const { assertRevert } = require('../../helpers/assertThrow')
 const { getEventArgument, getNewProxyAddress } = require('../../helpers/events')
 
 const ACL = artifacts.require('ACL')
@@ -7,10 +8,10 @@ const Relayer = artifacts.require('Relayer')
 const DAOFactory = artifacts.require('DAOFactory')
 const SampleApp = artifacts.require('RelayedAppMock')
 
-contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService]) => {
-  let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 1
+contract('RelayedApp', ([_, root, member, anyone, vault, offChainRelayerService]) => {
+  let daoFactory, dao, acl, app, relayer, relayedTx, nonce = 0
   let kernelBase, aclBase, sampleAppBase, relayerBase
-  let WRITING_ROLE, APP_MANAGER_ROLE, RELAYER_ROLE, ALLOW_OFF_CHAIN_SERVICE_ROLE
+  let WRITING_ROLE, APP_MANAGER_ROLE, ALLOW_OFF_CHAIN_SERVICE_ROLE, RELAYER_APP_ID
 
   before('deploy base implementations', async () => {
     aclBase = await ACL.new()
@@ -20,9 +21,9 @@ contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService]
     daoFactory = await DAOFactory.new(kernelBase.address, aclBase.address, '0x0')
   })
 
-  before('load roles', async () => {
+  before('load constants', async () => {
+    RELAYER_APP_ID = await kernelBase.DEFAULT_RELAYER_APP_ID()
     WRITING_ROLE = await sampleAppBase.WRITING_ROLE()
-    RELAYER_ROLE = await sampleAppBase.RELAYER_ROLE()
     APP_MANAGER_ROLE = await kernelBase.APP_MANAGER_ROLE()
     ALLOW_OFF_CHAIN_SERVICE_ROLE = await relayerBase.ALLOW_OFF_CHAIN_SERVICE_ROLE()
   })
@@ -36,7 +37,7 @@ contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService]
   })
 
   before('create relayer instance', async () => {
-    const receipt = await dao.newAppInstance('0x11111', relayerBase.address, '0x', false, { from: root })
+    const receipt = await dao.newAppInstance(RELAYER_APP_ID, relayerBase.address, '0x', true, { from: root })
     relayer = Relayer.at(getNewProxyAddress(receipt))
     await relayer.initialize()
 
@@ -51,32 +52,48 @@ contract('VolatileRelayedApp', ([_, root, sender, vault, offChainRelayerService]
     app = SampleApp.at(getNewProxyAddress(receipt))
     await app.initialize()
 
-    await acl.createPermission(sender, app.address, WRITING_ROLE, root, { from: root })
-    await acl.createPermission(relayer.address, app.address, RELAYER_ROLE, root, { from: root })
+    await acl.createPermission(member, app.address, WRITING_ROLE, root, { from: root })
   })
 
-  beforeEach('relay transaction', async () => {
-    const calldata = app.contract.write.getData(10)
-    const messageHash = soliditySha3(sha3(calldata), nonce)
-    const signature = web3.eth.sign(sender, messageHash)
+  beforeEach('increment nonce', () => nonce++)
 
-    relayedTx = await relayer.relay(sender, app.address, nonce, calldata, signature, { from: offChainRelayerService })
-    nonce++
-  })
+  context('when the sender is authorized', () => {
+    const sender = member
+
+    beforeEach('relay transaction', async () => {
+      const calldata = app.contract.write.getData(10)
+      const messageHash = soliditySha3(sha3(calldata), nonce)
+      const signature = web3.eth.sign(sender, messageHash)
+
+      relayedTx = await relayer.relay(sender, app.address, nonce, calldata, signature, { from: offChainRelayerService })
+    })
+
+    it('relays transactions to app', async () => {
+      assert.equal((await app.read()).toString(), 10, 'app value does not match')
+    })
+
+    it('overloads a transaction with ~34k of gas', async () => {
+      const { receipt: { cumulativeGasUsed: relayedGasUsed } } = relayedTx
+      const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: sender })
+
+      const gasOverload = relayedGasUsed - nonRelayerGasUsed
+      console.log('relayedGasUsed:', relayedGasUsed)
+      console.log('nonRelayerGasUsed:', nonRelayerGasUsed)
+      console.log('gasOverload:', gasOverload)
 
-  it('relays transactions to app', async () => {
-    assert.equal((await app.read()).toString(), 10, 'app value does not match')
+      assert.isBelow(gasOverload, 34000, 'relayed txs gas overload is higher than 34k')
+    })
   })
 
-  it('overloads a transaction with ~78k of gas', async () => {
-    const { receipt: { cumulativeGasUsed: relayedGasUsed } } = relayedTx
-    const { receipt: { cumulativeGasUsed: nonRelayerGasUsed } } = await app.write(10, { from: sender })
+  context('when the sender is not authorized', () => {
+    const sender = anyone
 
-    const gasOverload = relayedGasUsed - nonRelayerGasUsed
-    console.log('relayedGasUsed:', relayedGasUsed)
-    console.log('nonRelayerGasUsed:', nonRelayerGasUsed)
-    console.log('gasOverload:', gasOverload)
+    it('reverts', async () => {
+      const calldata = app.contract.write.getData(10)
+      const messageHash = soliditySha3(sha3(calldata), nonce)
+      const signature = web3.eth.sign(sender, messageHash)
 
-    assert.isBelow(gasOverload, 78000, 'relayed txs gas overload is higher than 78k')
+      await assertRevert(relayer.relay(sender, app.address, nonce, calldata, signature, { from: offChainRelayerService }), 'APP_AUTH_FAIL')
+    })
   })
 })