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 logic #210

Merged
merged 21 commits into from
Nov 1, 2024
Merged

feat: ChonkyBFT logic #210

merged 21 commits into from
Nov 1, 2024

Conversation

brunoffranca
Copy link
Member

@brunoffranca brunoffranca commented Oct 22, 2024

  • Everything in chonky_bft folder is basically new.
  • UTHarness was moved to chonky_bft/testonly.rs. Several changes to it though.
  • All tests that use UTHarness were moved to chonky_bft/tests. Some of the old unit tests were repurposed, but a fair amount of the tests in chonky_bft/tests are new.
  • The tests in tests.rs were split between tests/mod.rs and tests/twins.rs (except for a few that used UTHarness and were moved as said before). There were no changes to them though.

Part of BFT-452

@brunoffranca brunoffranca changed the title ChonkyBFT logic feat: ChonkyBFT logic Oct 30, 2024
@brunoffranca brunoffranca marked this pull request as ready for review October 30, 2024 18:20
@brunoffranca
Copy link
Member Author

One thing I wanted to note is that I don't actually check anywhere that reproposals don't have a payload set. This technically is not necessary since the consensus just ignores the payload. But maybe we want to do that check anyway.

@pompon0
Copy link
Collaborator

pompon0 commented Oct 31, 2024

Yes, for defence in depth, please check that payload is included if and only if it is required.

@brunoffranca brunoffranca merged commit 00f6214 into bf-chonky Nov 1, 2024
5 of 7 checks passed
@brunoffranca brunoffranca deleted the bf-chonky-logic branch November 1, 2024 19:29
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