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: some orders are ignored when computing total surplus #2289

Closed
fedgiac opened this issue Jan 15, 2024 · 0 comments · Fixed by #2305
Closed

bug: some orders are ignored when computing total surplus #2289

fedgiac opened this issue Jan 15, 2024 · 0 comments · Fixed by #2305
Assignees
Labels
bug Something isn't working E:4.2 Protocol Fee Implementation See https://github.com/cowprotocol/pm/issues/29 for details

Comments

@fedgiac
Copy link
Contributor

fedgiac commented Jan 15, 2024

Problem

We noticed some instances in Sepolia where total_surplus is not being computed as expected.

While debugging the corresponding DB query, I noticed that this happens because some order uids aren't available in the order_execution table and this causes these orders to be ignored when computing the surplus.

As we recently changed how the order_execution table works, the query might need to be updated accordingly.

Example of orders that are ignored for the surplus: this one or this one. This user has many orders but only the most recent orders count for it.

Impact

/api/v1/users/{address}/total_surplus returns lower values than expected.

To reproduce

I used the following simplified version of the total_surplus database query. first with the last join and then without to see that some useful orders are missing. (Tried in Sepolia staging.)

trade_components AS (
    SELECT
       o.uid
    FROM orders o
    JOIN trades t ON o.uid = t.order_uid
    JOIN order_execution oe ON o.uid = oe.order_uid
)
SELECT *
FROM trade_components;

Expected behaviour

The query should include the surplus of the two example orders in the description.

services version/commit hash and environment

72ce082

@fedgiac fedgiac added the bug Something isn't working label Jan 15, 2024
@sunce86 sunce86 added the E:4.2 Protocol Fee Implementation See https://github.com/cowprotocol/pm/issues/29 for details label Jan 16, 2024
@squadgazzz squadgazzz self-assigned this Jan 18, 2024
squadgazzz added a commit that referenced this issue Jan 19, 2024
# Description
This PR addresses the issue identified in Sepolia where `total_surplus`
was not computed correctly due to missing `order_uids` in the
`order_execution` table. This miscomputation was a consequence of
[recent changes](#2190) in
the workings of the `order_execution` table. The identified problem
impacted the `/api/v1/users/{address}/total_surplus` endpoint, leading
to lower-than-expected surplus values. This fix aims to ensure accurate
surplus computation by addressing the data retrieval process in the
database.

# Changes
After thorough consideration, it was decided to go with the option of
reverting the change and saving `order_executions` for all trades. The
alternative of building a complicated subquery was deemed overly complex
and unnecessary, especially given the context of moving towards a system
where all orders are limit orders. This simplification aligns with the
future direction and maintains system efficiency.

## How to test

#2267

## Related Issues

Fixes #2289
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working E:4.2 Protocol Fee Implementation See https://github.com/cowprotocol/pm/issues/29 for details
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants