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: Surplus of CoW AMM orders in settlement_scores table is zero #2808

Closed
fhenneke opened this issue Jul 10, 2024 · 5 comments · Fixed by #2818
Closed

bug: Surplus of CoW AMM orders in settlement_scores table is zero #2808

fhenneke opened this issue Jul 10, 2024 · 5 comments · Fixed by #2818
Assignees
Labels
bug Something isn't working

Comments

@fhenneke
Copy link

Problem

The surplus of JIT orders is not accounted for correctly in the autopilot when populating the settlement_scores table. The problem is present in prod and staging. Scores from the driver, however, are correct.

Impact

This error impacts reward computations as the reward script uses surplus from the settlement_scores table. This lead to small discrepancies in payments mostly to Quasilabs which were corrected manually. Other solvers are also affected, but the overall damage is small as of now.

Wrong data is currently uploaded to Dune.

To reproduce

This is easiest to see in examples of settling only one order in a batch as for this settlement (staging, auction id 10859238, block number 20261350)
The query select * from settlement_observations where block_number = 20261350 gives a surplus of 0, while the query select * from settlement_scores where auction_id = 10859238 gives a winning score of 57387933137402 which equals the surplus of that order since there are no protocol fees. The score (and surplus) are double checked using the circuit breaker which contains an independent implementation of scores (and surplus).

The problem is less obvious for other settlement (e.g. this settlement, auction id: 20272499, block number: 9116222), but the same arguments apply: surplus is stored as 46523400998314531 but the score suggests a surplus of 46624022071370740 which is larger by the surplus of the CoW AMM order.

Alternative solutions

I think that the rewards script could get by with just using the winning score directly instead of recomputing it from surplus and protocol fees. The correctness of the scores themselves is checked by the circuit breaker (with the next release).
We do upload surplus to Dune and for that it would be convenient to have a correct surplus computation in the autopilot. This is almost required since surplus for JIT orders cannot be directly computed from the database except by parsing the stored calldata.

cc @harisang

@fhenneke fhenneke added the bug Something isn't working label Jul 10, 2024
@fleupold
Copy link
Contributor

cc @Mateo-mro is this on your radar (as far as I know you are finishing up CoW AMM integration work)?

@m-lord-renkse
Copy link
Contributor

cc @Mateo-mro is this on your radar (as far as I know you are finishing up CoW AMM integration work)?

@MartinquaXD and I worked on the CoW AMM. I can definitely take a look into this, I am doing some CoW AMM related tasks so I can add it to the pile. The only thing is I can only start working full on it from next week, is that is ok @fhenneke .

@fleupold
Copy link
Contributor

Also cc @sunce86

@sunce86
Copy link
Contributor

sunce86 commented Jul 12, 2024

Since cow amm orders are no longer part of the autopilot Auction, the code assumes decoded cow amm orders are not eligible for surplus: https://github.com/cowprotocol/services/blob/main/crates/autopilot/src/on_settlement_event_updater.rs#L215

To fix this, we need #2793 implemented, so that we have a mechanism how to differentiate between cow amm orders and jit liquidity orders in the autopilot.

@m-lord-renkse
Copy link
Contributor

Being that the case, I will prioritize that task next week.

harisang added a commit to cowprotocol/solver-rewards that referenced this issue Jul 12, 2024
There is a bug with surplus computations for JIT orders,
cowprotocol/services#2808.

This PR changes the computation of rewards to use winning and reference
scores. Before, the winning score was recomputed using surplus and
protocol fees. Under normal operation, this change should only affect
rewards do to different rounding in the driver and the SQL query. With
the current bug in surplus, the PR will produce correct rewards.

This simplifies the reward computation. One drawback is that misreported
scores are now used for computing rewards. Fortunately, the circuit
breaker will trigger in such cases and a manual transaction can be
issued to reverse the wrong reward.

Tests have been adepted to only use scores for rewards.

Co-authored-by: Haris Angelidakis <[email protected]>
@sunce86 sunce86 self-assigned this Jul 17, 2024
sunce86 added a commit that referenced this issue Jul 18, 2024
# Description
Fixes #2808

# Changes
<!-- List of detailed changes (how the change is accomplished) -->

- [ ] Fetch surplus capturing jit order owners from database and
calculate surplus for orders which owners are part of it.

## How to test
Added unit test total_surplus_cow_amm_order_test that calculates the
surplus for the problematic settlement containing one cow amm order from
#2808

<!--
## Related Issues

Fixes #
-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants