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

Qbft cleanup #131

Merged
merged 18 commits into from
Feb 14, 2025
Merged

Qbft cleanup #131

merged 18 commits into from
Feb 14, 2025

Conversation

Zacholme7
Copy link
Member

Issue Addressed

#127

Proposed Changes

Some basic cleanup and small optimizations for QBFT. Gets rid of a few expects, introduces ValidData which represents validated data, includes roots in prepare and commit state, and a few other nitpicks.

Currently a draft as this needs to be tested once #128 is merged in.

@Zacholme7 Zacholme7 added the QBFT label Feb 10, 2025
anchor/qbft_manager/src/lib.rs Outdated Show resolved Hide resolved
anchor/common/qbft/src/qbft_types.rs Show resolved Hide resolved
anchor/common/qbft/src/lib.rs Outdated Show resolved Hide resolved
anchor/common/qbft/src/lib.rs Outdated Show resolved Hide resolved
anchor/common/qbft/src/lib.rs Outdated Show resolved Hide resolved
@Zacholme7 Zacholme7 marked this pull request as ready for review February 14, 2025 13:46
@Zacholme7
Copy link
Member Author

Zacholme7 commented Feb 14, 2025

investigating test failure. I think it may be some weird future forwarding with the start_paused. Had to run this test 43 times before it failed locally

Edit: Race condition in tester. Solved

@Zacholme7 Zacholme7 requested a review from dknopik February 14, 2025 15:46
Copy link
Member

@dknopik dknopik left a comment

Choose a reason for hiding this comment

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

nice, thanks!

@Zacholme7 Zacholme7 merged commit fdba58d into sigp:unstable Feb 14, 2025
10 checks passed
@Zacholme7 Zacholme7 deleted the qbft-cleanup branch February 14, 2025 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants