ACL: optimize checkOracle()

contracts/acl/ACL.sol

@@ -419,31 +419,39 @@ contract ACL is IACL, TimeHelpers, AragonApp, ACLHelpers {
     function checkOracle(IACLOracle _oracleAddr, address _who, address _where, bytes32 _what, uint256[] _how) internal view returns (bool) {
         bytes4 sig = _oracleAddr.canPerform.selector;
 
-        // a raw call is required so we can return false if the call reverts, rather than reverting
         bytes memory checkCalldata = abi.encodeWithSelector(sig, _who, _where, _what, _how);
         uint256 oracleCheckGas = ORACLE_CHECK_GAS;
-
-        bool ok;
-        assembly {
-            ok := staticcall(oracleCheckGas, _oracleAddr, add(checkCalldata, 0x20), mload(checkCalldata), 0, 0)
-        }
-
-        if (!ok) {
-            return false;
-        }
-
-        uint256 size;
-        assembly { size := returndatasize }
-        if (size != 32) {
-            return false;
-        }
-
         bool result;
+
         assembly {
-            let ptr := mload(0x40)       // get next free memory ptr
-            returndatacopy(ptr, 0, size) // copy return from above `staticcall`
-            result := mload(ptr)         // read data at ptr and set it to result
-            mstore(ptr, 0)               // set pointer memory to 0 so it still is the next free ptr
+            let ptr := mload(0x40)        // free memory pointer
+
+            // A raw staticcall is required so we can return false if the call reverts, rather than reverting
+            result := staticcall(
+                oracleCheckGas,           // gas forwarded
+                _oracleAddr,              // address
+                add(checkCalldata, 0x20), // calldata start
+                mload(checkCalldata),     // calldata length
+                ptr,                      // write output over free memory
+                0x20                      // uint256 return
+            )
+
+            // solidity-coverage fails on assembly `if`
+            switch result case 0 { }
+            default {
+                // Check number of bytes returned from last external call
+                switch returndatasize
+
+                // 32 bytes returned: check if true
+                case 0x20 {
+                    // Only return success if returned data was true
+                    // Already have output in ptr
+                    result := eq(mload(ptr), 1)
+                }
+
+                // Not sure what was returned: don't mark as success
+                default { }
+            }
         }
 
         return result;

contracts/common/SafeERC20.sol

@@ -29,7 +29,7 @@ library SafeERC20 {
                 add(_calldata, 0x20), // calldata start
                 mload(_calldata),     // calldata length
                 ptr,                  // write output over free memory
-                0x20                  // uint256 return
+                0x20                  // write 32 bytes
             )
 
             if gt(success, 0) {
@@ -41,7 +41,7 @@ library SafeERC20 {
                     ret := 1
                 }
 
-                // 32 bytes returned: check if non-zero
+                // 32 bytes returned: check if true
                 case 0x20 {
                     // Only return success if returned data was true
                     // Already have output in ptr

contracts/test/TestACLInterpreter.sol

@@ -86,6 +86,7 @@ contract TestACLInterpreter is ACL, ACLHelper {
         assertEval(arr(), ORACLE_PARAM_ID, Op.EQ, uint256(new StateModifyingOracle()), false);
         // if returned data size is not correct, returns false
         assertEval(arr(), ORACLE_PARAM_ID, Op.EQ, uint256(new EmptyDataReturnOracle()), false);
+        assertEval(arr(), ORACLE_PARAM_ID, Op.EQ, uint256(new LargeDataReturnOracle()), false);
 
         // conditional oracle returns true if first param > 0
         ConditionalOracle conditionalOracle = new ConditionalOracle();

contracts/test/helpers/ACLHelper.sol

@@ -52,6 +52,18 @@ contract EmptyDataReturnOracle is IACLOracle {
     }
 }
 
+contract LargeDataReturnOracle is IACLOracle {
+    function canPerform(address, address, bytes32, uint256[]) external view returns (bool) {
+        uint256[] memory largeData = new uint256[](2);
+        largeData[0] = 1;
+        largeData[1] = 2;
+        assembly {
+            // Return two uint256s
+            return(largeData, 0x40)
+        }
+    }
+}
+
 contract ConditionalOracle is IACLOracle {
     function canPerform(address, address, bytes32, uint256[] how) external view returns (bool) {
         return how[0] > 0;