New database tables for auction and solver competition #2980

sunce86 · 2024-09-12T14:14:23Z

Description

Database changes internal discussion: https://www.notion.so/cownation/Database-26th-September-2024-10d8da5f04ca801ab087f00f6a6d608f

@fhenneke would this be appropriate design change for you?

Update 02 Oct 2024

This PR proposes new tables that should eventually replace solver_competitions table and also give enough information for core protocol and external tools to reconstruct the auction and competition for historical entries.
Plan to execute:

Create new tables (implemented in this PR)
Start populating new tables (implemented in this PR)
Do a one time migration of data from existing tables (solver_competitions, settlement_scores etc) into new tables.
Start using new tables instead of old tables in services repo. From now on, old tables are no longer used by backend.
Give time to solver team, frontend team etc to switch to new tables
Cleanup - remove one time migration code, remove old tables, remove code that was updating old tables etc.

Changes

Defines tables for auction and proposed solutions
Populates new tables from autopilot

github-actions · 2024-09-12T14:14:40Z

Reminder: Please update the DB Readme.

Caused by:

database/sql/V072__auction_solution_orders.sql

fhenneke · 2024-09-12T16:25:15Z

I left a comment on some google doc, but it seems to be relevant here as well. The discussion was around redesigning the settlement_scores table to compute rewards:

A competition table with the following columns would be compatible with all variants of the comb. auction mechanism we are currently thinking about:
auction_id, solution_id, solver_address, solution, deadline

where solution should be equivalent to (but need not be the same as) the first part of call data (i.e. tokens, prices, trades) with additional data in other tables (e.g. an auctions table with historic data) on native prices and protocol fees to reconstruct scores.

the deadline would set if the solution is selected as winning.
additionally there would be some indexing of solutions on chain (tx_hash, auction_id, solution_id). actual observations on fees or surplus are not required. checking that solutions are valid would be done by a circuit breaker.

Some example code we might base experiments on: https://github.com/fhenneke/comb_auctions

database/sql/V072__auction_solution_orders.sql

fleupold · 2024-09-17T07:07:02Z

database/sql/V072__auction_solution_orders.sql

+   -- Not NULL for winning orders.
+   deadline bigint,
+
+   -- Order details


