Notify solvers if driver fails for some reason #2186
Conversation
| SimulatorError::Blockchain(blockchain::Error::Gas(_)) => None, | ||
| SimulatorError::Blockchain(blockchain::Error::Response(_)) => None, | ||
| SimulatorError::Blockchain(blockchain::Error::Gas(_)) => Some(tx), | ||
| SimulatorError::Blockchain(blockchain::Error::Response(_)) => Some(tx), |
There was a problem hiding this comment.
Why these three are now classified as revert errors?
There was a problem hiding this comment.
Yeah, these seem more like they should be Other errors. Meaning it's entirely possible that the simulated transaction is fine but we just encountered an internal error.
There was a problem hiding this comment.
You are right about Gas (I thought it was gas amount estimation related, but it's purely gas price related which the transaction has no influence on).
However for blockchain::Error::Response it only happens when we fail to estimate the accessList which happens if the transaction doesn't simulate properly.
Will rename these variants to make that more clear.
| SimulatorError::Blockchain(blockchain::Error::Gas(_)) => None, | ||
| SimulatorError::Blockchain(blockchain::Error::Response(_)) => None, | ||
| SimulatorError::Blockchain(blockchain::Error::Gas(_)) => Some(tx), | ||
| SimulatorError::Blockchain(blockchain::Error::Response(_)) => Some(tx), |
There was a problem hiding this comment.
Yeah, these seem more like they should be Other errors. Meaning it's entirely possible that the simulated transaction is fine but we just encountered an internal error.
Description
We don't always notify solvers if their solutions fails in certain cases (e.g. if we cannot generate an access list for their solution). However for debugging, a solver should receive a /notify request for every solution it provided that doesn't end up ranking.
Changes
How to test
CI
Related Issues
Fixes #2140