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

bug: Inconsistency in Order not Valid semantics #107

Closed
3 tasks
anxolin opened this issue Oct 11, 2023 · 1 comment
Closed
3 tasks

bug: Inconsistency in Order not Valid semantics #107

anxolin opened this issue Oct 11, 2023 · 1 comment
Labels
blocked This issue is blocked by some other work bug Something isn't working

Comments

@anxolin
Copy link
Contributor

anxolin commented Oct 11, 2023

Problem

Currently, we re-try on every block for INVALID ORDERS, however this is incoherent with how we handle the error in the SDK

See here: https://github.com/cowprotocol/cow-sdk/blob/main/src/composable/ConditionalOrder.ts#L260

And the TEST: https://github.com/cowprotocol/cow-sdk/blob/main/src/composable/ConditionalOrder.spec.ts#L246

Here we have a bunch of validations we do for TWAPs, where isValid means, the order has some parameters that are wrong, so there's no point to poll in the future: https://github.com/cowprotocol/cow-sdk/blob/main/src/composable/orderTypes/Twap.spec.ts#L135

For this reason, I created this PR to make sure we stop watching the order #105

Proposal

The proposal is to make it more explicit in the doc that watch tower will stop indexing for any of the below errors except POLL_TRY_NEXT_BLOCK, POLL_TRY_AT_BLOCK or POLL_TRY_AT_EPOCH

# Watch Tower will check at the instructed time
POLL_TRY_NEXT_BLOCK: PollTryNextBlock(string)
POLL_TRY_AT_BLOCK: PollTryAtBlock(uint256,string)
POLL_TRY_AT_EPOCH: PollTryAtEpoch(uint256,string)

# Watch Tower will stop polling the order for future blocks
POLL_NEVER: PollNever(string)

# Effectivelly same as `POLL_NEVER` but with specific semantics on what's the reason to stop polling
PROOF_NOT_AUTHED: ProofNotAuthed()
SINGLE_ORDER_NOT_AUTHED: SingleOrderNotAuthed()
SWAP_GUARD_RESTRICTED: SwapGuardRestricted()
INVALID_HANDLER: InvalidHandler()
INVALID_FALLBACK_HANDLER: InvalidFallbackHandler()
INTERFACE_NOT_SUPPORTED: InterfaceNotSupported()
ORDER_NOT_VALID: OrderNotValid(string) 

Therefore, in particular ORDER_NOT_VALID should translate to a POLL_NEVER

🚨 Blocker: We can't make it coherent right now

Currently, there's an order for safe 0xAB8E66516e189ECf57415562183584659A814e4b which is meant to be checked in every block, but it raises the error ORDER_NOT_VALID instead of POLL_TRY_NEXT_BLOCK.

TO DO:

  • We could add an exception for this order, or ask nicely to the dev to use POLL_TRY_NEXT_BLOCK if that's the behaviour they seek
  • Document nicely so DEVs have the above mentioned behaviour clear
  • Merge fix: drop invalid orders #105
@anxolin anxolin added the bug Something isn't working label Oct 11, 2023
@mfw78 mfw78 added the blocked This issue is blocked by some other work label Oct 11, 2023
@mfw78
Copy link
Contributor

mfw78 commented Jan 25, 2024

Can this be closed given the new documentation on polling?

@anxolin anxolin closed this as completed Jan 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked This issue is blocked by some other work bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants