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: enhancement of the current block generation logic. #1186

Merged
merged 4 commits into from
Nov 21, 2022

Conversation

setunapo
Copy link
Contributor

@setunapo setunapo commented Nov 17, 2022

Description

Currently, validator only try once to get transactions from TxPool to produce the block.
However, new transactions could arrive while the validator is committing transaction.
Validator should be allowed to add these new arrived transactions as long as Header.Timestamp is not reached.

Rationale

This PR will:

  • commitTransactions return with error code.
  • drop current mining block on new block imported or resubmit timer(default 10s) is triggered.
  • try fillTransactions several times for the best, not use append mode to follow the GasPrice rule.
  • an algorithm to determine new fillTransactions, based on: the number of new arrived transaction, the time left, the previous cost of fillTransactions, to avoid do fillTransactions too frequently.

And the general workflow of new implementation could be described as:
image

Example

NA.

Changes

It could effect the mining logic, validators will be update to align the mine logic.

@setunapo setunapo changed the title worker: some block produce enhancement of work.go worker: enhancement of the current block generation logic. Nov 17, 2022
@brilliant-lx brilliant-lx added the r4r ready for review label Nov 17, 2022
Currently, validator only try once to get transactions from TxPool to produce the block.
However, new transactions could arrive while the validator is committing transaction.
Validator should be allowed to add these new arrived transactions as long as
Header.Timestamp is not reached

This commit will:
** commitTransactions return with error code
** drop current mining block on new block imported
** try fillTransactions several times for the best
   not use append mode to follow the GasPrice rule.
** check if there is enough time for another fillTransactions.
miner/worker.go Outdated
return
}
}
if len(remoteTxs) > 0 {
txs := types.NewTransactionsByPriceAndNonce(env.signer, remoteTxs, env.header.BaseFee)
if w.commitTransactions(env, txs, interruptCh, stopTimer) {
err = w.commitTransactions(env, txs, interruptCh, stopTimer)
if err == errBlockInterruptedByNewHead || err == errBlockInterruptedByOutOfGas || err == errBlockInterruptedByTimeout {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if seems useless here, would return anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

accepted.

It may not efficient if schedule fillTransactions when new transactions arrive.
It could make the CPU keep running.
To make is more efficient:
  1.schedule fillTransactions when a certain amount of transaction are arrived.
  2.or there is not much time left.
miner/worker.go Outdated Show resolved Hide resolved
miner/worker.go Outdated Show resolved Hide resolved
@unclezoro unclezoro added the enhancement New feature or request label Nov 20, 2022
unclezoro
unclezoro previously approved these changes Nov 21, 2022
When new block is imported, there is no need to commit the current
work, even the new imported block is offturn and itself is inturn.

That is because when offturn block is received, the inturn block is
already later to broadcast block, deliver the later block will cause
many reorg, which is not reasonable.

And also make sure all useless work can be discarded, to avoid goroutine leak.
it is not necssary to add more transaction when resubmit is fired.
the resubmit logic was for PoW and can be removed later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request r4r ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants