continue processing orders on error #506
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📝 Summary
When testing #489, I noticed that when processing new orders, if one of them fails, we stop further processing.
This PR addresses this issue.
💡 Motivation and Context
The effect of the above-explained behaviour, in the worst case scenario, is that the whole slot can fail, meaning if the processing of the new order fails for the very first order, the whole slot ends up being empty, and in non-worse scenario we loose % of transactions, depending on when the processing was stopped.
When processing new order fails?
The processing of the new orders will fail only if the StateProvider was unable to provide the Nonce, i.e. the StateProvider errors.
This is edge-case, but this can happen.
I don't see any downsides with this approach, so this is why I opened PR instead of an issue. (I'm not saying hat there aren't any, just that I don't see them 🙂)
Process new simulations
Similar argument can be made for the processing new simulations, specifically for the following:
The code above only errors in case of the nonce issues, and in that scenario we are stopping processing of all simulation tasks.
I haven't pushed this change because I wanted so se how you feel about the this, especially since simulations seem more serious than orders.
✅ I have completed the following steps:
make lint
make test