-
Notifications
You must be signed in to change notification settings - Fork 153
Replace RPC mempool API with in-memory tracking #4086
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
Conversation
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.
Code Review
This PR removes the usage of the node's mempool API for determining replacement gas prices by caching the last submitted transaction in memory. While this successfully reduces latency, it introduces a potential issue. The in-memory cache is not persistent across driver restarts, which can lead to transaction submission failures if a transaction was pending before a restart. I have added a comment with details on this issue.
squadgazzz
left a comment
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.
How would it work with the service restarts? If there is a pending tx in the mempool, we will never know the proper gas price, right? Should we fallback to the legacy approach then?
Description
In order to know which gas price we have to beat at least (in case of cancellations) we made the driver scan the RPC node's mempool using the respective API as this is the ultimate source of truth.
However, this has 2 issues:
Especially the latency seemingly causes us to not notify the connected solver about the tx submission at times. The submission process works as follows:
/settlecall and starts the submission/settlecall/settlecall the driver only polls the submission future for 1 more second but otherwise simply stops polling it (code)Usually the submission future and autopilot detect the breach of the submission deadline at the same time so the settle future naturally executes the cancellation logic during that grace period. However, with the latency introduced by the mempool API this grace period is often not sufficient anymore (especially on mainnet). Doing some back of the napkin calculation using logs it appears as if the driver is currently not cancelling and submitting the respective notification for ~40% of the
/settlecalls.There is an argument to be made that the submission strategy should be refactored more broadly to ensure that cancellations always get initiated (instead of just stopping to poll the settle future) but this PR should at least already resolve the current issue.
Changes
Instead of using the RPC's
mempoolAPI we simply store the last successfully submitted transactions in memory. Now that we only have to lookup a key in aDashmapthe latency will be as it was before.