-
Notifications
You must be signed in to change notification settings - Fork 90
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
Risk calculation in solvers
crate
#1919
Conversation
This reverts commit d43b05c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while Naive solver returns success_probability=1 since it does not have any gas estimate.
The naive solver always operates on a single liquidity instance, which has a gas estimate attached to it (requires adding the constant settlement overhead). Can we add this logic to the PR (should be quite small)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Approving assuming:
- The
tests::Config::None
variant is removed if it is indeed no longer used - @fleupold's comment of getting the gas estimate for the naive solver is implemented (should be doable in the same/similar way to the Baseline solver)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just an issue with the gas math imo.
Description
Implements #1899
Changes
Risk parameters are now part of
solvers
crate configuration.Based on risk parameters,
success_probability
is calculated and forwarded todriver
.This PR is aware of bad gas estimates used. Better gas estimates might come from #1531
Currently,
Dex
andBaseline
andNaive
solvers are using these bad gas estimatesRelease
Risk parameters need to be defined in the configuration. Risk parameters should be extracted from https://github.com/cowprotocol/services/blob/main/crates/solver/src/solver/risk_computation.rs#L53
IF we want to stop using bad gas estimates and just fallback to
success_probability=1
, then risk parameters need to be set as[0,0,0,f64::MAX]
.