Prepare workflow for migration #2
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nlordell
pushed a commit
that referenced
this pull request
Apr 4, 2022
This PR just changes our GitHub workflow configurations related to the fork. Will be testing this out with @giacomolicari 😄. Docker images are now pulled from the GitHub container registry instead of Docker Hub.
nlordell
pushed a commit
that referenced
this pull request
Apr 4, 2022
This PR just changes our GitHub workflow configurations related to the fork. Will be testing this out with @giacomolicari 😄. Docker images are now pulled from the GitHub container registry instead of Docker Hub.
nlordell
pushed a commit
that referenced
this pull request
Apr 4, 2022
This PR just changes our GitHub workflow configurations related to the fork. Will be testing this out with @giacomolicari 😄. Docker images are now pulled from the GitHub container registry instead of Docker Hub.
nlordell
pushed a commit
that referenced
this pull request
Apr 4, 2022
This PR just changes our GitHub workflow configurations related to the fork. Will be testing this out with @giacomolicari 😄. Docker images are now pulled from the GitHub container registry instead of Docker Hub.
nlordell
pushed a commit
to cowprotocol/dune-bridge
that referenced
this pull request
Apr 7, 2022
This PR migrates the `deploy.sh` script to a GitHub action like we did for cowprotocol/services#2. I also noticed that this repo does not auto-redeploy on new commits like we have in the services repo. I will leave it as-is if this is intended. If not we can use cowprotocol/autodeploy-action to trigger a re-deploy of the pods on new commits. I also fixed some small `cargo clippy` lints to make CI pass. ### Test Plan Build a docker image when this merges!
nlordell
pushed a commit
to cowprotocol/dune-bridge
that referenced
this pull request
Apr 7, 2022
This PR migrates the `deploy.sh` script to a GitHub action like we did for cowprotocol/services#2. I also noticed that this repo does not auto-redeploy on new commits like we have in the services repo. I will leave it as-is if this is intended. If not we can use cowprotocol/autodeploy-action to trigger a re-deploy of the pods on new commits. I also fixed some small `cargo clippy` lints to make CI pass. Build a docker image when this merges!
Merged
2 tasks
fleupold
added a commit
that referenced
this pull request
Dec 13, 2023
…2158) # Description Context: https://cowservices.slack.com/archives/C0375NV72SC/p1702383991059619 In [this settlement](https://explorer.phalcon.xyz/tx/eth/0xd881e90f4afb020d92b8fa1b4931d2352aab4179e4f8d9a4aeafd01ebc75f808?line=0&debugLine=0) three orders with the exact same signature got matched: <img width="546" alt="image" src="https://github.com/cowprotocol/services/assets/1200333/f9fc1847-3815-49c3-adf7-4545a0b2bda0"> While this is fine (presign orders use the owner as a signer), this was not accounted for in the settlement decoding code. There we assume trades within a settlement have a unique signature. As a consequence, this is messing with our settlement post-processing which is trying to match fill-or-kill trades and order executions solely based on signature. Incorrectly connecting trade #2 with order execution #1 may lead to using incorrect indices to look up the adjusted price for the trade and thus create incorrect "surplus fee" values (cf. [this trade](https://explorer.cow.fi/orders/0x77425bd23d5fbb24d32229b1c343807bee572f0555429632161350a56811d263c001d00d425fa92c4f840baa8f1e0c27c4297a0b65782608?tab=overview), where fee amount seems to exceed the sell amount). For additional context on how to interpret the order flags, cf. https://github.com/cowprotocol/contracts/blob/main/src/contracts/libraries/GPv2Trade.sol#L95 # Changes - [ ] Make the matching based on orderUID which - for FOK orders - has to be unique - [ ] For partially fillable orders we continue to check the executed. In case a partially fillable order is executed twice in the same batch with exactly the same amount but different fees (very unlikely) computation may still be off. ## How to test Added a unit test for the transaction in question and updated the existing tests.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR just changes our GitHub workflow configurations related to the fork.
Test Plan
Will be testing this out with @giacomolicari 😄.
Release notes
Docker images are now pulled from the GitHub container registry instead of Docker Hub.