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.

Sign up for GitHub

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

Jump to bottom

Executed total fee token #2966

Merged

sunce86 merged 21 commits into main from executed-surplus-fee-token

Dec 24, 2024

Contributor

sunce86 commented Sep 10, 2024 •

edited

Loading

Description

Implements (1) and (2) from #2963.
Also, renames surplus_fee to executed_fee in database.

Changes

Added new field order_execution.executed_fee_token to database
Populated this field with order sell token, otherwise default value is 0x0
Token is exposed over API the same way as executed_fee.

Note: all places where sell token needs to be substituted with surplus token are marked with TODO surplus token and this will be tackled as part of (3) from #2963.

How to test

Existing e2e tests. Observed values during the tests.

sunce86 added 7 commits

September 9, 2024 15:16


          Initial commit

d6e51f6


          db migration

bb686b0


          fix unit tests

33a9381


          update readme

6768adc


          openapi

5f3d670


          fix bugs

ad453ea


          update docs

281b94e

sunce86 self-assigned this

sunce86 requested a review from a team as a code owner

September 10, 2024 14:05

github-actions bot commented Sep 10, 2024 •

edited

Loading

Reminder: Please update the DB Readme.

Caused by:

database/sql/V072__total_fee_token.sql


          rename surplus to total

5fe2851

sunce86 changed the title ~~Executed surplus fee token~~ Executed total fee token

sunce86 added 2 commits

September 11, 2024 14:57


          fix tests

3df7ce2


          fix migration

d78ec96

MartinquaXD reviewed

View reviewed changes

crates/orderbook/openapi.yml Outdated Show resolved Hide resolved

crates/database/src/order_execution.rs Outdated Show resolved Hide resolved

crates/database/src/order_execution.rs Outdated Show resolved Hide resolved

database/README.md Outdated Show resolved Hide resolved

crates/orderbook/openapi.yml Outdated Show resolved Hide resolved


          fix comments

50dc2d3

m-lord-renkse reviewed

View reviewed changes

crates/autopilot/src/infra/persistence/mod.rs

-                                  &total_fee,
-                                  &executed_protocol_fees,
+                                  Asset {
+                                      token: ByteArray(order_fee.total.token.0 .0),

Contributor

m-lord-renkse Sep 12, 2024 •

edited

Loading

Maybe worth having From<TokenAddress> for ByteArray as it appears in more occurrences 🤔

m-lord-renkse approved these changes

View reviewed changes

sunce86 added 2 commits

September 13, 2024 10:48


          Merge branch 'main' into executed-surplus-fee-token


          rename

a6ac8aa

sunce86 commented

View reviewed changes

crates/database/src/orders.rs Outdated Show resolved Hide resolved

sunce86 added 2 commits

September 13, 2024 13:35


          renaming and soft landing for api

3994e53


          remove Some

8ad4105

squadgazzz reviewed

View reviewed changes

crates/autopilot/src/domain/settlement/trade/math.rs Outdated Show resolved Hide resolved

MartinquaXD reviewed

View reviewed changes

database/sql/V072__total_fee_token.sql Outdated Show resolved Hide resolved

database/sql/V072__total_fee_token.sql Outdated

+              ADD COLUMN total_fee_token bytea NOT NULL DEFAULT '\x0000000000000000000000000000000000000000';
+              -- Now populate existing rows with the sell token taken from the 'orders' table, or if it doesn't exist, try from 'jit_orders'.
+              UPDATE order_execution oe

Contributor

MartinquaXD Sep 19, 2024

Did you test that this query does not leave any surplus_token uninitialized?

Contributor Author

sunce86 Dec 24, 2024

Here are test results on mainnet prod db:

Number of rows in order_execution: 1466450
Number of rows being updated: 1460019

So, there are ~6k entries in order_execution that can't be found in orders or jit_orders (but all missing orders are older than 2 months).
My assumptions is that those are some jit orders from before we started saving them in the library. Example:
https://api.cow.fi/mainnet/api/v1/solver_competition/by_tx_hash/0x59ddfcab4910ad48b3550658f0c46c8892f114e6791dbc032c05a42c1e858b63

Note how the order https://explorer.cow.fi/search/0x0088e45fff8487b9d4c5edcf2ad6580cdfc3a689c3c3022d7282aebf6446a8a99bd702e05b9c97e4a4a3e47df1e0fe7a0c26d2f166ff4487 can't be found in the explorer.
So, for these, I am not too worried if their token is not populated since those can't be found in explorer anyway.

github-actions bot commented Sep 27, 2024

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions bot added the stale label

sunce86 removed the stale label

github-actions bot commented Oct 5, 2024

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions bot added the stale label

sunce86 reopened this

github-actions bot removed the stale label

github-actions bot commented Nov 9, 2024

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions bot added the stale label

github-actions bot closed this

sunce86 reopened this

github-actions bot removed the stale label

github-actions bot commented Dec 6, 2024

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions bot added the stale label

fleupold removed the stale label

github-actions bot commented Dec 14, 2024

This pull request has been marked as stale because it has been inactive a while. Please update this pull request or it will be automatically closed.

github-actions bot added the Stale label

sunce86 removed the Stale label


          Merge branch 'main' into executed-surplus-fee-token

6f1d3f4

github-actions bot commented Dec 19, 2024

Reminder: Please update the DB Readme and comment whether migrations are reversible (include rollback scripts if applicable).

Caused by:

database/sql/V076__total_fee_token.sql

github-actions bot commented Dec 19, 2024

Reminder: Please consider backward compatibility when modifying the API specification.
If breaking changes are unavoidable, ensure:

You explicitly pointed out breaking changes.
You communicate the changes to affected teams.
You provide proper versioning and migration mechanisms.

Caused by:

crates/orderbook/openapi.yml

sunce86 added 2 commits

December 23, 2024 15:47


          small fixes

52b194d


          renaming to executed fee

079492b

sunce86 mentioned this pull request

Rename surplus to executed cowprotocol/solver-rewards#471

Merged

sunce86 requested a review from squadgazzz

December 24, 2024 12:36

squadgazzz reviewed

View reviewed changes

crates/model/src/order.rs Show resolved Hide resolved

squadgazzz approved these changes

View reviewed changes

Contributor

squadgazzz left a comment

LGTM

sunce86 added 2 commits

December 24, 2024 16:22


          fix e2e test

a8a5038


          fix openapi

28ec783

sunce86 mentioned this pull request

chore: Remove executed_surplus_fee #3180

Open


          Merge branch 'main' into executed-surplus-fee-token

5ae4d31

sunce86 enabled auto-merge (squash)

December 24, 2024 15:56

sunce86 merged commit 37300c4 into main

11 checks passed

sunce86 deleted the executed-surplus-fee-token branch

December 24, 2024 16:01

github-actions bot locked and limited conversation to collaborators

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet