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

Allow to create filtering policy rules by conditional order id or transaction hashes #146

Merged
merged 5 commits into from
Feb 14, 2024

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Feb 14, 2024

Description

It's possible that we need to quickly ignore one specific order, but we don't want to neither ignore the whole handler (i.e. all TWAP orders), nor a whole user.

For this reason, I included in this PR support for creating rules to ignore or drop orders by ID.

Test

One way to test this is to check how, current version of watch tower shows a ton of errors and can't process blocks in Sepolia with this config:

{
  "networks": [
    {
      "name": "sepolia",
      "rpc": "...",
      "deploymentBlock": 5072748,
      "filterPolicy": {
        "defaultAction": "DROP",
        "handlers": {
          "0x44569Cbd4E10dd5e97293337964Eff32d58ed352": "ACCEPT",
          "0x519BA24e959E33b3B6220CA98bd353d8c2D89920": "ACCEPT",
          "0x6cF1e9cA41f7611dEf408122793c358a3d11E5a5": "ACCEPT",
          "0xd3338f21c89745e46af56aeaf553cf96ba9bc66f": "ACCEPT",
          "0xE8212F30C28B4AAB467DF3725C14d6e89C2eB967": "ACCEPT"
        }
      },
      "watchdogTimeout": 120
    }
  ]
}

However, with this config it works:

{
  "networks": [
    {
      "name": "sepolia",
      "rpc": "...",
      "deploymentBlock": 5072748,
      "filterPolicy": {
        "defaultAction": "DROP",
        "conditionalOrderIds": {
          "0x5b3cdb6ffa3c95507cbfc459162609007865c2e87340312d3cd469c4ffbfae81": "SKIP"
        },
        "handlers": {
          "0x44569Cbd4E10dd5e97293337964Eff32d58ed352": "ACCEPT",
          "0x519BA24e959E33b3B6220CA98bd353d8c2D89920": "ACCEPT",
          "0x6cF1e9cA41f7611dEf408122793c358a3d11E5a5": "ACCEPT",
          "0xd3338f21c89745e46af56aeaf553cf96ba9bc66f": "ACCEPT",
          "0xE8212F30C28B4AAB467DF3725C14d6e89C2eB967": "ACCEPT"
        }
      },
      "watchdogTimeout": 120
    }
  ]
}
image

@anxolin anxolin requested a review from mfw78 February 14, 2024 16:50
@anxolin anxolin changed the base branch from main to add-order-id February 14, 2024 16:56
@anxolin anxolin force-pushed the add-id-and-txs-to-filter branch 2 times, most recently from 3b56f1f to af3c3c8 Compare February 14, 2024 17:05
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.

Minor nits. Additionally, I'm not sure why we are introducing different variable naming within this PR (which seems orthogonal, and should be done across the entire repo).

"defaultAction": "ACCEPT"
"defaultAction": "ACCEPT",
"conditionalOrderIds": {
"0xd3338f21c89745e46af56aeaf553cf96ba9bc66f": "DROP",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: full-length example order id

"0xd3338f21c89745e46af56aeaf553cf96ba9bc66f": "DROP",
},
"transactions": {
"0x33ef06af308d1e4f94dd61fa8df43fe52b67e8a485f4e4fff75235080e663bfa": "DROP",
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: we should warn on this as a single transaction may actually spawn multiple conditional order ids (and therefore kill multiple)

Base automatically changed from add-order-id to main February 14, 2024 17:20
@anxolin anxolin force-pushed the add-id-and-txs-to-filter branch from 1ced1d5 to c00234b Compare February 14, 2024 17:37
@anxolin anxolin merged commit 8226fc0 into main Feb 14, 2024
4 checks passed
@anxolin anxolin deleted the add-id-and-txs-to-filter branch February 14, 2024 18:06
@github-actions github-actions bot locked and limited conversation to collaborators Feb 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants