Add flag to BatchExecutor to enable or disable partial failures

.gas-snapshot

@@ -1,8 +1,10 @@
 ApproveAndSwapTest:testSwap() (gas: 286150)
 ApproveAndSwapTest:testSwapFailsIfWeExpectedTooMuch() (gas: 365405)
 ApproveAndSwapTest:testSwapFailsWithNoApproval() (gas: 122970)
-BatchExecutorTest:testBatchExecute() (gas: 129560)
-BatchExecutorTest:testBatchExecuteDoesNotRevertIfAnyCallsRevert() (gas: 122704)
+BatchExecutorTest:testBatchExecuteWithPartialFailures() (gas: 129505)
+BatchExecutorTest:testBatchExecuteWithPartialFailuresDoesNotRevertIfAnyCallsRevert() (gas: 122999)
+BatchExecutorTest:testBatchExecuteWithoutPartialFailures() (gas: 129769)
+BatchExecutorTest:testBatchExecuteWithoutPartialFailuresRevertsIfAnyCallsRevert() (gas: 120320)
 CallbacksTest:testAllowNestedCallbacks() (gas: 157273)
 CallbacksTest:testCallbackFromCounter() (gas: 100510)
 CallbacksTest:testCallcodeReentrancyExploitWithProtectedScript() (gas: 104263)
@@ -81,9 +83,9 @@ PaycallTest:testRevertsForInvalidCallContext() (gas: 16800)
 PaycallTest:testSimpleCounterAndPayWithUSDC() (gas: 144858)
 PaycallTest:testSimpleTransferTokenAndPayWithUSDC() (gas: 140402)
 PaycallTest:testSupplyWETHWithdrawUSDCOnCometAndPayWithUSDC() (gas: 301158)
-QuarkFactoryTest:testInvariantAddressesBetweenNonces() (gas: 2872802)
-QuarkFactoryTest:testQuarkFactoryDeployToDeterministicAddresses() (gas: 2872535)
-QuarkFactoryTest:testQuarkFactoryDeployTwice() (gas: 2953514)
+QuarkFactoryTest:testInvariantAddressesBetweenNonces() (gas: 2890687)
+QuarkFactoryTest:testQuarkFactoryDeployToDeterministicAddresses() (gas: 2890420)
+QuarkFactoryTest:testQuarkFactoryDeployTwice() (gas: 2971836)
 QuarkMinimalProxyTest:testSignerExecutor() (gas: 75271)
 QuarkStateManagerTest:testNonceZeroIsValid() (gas: 91471)
 QuarkStateManagerTest:testReadStorageForWallet() (gas: 148583)

src/quark-core/src/periphery/BatchExecutor.sol

