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.

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

chore: Remove score from boundary::settlement #1973

Closed
sunce86 opened this issue Oct 16, 2023 · 5 comments
Closed

chore: Remove score from boundary::settlement #1973

sunce86 opened this issue Oct 16, 2023 · 5 comments
Assignees
@sunce86
Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details

Comments

@sunce86
Copy link
Contributor

sunce86 commented Oct 16, 2023 •
edited
Loading

Background

Original comment: #1901 (comment)

Details

Basically try to decouple boundary::settlement and score as much as possible, so we could move scoring away from boundary. This is also related to #1494, but this issue is a prerequisite for #1494.
@sunce86 sunce86 added the E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details label Oct 16, 2023
@sunce86 sunce86 self-assigned this Oct 16, 2023
@sunce86 sunce86 mentioned this issue Oct 19, 2023
Merged
@sunce86 sunce86 changed the title chore: Remove score from settlement chore: Remove score from boundary::settlement Oct 19, 2023
sunce86 added a commit that referenced this issue Oct 20, 2023
@sunce86
Refactor score calculation (#1993)
f63fa1b 
# Description
No new functionality (actually fixes bug, because before this we did not
check the validity of score if provided by solver as is, we did the
checks only for risk adjusted scores).

Moves out score calculation out of the boundary settlement.
A step forward for #1973
and prerequisite for implementing task #4 from
#1891

# Changes
- Added objective_value calculation on boundary settlement
- Added score forwarding on boundary settlement
- With these two, score calculation is moved out to domain::settlement
- Score calculator moved to boundary::score and used by ☝️ 
- These two allowed to define domain::score::Error and propagate errors
properly.
@fleupold
Copy link
Contributor

Will tackle this as part of #1494

@sunce86
Copy link
Contributor Author

sunce86 commented Dec 27, 2023

Are we removing the score completely with ranking by surplus? I tried to glance over the forum post but couldn't find it.

@fleupold
Copy link
Contributor

@harisang correct me if I'm wrong but I understood that given we submit the clearing prices and executed amounts with the solutions we no longer need to report a score but can instead compute it in the autopilot for each submission.

I'm not sure we want to wait for "rank by surplus" though to remove the legacy solver crate.

@sunce86
Copy link
Contributor Author

sunce86 commented Dec 27, 2023 •
edited
Loading

I even imagined driver submits encoded settlement (but with cleared interaction) so we can use the same logic for ranking in both autopilot and driver (driver still needs to do the internal ranking to see which solution to submit).

@fleupold
Copy link
Contributor

I even imagined driver submits encoded settlement (but with cleared interaction)

What data in addition to than clearing prices and order executions would be needed for that.

I even imagined driver submits encoded settlement (but with cleared interaction)

That's a very good point. In this case it may be simpler to continue sending a score from the driver to the autopilot and leave it up to the off chain verification to make sure it was correct.

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

@sunce86 sunce86

Labels
E:3.1 Driver Colocation See https://github.com/cowprotocol/pm/issues/14 for details
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@fleupold @sunce86