Parlia: Some updates of the miner worker #1182
Merged
Conversation
This file contains 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
setunapo
force-pushed
the
worker_delay
branch
from
3b0f461
to
7832b73
Compare
Right now, DelayLeftOver is used to reserve time for block finalize, not block broadcast. And the code does not work as expected. The general block generation could be described as: |- fillTransactions -|- finalize a block -|- wait until the period(3s) reached -|- broadcast -|
setunapo
force-pushed
the
worker_delay
branch
from
7832b73
to
c80586c
Compare
setunapo
changed the title
setunapo
force-pushed
the
worker_delay
branch
from
6fab891
to
b903124
Compare
setunapo
changed the title
It could be a very old PoW logic, which try to add more transaction into the pending block when mining is stopped. Mining can be stopped when: 1.download started. 2.manually stopped by RPC. It is unnecessary to add more transaction into the pending block if a validator is stopped. And updateSnapshot() is not needed as well, it is to get the pending mining snapshot.
setunapo
force-pushed
the
worker_delay
branch
from
b903124
to
4c7f004
Compare
brilliant-lx
requested review from
kyrie-yl,
j75689,
unclezoro,
keefel,
qinglin89 and
yutianwu
qinglin89
approved these changes
Nov 16, 2022
unclezoro
reviewed
Nov 16, 2022
| if len(localTxs) > 0 { | ||
| txs := types.NewTransactionsByPriceAndNonce(env.signer, localTxs, env.header.BaseFee) | ||
| if w.commitTransactions(env, txs, interruptCh) { | ||
| if w.commitTransactions(env, txs, interruptCh, stopTimer) { |
There was a problem hiding this comment.
x := time.NewTimer(time.Second)
<-x.C
fmt.Println("read delay 1")
<-x.C
fmt.Println("read delay 2")
It turns out that fmt.Println("read delay 2") is not reachable.
So if the timer trigger during the first commitTransactions for local tx, then commitTransactions will have no time delay limitation .
There was a problem hiding this comment.
yes, it do have the problem, will fix it later.
This was referenced Nov 29, 2022
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.
Description
Some update of the worker.go
1.Delay() with DelayLeftOver
Right now, DelayLeftOver is used to reserve time for block finalize, not block broadcast.
The general block generation could be described as:
|- fillTransactions -|- finalize a block -|- wait until the period(3s) reached -|- broadcast -|And the code does not work as expected, some code improve on the Delay() logic.
2.no need to add transaction to the current work if mining is stopped.
It seems that it is unnecessary to add transaction when mining is stopped.
It is a old logic of PoW, which is no longer need for BSC.
Rationale
none
Example
none
Changes
none