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

Threshold check in PBFT #2173

Closed
edsko opened this issue May 29, 2020 · 4 comments
Closed

Threshold check in PBFT #2173

edsko opened this issue May 29, 2020 · 4 comments
Labels
byron Required for a Byron mainnet: replace the old core nodes with cardano-node consensus issues related to ouroboros-consensus

Comments

@edsko
Copy link
Contributor

edsko commented May 29, 2020

After #2169 , nodes can now indicate that they are a leader for a slot, but cannot lead due to some problem. For PBFT, this problem could be that the key does not match the delegation state (#2172), but it could also mean that the block would exceed the PBFT window threshold.

Right now we don't check for this in PBFT. Initially I thought that doing so would be wrong, because it means that I could force you to give up your slot simply by producing few blocks myself. However, if a node produces a block ignoring this, that block would anyway promptly be discarded (by its own node!) as invalid, so there is no reason not to do the check.

The absence of the check means that the consensus tests need to make sure that the threshold does not get exceeded, or else they end up with invalid blocks. Introducing this check would allow us to lift that restriction. It would also give a better log message should this ever happen in real life, although in reality we don't expect this to ever happen, which is why I've marked this purely as a testing ticket.

@edsko edsko added consensus issues related to ouroboros-consensus protocol testing byron Required for a Byron mainnet: replace the old core nodes with cardano-node labels May 29, 2020
@nfrisby
Copy link
Contributor

nfrisby commented May 29, 2020

To clarify: this ticket is a follow-on to Issue 2172 that expands the task therein from just checking the key is OK (up-to-date delegation map, etc) to also checking that the PBFT window threshold is satisfied, right?

@edsko
Copy link
Contributor Author

edsko commented May 29, 2020

Correct.

@edsko
Copy link
Contributor Author

edsko commented Jun 1, 2020

because it means that I could force you to give up your slot simply by producing few blocks myself.

actually, this is doubly incorrect. Not only would producing a block be pointless in this case anyway (as I mention in the last paragraph in the ticket description), but also, the threshold is computed over the window size, independent from the slots filled by other nodes.

@edsko
Copy link
Contributor Author

edsko commented Jun 1, 2020

Closed in #2176 .

@edsko edsko closed this as completed Jun 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
byron Required for a Byron mainnet: replace the old core nodes with cardano-node consensus issues related to ouroboros-consensus
Projects
None yet
Development

No branches or pull requests

2 participants