Cleanup score #2585
Cleanup score #2585
Conversation
There was a problem hiding this comment.
My understanding is that unfortunately the correct bidding behavior still requires the risk parameters to adjust the computed solution / reported score based on the revert risk (see slack).
Thanks. I reverted everything related to risk parameters. Asked in slack to define how are we supposed to use risk parameters to adjust the reported fee of the solution (This is not strictly related to this PR since this PR refactors the score). |
| score::Error::RiskAdjusted(score::risk::Error::Boundary(_)) => return, | ||
| score::Error::Boundary(_) => return, | ||
| score::Error::Scoring(_) => return, // TODO: should we notify? | ||
| _ => return, |
There was a problem hiding this comment.
Can we keep listing all other variants explicitly? That would help to avoid mistakes in the future if a new variant is added, for example.
There was a problem hiding this comment.
I simplified the error enum and listed all of them. Can be additionally cleaned up as part of
#2597
There was a problem hiding this comment.
Actually forgot to commit 😶🌫️ will do it in a follow up PR
| /// No clearing prices are present for all trades. | ||
| InvalidClearingPrices, |
There was a problem hiding this comment.
Do we still need the enum? Can we use Kind::ScoringFailed directly without inner value?
There was a problem hiding this comment.
Actually we don't need it but I guessed it's less work to leave it like this since we will add more variants in #2597
|
Is it still used? services/crates/solvers/src/domain/solver/baseline.rs Lines 61 to 62 in bc6d339 |
Since the consensus on slack seems to be that the risk parameters can be replaced by offsetting the settlement gas cost it can be removed as well after all. Sorry for the back and forth. 😅 |
Description
Fixes #2493
Fixes #1494
Important notes:
Removed risk parameter config from solvers - requires cleanup of "riskParameters" in infrastructure repo- will be done in a separate PRFollow ups:
How to test
Existing tests. No change in behaviour is expected.