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

Wrong base block used when fast-forwarding #51

Closed
loongy opened this issue Mar 23, 2020 · 1 comment
Closed

Wrong base block used when fast-forwarding #51

loongy opened this issue Mar 23, 2020 · 1 comment
Assignees
Labels
bug Something isn't working
Milestone

Comments

@loongy
Copy link
Contributor

loongy commented Mar 23, 2020

Fast-forwarding allows nodes to catch up with the latest block if they have been offline for a substantial amount of time.

Currently, syncLatestCommit(lastestCommit LastestCommit) implements this functionality. It uses p.blockchain.BlockAtHeight(0) to get the “latest base block” to confirm that the latestCommit.Precommits are actually valid. This is incorrect, because rebases are expected to happen which would change the signatory set from the one found in block 0. Furthermore, as of #49, the Process will no longer receive messages from future heights. This means it would never see messages prompting it to fast-forward.

Requirements

The actual problem here: “always using the latest base block to validate the latest block”. Currently, the syncLatestCommit is obviously wrong but the solution is more complex than naïvely introducing a function like LatestBaseBlock to the process.Blockchain interface.

What happens if the node misses a rebase block? For example: the last block it has seen is at height H, then a rebase happens at H+1, and then it sees a bad proposal at height H+2 from the old signatories from H in an attempt to fast-forward the node to a bad fork.

To protect against this, the node needs to know whether or not it has missed rebasing block. To know this, there must be an interface provided by the user that gives it this information (because, currently, rebasing is controlled entirely by user implemented interfaces via block proposal and block validation).

Solution

Introduce the BaseBlocksInRange(h1, h2 Height) int method to the Rebaser interface. This method must return the number of base blocks between h1 and h2.

When receiving a Propose, the Replica calls BaseBlocksInRange(Process.CurrentHeight(), Propose.Height()) and checks whether or not the return value is 0. If yes, then it forwards the Propose to the Process and lets it fast-forward correctly (we need to add LatestBaseBlock to the process.Blockchain interface). If no, then the Replica engages in block syncing.

Block syncing will require some new messages. A Replica can send a ResyncBaseBlocks message and get back the latest N base blocks (and their associated precommits). Generally, we should not need to synchronise more than 2 base blocks. In response to receiving this message, an honest Replica will reply with a LatestCommit (we need to promote LatestCommit to its own message type).

@loongy loongy added this to the v1.3.0 milestone Mar 25, 2020
@loongy loongy self-assigned this Mar 30, 2020
@loongy loongy added the bug Something isn't working label Mar 30, 2020
@loongy
Copy link
Contributor Author

loongy commented Apr 1, 2020

This was implemented without explicit resynching of missing base blocks. See #77 for more information.

@loongy loongy closed this as completed Apr 1, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant