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

consensus: Ignore candidate with wrong prev_block_hash #1183

Closed
fed-franz opened this issue Dec 5, 2023 · 6 comments · Fixed by #1355
Closed

consensus: Ignore candidate with wrong prev_block_hash #1183

fed-franz opened this issue Dec 5, 2023 · 6 comments · Fixed by #1355
Labels
module:consensus Issues related to consensus module

Comments

@fed-franz
Copy link
Contributor

Summary

Currently, if we receive a candidate block with prev_block_hash different from our tip's hash, we consider the block as invalid and hence vote Invalid for it.
However, it might be the case that the block generator is actually on a fork (and happens to be the generator for the same round and iteration) and hence produces a block on top of a different tip.

Possible solution design or implementation

Instead of voting Invalid, a candidate with a wrong prev_block_hash can be simply discarded (as it's not on our chain).
A Provisioner participating in the block validation would then timeout instead of voting Invalid.

Additional context

If we implement slashing and apply it to generators producing invalid blocks, we might mistakenly punish the block producer just for being on a fork.

@fed-franz fed-franz added the module:consensus Issues related to consensus module label Dec 5, 2023
@autholykos
Copy link
Member

autholykos commented Dec 6, 2023

I also do not like to vote NIL here (or vote INVALID as per #1176). since it might contribute to create an attested block on a fork the node is not partaking into

@fed-franz
Copy link
Contributor Author

I also do not like to vote NIL here (or vote INVALID as per #1176). since it might contribute to create an attested block on a fork the node is not partaking into

In this case the received block would simply be ignored. But since the node is waiting for a block from this block producer (which is not received because the producer is on a fork) it will timeout and vote accordingly

@herr-seppia
Copy link
Member

if we receive a candidate block with prev_block_hash different from our tip's hash, we consider the block as invalid and hence vote Invalid for it

Can you show the code that does this?
Seems to me that if a VTS fails, the message is simply ignored

@fed-franz
Copy link
Contributor Author

if we receive a candidate block with prev_block_hash different from our tip's hash, we consider the block as invalid and hence vote Invalid for it

Can you show the code that does this? Seems to me that if a VTS fails, the message is simply ignored

Apparently the header is only checked when receiving a block... plus, as you say, a failure on VST makes the provisioner ignore the candidate... does this mean we don't vote for an invalid candidate?

@herr-seppia
Copy link
Member

In addition to the skip of the proposal phase. Should we skip every "present" message with prev_block_hash different from our chain tip?
If so, should we skip the repropagation too?

@fed-franz
Copy link
Contributor Author

In addition to the skip of the proposal phase. Should we skip every "present" message with prev_block_hash different from our chain tip? If so, should we skip the repropagation too?

IMO, yes and yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module:consensus Issues related to consensus module
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants