Merge pull request #25 from gnosis/executeTransactionReturnData

fix: adds `executeTransactionReturnData()`

contracts/Delay.sol

@@ -41,8 +41,13 @@ contract Delay is Modifier {
         uint256 _cooldown,
         uint256 _expiration
     ) {
-        bytes memory initParams =
-            abi.encode(_owner, _avatar, _target, _cooldown, _expiration);
+        bytes memory initParams = abi.encode(
+            _owner,
+            _avatar,
+            _target,
+            _cooldown,
+            _expiration
+        );
         setUp(initParams);
     }
 
@@ -53,8 +58,7 @@ contract Delay is Modifier {
             address _target,
             uint256 _cooldown,
             uint256 _expiration
-        ) =
-            abi.decode(
+        ) = abi.decode(
                 initParams,
                 (address, address, address, uint256, uint256)
             );
@@ -121,27 +125,50 @@ contract Delay is Modifier {
     /// @param value Ether value of module transaction
     /// @param data Data payload of module transaction
     /// @param operation Operation type of module transaction
+    /// @return success Whether or not the call was successfully queued for execution
     /// @notice Can only be called by enabled modules
     function execTransactionFromModule(
         address to,
         uint256 value,
         bytes calldata data,
         Enum.Operation operation
     ) public override moduleOnly returns (bool success) {
-        txHash[queueNonce] = getTransactionHash(to, value, data, operation);
+        bytes32 hash = getTransactionHash(to, value, data, operation);
+        txHash[queueNonce] = hash;
         txCreatedAt[queueNonce] = block.timestamp;
-        emit TransactionAdded(
-            queueNonce,
-            txHash[queueNonce],
-            to,
-            value,
-            data,
-            operation
-        );
+        emit TransactionAdded(queueNonce, hash, to, value, data, operation);
         queueNonce++;
         success = true;
     }
 
+    /// @dev Adds a transaction to the queue (same as avatar interface so that this can be placed between other modules and the avatar).
+    /// @param to Destination address of module transaction
+    /// @param value Ether value of module transaction
+    /// @param data Data payload of module transaction
+    /// @param operation Operation type of module transaction
+    /// @return success Whether or not the call was successfully queued for execution
+    /// @return returnData ABI encoded queue nonce (uint256), transaction hash (bytes32), and block.timestamp (uint256)
+    /// @notice Can only be called by enabled modules
+    function execTransactionFromModuleReturnData(
+        address to,
+        uint256 value,
+        bytes calldata data,
+        Enum.Operation operation
+    )
+        public
+        override
+        moduleOnly
+        returns (bool success, bytes memory returnData)
+    {
+        bytes32 hash = getTransactionHash(to, value, data, operation);
+        txHash[queueNonce] = hash;
+        txCreatedAt[queueNonce] = block.timestamp;
+        emit TransactionAdded(queueNonce, hash, to, value, data, operation);
+        success = true;
+        returnData = abi.encode(queueNonce, hash, block.timestamp);
+        queueNonce++;
+    }
+
     /// @dev Executes the next transaction only if the cooldown has passed and the transaction has not expired
     /// @param to Destination address of module transaction
     /// @param value Ether value of module transaction
@@ -155,13 +182,14 @@ contract Delay is Modifier {
         Enum.Operation operation
     ) public {
         require(txNonce < queueNonce, "Transaction queue is empty");
+        uint256 txCreationTimestamp = txCreatedAt[txNonce];
         require(
-            block.timestamp - txCreatedAt[txNonce] >= txCooldown,
+            block.timestamp - txCreationTimestamp >= txCooldown,
             "Transaction is still in cooldown"
         );
         if (txExpiration != 0) {
             require(
-                txCreatedAt[txNonce] + txCooldown + txExpiration >=
+                txCreationTimestamp + txCooldown + txExpiration >=
                     block.timestamp,
                 "Transaction expired"
             );

src/tasks/setup.ts

@@ -97,6 +97,12 @@ task("verifyEtherscan", "Verifies the contract on etherscan")
     undefined,
     types.string
   )
+  .addParam(
+    "target",
+    "Address that this module will send to",
+    undefined,
+    types.string
+  )
   .addParam(
     "cooldown",
     "Cooldown in seconds that should be required after a oracle provided answer",

test/Delay.spec.ts

@@ -1,6 +1,7 @@
 import { expect } from "chai";
-import hre, { deployments, waffle } from "hardhat";
+import hre, { deployments, ethers, waffle } from "hardhat";
 import "@nomiclabs/hardhat-ethers";
+import exp from "constants";
 
 const ZeroState =
   "0x0000000000000000000000000000000000000000000000000000000000000000";
@@ -345,6 +346,125 @@ describe("DelayModifier", async () => {
     });
   });
 
+  describe("execTransactionFromModuleReturnData()", async () => {
+    it("throws if not authorized", async () => {
+      const { modifier } = await setupTestWithTestAvatar();
+      await expect(
+        modifier.execTransactionFromModuleReturnData(user1.address, 0, "0x", 0)
+      ).to.be.revertedWith("Module not authorized");
+    });
+
+    it("increments queueNonce", async () => {
+      const { avatar, modifier } = await setupTestWithTestAvatar();
+      const tx = await modifier.populateTransaction.enableModule(user1.address);
+      await avatar.exec(modifier.address, 0, tx.data);
+      let queueNonce = await modifier.queueNonce();
+
+      await expect(queueNonce._hex).to.be.equals("0x00");
+      await modifier.execTransactionFromModuleReturnData(
+        user1.address,
+        0,
+        "0x",
+        0
+      );
+      queueNonce = await modifier.queueNonce();
+      await expect(queueNonce._hex).to.be.equals("0x01");
+    });
+
+    it("sets txHash", async () => {
+      const { avatar, modifier } = await setupTestWithTestAvatar();
+      const tx = await modifier.populateTransaction.enableModule(user1.address);
+      await avatar.exec(modifier.address, 0, tx.data);
+
+      let txHash = await modifier.getTransactionHash(user1.address, 0, "0x", 0);
+
+      await expect(await modifier.getTxHash(0)).to.be.equals(ZeroState);
+      await modifier.execTransactionFromModuleReturnData(
+        user1.address,
+        0,
+        "0x",
+        0
+      );
+      await expect(await modifier.getTxHash(0)).to.be.equals(txHash);
+    });
+
+    it("sets txCreatedAt", async () => {
+      const { avatar, modifier } = await setupTestWithTestAvatar();
+      const tx = await modifier.populateTransaction.enableModule(user1.address);
+      let expectedTimestamp = await modifier.getTxCreatedAt(0);
+      await avatar.exec(modifier.address, 0, tx.data);
+
+      await expect(expectedTimestamp._hex).to.be.equals("0x00");
+      let receipt = await modifier.execTransactionFromModuleReturnData(
+        user1.address,
+        0,
+        "0x",
+        0
+      );
+      let blockNumber = receipt.blockNumber;
+
+      let block = await hre.network.provider.send("eth_getBlockByNumber", [
+        "latest",
+        false,
+      ]);
+
+      expectedTimestamp = await modifier.getTxCreatedAt(0);
+      await expect(block.timestamp).to.be.equals(expectedTimestamp._hex);
+    });
+
+    it("emits transaction details", async () => {
+      const { avatar, modifier } = await setupTestWithTestAvatar();
+      const tx = await modifier.populateTransaction.enableModule(user1.address);
+      await avatar.exec(modifier.address, 0, tx.data);
+      const expectedQueueNonce = await modifier.queueNonce();
+
+      expect(
+        await modifier.execTransactionFromModuleReturnData(
+          user1.address,
+          42,
+          "0x",
+          0
+        )
+      )
+        .to.emit(modifier, "TransactionAdded")
+        .withArgs(
+          expectedQueueNonce,
+          await modifier.getTransactionHash(user1.address, 42, "0x", 0),
+          user1.address,
+          42,
+          "0x",
+          0
+        );
+    });
+
+    it("returns ABI encoded nonce, hash, and timestamp", async () => {
+      const { avatar, modifier } = await setupTestWithTestAvatar();
+      const tx = await modifier.populateTransaction.enableModule(user1.address);
+      await avatar.exec(modifier.address, 0, tx.data);
+      const expectedQueueNonce = await modifier.queueNonce();
+      const expectedHash = await modifier.getTransactionHash(
+        user1.address,
+        42,
+        "0xbadfed",
+        0
+      );
+
+      const data = await modifier.callStatic.execTransactionFromModuleReturnData(
+        user1.address,
+        42,
+        "0xbadfed",
+        0
+      );
+      // console.log(data);
+      const decodedData = await ethers.utils.defaultAbiCoder.decode(
+        ["uint256", "bytes32", "uint256"],
+        data.returnData
+      );
+      expect(decodedData[0]).to.equal(expectedQueueNonce);
+      expect(decodedData[1]).to.equal(expectedHash);
+    });
+  });
+
   describe("executeNextTx()", async () => {
     it("throws if there is nothing in queue", async () => {
       const { avatar, modifier } = await setupTestWithTestAvatar();