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

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

Closed
sunce86 opened this issue Sep 12, 2024 · 7 comments · Fixed by #2980
Closed

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

sunce86 opened this issue Sep 12, 2024 · 7 comments · Fixed by #2980
Labels
E:6.2 Time to Happy Moo See https://github.com/cowprotocol/pm/issues/77 for details

Comments

@sunce86
Copy link
Contributor

sunce86 commented Sep 12, 2024

Problem

Database table settlement_score stores only details about the global winner. We need to redesign the table so multiple winners can be stored.

Suggested solution

Winner, winning_score and reference_score to become arrays.

CREATE TABLE auction_winners (
  auction_id bigint PRIMARY KEY,
  winners bytea[] NOT NULL,
  winning_scores numeric(78,0)[] NOT NULL,
  reference_scores numeric(78,0)[] NOT NULL,
  block_deadline bigint NOT NULL,
  simulation_block bigint NOT NULL
);

Alternatives considered

Have a row for each winner but then auction_id can't be primary key, so introducing a new field to act as a primary key and to determine the ordering would be needed. Not worth it IMO since we always insert and fetch all data for a single auction.

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

Why don't we follow @fhenneke's suggestion (store all solutions together with some committment of what they are supposed to settle - ie. order ids, executed amounts and clearing prices)?

I think for instance committing to a reference score is dangerous because it depends on the auction mechanism.

@sunce86
Copy link
Contributor Author

sunce86 commented Sep 13, 2024

Why don't we follow @fhenneke's suggestion (store all solutions together with some committment of what they are supposed to settle - ie. order ids, executed amounts and clearing prices)?

Isn't this standardizing the solver competition table which already contains most of this information? and we already have auction prices and fee policies stored in db. We can go that route but it adds another week or two of work for this feature.

@fleupold
Copy link
Contributor

Isn't this standardizing the solver competition table which already contains most of this information?

Yes, basically. It's making it so that the role of the autopilot is not to know any exact reward scheme but simply be aware of all submitted solutions and record which of those it has seen on chain (so that another component can check it's according to the protocol rules). It will still contain some logic on which should execute but eventually this decision can also be taken over by the drivers (assuming they have visibility into all bids)

We can go that route but it adds another week or two of work for this feature.

Really? How is having a slightly different schema with data we already have and store during the post processing step creating such a large difference in time it takes?

@sunce86
Copy link
Contributor Author

sunce86 commented Sep 16, 2024

Really? How is having a slightly different schema with data we already have and store during the post processing step creating such a large difference in time it takes?

It's a non trivial decision for how the schema would look like for it so it would require few rounds of discusion/reviews probably. We also need to deprecate the solver competition table and use the new one (to avoid saving the same data twice) which is also a non trivial task. I mean, it's not two weeks itself but rather a delay of week or two is what I wanted to point out considering we are working on other tasks as well.

The PR we already have is a straight forward bare minimum change that is already implemented and needed to satisfy the feature requirements and unblock other tasks. All other nice to have tasks should be optional if we want to finish up with this in Q3.

I'm fine with both, just wanted to make sure we accomplish the Q3 goals @MartinquaXD @fleupold let me know what would you prefer.

@fleupold
Copy link
Contributor

Is the current table only used for rewards or also elsewhere? Would we have to migrate past data or can we simply transition (and not have this data for old settlements). Overall I feel the proposed PR is just going to shoot us in the foot when we start implementing an actual rewards scheme for multi-solution auctions (e.g. it's not clear if the concept of a reference score is still around then).

@sunce86
Copy link
Contributor Author

sunce86 commented Sep 16, 2024

Ah I understand, then it's not actually nice to have. Ok will sync with solver team and refactor the PR. Thanks.

@MartinquaXD
Copy link
Contributor

I agree that splitting the responsibilities of recording what happened from how this should result in rewards would be nice as it's more flexible. Also I think we should not split the implementation work in Q3 and Q4 too much. I mean shortcuts we take in Q3 will probably make the work in Q4 slower and since submitting multiple solutions and having a reasonable rewards mechanism for that depend on each other we should not see every trade-off as moving complexity from one quarter to the other as the overall complexity will not decrease.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
E:6.2 Time to Happy Moo See https://github.com/cowprotocol/pm/issues/77 for details
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants