From f2280e62c08f0dea08d23a09dd37e6b52b8d0ffb Mon Sep 17 00:00:00 2001 From: Anxo Rodriguez Date: Fri, 18 Aug 2023 08:48:47 +0100 Subject: [PATCH 1/5] Don't fail if simulation fails --- actions/checkForAndPlaceOrder.ts | 44 +++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/actions/checkForAndPlaceOrder.ts b/actions/checkForAndPlaceOrder.ts index a7b8c83..b5067c2 100644 --- a/actions/checkForAndPlaceOrder.ts +++ b/actions/checkForAndPlaceOrder.ts @@ -19,6 +19,7 @@ import { writeRegistry, } from "./utils"; import { ChainContext, ConditionalOrder, OrderStatus } from "./model"; +import { GPv2Order } from "./types/ComposableCoW"; const GPV2SETTLEMENT = "0x9008D19f58AAbD9eD0D60971565AA8510560ab41"; @@ -115,12 +116,19 @@ async function _processConditionalOrder( ): Promise<{ deleteConditionalOrder: boolean; error: boolean }> { let error = false; try { - const { order, signature } = await _getTradeableOrderWithSignature( + const result = await _getTradeableOrderWithSignature( owner, conditionalOrder, contract ); + if (!result.success) { + // The simulation failed, this only means the order is not ready to be placed in the orderbook. Will check again in the next block + return { error: false, deleteConditionalOrder: false }; + } + + const { order, signature } = result.data; + const orderToSubmit: Order = { ...order, kind: kindToString(order.kind), @@ -299,11 +307,21 @@ function _handleOrderBookError( return { shouldThrow: true }; } +type GetTradeableOrderWithSignatureResult = + | { + success: true; + data: { + order: GPv2Order.DataStructOutput; + signature: string; + }; + } + | { success: false; error: any }; + async function _getTradeableOrderWithSignature( owner: string, conditionalOrder: ConditionalOrder, contract: ComposableCoW -) { +): Promise { const proof = conditionalOrder.proof ? conditionalOrder.proof.path : []; const offchainInput = "0x"; const { to, data } = @@ -319,20 +337,22 @@ async function _getTradeableOrderWithSignature( data, }); - return contract.callStatic - .getTradeableOrderWithSignature( + try { + const data = await contract.callStatic.getTradeableOrderWithSignature( owner, conditionalOrder.params, offchainInput, proof - ) - .catch((e) => { - console.error( - '[getTradeableOrderWithSignature] Error during "getTradeableOrderWithSignature" call: ', - e - ); - throw e; - }); + ); + + return { success: true, data }; + } catch (error) { + console.error( + '[getTradeableOrderWithSignature] "getTradeableOrderWithSignature" failed. Order might not be ready to be placed in the orderbook yet', + error + ); + return { success: false, error }; + } } /** From b68dfd65d0b5be24f0da6a73d21c25d608f23d11 Mon Sep 17 00:00:00 2001 From: Anxo Rodriguez Date: Fri, 18 Aug 2023 12:17:10 +0100 Subject: [PATCH 2/5] Improve handling of CALLS --- actions/checkForAndPlaceOrder.ts | 131 +++++++++++++++++++++---------- 1 file changed, 90 insertions(+), 41 deletions(-) diff --git a/actions/checkForAndPlaceOrder.ts b/actions/checkForAndPlaceOrder.ts index b5067c2..4d7c93a 100644 --- a/actions/checkForAndPlaceOrder.ts +++ b/actions/checkForAndPlaceOrder.ts @@ -116,18 +116,22 @@ async function _processConditionalOrder( ): Promise<{ deleteConditionalOrder: boolean; error: boolean }> { let error = false; try { - const result = await _getTradeableOrderWithSignature( + const tradableOrderResult = await _getTradeableOrderWithSignature( owner, conditionalOrder, contract ); - if (!result.success) { - // The simulation failed, this only means the order is not ready to be placed in the orderbook. Will check again in the next block - return { error: false, deleteConditionalOrder: false }; + // Return early if the simulation fails + if (tradableOrderResult.result != CallResult.Success) { + const { deleteConditionalOrder, result } = tradableOrderResult; + return { + error: result !== CallResult.FailedButIsExpected, // If we expected the call to fail, then we don't consider it an error + deleteConditionalOrder, + }; } - const { order, signature } = result.data; + const { order, signature } = tradableOrderResult.data; const orderToSubmit: Order = { ...order, @@ -157,26 +161,6 @@ async function _processConditionalOrder( ); } } catch (e: any) { - error = true; - if (e.code === Logger.errors.CALL_EXCEPTION) { - switch (e.errorName) { - case "OrderNotValid": - // The conditional order has not expired, or been cancelled, but the order is not valid - // For example, with TWAPs, this may be after `span` seconds have passed in the epoch. - return { deleteConditionalOrder: false, error }; - case "SingleOrderNotAuthed": - console.log( - `Single order on safe ${owner} not authed. Unfilled orders:` - ); - case "ProofNotAuthed": - console.log(`Proof on safe ${owner} not authed. Unfilled orders:`); - } - _printUnfilledOrders(conditionalOrder.orders); - console.log( - "Removing conditional order from registry due to CALL_EXCEPTION" - ); - return { deleteConditionalOrder: true, error }; - } console.error(`Unexpected error while processing order:`, e); } @@ -202,10 +186,9 @@ function _getOrderUid(network: string, orderToSubmit: Order, owner: string) { ); } -// --- Helpers --- - /** * Print a list of all the orders that were placed and not filled + * * @param orders All the orders that are being tracked */ export const _printUnfilledOrders = (orders: Map) => { @@ -308,14 +291,28 @@ function _handleOrderBookError( } type GetTradeableOrderWithSignatureResult = - | { - success: true; - data: { - order: GPv2Order.DataStructOutput; - signature: string; - }; - } - | { success: false; error: any }; + | GetTradeableOrderWithSignatureSuccess + | GetTradeableOrderWithSignatureError; + +enum CallResult { + Success, + Failed, + FailedButIsExpected, +} + +type GetTradeableOrderWithSignatureSuccess = { + result: CallResult.Success; + deleteConditionalOrder: boolean; + data: { + order: GPv2Order.DataStructOutput; + signature: string; + }; +}; +type GetTradeableOrderWithSignatureError = { + result: CallResult.Failed | CallResult.FailedButIsExpected; + deleteConditionalOrder: boolean; + errorObj: any; +}; async function _getTradeableOrderWithSignature( owner: string, @@ -345,16 +342,68 @@ async function _getTradeableOrderWithSignature( proof ); - return { success: true, data }; - } catch (error) { - console.error( - '[getTradeableOrderWithSignature] "getTradeableOrderWithSignature" failed. Order might not be ready to be placed in the orderbook yet', - error + return { result: CallResult.Success, deleteConditionalOrder: false, data }; + } catch (error: any) { + // Print and handle the error + // We need to decide if the error is final or not (if a re-attempt might help). If it doesn't, we delete the order + const { result, deleteConditionalOrder } = _handleGetTradableOrderCall( + error, + owner ); - return { success: false, error }; + return { + result, + deleteConditionalOrder, + errorObj: error, + }; } } +function _handleGetTradableOrderCall( + error: any, + owner: string +): { + result: CallResult.Failed | CallResult.FailedButIsExpected; + deleteConditionalOrder: boolean; +} { + if (error.code === Logger.errors.CALL_EXCEPTION) { + const errorMessagePrefix = + "[getTradeableOrderWithSignature] Call Exception"; + switch (error.errorName) { + case "OrderNotValid": + // The conditional order has not expired, or been cancelled, but the order is not valid + // For example, with TWAPs, this may be after `span` seconds have passed in the epoch. + return { + result: CallResult.FailedButIsExpected, + deleteConditionalOrder: false, + }; + case "SingleOrderNotAuthed": + // If there's no autorization we delete the order + // for now it doesn't support more advance cases where the order is auth during a pre-interaction + + console.error( + `${errorMessagePrefix}: Single order on safe ${owner} not authed. Deleting order...` + ); + return { result: CallResult.Failed, deleteConditionalOrder: true }; + case "ProofNotAuthed": + // If there's no autorization we delete the order + // for now it doesn't support more advance cases where the order is auth during a pre-interaction + + console.error( + `${errorMessagePrefix}: Proof on safe ${owner} not authed. Deleting order...` + ); + return { result: CallResult.Failed, deleteConditionalOrder: true }; + } + + console.error(errorMessagePrefix + " for unexpected reasons", error); + // If we don't know the reason, is better to not delete the order + return { result: CallResult.Failed, deleteConditionalOrder: false }; + } + + console.error("[getTradeableOrderWithSignature] Unexpected error", error); + // If we don't know the reason, is better to not delete the order + return { result: CallResult.Failed, deleteConditionalOrder: false }; +} + /** * Convert an order kind hash to a string * @param kind of order in hash format From c38a26c7c3b67d89af915b90856e1220eec1af89 Mon Sep 17 00:00:00 2001 From: Anxo Rodriguez Date: Fri, 18 Aug 2023 13:36:41 +0100 Subject: [PATCH 3/5] Do not consider error if the order has no auth --- actions/checkForAndPlaceOrder.ts | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/actions/checkForAndPlaceOrder.ts b/actions/checkForAndPlaceOrder.ts index 4d7c93a..860c37a 100644 --- a/actions/checkForAndPlaceOrder.ts +++ b/actions/checkForAndPlaceOrder.ts @@ -378,20 +378,28 @@ function _handleGetTradableOrderCall( }; case "SingleOrderNotAuthed": // If there's no autorization we delete the order - // for now it doesn't support more advance cases where the order is auth during a pre-interaction + // - One reason could be, because the user CACTIONed the order + // - for now it doesn't support more advance cases where the order is auth during a pre-interaction - console.error( + console.info( `${errorMessagePrefix}: Single order on safe ${owner} not authed. Deleting order...` ); - return { result: CallResult.Failed, deleteConditionalOrder: true }; + return { + result: CallResult.FailedButIsExpected, + deleteConditionalOrder: true, + }; case "ProofNotAuthed": // If there's no autorization we delete the order - // for now it doesn't support more advance cases where the order is auth during a pre-interaction + // - One reason could be, because the user CACTIONed the order + // - for now it doesn't support more advance cases where the order is auth during a pre-interaction - console.error( + console.info( `${errorMessagePrefix}: Proof on safe ${owner} not authed. Deleting order...` ); - return { result: CallResult.Failed, deleteConditionalOrder: true }; + return { + result: CallResult.FailedButIsExpected, + deleteConditionalOrder: true, + }; } console.error(errorMessagePrefix + " for unexpected reasons", error); From d4ab332a77e10e7eda1a7cd478ae16aa3138d8a8 Mon Sep 17 00:00:00 2001 From: Anxo Rodriguez Date: Fri, 18 Aug 2023 13:40:40 +0100 Subject: [PATCH 4/5] Fix typo --- actions/checkForAndPlaceOrder.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actions/checkForAndPlaceOrder.ts b/actions/checkForAndPlaceOrder.ts index 860c37a..0a66bb5 100644 --- a/actions/checkForAndPlaceOrder.ts +++ b/actions/checkForAndPlaceOrder.ts @@ -378,7 +378,7 @@ function _handleGetTradableOrderCall( }; case "SingleOrderNotAuthed": // If there's no autorization we delete the order - // - One reason could be, because the user CACTIONed the order + // - One reason could be, because the user CANCELLED the order // - for now it doesn't support more advance cases where the order is auth during a pre-interaction console.info( @@ -390,7 +390,7 @@ function _handleGetTradableOrderCall( }; case "ProofNotAuthed": // If there's no autorization we delete the order - // - One reason could be, because the user CACTIONed the order + // - One reason could be, because the user CANCELLED the order // - for now it doesn't support more advance cases where the order is auth during a pre-interaction console.info( From c2901f9de19f975faa39d946ee153a9d456d6d40 Mon Sep 17 00:00:00 2001 From: Anxo Rodriguez Date: Sat, 19 Aug 2023 21:35:04 +0100 Subject: [PATCH 5/5] Correct typos Co-authored-by: mfw78 <53399572+mfw78@users.noreply.github.com> --- actions/checkForAndPlaceOrder.ts | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/actions/checkForAndPlaceOrder.ts b/actions/checkForAndPlaceOrder.ts index 0a66bb5..dc9dd50 100644 --- a/actions/checkForAndPlaceOrder.ts +++ b/actions/checkForAndPlaceOrder.ts @@ -116,22 +116,22 @@ async function _processConditionalOrder( ): Promise<{ deleteConditionalOrder: boolean; error: boolean }> { let error = false; try { - const tradableOrderResult = await _getTradeableOrderWithSignature( + const tradeableOrderResult = await _getTradeableOrderWithSignature( owner, conditionalOrder, contract ); // Return early if the simulation fails - if (tradableOrderResult.result != CallResult.Success) { - const { deleteConditionalOrder, result } = tradableOrderResult; + if (tradeableOrderResult.result != CallResult.Success) { + const { deleteConditionalOrder, result } = tradeableOrderResult; return { error: result !== CallResult.FailedButIsExpected, // If we expected the call to fail, then we don't consider it an error deleteConditionalOrder, }; } - const { order, signature } = tradableOrderResult.data; + const { order, signature } = tradeableOrderResult.data; const orderToSubmit: Order = { ...order, @@ -377,9 +377,9 @@ function _handleGetTradableOrderCall( deleteConditionalOrder: false, }; case "SingleOrderNotAuthed": - // If there's no autorization we delete the order + // If there's no authorization we delete the order // - One reason could be, because the user CANCELLED the order - // - for now it doesn't support more advance cases where the order is auth during a pre-interaction + // - for now it doesn't support more advanced cases where the order is auth during a pre-interaction console.info( `${errorMessagePrefix}: Single order on safe ${owner} not authed. Deleting order...` @@ -389,9 +389,9 @@ function _handleGetTradableOrderCall( deleteConditionalOrder: true, }; case "ProofNotAuthed": - // If there's no autorization we delete the order + // If there's no authorization we delete the order // - One reason could be, because the user CANCELLED the order - // - for now it doesn't support more advance cases where the order is auth during a pre-interaction + // - for now it doesn't support more advanced cases where the order is auth during a pre-interaction console.info( `${errorMessagePrefix}: Proof on safe ${owner} not authed. Deleting order...`