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

feat: ChonkyBFT types #209

Merged
merged 10 commits into from
Oct 21, 2024
Merged

feat: ChonkyBFT types #209

merged 10 commits into from
Oct 21, 2024

Conversation

brunoffranca
Copy link
Member

@brunoffranca brunoffranca commented Oct 15, 2024

Implemented all types for ChonkyBFT. Work on this is all on the roles crate. Tests in roles crate are passing. Note that I'm not keeping the old types, that's because ReplicaCommit has no changes to it (I checked and serialization is the same for old and new, old signatures still work) as well as CommitQC and FinalBlock. Any type that an EN could receive, or that ends on our database, doesn't suffer any alterations. So it seems that the upgrade to ChonkyBFT is backwards-compatible for ENs. Since we only have one validator, we can upgrade the whole network without a planned hard fork.

Part of BFT-452

@matter-labs matter-labs deleted a comment from github-actions bot Oct 15, 2024
@brunoffranca brunoffranca changed the title ChonkyBFT types feat: ChonkyBFT types Oct 15, 2024
@brunoffranca brunoffranca marked this pull request as ready for review October 16, 2024 01:21
@brunoffranca
Copy link
Member Author

I still need to add tests for some new error types that I introduced.

@pompon0
Copy link
Collaborator

pompon0 commented Oct 16, 2024

Please do not remove the fastest hotstuff logic, we need both to coexist for the time of the migration.

@brunoffranca
Copy link
Member Author

Why do we need the old logic?

@pompon0
Copy link
Collaborator

pompon0 commented Oct 16, 2024

for transition?

@brunoffranca
Copy link
Member Author

But as I've wrote in the description, the change is backwards compatible for ENs. And we only have one validator, so it doesn't need to be backwards compatible for validators. Unless I'm missing something here, it seems like it should work.

/// A timeout QC is just a collection of timeout votes (with at least
/// QUORUM_WEIGHT) for the previous view. Unlike with the Commit QC,
/// timeout votes don't need to be identical.
/// The first proposal, for view 0, will always be a timeout.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it is in fact a good idea to make view 0 always time out, as this will be the initial synchronization point of the consensus. However please make sure that this doesn't create an artificial delay - i.e. validators should immediately cast timeout vote for view 0 on startup, rather than waiting for the actual timeout delay.

@brunoffranca brunoffranca merged commit f6168f8 into bf-chonky Oct 21, 2024
4 of 7 checks passed
@brunoffranca brunoffranca deleted the bf-chonky-types branch October 21, 2024 01:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants