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

[WIP] consensus, core, eth, miner: use the same insert behavior between proposer and validators #123

Open
wants to merge 20 commits into
base: istanbul/develop
Choose a base branch
from

Conversation

markya0616
Copy link

…poser and validators

c.sendEvent(backlogEvent{
src: src,
msg: msg,
if !c.valSet.IsProposer(c.Address()) {
Copy link
Author

Choose a reason for hiding this comment

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

core isProposer()

Copy link
Author

Choose a reason for hiding this comment

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

OK

Copy link
Author

Choose a reason for hiding this comment

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

OK

@@ -838,6 +838,11 @@ func (bc *BlockChain) WriteBlock(block *types.Block) (status WriteStatus, err er
bc.mu.Lock()
defer bc.mu.Unlock()

if bc.HasBlock(block.Hash()) {
log.Info("Block exists", "hash", block.Hash())
Copy link
Author

Choose a reason for hiding this comment

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

log.Debug

Copy link
Author

Choose a reason for hiding this comment

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

OK

Copy link
Author

Choose a reason for hiding this comment

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

OK

@markya0616 markya0616 changed the base branch from improve_performance to feature/improve_performance July 18, 2017 02:29
@yutelin yutelin changed the title consensus, core, eth, miner: use the same insert behavior between pro… consensus, core, eth, miner: use the same insert behavior between proposer and validators Jul 21, 2017
logger log.Logger
db ethdb.Database
chain consensus.ChainReader
commitProposedWork func(*types.Block) error
Copy link

Choose a reason for hiding this comment

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

comment more on commitProposedWork and writeProposedWork

Copy link
Author

Choose a reason for hiding this comment

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

OK

@yutelin
Copy link

yutelin commented Jul 21, 2017

Please elaborate what the major difference from previous implementation is.
What we did before and what we do right now.

For example, previously proposer inserts the block through __ and validator insert the block through __.
And now, proposer and validators are the same. They call commitProposedWork during __ step to __? And call writeProposedWork during __ step for __ ?

@markya0616 markya0616 changed the base branch from feature/improve_performance to istanbul/develop July 24, 2017 06:45
@markya0616
Copy link
Author

OK. In previous implementation, we only verify block's header in Verify() and call InsertChain to insert committed blocks. We don't check the correctness of the block body in Verfiy(), so we may try to lock or commit a block with incorrect body. In this PR, we created an Istanbul worker to improve system performance and check block body (i.e., transactions) in Verify().
The procedure are as follows:

  1. Proposer call commitNewWork() to create a block after receiving NewBlockEvent event
  2. Proposer broadcasts the proposed blocks to other validators
  3. After validators received the proposed block, they call commitProposedWork() and then apply the transactions in the block to check the correctness of the block's body and header.
  4. If we received 2F+1 commits, we call writeProposedWork() to write the committed block into chain db.

markya0616 and others added 2 commits August 1, 2017 14:41
* consensus/istanbul: handle future preprepare
* consensus/istanbul: handle request timeout in evnet loop
* consensus, eth: start/stop core engine while start/stop mining
* eth, ethstats: fix crash while reporting to ethstats
* consensus/istanbul, miner: add new event to trigger new block creation
* eth, consensus/istanbul: improve sending messages
* consensus/istanbul: stop future preprepare timer while stop core
* consensus/istanbul: add cache in ecrecover()
@markya0616 markya0616 changed the title consensus, core, eth, miner: use the same insert behavior between proposer and validators [WIP] consensus, core, eth, miner: use the same insert behavior between proposer and validators Aug 8, 2017
@markya0616
Copy link
Author

This PR changes too many stuffs. We need more tests for this PR before merge

@stevenroose
Copy link

stevenroose commented Jan 24, 2018

Where can I report issues with Istanbul? I found one that might be slightly related to this change.

It's explained here in more detail: ethereum/EIPs#650 (comment)

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