Are these needed? Given we store trades for all orders (jit included), do we need to store this information? Also, I think the order uid commits to those values (so you wouldn't be able to change the limit price for a fixed order uid and it can just be read from the settlement).

do we need to store this information? I think the order uid commits to those values

Ah I see, yes, order_uid is actually a checksum for OrderData. If the surplus capturing surplus JIT order is promised, there are only three outcomes in circuit breaker:

Solution not delivered at all - violation.

Solution delivered but it doesn't contain promised order_uid of JIT order - violation

Solution delivered with the same order uid - then just compare the executed amounts to check if the prices are >= promised.

@fhenneke will circuit breaker require to calculate the score of promised (non-winning) solutions (for reference scores for example)? If not, then we can remove this order data and get them from orders table and jit_orders table if needed. But note the comment 👇 that I would rather not save the scores themselves but instead I would save the input for calculating score or whichever scoring criteria we use.

Ok, after meeting with @fhenneke we concluded that we need this data for all proposed solutions (which include surplus capturing JIT orders and potentially regular JIT orders in the future) and also it makes sense to store this data to show it on the solver competition endpoint.

For regular orders this data is already available in a different table (so basic SQL normalisation theory says we shouldn't duplicate). For jit orders this may not be the case. Can you explain a bit more detailed why we need this information for jit orders even if there ends up not being a settlement observation?

Note that the current competition endoint exposes order ids and executed amounts, nothing more: https://api.cow.fi/mainnet/api/v1/solver_competition/by_tx_hash/0xe987ca2672c8330398750c73e38ed6375c3e18b29172b806cfc2d66f33eaaf0d

Can you explain a bit more detailed why we need this information for jit orders even if there ends up not being a settlement observation?

If we have two solutions, and one of them is declared a winner and allowed to settle, and IF the rewards scheme stays the same (difference between winning score and reference score), then we need all data of a second best solution (which might contain surplus capturing JIT order) available at postprocessing time, so that we could calculate the score of it to use it as a reference score.

And all of this stands because we don't want to save score as a field into database. We want to save solutions (input for ranking and everything else) and not scores (output of ranking) into db, for greater flexibility in the future. By flexibility I mean being possible to change the scoring rules of the protocol without changing the db scheme.

fhenneke · 2024-09-17T07:26:38Z

I think it is good to have more data available on orders to compute surplus and trade directions.

One thing which would need to be added is scores of all solutions. Some of the reward mechanisms might even require scores per order.

We can either store scores directly or make enough information available to compute scores ourselves.
Data required to compute scores would include native prices for all surplus tokens (instead of just the tokens of the winner). For protocol fees, we would need fee policies for all orders with a proposed solution (instead of just protocol fees for executed solutions).

database/sql/V072__auction_solution_orders.sql

MartinquaXD · 2024-09-17T07:51:16Z

database/sql/V072__auction_solution_orders.sql

+   -- The block number until which the order should be settled.
+   -- Not NULL for winning orders.
+   deadline bigint,


The deadline is a property of the auction and not the order.

Deadline is also optional to indicate if a solution is chosen as a winner or not.

Would you rather have:

Deadline as property of auction and non-optional. Then, another property winner: bool on each solution to indicate if winner.

Deadline as a property of solution and optional so that it indicates if winner.

I would prefer to have less implicit state so deadline as required on auction.
This could then lead to a separate table:

solutions - id (populated by sequence) - auction_id - solver - solution_id - is_winner

And then auction_solution_orders (I prefer the name proposed_order_executions) could reference the solutions.id.
Not sure if this would actually result in good queries since a shared key of multiple properties might be easier to use than solutions.id but that would at least have some logical consistency where:

auction has solutions

solution contains orders

OTOH do we need to store who the winner is? Assuming there is no bug we should be able to reconstruct who (should have) won with all the data we have, no? Just a conceptual question as it probably doesn't make sense to cheap out on a bool here.

Assuming there is no bug we should be able to reconstruct who (should have) won with all the data we have, no?

The criteria might change even on each restart of the autopilot (for example, if we enable/disable multiple winners feature several times). In this case we would need to know who was supposed to settle solutions.

I've refactored the tables before you posted a comment. Can you check if it's more acceptable now?

The criteria might change even on each restart of the autopilot (for example, if we enable/disable multiple winners feature several times). In this case we would need to know who was supposed to settle solutions.

Alternative to this is to also save the information "which type of competition" was executed for each auction. With this, we would have an input and would be able to determine the winners so "is_winner" would not be needed.

database/sql/V072__auction_solution_orders.sql

sunce86 · 2024-09-17T09:16:02Z

I think it is good to have more data available on orders to compute surplus and trade directions.

One thing which would need to be added is scores of all solutions. Some of the reward mechanisms might even require scores per order.

We can either store scores directly or make enough information available to compute scores ourselves. Data required to compute scores would include native prices for all surplus tokens (instead of just the tokens of the winner). For protocol fees, we would need fee policies for all orders with a proposed solution (instead of just protocol fees for executed solutions).

I'd go with more general approach of storing enough information to compute whatever scoring criteria we use.

Data required to compute scores would include native prices for all surplus tokens

We have that in auction_prices db table.

we would need fee policies for all orders with a proposed solution (instead of just protocol fees for executed solutions)

We WILL have that in fee_policies db table. Out of curiosity, why do you calculate the score for proposed solution that was not delivered? Is it because you might need reference scores?

database/sql/V072__auction_solution_orders.sql

fleupold · 2024-09-17T13:43:36Z

database/sql/V072__auction_solution_orders.sql

+   -- Not NULL for winning orders.
+   deadline bigint,
+
+   -- Order details


For regular orders this data is already available in a different table (so basic SQL normalisation theory says we shouldn't duplicate). For jit orders this may not be the case. Can you explain a bit more detailed why we need this information for jit orders even if there ends up not being a settlement observation?

Note that the current competition endoint exposes order ids and executed amounts, nothing more: https://api.cow.fi/mainnet/api/v1/solver_competition/by_tx_hash/0xe987ca2672c8330398750c73e38ed6375c3e18b29172b806cfc2d66f33eaaf0d

database/sql/V072__auction_solution_orders.sql

# Description Currently fee policies are saved only for winning solution. This PR saves fee policies for all auction orders. This is needed for at least two reasons: 1. As discussed [in the PR](#2980 (comment)), fee policies will be needed for all proposed solutions during a competition so that the score could be reconstructed in circuit breaker. 2. [For historical get_auction](#2844).

# Description Fixes #2992 `settlement_scores::fetch` will be updated once the #2980 is merged. ## How to test Existing univ2 e2e test.

github-actions · 2024-09-26T00:07:49Z

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.

crates/autopilot/src/run_loop.rs

MartinquaXD · 2024-10-11T07:37:45Z

crates/database/src/solver_competition.rs

+    pub uid: i64,
+    // Id as reported by the solver (solvers are unaware of how other solvers are numbering their
+    // solutions)
+    pub id: i64,


Id is a string on the API level. It just happens to be that all solvers currently report a number.
It's probably okay to make this an integer on the API level but that has to be adjusted and communicated first.

id is not supposed to replace solver name on the solver_competition API. It's here for completness of saving the whole solution object but not necessary for functionality to work. We can remove it as well if we are sure we won't need it.

I think we agreed somewhere that solver name is something we don't care too much about. We have a solver address which is supposed to uniquely identify the solver.

Not sure what the solver name has to do with this. This id is the id that solvers return for each individual solution, right?

Id is a string on the API level

I thought you referred to solver name here.

This id is the id that solvers return for each individual solution, right?

Yes.

So then the point still stands that the ID is currently a string on the API level and only an integer by convention. If we want to store it in the DB I think we should make sure the data types align and make sense.

Ok. Will add an issue to switch ID to being u64 as it used that way in both driver and autopilot domains.

#3064

crates/database/src/solver_competition.rs

MartinquaXD · 2024-10-11T08:08:48Z

database/sql/V072__auction_solution_orders.sql

@@ -0,0 +1,66 @@
+-- All auctions ran by autopilot
+CREATE TABLE competition_auctions (


Let's try to avoid these composite table names if possible as I think they mostly cause confusion.
I'd say this should be called auctions and the current auctions table would become current_auction (singular as it's supposed to only store a single row at all times).

I'd like to avoid touching existing code with this PR. Table renaming is particularly risky, and even though I initially wanted to do renaming, I went with defining a new name after all.
And if you assume that, it's really hard to figure out a new name for this table.

Then at least make sure the tables are properly renamed when we finalize this refactor and remove the old tables.

MartinquaXD · 2024-10-11T08:14:40Z

database/sql/V072__auction_solution_orders.sql

@@ -0,0 +1,66 @@
+-- All auctions ran by autopilot
+CREATE TABLE competition_auctions (
+   id bigint PRIMARY KEY,


I think we are supposed to use identity columns to have the DB automatically generate these unique values for us.

Suggested change

id bigint PRIMARY KEY,

id bigint PRIMARY KEY GENERATED ALWAYS AS IDENTITY,

I understand it looks neat to use DB generated ID, but why would we use lock us in with using it?
Besides, right now auctions and competition_auctions need to be aligned with ids.

Besides, right now auctions and competition_auctions need to be aligned with ids.
Sorry, the comment was supposed to be on proposed_solutions.id.

I understand it looks neat to use DB generated ID, but why would we use lock us in with using it?

For Ids that have no other purpose than being unique and identifying rows I think it makes the most sense to let the DB make sure that things are unique instead of relying on domain code that can have bugs in that regard. Since we don't expect any additional information in the ID any value is as good as any other value as long as it's unique so why should we bother with maintaining that uniqueness ourselves?

For Ids that have no other purpose than being unique and identifying rows

But this is not actually true in this case right? Auction id is read directly by client and used to fetch data from other tables etc. It's not like it's inserted only once and never used by client but only by database internally to join on other tables etc.

But anyway, in this case we have to go with client defined Ids because of:

right now auctions and competition_auctions need to be aligned with ids.

database/sql/V072__auction_solution_orders.sql

MartinquaXD · 2024-10-11T08:23:30Z

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

+                .enumerate()
+                .map(|(uid, participant)| {
+                    let solution = Solution {
+                        uid: uid.try_into().context("uid overflow")?,


The db docs made it seem like this uid is supposed to be globally unique, which I think is a nicer property than just having it be the index within one auction.

unique id of the proposed solution within a single auction this is in the docs

Global uniqueness is not required.

database/sql/V072__auction_solution_orders.sql

database/README.md

squadgazzz

For the current state, I don't see any blockers.

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

m-lord-renkse · 2024-10-14T09:19:12Z

crates/database/src/solver_competition.rs

+                    ON CONFLICT (auction_id, solution_uid, order_uid) DO NOTHING
+                "#;
+
+                sqlx::query(QUERY_JIT)


Could we do this in one roundtrip?

God is my witness I tried but couldn't come up with a query that would properly handle WHERE NOT EXISTS part.

MartinquaXD

I think all my comments got addressed.

m-lord-renkse

Did another round, LGTM! nice PR!

Redesign settlement_scores table

10a1bc1

sunce86 added the E:6.2 Time to Happy Moo See https://github.com/cowprotocol/pm/issues/77 for details label Sep 12, 2024

sunce86 self-assigned this Sep 12, 2024

sunce86 mentioned this pull request Sep 12, 2024

feat: Allow multiple solution submissions for one auction #2830

Closed

This was referenced Sep 16, 2024

feat: Redesign database table settlement_score to support multiple winners #2979

Closed

[EASY] Update most_recent_cip_20_data #2997

Merged

new table for storing proposed solutions

97aa50e

fleupold reviewed Sep 17, 2024

View reviewed changes

MartinquaXD reviewed Sep 17, 2024

View reviewed changes

split into two tables

8c82c6a

sunce86 mentioned this pull request Sep 17, 2024

Save fee policies for all auction orders #2999

Merged

fleupold reviewed Sep 17, 2024

View reviewed changes

MartinquaXD reviewed Sep 17, 2024

View reviewed changes

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

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

cr fixes

9608b4b

sunce86 commented Sep 18, 2024

View reviewed changes

database/sql/V072__auction_solution_orders.sql Show resolved Hide resolved

sunce86 added 4 commits September 18, 2024 10:24

don't force solution_id to be unique globally

70152f4

fix index

b387a16

remove obsolete index

9a39a0f

removed deadline from solutions

10197dc

sunce86 commented Sep 18, 2024

View reviewed changes

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

sunce86 requested review from fleupold and MartinquaXD September 18, 2024 08:41

sunce86 added a commit that referenced this pull request Sep 24, 2024

[EASY] Update most_recent_cip_20_data (#2997)

4b985bb

# Description Fixes #2992 `settlement_scores::fetch` will be updated once the #2980 is merged. ## How to test Existing univ2 e2e test.

github-actions bot added the stale label Sep 26, 2024

sunce86 marked this pull request as ready for review October 10, 2024 15:00

sunce86 requested a review from a team as a code owner October 10, 2024 15:00

sunce86 changed the title ~~[WIP] New database tables for auction and solver competition~~ New database tables for auction and solver competition Oct 10, 2024

sunce86 requested review from fleupold, fhenneke, squadgazzz and m-lord-renkse October 10, 2024 15:00

MartinquaXD reviewed Oct 11, 2024

View reviewed changes

cr fixes

a4f4d9c

squadgazzz reviewed Oct 11, 2024

View reviewed changes

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

squadgazzz reviewed Oct 11, 2024

View reviewed changes

database/README.md Outdated Show resolved Hide resolved

squadgazzz reviewed Oct 11, 2024

View reviewed changes

database/README.md Outdated Show resolved Hide resolved

squadgazzz reviewed Oct 11, 2024

View reviewed changes

database/README.md Outdated Show resolved Hide resolved

squadgazzz reviewed Oct 11, 2024

View reviewed changes

database/README.md Outdated Show resolved Hide resolved

squadgazzz approved these changes Oct 11, 2024

View reviewed changes

m-lord-renkse reviewed Oct 14, 2024

View reviewed changes

crates/autopilot/src/infra/persistence/mod.rs Show resolved Hide resolved

m-lord-renkse reviewed Oct 14, 2024

View reviewed changes

sunce86 added 3 commits October 14, 2024 12:12

cr fixes from ilya

0cc9f39

Split into functions

8fb30ec

Merge branch 'main' into auction-winners-table

166da4d

This was referenced Oct 14, 2024

chore: Populate historic entries for new auction table #3055

Closed

chore: Populate historic entries for new solver competition tables #3056

Open

chore: Read competition data from new tables #3057

Open

MartinquaXD approved these changes Oct 17, 2024

View reviewed changes

m-lord-renkse approved these changes Oct 17, 2024

View reviewed changes

Merge branch 'main' into auction-winners-table

3870108

sunce86 enabled auto-merge (squash) October 17, 2024 13:23

sunce86 merged commit 1812dd9 into main Oct 17, 2024
11 checks passed

sunce86 deleted the auction-winners-table branch October 17, 2024 13:24

github-actions bot locked and limited conversation to collaborators Oct 17, 2024

		@@ -0,0 +1,66 @@
		-- All auctions ran by autopilot
		CREATE TABLE competition_auctions (

	id bigint PRIMARY KEY,
	id bigint PRIMARY KEY GENERATED ALWAYS AS IDENTITY,

New database tables for auction and solver competition #2980

New database tables for auction and solver competition #2980

Conversation

sunce86 commented Sep 12, 2024 • edited Loading

Description

Update 02 Oct 2024

Changes

github-actions bot commented Sep 12, 2024 • edited Loading

fhenneke commented Sep 12, 2024

Choose a reason for hiding this comment

sunce86 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

fhenneke commented Sep 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 commented Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

github-actions bot commented Sep 26, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 Oct 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 Oct 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sunce86 Oct 11, 2024 • edited Loading

Choose a reason for hiding this comment

squadgazzz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MartinquaXD left a comment

Choose a reason for hiding this comment

m-lord-renkse left a comment

Choose a reason for hiding this comment

sunce86 commented Sep 12, 2024 •

edited

Loading

github-actions bot commented Sep 12, 2024 •

edited

Loading

sunce86 Sep 17, 2024 •

edited

Loading

sunce86 Sep 17, 2024 •

edited

Loading

sunce86 Sep 17, 2024 •

edited

Loading

sunce86 commented Sep 17, 2024 •

edited

Loading

sunce86 Oct 17, 2024 •

edited

Loading

sunce86 Oct 15, 2024 •

edited

Loading

sunce86 Oct 11, 2024 •

edited

Loading