Orchestrator Updates

contracts/Orchestrator.sol

@@ -1,53 +1,72 @@
-pragma solidity 0.7.6;
+// SPDX-License-Identifier: GPL-3.0-or-later
+pragma solidity 0.8.2;
 
-import "./_external/Ownable.sol";
+import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";
+import {BytesLib} from "./_external/BytesLib.sol";
+import {StringUtils} from "./_external/StringUtils.sol";
 
-import "./UFragmentsPolicy.sol";
+interface IUFragmentsPolicy {
+    function rebase() external;
+}
 
 /**
  * @title Orchestrator
  * @notice The orchestrator is the main entry point for rebase operations. It coordinates the policy
  * actions with external consumers.
  */
 contract Orchestrator is Ownable {
+    using BytesLib for bytes;
+    using StringUtils for uint256;
+
+    // Reference to the Ampleforth Policy
+    address public policy;
+
     struct Transaction {
         bool enabled;
+        bool critical;
         address destination;
         bytes data;
     }
 
     // Stable ordering is not guaranteed.
     Transaction[] public transactions;
 
-    UFragmentsPolicy public policy;
+    // events
+    event TransactionFailed(uint256 index);
 
     /**
      * @param policy_ Address of the UFragments policy.
      */
-    constructor(address policy_) public {
-        Ownable.initialize(msg.sender);
-        policy = UFragmentsPolicy(policy_);
+    constructor(address policy_) {
+        policy = policy_;
     }
 
     /**
      * @notice Main entry point to initiate a rebase operation.
      *         The Orchestrator calls rebase on the policy and notifies downstream applications.
      *         Contracts are guarded from calling, to avoid flash loan attacks on liquidity
      *         providers.
-     *         If a transaction in the transaction list fails, Orchestrator will stop execution
-     *         and revert to prevent a gas underprice attack.
+     *         If a transaction marked 'critical' in the transaction list fails,
+     *         Orchestrator will stop execution and revert.
      */
     function rebase() external {
         require(msg.sender == tx.origin); // solhint-disable-line avoid-tx-origin
 
-        policy.rebase();
+        IUFragmentsPolicy(policy).rebase();
 
         for (uint256 i = 0; i < transactions.length; i++) {
             Transaction storage t = transactions[i];
             if (t.enabled) {
-                (bool result, ) = t.destination.call(t.data);
-                if (!result) {
-                    revert("Transaction Failed");
+                (bool success, bytes memory reason) = t.destination.call(t.data);
+
+                // Critical transaction failed, revert with message
+                if (!success && t.critical) {
+                    revert(buildRevertReason(i, reason));
+                }
+
+                // Non-Critical transaction failed, log error and continue
+                if (!success) {
+                    emit TransactionFailed(i);
                 }
             }
         }
@@ -58,16 +77,22 @@ contract Orchestrator is Ownable {
      * @param destination Address of contract destination
      * @param data Transaction data payload
      */
-    function addTransaction(address destination, bytes memory data) external onlyOwner {
-        transactions.push(Transaction({enabled: true, destination: destination, data: data}));
+    function addTransaction(
+        bool critical,
+        address destination,
+        bytes memory data
+    ) external onlyOwner {
+        transactions.push(
+            Transaction({enabled: true, critical: critical, destination: destination, data: data})
+        );
     }
 
     /**
      * @param index Index of transaction to remove.
      *              Transaction ordering may have changed since adding.
      */
     function removeTransaction(uint256 index) external onlyOwner {
-        require(index < transactions.length, "index out of bounds");
+        require(index < transactions.length, "Orchestrator: index out of bounds");
 
         if (index < transactions.length - 1) {
             transactions[index] = transactions[transactions.length - 1];
@@ -81,14 +106,67 @@ contract Orchestrator is Ownable {
      * @param enabled True for enabled, false for disabled.
      */
     function setTransactionEnabled(uint256 index, bool enabled) external onlyOwner {
-        require(index < transactions.length, "index must be in range of stored tx list");
+        require(
+            index < transactions.length,
+            "Orchestrator: index must be in range of stored tx list"
+        );
         transactions[index].enabled = enabled;
     }
 
+    /**
+     * @param index Index of transaction. Transaction ordering may have changed since adding.
+     * @param critical True for critical, false for non-critical.
+     */
+    function setTransactionCritical(uint256 index, bool critical) external onlyOwner {
+        require(
+            index < transactions.length,
+            "Orchestrator: index must be in range of stored tx list"
+        );
+        transactions[index].critical = critical;
+    }
+
     /**
      * @return Number of transactions, both enabled and disabled, in transactions list.
      */
     function transactionsSize() external view returns (uint256) {
         return transactions.length;
     }
+
+    /**
+     * @param txIndex The index of the failing transaction in the transaction array.
+     * @param reason The revert reason in bytes.
+     * @return Number of transactions, both enabled and disabled, in transactions list.
+     */
+    function buildRevertReason(uint256 txIndex, bytes memory reason)
+        internal
+        pure
+        returns (string memory)
+    {
+        return
+            string(
+                abi.encodePacked(
+                    "Orchestrator: critical {job} reverted with {reason}:",
+                    txIndex.uintToBytes(),
+                    "|",
+                    revertReasonToString(reason)
+                )
+            );
+    }
+
+    /**
+     * @dev github.com/authereum/contracts/account/BaseAccount.sol#L132
+     * @param reason The revert reason in bytes.
+     * @return The revert reason as a string.
+     */
+    function revertReasonToString(bytes memory reason) internal pure returns (string memory) {
+        // If the reason length is less than 68, then the transaction failed
+        // silently (without a revert message)
+        if (reason.length < 68) return "Transaction reverted silently";
+
+        // Remove the selector which is the first 4 bytes
+        bytes memory revertData = reason.slice(4, reason.length - 4);
+
+        // All that remains is the revert string
+        return abi.decode(revertData, (string));
+    }
 }

contracts/UFragments.sol

@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity 0.7.6;
 
 import "./_external/SafeMath.sol";

contracts/UFragmentsPolicy.sol

@@ -1,3 +1,4 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
 pragma solidity 0.7.6;
 
 import "./_external/SafeMath.sol";

contracts/_external/BytesLib.sol

@@ -0,0 +1,82 @@
+// SPDX-License-Identifier: Unlicensed
+pragma solidity ^0.8.0;
+
+// https://github.com/GNSPS/solidity-bytes-utils/blob/master/contracts/BytesLib.sol
+/*
+ * @title Solidity Bytes Arrays Utils
+ * @author Gonçalo Sá <[email protected]>
+ *
+ * @dev Bytes tightly packed arrays utility library for ethereum contracts written in Solidity.
+ *      The library lets you concatenate, slice and type cast bytes arrays both in memory
+ *      and storage.
+ */
+library BytesLib {
+    function slice(
+        bytes memory _bytes,
+        uint256 _start,
+        uint256 _length
+    ) internal pure returns (bytes memory) {
+        require(_length + 31 >= _length, "slice_overflow");
+        require(_start + _length >= _start, "slice_overflow");
+        require(_bytes.length >= _start + _length, "slice_outOfBounds");
+
+        bytes memory tempBytes;
+
+        assembly {
+            switch iszero(_length)
+                case 0 {
+                    // Get a location of some free memory and store it in tempBytes as
+                    // Solidity does for memory variables.
+                    tempBytes := mload(0x40)
+
+                    // The first word of the slice result is potentially a partial
+                    // word read from the original array. To read it, we calculate
+                    // the length of that partial word and start copying that many
+                    // bytes into the array. The first word we copy will start with
+                    // data we don't care about, but the last `lengthmod` bytes will
+                    // land at the beginning of the contents of the new array. When
+                    // we're done copying, we overwrite the full first word with
+                    // the actual length of the slice.
+                    let lengthmod := and(_length, 31)
+
+                    // The multiplication in the next line is necessary
+                    // because when slicing multiples of 32 bytes (lengthmod == 0)
+                    // the following copy loop was copying the origin's length
+                    // and then ending prematurely not copying everything it should.
+                    let mc := add(add(tempBytes, lengthmod), mul(0x20, iszero(lengthmod)))
+                    let end := add(mc, _length)
+
+                    for {
+                        // The multiplication in the next line has the same exact purpose
+                        // as the one above.
+                        let cc := add(
+                            add(add(_bytes, lengthmod), mul(0x20, iszero(lengthmod))),
+                            _start
+                        )
+                    } lt(mc, end) {
+                        mc := add(mc, 0x20)
+                        cc := add(cc, 0x20)
+                    } {
+                        mstore(mc, mload(cc))
+                    }
+
+                    mstore(tempBytes, _length)
+
+                    //update free-memory pointer
+                    //allocating the array padded to 32 bytes like the compiler does now
+                    mstore(0x40, and(add(mc, 31), not(31)))
+                }
+                //if we want a zero-length slice let's just return a zero-length array
+                default {
+                    tempBytes := mload(0x40)
+                    //zero out the 32 bytes slice we are about to return
+                    //we need to do it because Solidity does not garbage collect
+                    mstore(tempBytes, 0)
+
+                    mstore(0x40, add(tempBytes, 0x20))
+                }
+        }
+
+        return tempBytes;
+    }
+}

contracts/_external/StringUtils.sol

@@ -0,0 +1,23 @@
+// SPDX-License-Identifier: Unlicensed
+pragma solidity ^0.8.0;
+
+// https://github.com/pipermerriam/ethereum-string-utils/blob/master/contracts/StringLib.sol
+// String Utils v0.1
+/// @title String Utils - String utility functions
+/// @author Piper Merriam - <[email protected]>
+library StringUtils {
+    /// @dev Converts an unsigned integert to its string representation.
+    /// @param v The number to be converted.
+    function uintToBytes(uint256 v) internal pure returns (bytes32 ret) {
+        if (v == 0) {
+            ret = "0";
+        } else {
+            while (v > 0) {
+                ret = bytes32(uint256(ret) / (2**8));
+                ret |= bytes32(((v % 10) + 48) * 2**(8 * 31));
+                v /= 10;
+            }
+        }
+        return ret;
+    }
+}

contracts/mocks/ConstructorRebaseCallerContract.sol

@@ -1,12 +1,14 @@
 pragma solidity 0.7.6;
 
-import "../Orchestrator.sol";
+interface IOrchestrator {
+    function rebase() external;
+}
 
 contract ConstructorRebaseCallerContract {
     constructor(address orchestrator) public {
         // Take out a flash loan.
         // Do something funky...
-        Orchestrator(orchestrator).rebase(); // should fail
+        IOrchestrator(orchestrator).rebase(); // should fail
         // pay back flash loan.
     }
 }

contracts/mocks/MockDownstream.sol

@@ -39,4 +39,14 @@ contract MockDownstream is Mock {
 
         require(false, "reverted");
     }
+
+    function revertsWithoutMessage() external {
+        emit FunctionCalled("MockDownstream", "reverts", msg.sender);
+
+        uint256[] memory uintVals = new uint256[](0);
+        int256[] memory intVals = new int256[](0);
+        emit FunctionArguments(uintVals, intVals);
+
+        revert();
+    }
 }

contracts/mocks/RebaseCallerContract.sol

@@ -1,12 +1,14 @@
 pragma solidity 0.7.6;
 
-import "../Orchestrator.sol";
+interface IOrchestrator {
+    function rebase() external;
+}
 
 contract RebaseCallerContract {
     function callRebase(address orchestrator) public returns (bool) {
         // Take out a flash loan.
         // Do something funky...
-        Orchestrator(orchestrator).rebase(); // should fail
+        IOrchestrator(orchestrator).rebase(); // should fail
         // pay back flash loan.
         return true;
     }

hardhat.config.ts

@@ -3,26 +3,26 @@ import { HardhatUserConfig } from 'hardhat/config'
 import '@nomiclabs/hardhat-ethers'
 import '@nomiclabs/hardhat-waffle'
 import '@openzeppelin/hardhat-upgrades'
-import "@nomiclabs/hardhat-etherscan";
+import '@nomiclabs/hardhat-etherscan'
 import 'solidity-coverage'
 import 'hardhat-gas-reporter'
 
 require('./scripts/deploy')
 
 export default {
   etherscan: {
-    apiKey: process.env.ETHERSCAN_API_KEY
+    apiKey: process.env.ETHERSCAN_API_KEY,
   },
   networks: {
     rinkeby: {
-      url: `https://rinkeby.infura.io/v3/${process.env.INFURA_SECRET}`
+      url: `https://rinkeby.infura.io/v3/${process.env.INFURA_SECRET}`,
     },
     kovan: {
-      url: `https://kovan.infura.io/v3/${process.env.INFURA_SECRET}`
+      url: `https://kovan.infura.io/v3/${process.env.INFURA_SECRET}`,
     },
     mainnet: {
-      url: `https://mainnet.infura.io/v3/${process.env.INFURA_SECRET}`
-    }
+      url: `https://mainnet.infura.io/v3/${process.env.INFURA_SECRET}`,
+    },
   },
   solidity: {
     compilers: [
@@ -36,7 +36,13 @@ export default {
         },
       },
       {
-        version: '0.4.24',
+        version: '0.8.2',
+        settings: {
+          optimizer: {
+            enabled: true,
+            runs: 200,
+          },
+        },
       },
     ],
   },