Remove option to swallow reverts in Paycall and Quotecall (#199)

The `propagateReverts` constructor arg was introduced recently to
`Paycall` and `Quotecall` to support swallowing of reverts in the two
scripts. This was done to allow submitters to safely submit
`Paycall/Quotecall` operations on networks without Flashbots (e.g. all
L2s). Otherwise, without Flashbots' no-revert guarantee for
transactions, submitters will eat the cost for any reverts in
`Paycall/Quotecall`.

However, this option to swallow reverts introduces a potential gas
griefing vector. When reverts are swallowed, the submitter will still
get paid the payment token, at the expense of the user that signed the
Quark Operation. This is normally fine and expected, since users should
be paying for failing transactions and submitters need to be compensated
for doing the submission. This becomes problematic when the reverting
transaction is a replayable Quark Operation. In this case, a malicious
submitter could repeatedly submit the reverting replayable transaction,
draining the Quark wallet of its payment token in the process.

In short, we remove the option to swallow reverts in `Paycall/Quotecall`
to prevent this case from happening. Submitters on networks without
Flashbots are encouraged to simulate transactions first before
submitting them on-chain.

.gas-snapshot

@@ -66,19 +66,19 @@ MulticallTest:testSupplyWETHWithdrawUSDCOnComet() (gas: 260802)
 NonceTest:testIsSet() (gas: 30425)
 NonceTest:testNextUnusedNonce() (gas: 40315)
 NonceTest:testNonLinearNonce() (gas: 30757)
-PaycallTest:testInitializeProperlyFromConstructor() (gas: 6606)
-PaycallTest:testPaycallForPayWithUSDT() (gas: 126144)
-PaycallTest:testPaycallForPayWithWBTC() (gas: 120640)
-PaycallTest:testPaycallNonRevertyDoesNotRevertWhenCallReverts() (gas: 112591)
-PaycallTest:testReturnCallResult() (gas: 93192)
-PaycallTest:testRevertsForInvalidCallContext() (gas: 16767)
-PaycallTest:testRevertsWhenCostIsMoreThanMaxPaymentCost() (gas: 120368)
-PaycallTest:testSimpleCounterAndPayWithUSDC() (gas: 150073)
-PaycallTest:testSimpleTransferTokenAndPayWithUSDC() (gas: 145617)
-PaycallTest:testSupplyWETHWithdrawUSDCOnCometAndPayWithUSDC() (gas: 306416)
-QuarkFactoryTest:testInvariantAddressesBetweenNonces() (gas: 3359064)
-QuarkFactoryTest:testQuarkFactoryDeployToDeterministicAddresses() (gas: 3358797)
-QuarkFactoryTest:testQuarkFactoryDeployTwice() (gas: 3452021)
+PaycallTest:testInitializeProperlyFromConstructor() (gas: 6567)
+PaycallTest:testPaycallForPayWithUSDT() (gas: 126172)
+PaycallTest:testPaycallForPayWithWBTC() (gas: 120663)
+PaycallTest:testPaycallRevertsWhenCallReverts() (gas: 73223)
+PaycallTest:testReturnCallResult() (gas: 93230)
+PaycallTest:testRevertsForInvalidCallContext() (gas: 16811)
+PaycallTest:testRevertsWhenCostIsMoreThanMaxPaymentCost() (gas: 120410)
+PaycallTest:testSimpleCounterAndPayWithUSDC() (gas: 150109)
+PaycallTest:testSimpleTransferTokenAndPayWithUSDC() (gas: 145456)
+PaycallTest:testSupplyWETHWithdrawUSDCOnCometAndPayWithUSDC() (gas: 306454)
+QuarkFactoryTest:testInvariantAddressesBetweenNonces() (gas: 3361264)
+QuarkFactoryTest:testQuarkFactoryDeployToDeterministicAddresses() (gas: 3360997)
+QuarkFactoryTest:testQuarkFactoryDeployTwice() (gas: 3454221)
 QuarkMinimalProxyTest:testSignerExecutor() (gas: 75518)
 QuarkStateManagerTest:testNonceZeroIsValid() (gas: 91493)
 QuarkStateManagerTest:testReadStorageForWallet() (gas: 148724)
@@ -89,14 +89,14 @@ QuarkStateManagerTest:testSetActiveNonceAndCallbackNotImplemented() (gas: 92067)
 QuarkStateManagerTest:testSetsAndGetsNextNonces() (gas: 810598)
 QuarkWalletProxyFactoryTest:testCreateAdditionalWalletWithSalt() (gas: 212726)
 QuarkWalletProxyFactoryTest:testCreateAndExecuteCreatesWallet() (gas: 412006)
-QuarkWalletProxyFactoryTest:testCreateAndExecuteMultiCreatesWallet() (gas: 446513)
-QuarkWalletProxyFactoryTest:testCreateAndExecuteMultiWithSalt() (gas: 450686)
+QuarkWalletProxyFactoryTest:testCreateAndExecuteMultiCreatesWallet() (gas: 445108)
+QuarkWalletProxyFactoryTest:testCreateAndExecuteMultiWithSalt() (gas: 449281)
 QuarkWalletProxyFactoryTest:testCreateAndExecuteSetsMsgSender() (gas: 223967)
 QuarkWalletProxyFactoryTest:testCreateAndExecuteWithSalt() (gas: 416581)
 QuarkWalletProxyFactoryTest:testCreateAndExecuteWithSaltSetsMsgSender() (gas: 396196)
 QuarkWalletProxyFactoryTest:testCreateRevertsOnRepeat() (gas: 8937393460516733241)
 QuarkWalletProxyFactoryTest:testCreatesWalletAtDeterministicAddress() (gas: 222644)
-QuarkWalletProxyFactoryTest:testExecuteMultiOnExistingWallet() (gas: 444835)
+QuarkWalletProxyFactoryTest:testExecuteMultiOnExistingWallet() (gas: 443430)
 QuarkWalletProxyFactoryTest:testExecuteOnExistingWallet() (gas: 410716)
 QuarkWalletProxyFactoryTest:testExecutorIsOtherWallet() (gas: 313753)
 QuarkWalletProxyFactoryTest:testExecutorSetInCreate() (gas: 103646)
@@ -116,7 +116,7 @@ QuarkWalletTest:testGetCodeJar() (gas: 11204)
 QuarkWalletTest:testGetExecutor() (gas: 5469)
 QuarkWalletTest:testGetSigner() (gas: 8500)
 QuarkWalletTest:testGetStateManager() (gas: 10612)
-QuarkWalletTest:testMultiQuarkOperationCanCallMultipleOperationsWithOneSignature() (gas: 108861)
+QuarkWalletTest:testMultiQuarkOperationCanCallMultipleOperationsWithOneSignature() (gas: 107739)
 QuarkWalletTest:testPrecompileBigModExp() (gas: 51036)
 QuarkWalletTest:testPrecompileBlake2F() (gas: 53508)
 QuarkWalletTest:testPrecompileBn256Add() (gas: 52522)
@@ -127,16 +127,26 @@ QuarkWalletTest:testPrecompileRipemd160() (gas: 52524)
 QuarkWalletTest:testPrecompileSha256() (gas: 52967)
 QuarkWalletTest:testQuarkOperationRevertsIfCallReverts() (gas: 67575)
 QuarkWalletTest:testRevertOnAllPrecompilesDirectCall() (gas: 633031)
-QuarkWalletTest:testRevertsForBadInputsInMultiQuarkOperation() (gas: 13531)
+QuarkWalletTest:testRevertsForBadInputsInMultiQuarkOperation() (gas: 13452)
 QuarkWalletTest:testRevertsForDirectExecuteByNonExecutorSigner() (gas: 13835)
-QuarkWalletTest:testRevertsForNonceReuse() (gas: 93063)
+QuarkWalletTest:testRevertsForNonceReuse() (gas: 91723)
 QuarkWalletTest:testRevertsForRandomEmptyScriptAddress() (gas: 117042)
 QuarkWalletTest:testRevertsForReplayOfCanceledScript() (gas: 209767)
 QuarkWalletTest:testRevertsForReusedNonceWithChangedScript() (gas: 106673)
 QuarkWalletTest:testRevertsForUnauthorizedDirectExecuteByRandomAddress() (gas: 16023)
 QuarkWalletTest:testSetsMsgSender() (gas: 51350)
 QuarkWalletTest:testSetsMsgSenderDuringDirectExecute() (gas: 45371)
 QuarkWalletTest:testSingleNonceCanCallDifferentScriptsAndCacheNonTargetScript() (gas: 135418)
+QuotecallTest:testInitializeProperlyFromConstructor() (gas: 7213)
+QuotecallTest:testQuotecallForPayWithUSDT() (gas: 126304)
+QuotecallTest:testQuotecallForPayWithWBTC() (gas: 120860)
+QuotecallTest:testQuotecallRevertsWhenCallReverts() (gas: 109497)
+QuotecallTest:testReturnCallResult() (gas: 113327)
+QuotecallTest:testRevertsForInvalidCallContext() (gas: 16792)
+QuotecallTest:testRevertsWhenQuoteTooHigh() (gas: 156348)
+QuotecallTest:testRevertsWhenQuoteTooLow() (gas: 156190)
+QuotecallTest:testSimpleCounterAndPayWithUSDC() (gas: 150274)
+QuotecallTest:testSimpleTransferTokenAndPayWithUSDC() (gas: 145818)
 ReplayableTransactionsTest:testCancelRecurringPurchase() (gas: 271116)
 ReplayableTransactionsTest:testRecurringPurchaseHappyPath() (gas: 211835)
 ReplayableTransactionsTest:testRecurringPurchaseMultiplePurchases() (gas: 370049)

src/quark-core-scripts/src/Paycall.sol

@@ -25,9 +25,6 @@ contract Paycall {
     /// @notice Payment token address
     address public immutable paymentTokenAddress;
 
-    /// @notice Flag for indicating if reverts from the call should be propagated or swallowed
-    bool public immutable propagateReverts;
-
     /// @notice This contract's address
     address internal immutable scriptAddress;
 
@@ -45,12 +42,10 @@ contract Paycall {
      * @notice Constructor
      * @param nativeTokenBasedPriceFeedAddress_ Native token price feed address that follows Chainlink's AggregatorV3Interface correlated to the payment token
      * @param paymentTokenAddress_ Payment token address
-     * @param propagateReverts_ Flag for indicating if reverts from the call should be propagated or swallowed
      */
-    constructor(address nativeTokenBasedPriceFeedAddress_, address paymentTokenAddress_, bool propagateReverts_) {
+    constructor(address nativeTokenBasedPriceFeedAddress_, address paymentTokenAddress_) {
         nativeTokenBasedPriceFeedAddress = nativeTokenBasedPriceFeedAddress_;
         paymentTokenAddress = paymentTokenAddress_;
-        propagateReverts = propagateReverts_;
         scriptAddress = address(this);
 
         divisorScale = 10
@@ -78,7 +73,7 @@ contract Paycall {
         }
 
         (bool success, bytes memory returnData) = callContract.delegatecall(callData);
-        if (!success && propagateReverts) {
+        if (!success) {
             assembly {
                 revert(add(returnData, 32), mload(returnData))
             }

src/quark-core-scripts/src/Quotecall.sol

@@ -28,9 +28,6 @@ contract Quotecall {
     /// @notice The max delta precentage allowed between the quoted cost and actual cost of the call
     uint256 public immutable maxDeltaPercentage;
 
-    /// @notice Flag for indicating if reverts from the call should be propagated or swallowed
-    bool public immutable propagateReverts;
-
     /// @notice This contract's address
     address internal immutable scriptAddress;
 
@@ -52,18 +49,11 @@ contract Quotecall {
      * @param nativeTokenBasedPriceFeedAddress_ Native token based price feed address that follows Chainlink's AggregatorV3Interface correlated to the payment token
      * @param paymentTokenAddress_ Payment token address
      * @param maxDeltaPercentage_ Maximum allowed delta percentage between the quoted cost and actual cost of the call (1e18 = 100%)
-     * @param propagateReverts_ Flag for indicating if reverts from the call should be propagated or swallowed
      */
-    constructor(
-        address nativeTokenBasedPriceFeedAddress_,
-        address paymentTokenAddress_,
-        uint256 maxDeltaPercentage_,
-        bool propagateReverts_
-    ) {
+    constructor(address nativeTokenBasedPriceFeedAddress_, address paymentTokenAddress_, uint256 maxDeltaPercentage_) {
         nativeTokenBasedPriceFeedAddress = nativeTokenBasedPriceFeedAddress_;
         paymentTokenAddress = paymentTokenAddress_;
         maxDeltaPercentage = maxDeltaPercentage_;
-        propagateReverts = propagateReverts_;
         scriptAddress = address(this);
 
         // Note: Assumes the native token has 18 decimals
@@ -92,7 +82,7 @@ contract Quotecall {
         emit PayForGas(address(this), tx.origin, paymentTokenAddress, quotedAmount);
 
         (bool success, bytes memory returnData) = callContract.delegatecall(callData);
-        if (!success && propagateReverts) {
+        if (!success) {
             assembly {
                 revert(add(returnData, 32), mload(returnData))
             }

test/quark-core-scripts/Paycall.t.sol

@@ -57,7 +57,6 @@ contract PaycallTest is Test {
     bytes paycall;
     bytes paycallUSDT;
     bytes paycallWBTC;
-    bytes paycallNonReverty;
 
     bytes legendCometSupplyScript = new YulHelper().getCode("DeFiScripts.sol/CometSupplyActions.json");
 
@@ -71,7 +70,6 @@ contract PaycallTest is Test {
     address paycallAddress;
     address paycallUSDTAddress;
     address paycallWBTCAddress;
-    address paycallNonRevertyAddress;
     address legendCometSupplyScriptAddress;
     address legendCometWithdrawScriptAddress;
     address legendUniswapSwapScriptAddress;
@@ -91,15 +89,13 @@ contract PaycallTest is Test {
         ethcallAddress = codeJar.saveCode(ethcall);
         multicallAddress = codeJar.saveCode(multicall);
         revertsAddress = codeJar.saveCode(reverts);
-        paycall = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC, true));
-        paycallUSDT = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDT, true));
-        paycallWBTC = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_BTC_PRICE_FEED, WBTC, true));
-        paycallNonReverty = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC, false));
+        paycall = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC));
+        paycallUSDT = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDT));
+        paycallWBTC = abi.encodePacked(type(Paycall).creationCode, abi.encode(ETH_BTC_PRICE_FEED, WBTC));
 
         paycallAddress = codeJar.saveCode(paycall);
         paycallUSDTAddress = codeJar.saveCode(paycallUSDT);
         paycallWBTCAddress = codeJar.saveCode(paycallWBTC);
-        paycallNonRevertyAddress = codeJar.saveCode(paycallNonReverty);
 
         legendCometSupplyScriptAddress = codeJar.saveCode(legendCometSupplyScript);
         legendCometWithdrawScriptAddress = codeJar.saveCode(legendCometWithdrawScript);
@@ -111,11 +107,9 @@ contract PaycallTest is Test {
     function testInitializeProperlyFromConstructor() public {
         address storedPriceFeedAddress = Paycall(paycallAddress).nativeTokenBasedPriceFeedAddress();
         address storedPaymentTokenAddress = Paycall(paycallAddress).paymentTokenAddress();
-        bool storedPropagateReverts = Paycall(paycallAddress).propagateReverts();
 
         assertEq(storedPriceFeedAddress, ETH_USD_PRICE_FEED);
         assertEq(storedPaymentTokenAddress, USDC);
-        assertEq(storedPropagateReverts, true);
     }
 
     function testRevertsForInvalidCallContext() public {
@@ -431,7 +425,7 @@ contract PaycallTest is Test {
         assertEq(IERC20(USDC).balanceOf(address(wallet)), 1000e6);
     }
 
-    function testPaycallNonRevertyDoesNotRevertWhenCallReverts() public {
+    function testPaycallRevertsWhenCallReverts() public {
         // gas: do not meter set-up
         vm.pauseGasMetering();
         vm.txGasPrice(32 gwei);
@@ -442,18 +436,16 @@ contract PaycallTest is Test {
         // Execute through paycall
         QuarkWallet.QuarkOperation memory op = new QuarkOperationHelper().newBasicOpWithCalldata(
             wallet,
-            paycallNonReverty,
+            paycall,
             abi.encodeWithSelector(Paycall.run.selector, revertsAddress, "", 20e6),
             ScriptType.ScriptSource
         );
         (uint8 v, bytes32 r, bytes32 s) = new SignatureHelper().signOp(alicePrivateKey, wallet, op);
 
-        // gas: meter execute
         vm.resumeGasMetering();
-        vm.expectEmit(true, true, true, false); // We ignore the amount because it will differ based on via-IR
-        emit PayForGas(address(wallet), tx.origin, USDC, 5_553_259);
+        vm.expectRevert(abi.encodeWithSelector(Reverts.Whoops.selector));
         wallet.executeQuarkOperation(op, v, r, s);
 
-        assertApproxEqAbs(IERC20(USDC).balanceOf(address(wallet)), 995e6, 1e6);
+        assertEq(IERC20(USDC).balanceOf(address(wallet)), 1000e6);
     }
 }

test/quark-core-scripts/Quotecall.t.sol

@@ -58,7 +58,6 @@ contract QuotecallTest is Test {
     bytes quotecall;
     bytes quotecallUSDT;
     bytes quotecallWBTC;
-    bytes quotecallNonReverty;
 
     bytes legendCometSupplyScript = new YulHelper().getCode("DeFiScripts.sol/CometSupplyActions.json");
 
@@ -72,7 +71,6 @@ contract QuotecallTest is Test {
     address quotecallAddress;
     address quotecallUSDTAddress;
     address quotecallWBTCAddress;
-    address quotecallNonRevertyAddress;
     address legendCometSupplyScriptAddress;
     address legendCometWithdrawScriptAddress;
     address legendUniswapSwapScriptAddress;
@@ -93,23 +91,16 @@ contract QuotecallTest is Test {
         multicallAddress = codeJar.saveCode(multicall);
         revertsAddress = codeJar.saveCode(reverts);
 
-        quotecall = abi.encodePacked(
-            type(Quotecall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC, MAX_DELTA_PERCENTAGE, true)
-        );
-        quotecallUSDT = abi.encodePacked(
-            type(Quotecall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDT, MAX_DELTA_PERCENTAGE, true)
-        );
-        quotecallWBTC = abi.encodePacked(
-            type(Quotecall).creationCode, abi.encode(ETH_BTC_PRICE_FEED, WBTC, MAX_DELTA_PERCENTAGE, true)
-        );
-        quotecallNonReverty = abi.encodePacked(
-            type(Quotecall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC, MAX_DELTA_PERCENTAGE, false)
-        );
+        quotecall =
+            abi.encodePacked(type(Quotecall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDC, MAX_DELTA_PERCENTAGE));
+        quotecallUSDT =
+            abi.encodePacked(type(Quotecall).creationCode, abi.encode(ETH_USD_PRICE_FEED, USDT, MAX_DELTA_PERCENTAGE));
+        quotecallWBTC =
+            abi.encodePacked(type(Quotecall).creationCode, abi.encode(ETH_BTC_PRICE_FEED, WBTC, MAX_DELTA_PERCENTAGE));
 
         quotecallAddress = codeJar.saveCode(quotecall);
         quotecallUSDTAddress = codeJar.saveCode(quotecallUSDT);
         quotecallWBTCAddress = codeJar.saveCode(quotecallWBTC);
-        quotecallNonRevertyAddress = codeJar.saveCode(quotecallNonReverty);
 
         legendCometSupplyScriptAddress = codeJar.saveCode(legendCometSupplyScript);
         legendCometWithdrawScriptAddress = codeJar.saveCode(legendCometWithdrawScript);
@@ -122,12 +113,10 @@ contract QuotecallTest is Test {
         address storedPriceFeedAddress = Quotecall(quotecallAddress).nativeTokenBasedPriceFeedAddress();
         address storedPaymentTokenAddress = Quotecall(quotecallAddress).paymentTokenAddress();
         uint256 storedMaxDeltaPercentage = Quotecall(quotecallAddress).maxDeltaPercentage();
-        bool storedPropagateReverts = Quotecall(quotecallAddress).propagateReverts();
 
         assertEq(storedPriceFeedAddress, ETH_USD_PRICE_FEED);
         assertEq(storedPaymentTokenAddress, USDC);
         assertEq(storedMaxDeltaPercentage, MAX_DELTA_PERCENTAGE);
-        assertEq(storedPropagateReverts, true);
     }
 
     function testRevertsForInvalidCallContext() public {
@@ -416,7 +405,7 @@ contract QuotecallTest is Test {
         assertEq(IERC20(USDC).balanceOf(address(wallet)), 1000e6);
     }
 
-    function testQuotecallNonRevertyDoesNotRevertWhenCallReverts() public {
+    function testQuotecallRevertsWhenCallReverts() public {
         // gas: do not meter set-up
         vm.pauseGasMetering();
         vm.txGasPrice(32 gwei);
@@ -427,18 +416,17 @@ contract QuotecallTest is Test {
         // Execute through quotecall
         QuarkWallet.QuarkOperation memory op = new QuarkOperationHelper().newBasicOpWithCalldata(
             wallet,
-            quotecallNonReverty,
+            quotecall,
             abi.encodeWithSelector(Quotecall.run.selector, revertsAddress, "", 8e6),
             ScriptType.ScriptSource
         );
         (uint8 v, bytes32 r, bytes32 s) = new SignatureHelper().signOp(alicePrivateKey, wallet, op);
 
         // gas: meter execute
         vm.resumeGasMetering();
-        vm.expectEmit(true, true, true, true);
-        emit PayForGas(address(wallet), tx.origin, USDC, 8e6);
+        vm.expectRevert(abi.encodeWithSelector(Reverts.Whoops.selector));
         wallet.executeQuarkOperation(op, v, r, s);
 
-        assertEq(IERC20(USDC).balanceOf(address(wallet)), 992e6);
+        assertEq(IERC20(USDC).balanceOf(address(wallet)), 1000e6);
     }
 }