Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: Handle cancellations: Reduce errors and logs #52

Merged
merged 2 commits into from
Aug 21, 2023

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Aug 20, 2023

This PR aims to handle cancelations of smart orders better.

Why

Currently, a big a mount of the errors in the logs comes from Smart Order cancelations

The error handling of getTradeableOrderWithSignature will try to guess if the issue with the CALL was because of the auth:

case "SingleOrderNotAuthed":

The problem is that sometimes (at least in Gnosis Chain), the EVM error will not give you this nice debug info, so basically you don't know why it failed.

The Problem

If we are not sure about the problem, we can't remove the order from the pending orders, so we will keep getting this error over and over for every block.

Additional problem

Debugging Smart Orders take a lot of time, and there are too many things that can be wrong. If we need to debug these situations manually, it will be very time consuming

This PR proposal

Before doing the getTradeableOrderWithSignature CALL, we check for singleOrders flow if the order is authorised or not.

If its not authorised, we remove the order from the pending orders, and continue (we don't consider this an error, this is part of the normal flow on cancelations)

Example

This Order was failing over and over:

As you can see in the history:
https://app.safe.global/transactions/history?safe=gno:0xe60d513f367326c70f447875984940ccf9348b3c

This order was created in NONCE 0 (https://gnosisscan.io/tx/0xcf62a4ff38ae24015269c41dbf3b9f70e3b4ac711d7a08c4b001f412f48e3ba5/), and canceled in NONCE 1 (https://gnosisscan.io/tx/0xf494ac79b5f7cb4fce8e6298f6ac6c6c73295ffb90e105914a6402934812dff0/)

@anxolin anxolin changed the title Handle cancellations Handle cancellations / Reduce errors / Reduce logs Aug 20, 2023
@anxolin anxolin changed the title Handle cancellations / Reduce errors / Reduce logs Handle cancellations: Reduce errors and logs Aug 20, 2023
@anxolin anxolin changed the title Handle cancellations: Reduce errors and logs feature: Handle cancellations: Reduce errors and logs Aug 20, 2023
@anxolin anxolin requested review from mfw78 and a team August 20, 2023 13:14
@anxolin anxolin force-pushed the handle-cancellations branch from 96e45a7 to 40c6ff9 Compare August 20, 2023 13:14
@anxolin anxolin changed the base branch from deploy-to-dev-and-prod to main August 20, 2023 13:15
@anxolin anxolin force-pushed the handle-cancellations branch from 40c6ff9 to 1df2426 Compare August 20, 2023 13:17
mfw78
mfw78 previously approved these changes Aug 20, 2023
Copy link
Contributor

@mfw78 mfw78 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! Minor comments.

actions/utils.ts Show resolved Hide resolved
actions/utils.ts Outdated Show resolved Hide resolved
@anxolin anxolin merged commit 9111d29 into main Aug 21, 2023
@mfw78 mfw78 deleted the handle-cancellations branch September 2, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants