From 66516f2aaebcefc199f56140644bbbc86e909b70 Mon Sep 17 00:00:00 2001
From: liamzebedee <liam.zebedee@keep.network>
Date: Wed, 22 Apr 2020 17:28:16 +1000
Subject: [PATCH] Forward nested revert messages in FundingScript

---
 solidity/contracts/scripts/FundingScript.sol  | 19 ++++++++---
 .../contracts/scripts/RedemptionScript.sol    |  1 +
 solidity/test/VendingMachineTest.js           | 33 +++++++++++++++++++
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/solidity/contracts/scripts/FundingScript.sol b/solidity/contracts/scripts/FundingScript.sol
index c3c76c3cd..90a4b3637 100644
--- a/solidity/contracts/scripts/FundingScript.sol
+++ b/solidity/contracts/scripts/FundingScript.sol
@@ -50,11 +50,20 @@ contract FundingScript {
         // Call the VendingMachine.
         // We could explictly encode the call to vending machine, but this would
         // involve manually parsing _extraData and allocating variables.
-        (bool success, bytes memory returnData) = address(vendingMachine).call(
-            // solium-disable-previous-line security/no-low-level-calls
-            _extraData
-        );
-        require(success, string(returnData));
+        // We capture the `returnData` in order to forward any nested revert message
+        // from the contract call.
+        (bool success, bytes memory returnData) = address(vendingMachine).call(_extraData);
+
+        string memory revertMessage;
+        assembly {
+            // A revert message is ABI-encoded as a call to Error(string).
+            // Slicing the Error() signature (4 bytes) and Data offset (4 bytes)
+            // leaves us with a pre-encoded string.
+            // We also slice off the ABI-coded length of returnData (32).
+            revertMessage := add(returnData, 0x44)
+        }
+
+        require(success, revertMessage);
 
         // Transfer the TBTC and feeRebateToken to the user.
         tbtcToken.transfer(_from, tbtcToken.balanceOf(address(this)));
diff --git a/solidity/contracts/scripts/RedemptionScript.sol b/solidity/contracts/scripts/RedemptionScript.sol
index af79c698e..ff5d2b60f 100644
--- a/solidity/contracts/scripts/RedemptionScript.sol
+++ b/solidity/contracts/scripts/RedemptionScript.sol
@@ -50,6 +50,7 @@ contract RedemptionScript {
 
         // We capture the `returnData` in order to forward any nested revert message
         // from the contract call.
+        // solium-disable-next-line security/no-low-level-calls
         (bool success, bytes memory returnData) = address(vendingMachine).call(_extraData);
 
         string memory revertMessage;
diff --git a/solidity/test/VendingMachineTest.js b/solidity/test/VendingMachineTest.js
index bc249e4a2..83def831e 100644
--- a/solidity/test/VendingMachineTest.js
+++ b/solidity/test/VendingMachineTest.js
@@ -592,6 +592,39 @@ describe("VendingMachine", async function() {
       expect(success).to.be.true
     })
 
+    it("forwards nested revert error messages", async () => {
+      const unqualifiedDepositToTbtcABI = vendingMachine.abi.filter(
+        x => x.name == "unqualifiedDepositToTbtc",
+      )[0]
+      const calldata = web3.eth.abi.encodeFunctionCall(
+        unqualifiedDepositToTbtcABI,
+        [
+          testDeposit.address,
+          _version,
+          _txInputVector,
+          _txOutputVector,
+          _txLocktime,
+          _fundingOutputIndex,
+          _merkleProof,
+          _txIndexInBlock,
+          _bitcoinHeaders,
+        ],
+      )
+
+      // To make the funding call fail.
+      await testDeposit.setState(states.ACTIVE)
+
+      await expectRevert(
+        tbtcDepositToken.approveAndCall.call(
+          fundingScript.address,
+          tdtId,
+          calldata,
+          {from: owner},
+        ),
+        "Not awaiting funding",
+      )
+    })
+
     it("reverts for unknown function calls encoded in _extraData", async () => {
       const unknownFunctionSignature = "0xCAFEBABE"