Add driver submission address to the autopilot #3065

m-lord-renkse · 2024-10-17T10:57:02Z

Description

This PR should solve: #3045 and #2780

Changes

Add driver submission address to the autopilot
Do not request /solve to drivers which have been deny listed
Filter the results by the driver submission address

How to test

Regression test (e2e modified to use the new config)
e2e test to check the filtering works

Related Issues

Fixes #3045
Fixes #2780

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

crates/shared/src/arguments.rs

MartinquaXD

The code to make things optional is pretty convoluted. Let's make it required from the start.
Would also be good to prepare the infra PR in the meantime.

crates/autopilot/src/run_loop.rs

sunce86

Can we also add a manual step (to the tutorial for solver submission keys rotation in notion) to

Update the infra configuration for autopilot
To make sure the solver switched their submission to use new account

crates/autopilot/src/run_loop.rs

crates/autopilot/src/arguments.rs

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

crates/autopilot/src/run_loop.rs

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

sunce86 · 2024-10-23T09:59:30Z

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

+                        .await
+                        .with_context(|| {
+                            let msg = format!("Unable to load KMS account {:?}", key_id);
+                            tracing::error!(?name, msg);


super nit: maybe move out tracing::error to the caller so that message formatting is cleaner

.await .with_context(|| format!("Unable to load KMS account {:?}", key_id))?;

@sunce86 I thought of that, but then we need to have the same trace in two places in the code, potentially creating inconsistencies in the future (if one log is reformatted, etc).

Actually this gets way better by introducing the error you suggested above 👌

crates/e2e/tests/e2e/solver_competition.rs

squadgazzz · 2024-10-25T08:55:08Z

crates/autopilot/src/run.rs

+                Ok(driver) => Some(Arc::new(driver)),
+                Err(_) => None,


Why do we need to introduce a new error when it is completely ignored on the caller's side? Either an Option should be returned or the error needs to be logged here instead.

Introducing an error is good for the function itself, that we don't use it now doesn't mean it won't be used. The logging is done within the function instead of the caller, mostly to avoid code repetition since this is instantiated in multiple times.

that we don't use it now doesn't mean it won't be used.

IMO, then it should only be introduced then. Currently, it is redundant.

I disagree. If we design a function that errors, then it has to emit an error. How the error is treated is responsibility of the caller.

github-actions · 2024-11-02T00:08:08Z

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.

fleupold

LGTM, just that this will cause issues with https://cowservices.slack.com/archives/C036JAGRQ04/p1732005732731009

fleupold · 2024-11-19T16:26:39Z

crates/autopilot/src/arguments.rs

@@ -372,6 +380,77 @@ impl std::fmt::Display for Arguments {
    }
 }

+/// External solver driver configuration


nit: does it make sense to move this type into it's own arguments module?

fleupold · 2024-11-19T16:27:10Z

crates/autopilot/src/arguments.rs

@@ -372,6 +380,77 @@ impl std::fmt::Display for Arguments {
    }
 }

