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

fix(code/core-consensus): Reproduce and fix consensus being stuck with byzantine proposer #853

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

ancazamfir
Copy link
Collaborator

@ancazamfir ancazamfir commented Feb 13, 2025

Closes: #850
See: #857


PR author checklist

For all contributors

For external contributors

Copy link

codecov bot commented Feb 13, 2025

❌ 10 Tests Failed:

Tests completed Failed Passed Skipped
149 10 139 0
View the top 3 failed test(s) by shortest run time
informalsystems-malachitebft-test::it wal::non_proposer_crashes_after_voting_parts_only
Stack Traces | 7.59s run time
No failure message available
informalsystems-malachitebft-test::it vote_sync_bcast::start_late
Stack Traces | 60s run time
No failure message available
informalsystems-malachitebft-test::it vote_sync_bcast::crash_restart_from_latest
Stack Traces | 60s run time
No failure message available

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@@ -50,12 +50,17 @@ pub struct State {
// order for each node to likely propose different values at
// each round.
fn seed_from_address(address: &Address) -> u64 {
address.into_inner().chunks(8).fold(0u64, |acc, chunk| {
Copy link
Member

Choose a reason for hiding this comment

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

Good idea! Maybe we should allow the test framework to trigger such behavior, what do you think? Will think about how to implement that properly

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, let's discuss. Also to figure out how to enable byzantine proposer.

code/crates/test/app/src/state.rs Outdated Show resolved Hide resolved
@romac romac changed the title chore(code): Reproduce and fix consensus being stuck with byzantine proposer fix(code/core-consensus): Reproduce and fix consensus being stuck with byzantine proposer Feb 17, 2025
@ancazamfir
Copy link
Collaborator Author

@romac what do you think about making a minimal PR with just the changes required to eliminate the vote equivocation.
Meaning only with the changes in:

core-consensus/src/handle/driver.rs
core-consensus/src/state.rs
core-driver/src/driver.rs

We can merge to main if CI is green and then we can continue on this PR.

@romac
Copy link
Member

romac commented Feb 19, 2025

Here is the PR with only those changes: #864

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.

Consensus stuck in votesync upon restart
2 participants