From 7c815d4a75ba0368aca29f1f269c90520968705e Mon Sep 17 00:00:00 2001 From: Pedro Yves Fracari <55461956+yvesfracari@users.noreply.github.com> Date: Mon, 5 Aug 2024 08:32:11 -0300 Subject: [PATCH] Refactor handler errors to try on next block watch tower pattern (#87) # Description Refactor Stop Loss, Trade Above Threshold, and Good After Time errors to watch tower try again pattern. # Changes - [x] Add `revertPollAtNextBlock`method on `ConditionalOrdersUtilsLib` - [x] Stop Loss Refactoring - [x] Trade Above Threshold Refactoring - [x] Good After Time Refactoring ## How to test 1. Run modified test files 2. Stop Loss Deployment on sepolia: `0x410c267baf50b36475B30051Da2aE734f6726F01` ## Related Issues - Fix #85 --------- Co-authored-by: mfw78 --- src/interfaces/IConditionalOrder.sol | 11 +++ src/types/GoodAfterTime.sol | 4 +- src/types/StopLoss.sol | 4 +- src/types/TradeAboveThreshold.sol | 2 +- test/ComposableCoW.gat.t.sol | 16 ++++- test/ComposableCoW.stoploss.t.sol | 21 +++++- test/ComposableCoW.tat.t.sol | 103 +++++++++++++++++++++++++++ 7 files changed, 150 insertions(+), 11 deletions(-) create mode 100644 test/ComposableCoW.tat.t.sol diff --git a/src/interfaces/IConditionalOrder.sol b/src/interfaces/IConditionalOrder.sol index 75f9067..c3b789c 100644 --- a/src/interfaces/IConditionalOrder.sol +++ b/src/interfaces/IConditionalOrder.sol @@ -10,10 +10,21 @@ import {IERC165} from "safe/interfaces/IERC165.sol"; * @author CoW Protocol Developers + mfw78 */ interface IConditionalOrder { + /// @dev This error is returned by the `getTradeableOrder` function if the order condition is not met. /// A parameter of `string` type is included to allow the caller to specify the reason for the failure. error OrderNotValid(string); + // --- errors specific for polling + // Signal to a watch tower that polling should be attempted again. + error PollTryNextBlock(string reason); + // Signal to a watch tower that polling should be attempted again at a specific block number. + error PollTryAtBlock(uint256 blockNumber, string reason); + // Signal to a watch tower that polling should be attempted again at a specific epoch (unix timestamp). + error PollTryAtEpoch(uint256 timestamp, string reason); + // Signal to a watch tower that the conditional order should not be polled again (delete). + error PollNever(string reason); + /** * @dev This struct is used to uniquely identify a conditional order for an owner. * H(handler || salt || staticInput) **MUST** be unique for an owner. diff --git a/src/types/GoodAfterTime.sol b/src/types/GoodAfterTime.sol index 47f0bfa..abae962 100644 --- a/src/types/GoodAfterTime.sol +++ b/src/types/GoodAfterTime.sol @@ -62,7 +62,7 @@ contract GoodAfterTime is BaseConditionalOrder { // Don't allow the order to be placed before it becomes valid. if (!(block.timestamp >= data.startTime)) { - revert IConditionalOrder.OrderNotValid(TOO_EARLY); + revert IConditionalOrder.PollTryAtEpoch(data.startTime, TOO_EARLY); } // Require that the sell token balance is above the minimum. @@ -82,7 +82,7 @@ contract GoodAfterTime is BaseConditionalOrder { // Don't allow the order to be placed if the buyAmount is less than the minimum out. if (!(buyAmount >= (_expectedOut * (Utils.MAX_BPS - p.allowedSlippage)) / Utils.MAX_BPS)) { - revert IConditionalOrder.OrderNotValid(PRICE_CHECKER_FAILED); + revert IConditionalOrder.PollTryNextBlock(PRICE_CHECKER_FAILED); } } diff --git a/src/types/StopLoss.sol b/src/types/StopLoss.sol index 2044949..d3a4d66 100644 --- a/src/types/StopLoss.sol +++ b/src/types/StopLoss.sol @@ -80,7 +80,7 @@ contract StopLoss is BaseConditionalOrder { && buyUpdatedAt >= block.timestamp - data.maxTimeSinceLastOracleUpdate ) ) { - revert IConditionalOrder.OrderNotValid(ORACLE_STALE_PRICE); + revert IConditionalOrder.PollTryNextBlock(ORACLE_STALE_PRICE); } // Normalize the decimals for basePrice and quotePrice, scaling them to 18 decimals @@ -90,7 +90,7 @@ contract StopLoss is BaseConditionalOrder { /// @dev Scale the strike price to 18 decimals. if (!(basePrice * SCALING_FACTOR / quotePrice <= data.strike)) { - revert IConditionalOrder.OrderNotValid(STRIKE_NOT_REACHED); + revert IConditionalOrder.PollTryNextBlock(STRIKE_NOT_REACHED); } } diff --git a/src/types/TradeAboveThreshold.sol b/src/types/TradeAboveThreshold.sol index f0d9e57..df815e5 100644 --- a/src/types/TradeAboveThreshold.sol +++ b/src/types/TradeAboveThreshold.sol @@ -45,7 +45,7 @@ contract TradeAboveThreshold is BaseConditionalOrder { uint256 balance = data.sellToken.balanceOf(owner); // Don't allow the order to be placed if the balance is less than the threshold. if (!(balance >= data.threshold)) { - revert IConditionalOrder.OrderNotValid(BALANCE_INSUFFICIENT); + revert IConditionalOrder.PollTryNextBlock(BALANCE_INSUFFICIENT); } // ensures that orders queried shortly after one another result in the same hash (to avoid spamming the orderbook) order = GPv2Order.Data( diff --git a/test/ComposableCoW.gat.t.sol b/test/ComposableCoW.gat.t.sol index 55e011c..6aec0e8 100644 --- a/test/ComposableCoW.gat.t.sol +++ b/test/ComposableCoW.gat.t.sol @@ -54,7 +54,7 @@ contract ComposableCoWGatTest is BaseComposableCoWTest { vm.warp(currentTime); // should revert when the current time is before the start time - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, TOO_EARLY)); + vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.PollTryAtEpoch.selector, startTime, TOO_EARLY)); gat.getTradeableOrder(address(safe1), address(0), bytes32(0), abi.encode(o), abi.encode(uint256(1e18))); } @@ -75,7 +75,12 @@ contract ComposableCoWGatTest is BaseComposableCoWTest { deal(address(o.sellToken), address(safe1), currentBalance); // should revert when the current balance is below the minimum balance - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, BALANCE_INSUFFICIENT)); + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.OrderNotValid.selector, + BALANCE_INSUFFICIENT + ) + ); gat.getTradeableOrder(address(safe1), address(0), bytes32(0), abi.encode(o), abi.encode(uint256(1e18))); } @@ -105,7 +110,12 @@ contract ComposableCoWGatTest is BaseComposableCoWTest { // set the current balance deal(address(o.sellToken), address(safe1), o.minSellBalance); - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, PRICE_CHECKER_FAILED)); + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.PollTryNextBlock.selector, + PRICE_CHECKER_FAILED + ) + ); gat.getTradeableOrder(address(safe1), address(0), bytes32(0), abi.encode(o), abi.encode(buyAmount)); } diff --git a/test/ComposableCoW.stoploss.t.sol b/test/ComposableCoW.stoploss.t.sol index cd0488a..5780eb5 100644 --- a/test/ComposableCoW.stoploss.t.sol +++ b/test/ComposableCoW.stoploss.t.sol @@ -63,7 +63,12 @@ contract ComposableCoWStopLossTest is BaseComposableCoWTest { createOrder(stopLoss, 0x0, abi.encode(data)); - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, STRIKE_NOT_REACHED)); + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.PollTryNextBlock.selector, + STRIKE_NOT_REACHED + ) + ); stopLoss.getTradeableOrder(safe, address(0), bytes32(0), abi.encode(data), bytes("")); } @@ -98,7 +103,12 @@ contract ComposableCoWStopLossTest is BaseComposableCoWTest { maxTimeSinceLastOracleUpdate: staleTime }); - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, STRIKE_NOT_REACHED)); + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.PollTryNextBlock.selector, + STRIKE_NOT_REACHED + ) + ); stopLoss.getTradeableOrder(safe, address(0), bytes32(0), abi.encode(data), bytes("")); } @@ -228,7 +238,12 @@ contract ComposableCoWStopLossTest is BaseComposableCoWTest { maxTimeSinceLastOracleUpdate: maxTimeSinceLastOracleUpdate }); - vm.expectRevert(abi.encodeWithSelector(IConditionalOrder.OrderNotValid.selector, ORACLE_STALE_PRICE)); + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.PollTryNextBlock.selector, + ORACLE_STALE_PRICE + ) + ); stopLoss.getTradeableOrder(safe, address(0), bytes32(0), abi.encode(data), bytes("")); } diff --git a/test/ComposableCoW.tat.t.sol b/test/ComposableCoW.tat.t.sol new file mode 100644 index 0000000..650f6fd --- /dev/null +++ b/test/ComposableCoW.tat.t.sol @@ -0,0 +1,103 @@ +// SPDX-License-Identifier: GPL-3.0 +pragma solidity >=0.8.0 <0.9.0; + +import {ERC1271} from "safe/handler/extensible/SignatureVerifierMuxer.sol"; + +import "./ComposableCoW.base.t.sol"; + +import "../src/types/TradeAboveThreshold.sol"; +import {ConditionalOrdersUtilsLib as Utils} from "../src/types/ConditionalOrdersUtilsLib.sol"; + +contract ComposableCoWTatTest is BaseComposableCoWTest { + using ComposableCoWLib for IConditionalOrder.ConditionalOrderParams[]; + using SafeLib for Safe; + + TradeAboveThreshold tat; + + function setUp() public virtual override(BaseComposableCoWTest) { + super.setUp(); + + // deploy the TAT handler + tat = new TradeAboveThreshold(); + } + + /** + * @dev Fuzz test revert on balance too low + */ + function test_getTradeableOrder_FuzzRevertBelowThreshold(uint256 currentBalance, uint256 threshold) public { + // Revert when the current balance is below the minimum balance + vm.assume(currentBalance < threshold); + + TradeAboveThreshold.Data memory o = _tatTest(); + o.threshold = threshold; + + // set the current balance + deal(address(o.sellToken), address(safe1), currentBalance); + + // should revert when the current balance is below the minimum balance + vm.expectRevert( + abi.encodeWithSelector( + IConditionalOrder.PollTryNextBlock.selector, + BALANCE_INSUFFICIENT + ) + ); + tat.getTradeableOrder(address(safe1), address(0), bytes32(0), abi.encode(o), bytes("")); + } + + function test_BalanceMet_fuzz( + address receiver, + uint256 threshold, + bytes32 appData, + uint256 currentBalance + ) public { + vm.assume(threshold > 0); + vm.assume(currentBalance >= threshold); + + // Use same time data from stop loss test + vm.warp(1687718451); + + TradeAboveThreshold.Data memory data = TradeAboveThreshold.Data({ + sellToken: token0, + buyToken: token1, + receiver: receiver, + validityBucketSeconds: 15 minutes, + threshold: threshold, + appData: appData + }); + + + // // set the current balance + deal(address(token0), address(safe1), currentBalance); + + // This should not revert + GPv2Order.Data memory order = + tat.getTradeableOrder(address(safe1), address(0), bytes32(0), abi.encode(data), bytes("")); + + + assertEq(address(order.sellToken), address(token0)); + assertEq(address(order.buyToken), address(token1)); + assertEq(order.sellAmount, currentBalance); + assertEq(order.buyAmount, 1); + assertEq(order.receiver, receiver); + assertEq(order.validTo, 1687718700); + assertEq(order.appData, appData); + assertEq(order.feeAmount, 0); + assertEq(order.kind, GPv2Order.KIND_SELL); + assertEq(order.partiallyFillable, false); + assertEq(order.sellTokenBalance, GPv2Order.BALANCE_ERC20); + assertEq(order.buyTokenBalance, GPv2Order.BALANCE_ERC20); + } + + // --- Helper functions --- + + function _tatTest() internal view returns (TradeAboveThreshold.Data memory) { + return TradeAboveThreshold.Data({ + sellToken: token0, + buyToken: token1, + receiver: address(0), + validityBucketSeconds: 15 minutes, + threshold: 200e18, + appData: keccak256("TradeAboveThreshold") + }); + } +}