Skip to content

Commit

Permalink
Fix decoding logic when orders with identical signatures got matched (#…
Browse files Browse the repository at this point in the history
…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.
  • Loading branch information
fleupold authored Dec 13, 2023
1 parent 138cadd commit 80b6006
Show file tree
Hide file tree
Showing 2 changed files with 425 additions and 44 deletions.
Loading

0 comments on commit 80b6006

Please sign in to comment.