@@ -10,6 +10,8 @@ import {QuarkWallet} from "quark-core/src/QuarkWallet.sol";
  * @author Compound Labs, Inc.
  */
 contract BatchExecutor {
+    error BatchExecutionError(uint256 opsIndex, bytes err);
+
     /// @notice The parameters used for processing a QuarkOperation
     struct OperationParams {
         address account;
@@ -23,20 +25,27 @@ contract BatchExecutor {
     /**
      * @notice Execute a list of QuarkOperations via signatures
      * @param ops List of QuarkOperations to execute via signature
+     * @param allowPartialFailures Whether or not to allow partial failures when any of the calls revert
      */
-    function batchExecuteOperations(OperationParams[] calldata ops) external {
-        for (uint256 i = 0; i < ops.length;) {
-            executeOperation(ops[i]);
-            unchecked {
-                ++i;
+    function batchExecuteOperations(OperationParams[] calldata ops, bool allowPartialFailures) external {
+        for (uint256 i = 0; i < ops.length; ++i) {
+            (bool success, bytes memory returnData) = executeOperation(ops[i]);
+            if (!allowPartialFailures && !success) {
+                revert BatchExecutionError(i, returnData);
             }
         }
     }
 
-    /// @notice Execute a single QuarkOperation via signature
-    function executeOperation(OperationParams memory op) internal {
+    /**
+     * @notice Execute a single QuarkOperation via signature
+     * @param op Quark Operation parameters to execute via signature
+     * @return Success and return value from the executed operation
+     */
+    function executeOperation(OperationParams memory op) internal returns (bool, bytes memory) {
         bytes memory data = abi.encodeWithSelector(QuarkWallet.executeQuarkOperation.selector, op.op, op.v, op.r, op.s);
         // We purposely ignore success and return values since the BatchExecutor will most likely be called by an EOA
-        op.account.call{gas: op.gasLimit}(data);
+        // Lower-level call is used to avoid reverting on failure
+        (bool success, bytes memory retData) = op.account.call{gas: op.gasLimit}(data);
+        return (success, retData);
     }
 }

test/quark-core/periphery/BatchExecutor.t.sol

@@ -61,7 +61,7 @@ contract BatchExecutorTest is Test {
         console.log("Bob wallet at: %s", address(aliceWallet));
     }
 
-    function testBatchExecute() public {
+    function testBatchExecuteWithPartialFailures() public {
         // We test multiple operations with different wallets
         // gas: do not meter set-up
         vm.pauseGasMetering();
@@ -104,12 +104,12 @@ contract BatchExecutorTest is Test {
         vm.resumeGasMetering();
         vm.expectEmit(false, false, false, true);
         emit Ping(55);
-        batchExecutor.batchExecuteOperations(ops);
+        batchExecutor.batchExecuteOperations(ops, true);
 
         assertEq(counter.number(), 3);
     }
 
-    function testBatchExecuteDoesNotRevertIfAnyCallsRevert() public {
+    function testBatchExecuteWithPartialFailuresDoesNotRevertIfAnyCallsRevert() public {
         // gas: do not meter set-up
         vm.pauseGasMetering();
         bytes memory ping = new YulHelper().getCode("Logger.sol/Logger.json");
@@ -154,7 +154,112 @@ contract BatchExecutorTest is Test {
 
         // gas: meter execute
         vm.resumeGasMetering();
-        batchExecutor.batchExecuteOperations(ops);
+        batchExecutor.batchExecuteOperations(ops, true);
+
+        // Note: We removed returning success as a gas optimization, but these are the expected successes
+        // assertEq(successes[0], true);
+        // assertEq(successes[1], false);
+        // // Should fail with OOG
+        // assertEq(successes[2], false);
+    }
+
+    function testBatchExecuteWithoutPartialFailures() public {
+        // We test multiple operations with different wallets
+        // gas: do not meter set-up
+        vm.pauseGasMetering();
+        bytes memory ping = new YulHelper().getCode("Logger.sol/Logger.json");
+        bytes memory incrementer = new YulHelper().getCode("Incrementer.sol/Incrementer.json");
+
+        QuarkWallet.QuarkOperation memory aliceOp =
+            new QuarkOperationHelper().newBasicOp(aliceWallet, ping, ScriptType.ScriptAddress);
+        (uint8 v0, bytes32 r0, bytes32 s0) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, aliceOp);
+        QuarkWallet.QuarkOperation memory bobOp = new QuarkOperationHelper().newBasicOpWithCalldata(
+            bobWallet,
+            incrementer,
+            abi.encodeWithSignature("incrementCounter(address)", counter),
+            ScriptType.ScriptSource
+        );
+        (uint8 v1, bytes32 r1, bytes32 s1) = new SignatureHelper().signOp(bobPrivateKey, bobWallet, bobOp);
+
+        // Construct list of operations and signatures
+        BatchExecutor.OperationParams[] memory ops = new BatchExecutor.OperationParams[](2);
+        ops[0] = BatchExecutor.OperationParams({
+            account: address(aliceWallet),
+            op: aliceOp,
+            v: v0,
+            r: r0,
+            s: s0,
+            gasLimit: 0.1 ether
+        });
+        ops[1] = BatchExecutor.OperationParams({
+            account: address(bobWallet),
+            op: bobOp,
+            v: v1,
+            r: r1,
+            s: s1,
+            gasLimit: 0.1 ether
+        });
+
+        assertEq(counter.number(), 0);
+
+        // gas: meter execute
+        vm.resumeGasMetering();
+        vm.expectEmit(false, false, false, true);
+        emit Ping(55);
+        batchExecutor.batchExecuteOperations(ops, false);
+
+        assertEq(counter.number(), 3);
+    }
+
+    function testBatchExecuteWithoutPartialFailuresRevertsIfAnyCallsRevert() public {
+        // gas: do not meter set-up
+        vm.pauseGasMetering();
+        bytes memory ping = new YulHelper().getCode("Logger.sol/Logger.json");
+        bytes memory reverts = new YulHelper().getCode("Reverts.sol/Reverts.json");
+
+        QuarkWallet.QuarkOperation memory aliceOp =
+            new QuarkOperationHelper().newBasicOp(aliceWallet, ping, ScriptType.ScriptAddress);
+        (uint8 v0, bytes32 r0, bytes32 s0) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, aliceOp);
+        QuarkWallet.QuarkOperation memory bobOp =
+            new QuarkOperationHelper().newBasicOp(bobWallet, reverts, ScriptType.ScriptSource);
+        (uint8 v1, bytes32 r1, bytes32 s1) = new SignatureHelper().signOp(bobPrivateKey, bobWallet, bobOp);
+        QuarkWallet.QuarkOperation memory aliceOp2 =
+            new QuarkOperationHelper().newBasicOp(aliceWallet, ping, ScriptType.ScriptAddress);
+        (uint8 v2, bytes32 r2, bytes32 s2) = new SignatureHelper().signOp(alicePrivateKey, aliceWallet, aliceOp2);
+
+        // Construct list of operations and signatures
+        BatchExecutor.OperationParams[] memory ops = new BatchExecutor.OperationParams[](3);
+        ops[0] = BatchExecutor.OperationParams({
+            account: address(aliceWallet),
+            op: aliceOp,
+            v: v0,
+            r: r0,
+            s: s0,
+            gasLimit: 0.1 ether
+        });
+        ops[1] = BatchExecutor.OperationParams({
+            account: address(bobWallet),
+            op: bobOp,
+            v: v1,
+            r: r1,
+            s: s1,
+            gasLimit: 0.1 ether
+        });
+        ops[2] = BatchExecutor.OperationParams({
+            account: address(aliceWallet),
+            op: aliceOp2,
+            v: v2,
+            r: r2,
+            s: s2,
+            gasLimit: 1 wei // To trigger OOG
+        });
+
+        vm.expectRevert(
+            abi.encodeWithSelector(BatchExecutor.BatchExecutionError.selector, 1, abi.encodeWithSignature("Whoops()"))
+        );
+        // gas: meter execute
+        vm.resumeGasMetering();
+        batchExecutor.batchExecuteOperations(ops, false);
 
         // Note: We removed returning success as a gas optimization, but these are the expected successes
         // assertEq(successes[0], true);