+/// External solver driver configuration
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub struct ExternalSolver {


naming nit (there is no such thing as an internal solver anymore)

Suggested change

pub struct ExternalSolver {

pub struct Solver {

fleupold · 2024-11-19T16:29:07Z

crates/autopilot/src/run_loop.rs

+            .into_iter()
+            .filter(|participant| {
+                let submission_address = participant.driver().submission_address;
+                let is_solution_from_driver = participant.solution().solver() == submission_address;


this will cause issues in case copium wants to use multiple different solvers, cc @bram-vdberg

github-actions · 2024-11-27T00:08:46Z

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.

m-lord-renkse commented Oct 17, 2024

View reviewed changes

crates/autopilot/src/infra/solvers/mod.rs Outdated Show resolved Hide resolved

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch 5 times, most recently from faf5c2a to b52a744 Compare October 17, 2024 11:59

Add driver submission driver to the autopilot

42bf53a

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from b52a744 to 42bf53a Compare October 17, 2024 12:00

m-lord-renkse marked this pull request as ready for review October 17, 2024 12:06

m-lord-renkse requested a review from a team as a code owner October 17, 2024 12:06

m-lord-renkse commented Oct 17, 2024

View reviewed changes

crates/shared/src/arguments.rs Outdated Show resolved Hide resolved

MartinquaXD reviewed Oct 17, 2024

View reviewed changes

crates/autopilot/src/run_loop.rs Outdated Show resolved Hide resolved

crates/autopilot/src/run_loop.rs Outdated Show resolved Hide resolved

add e2e test

17dbb88

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch 10 times, most recently from ef21e9d to 4910d52 Compare October 17, 2024 16:11

rework comments

df34e7e

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from 4910d52 to df34e7e Compare October 17, 2024 16:18

sunce86 reviewed Oct 17, 2024

View reviewed changes

crates/autopilot/src/run_loop.rs Outdated Show resolved Hide resolved

m-lord-renkse and others added 4 commits October 18, 2024 10:45

rework comments

e7d11c7

Merge branch 'main' into 3045/add-submission-address-autopilot

0014d0b

add arn to autopilot

bbf3814

lint

8371620

m-lord-renkse requested review from sunce86 and MartinquaXD October 18, 2024 12:46

sunce86 reviewed Oct 18, 2024

View reviewed changes

crates/autopilot/src/arguments.rs Outdated Show resolved Hide resolved

crates/autopilot/src/infra/solvers/mod.rs Outdated Show resolved Hide resolved

squadgazzz reviewed Oct 18, 2024

View reviewed changes

crates/autopilot/src/run_loop.rs Outdated Show resolved Hide resolved

squadgazzz reviewed Oct 18, 2024

View reviewed changes

crates/autopilot/src/run_loop.rs Outdated Show resolved Hide resolved

m-lord-renkse and others added 2 commits October 21, 2024 11:34

rework comments

beb2adb

Merge branch 'main' into 3045/add-submission-address-autopilot

69d8660

m-lord-renkse requested review from squadgazzz and sunce86 October 21, 2024 10:24

sunce86 approved these changes Oct 23, 2024

View reviewed changes

rework comments

02c199b

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from f556294 to 02c199b Compare October 25, 2024 07:34

Merge branch 'main' into 3045/add-submission-address-autopilot

09f5b75

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from 17886e0 to e7dd8ab Compare October 25, 2024 07:46

fix conflict

e3a6f7a

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from e7dd8ab to e3a6f7a Compare October 25, 2024 08:06

squadgazzz reviewed Oct 25, 2024

View reviewed changes

github-actions bot added the stale label Nov 2, 2024

github-actions bot closed this Nov 10, 2024

m-lord-renkse reopened this Nov 11, 2024

m-lord-renkse and others added 2 commits November 11, 2024 12:22

Merge branch 'main' into 3045/add-submission-address-autopilot

f1ab9a3

fix

bbf1691

m-lord-renkse force-pushed the 3045/add-submission-address-autopilot branch from 1137f42 to bbf1691 Compare November 11, 2024 12:06

github-actions bot removed the stale label Nov 12, 2024

Merge branch 'main' into 3045/add-submission-address-autopilot

e95cc90

fleupold approved these changes Nov 19, 2024

View reviewed changes

github-actions bot added the stale label Nov 27, 2024

m-lord-renkse removed the stale label Nov 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add driver submission address to the autopilot #3065

Add driver submission address to the autopilot #3065

m-lord-renkse commented Oct 17, 2024 •

edited

Loading

MartinquaXD left a comment •

edited

Loading

sunce86 left a comment •

edited

Loading

sunce86 Oct 23, 2024

m-lord-renkse Oct 25, 2024

m-lord-renkse Oct 25, 2024

squadgazzz Oct 25, 2024

m-lord-renkse Oct 25, 2024

squadgazzz Oct 25, 2024

m-lord-renkse Oct 25, 2024

github-actions bot commented Nov 2, 2024

fleupold left a comment

fleupold Nov 19, 2024

fleupold Nov 19, 2024

fleupold Nov 19, 2024

github-actions bot commented Nov 27, 2024

Add driver submission address to the autopilot #3065

Are you sure you want to change the base?

Add driver submission address to the autopilot #3065

Conversation

m-lord-renkse commented Oct 17, 2024 • edited Loading

Description

Changes

How to test

Related Issues

MartinquaXD left a comment • edited Loading

Choose a reason for hiding this comment

sunce86 left a comment • 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

Choose a reason for hiding this comment

github-actions bot commented Nov 2, 2024

fleupold left a 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

github-actions bot commented Nov 27, 2024

m-lord-renkse commented Oct 17, 2024 •

edited

Loading

MartinquaXD left a comment •

edited

Loading

sunce86 left a comment •

edited

Loading