Ban solvers based on the settlements success rate #3263
Conversation
# Conflicts: # crates/autopilot/src/domain/competition/participation_guard/db.rs
|
Reminder: Please consider backward compatibility when modifying the API specification.
Resolved |
squadgazzz
changed the title
| pub solver_blacklist_cache_ttl: Duration, | ||
|
|
||
| #[clap(flatten)] | ||
| pub non_settling_validator_config: NonSettlingValidatorConfig, |
There was a problem hiding this comment.
Please could you add comments?
There was a problem hiding this comment.
how does the term "validator" come into play here? Aren't these solvers?
| @@ -163,4 +163,26 @@ impl super::Postgres { | |||
| .await | |||
| .context("solver_competition::find_non_settling_solvers") | |||
| } | |||
|
|
|||
| pub async fn find_low_settling_solvers( | |||
There was a problem hiding this comment.
Please can you add a comment here too? 🙏
| ); | ||
| // Non-settling issue has a higher priority, remove duplicates from low-settling | ||
| // solvers. | ||
| low_settling_solvers.retain(|solver| !non_settling_solvers.contains(solver)); |
There was a problem hiding this comment.
This .contains() executes a O(n)search for each element of low_settling_solvers. Consider returning a HashSet from find_non_settling_solvers() and find_low_settling_solvers()
| self.0.non_settling_config.last_auctions_participation_count, | ||
| current_block, | ||
| ) | ||
| .await |
There was a problem hiding this comment.
Missing the debug trace tracing::debug!(solvers = ?non_settling_solver_names, "found non-settling solvers");
There was a problem hiding this comment.
Has this change been discussed with @harisang ? It's quite a steep intrusion into the arbitration cycle (any bonded solver is allowed to participate) and may require a CiP if we decide to selectively exclude solvers from the competition.
| pub solver_blacklist_cache_ttl: Duration, | ||
|
|
||
| #[clap(flatten)] | ||
| pub non_settling_validator_config: NonSettlingValidatorConfig, |
There was a problem hiding this comment.
how does the term "validator" come into play here? Aren't these solvers?
| /// participation in the competition. Otherwise, the solver will be | ||
| /// banned. | ||
| #[clap(long, env, default_value = "0.1")] | ||
| pub solver_min_settlement_success_rate: f64, |
There was a problem hiding this comment.
What happens if a solver has not won a single auction in the last n blocks? Then technically their success rate is 0 so they should be blocked. Does it make more sense to express this config in terms of a maximum failure rate instead?
| pub struct LowSettlingValidatorConfig { | ||
| /// Enables search of non-settling solvers. | ||
| #[clap( | ||
| id = "low_settling_solvers_blacklisting_enabled", |
| ); | ||
| self_.post_process( | ||
| &low_settling_solvers, | ||
| &dto::notify::Request::LowSettlingRate, |
There was a problem hiding this comment.
nit: HighSettleFailureRate might be more intuitive
Description
Another follow-up to #3257, which implements a suggested statistic-based approach #3257 (comment).
Changes
Introduces an additional DB-based validator, which searches for solvers with less than 10%(configurable) of successful settlements among 100 last auctions(configurable).
How to test
A new SQL query test.