Skip to content
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

Worker async sorting #1061

Closed
wants to merge 25 commits into from
Closed

Worker async sorting #1061

wants to merge 25 commits into from

Conversation

jdowning100
Copy link
Contributor

@jdowning100 jdowning100 commented Aug 29, 2023

@dominant-strategies/core-dev
The worker now asynchronously sorts the txpool (set to every three seconds in sortedPoolCacheInterval) which will allow higher priority transactions to be confirmed even if blocks are full. In addition, there is no longer any priority given to local txs vs remote ones. Note that this sorted list is cached in the worker and sorting is only done in a zone that is processing state.

When constructing a pending block, the worker checks and commits transactions from the sorted list, removing them from the list if they are valid. If the worker is called again quickly (GeneratePendingHeader), the sorted list will be in the next state, i.e. the sorted list will be in a state that assumes the previously constructed block was added to the chain. The sorted list may also be outdated in the case of a data race - that is, a new block is found and the sorter hasn't had a chance to re-sort. In the case of a data race, high-priority transactions that were included in the worker's candidate block but not included in the new current head will not be included in the next candidate block until the worker has a chance to re-sort.

In the case of the reorg, the sorted list is almost guaranteed to be wrong, so it's set to nil and the sorter waits the sortedPoolCacheInterval before attempting to sort again. A possible improvement is to listen to new chainhead events and set the sorted list to nil whenever a new block is found instead of just reorgs. In the case of a nil sorted list, the worker falls back to filling the block without sorting (randomized fill). Another possible improvement is for the worker to generate a new pending header immediately after async sorting so that the candidate block has the latest high-priority transactions (however, interrupting the miner in the middle of mining a valid block might not be a good idea).

One issue is that the sorted list is a heap, so when the top element is removed, it's possible that the heap needs to be re-sorted (worst case O(log n) ) which is not possible to do asynchronously. Currently, the worker pops the top element if there's an error with the transaction which requires a re-sort. If the transaction is a low nonce (data race) or does not have an error, the next transaction for the account, if there is one, is added to the block next without re-sorting.

Please let me know your thoughts.

@jdowning100 jdowning100 marked this pull request as draft September 5, 2023 16:34
@wizeguyy wizeguyy closed this Sep 7, 2023
@wizeguyy wizeguyy deleted the worker-sort branch September 7, 2023 17:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants