core/txpool: remove queue, implement temporary buffer (limbo)#27004
Closed
holiman wants to merge 11 commits into
Closed
core/txpool: remove queue, implement temporary buffer (limbo)#27004holiman wants to merge 11 commits into
holiman wants to merge 11 commits into
Conversation
Member
|
One idea would be to evict the transactions from limbo based on their arrival time. I believe we have that in the transaction object somewhere. This would provide a total ordering of the limbo and prevent us from evicting transactions that just arrived before we do the reorganize procedure |
Contributor
Author
|
I'll close this for now, might pick it up at a later point in time. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR implements the "Limbo" idea.
"Future" transactions are problematic, in that it can be used to push pending transactions out. The reason for even having future transactions is that we cannot guarantee stable delivery: a node has
[tx1, tx2], and announces it to a peer. During delivery, the peer may receive them in the order[tx2, tx1], and not necessarily within the same network packet/batch. When this occurs, it would be bad UX if we droppedtx2.To mitigate this, what we can do is have a 'limbo'. A transaction entering the node can take two paths:
Every
Nseconds, weAt the end of this loop, the limbo is empty again
In this system, the transaction pool itself is only ever aware of executable transactions. A user submitting multiple transactions may notice a degradation in propagation speeds of the "later" transactions, but should not experience them being dropped by the network.
This PR is a work in progress, which implements most of this. The removal of the
queuemakes a lot of code redundant. For now, I have chosen to leave the code mostly in place, but commenting it out. This is to make it easier for a reviewer to check and see that the removed code indeed was not useful any more.I also split up the validation in two phases:
A lot more work needs to be done on the PR itself, and also the tests are failing en masse.