This repository has been archived by the owner on Nov 6, 2020. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Include the seal when populating the header for a new block #11475
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
12abda9
debug signer
dvdplm f1acd9a
Merge branch 'master' into dp/fix/maximumEmptySteps-11445
dvdplm fc04c69
Don't panic if empty_steps_transition already happened
dvdplm 722ca99
Cleanup some code in Aura
dvdplm b1be876
Include the seal when populating the header for a new block (fixes #1…
dvdplm 917c094
Merge branch 'master' into dp/fix/maximumEmptySteps-11445
dvdplm 3103b2f
cleanup
dvdplm 47ee9b1
cleanup2
dvdplm a2c704a
Review grumbles
dvdplm 221afa7
Update ethcore/engines/authority-round/src/lib.rs
dvdplm 1298901
Update ethcore/engines/authority-round/src/lib.rs
dvdplm 2c984e7
Update ethcore/src/block.rs
dvdplm File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix for #11445 is here: without this the
LockedBlock
built byenact()
lacks the seal and this causes the state root to be wrong.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem right for me though. The idea with block types is that you go from:
OpenBlock -> ClosedBlock -> LockedBlock -> SealedBlock
and only the last type is guaranteed to have the seal.
How come the
seal
is included in the state root anyway? It's two separate things and really I don't see a way howseal
affects thestate
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added comment + ref to this PR. It still feels a bit dirty to do this here and I'm open to alternatives.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: is this not called from
Miner::seal_and_import_block_internally
code path? It seems likeMiner::prepare_block
doesn't callpopulate_from
, correct?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure I understand what you mean, but the reverse call-graph for
populate_from()
looks like this:In other words,
populate_from()
is only called on the block verification path, not for block producing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool, yeah that's what I meant.