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

Improve mempool extraction #1421

Closed
herr-seppia opened this issue Jun 20, 2022 · 1 comment · Fixed by #1422
Closed

Improve mempool extraction #1421

herr-seppia opened this issue Jun 20, 2022 · 1 comment · Fixed by #1422
Assignees

Comments

@herr-seppia
Copy link
Member

Describe what you want implemented
Transaction extracted from the mempool should be weighted with the average gas spent for that kind of transaction.
With this weight, the new prioritisation should be ForecastGasSpent*GasPrice.
Transactions should be extracted until Sum(ForecastGasSpent)>=GasLimit

This has no impact on rusk, if actual Gas is greater than the forecast, transactions will not be included the block (as per the current behaviour)

Describe "Why" this is needed
To extract transactions from the mempool, the only criteria currently used is GasLimit*GasPrice.
These leave the stage to two unwanted scenario:

  1. With higher GasLimit the prioritisation of a transfer transaction can be higher than a stake transaction, even if the latter will effectively use more gas
  2. All transactions that fit the BlockSize limit are executed to try to fit into a block (even if there is no reasonable gas left). This highly impact ExecuteStateTransition performance and, therefore, the whole consensus performance

Describe alternatives you've considered
N/A

Additional context
See also #1240

@robinmario90
Copy link

As discussed with @herr-seppia, it might actually make sense to only use the criteria of GasPrice for prioritization, as this would be most beneficial for node rewards. As long as I can add transactions up till the point that the block is full without running out of time adding transactions, prioritization on GasPrice would be most beneficial.

Example:

  • There are two transactions: Stake consume 10gas, and Transfer consume 1gas.
  • If I put my transfer with gasPrice=9, It should be prioritized over the stake even if I will pay 9 total instead of 10 total.
  • If I have 100 gas to fill in a block, I start with 9*1, claiming 9, and still having 99 gas to fill after this transaction.

The most important part is introducing: Sum(ForecastGasSpent)>=GasLimit

herr-seppia added a commit that referenced this issue Jun 20, 2022
herr-seppia added a commit that referenced this issue Jun 20, 2022
herr-seppia added a commit that referenced this issue Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants