Skip to content

Do not generate a seal if the block is generated from a past view #1807

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

Merged
merged 1 commit into from
Oct 8, 2019

Conversation

majecty
Copy link
Contributor

@majecty majecty commented Oct 7, 2019

If generating a block takes too much time, the view could be changed
before the miner module requests the signature. If the Tendermint
module receives a signature request of an old view, it should ignore
the message. Before this commit, the Tendermint module was
crashing when it gets a signature request from an old view.

It fixes #1805.

@majecty majecty added bug Something isn't working consensus labels Oct 7, 2019
@majecty majecty requested a review from foriequal0 October 7, 2019 07:53
assert!(self.is_signer_proposer(&parent_hash));
// We don't know at which view the node starts generating a block.
// If this node's signer is not proposer at the current view, return none.
if self.is_signer_proposer(&parent_hash) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if self.is_signer_proposer(&parent_hash) {
if !self.is_signer_proposer(&parent_hash) {

// We don't know at which view the node starts generating a block.
// If this node's signer is not proposer at the current view, return none.
if self.is_signer_proposer(&parent_hash) {
cwarn!(ENGINE, "Proposer is generated after view change");
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
cwarn!(ENGINE, "Proposer is generated after view change");
cwarn!(ENGINE, "View is changed after requesting the seal");

Copy link
Contributor Author

Choose a reason for hiding this comment

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

View is not changed after the seal request. View is changed after the block generation request.
What do you think about "Seal request for an old view"?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's good.

@majecty majecty force-pushed the f/do-not-generate-seal branch 2 times, most recently from d259882 to b6362cd Compare October 7, 2019 08:41
foriequal0
foriequal0 previously approved these changes Oct 7, 2019
@ScarletBlue ScarletBlue changed the title Do not generate a seal when the block is generated from a past view Do not generate a seal if the block is generated from a past view Oct 7, 2019
If generating a block takes too much time, the view could be changed
before the miner module requests the signature. If the Tendermint
module receives a signature request of an old view, it should ignore
the message. Before this commit, the Tendermint module was crashing
when it gets a signature request from an old view.
@majecty majecty force-pushed the f/do-not-generate-seal branch from b6362cd to d6cf766 Compare October 7, 2019 09:03
@majecty majecty merged commit 08f5b31 into CodeChain-io:master Oct 8, 2019
@majecty majecty deleted the f/do-not-generate-seal branch October 8, 2019 04:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stop generating a block if view is changed while creating the block.
2 participants