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 11 commits into
base: main
Choose a base branch
from
2 changes: 1 addition & 1 deletion code/crates/core-types/src/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ where
/// It is parameterized by a context type `Ctx` that defines the specific types used
/// for votes, proposals, and other consensus-related data structures.
///
/// Implementors of this trait are responsible for managing the private keys used for signing
/// Implementers of this trait are responsible for managing the private keys used for signing
/// and providing verification logic using the corresponding public keys.
pub trait SigningProvider<Ctx>
where
Expand Down
17 changes: 13 additions & 4 deletions code/crates/test/app/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,14 +82,23 @@ pub async fn run(
// we send back the very same value.
let proposal = match state.get_previously_built_value(height, round).await? {
Some(proposal) => {
info!(value = %proposal.value.id(), "Re-using previously built value");
proposal
let new_proposal = state.propose_value(height, round).await?;
error!(
"XXX Not Re-using previously built value {:} but a new one {:}",
proposal.value.id(),
new_proposal.value.id()
);
new_proposal
}
None => {
// If we have not previously built a value for that very same height and round,
// we need to create a new value to propose and send it back to consensus.
info!("Building a new value to propose");
state.propose_value(height, round).await?
let proposal = state.propose_value(height, round).await?;
error!(
"XXX Building a new value to propose {:}",
proposal.value.id()
);
proposal
}
};

Expand Down
9 changes: 7 additions & 2 deletions code/crates/test/app/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

romac marked this conversation as resolved.
Show resolved Hide resolved
let timestamp = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos() as u64;
let addr_seed = address.into_inner().chunks(8).fold(0u64, |acc, chunk| {
let term = chunk.iter().fold(0u64, |acc, &x| {
acc.wrapping_shl(8).wrapping_add(u64::from(x))
});
acc.wrapping_add(term)
})
});
addr_seed.wrapping_add(timestamp)
}

impl State {
Expand Down
71 changes: 71 additions & 0 deletions code/crates/test/tests/it/wal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -194,3 +194,74 @@ async fn non_proposer_crashes_after_voting(params: TestParams) {
)
.await
}

#[tokio::test]
async fn restart_with_byzantine_proposer() {
byzantine_proposer_crashes_after_proposing(TestParams {
value_payload: ValuePayload::ProposalAndParts,
..TestParams::default()
})
.await
}

async fn byzantine_proposer_crashes_after_proposing(params: TestParams) {
#[derive(Clone, Debug, Default)]
struct State {
first_proposed_value: Option<LocallyProposedValue<TestContext>>,
}

const CRASH_HEIGHT: u64 = 3;

let mut test = TestBuilder::<State>::new();

test.add_node()
.with_voting_power(10)
.start()
.wait_until(CRASH_HEIGHT)
.crash()
.restart_after(Duration::from_secs(5))
.success();

test.add_node()
.with_voting_power(10)
.start()
.wait_until(CRASH_HEIGHT)
.crash()
.restart_after(Duration::from_secs(5))
.success();

test.add_node()
.with_voting_power(10)
.start()
.wait_until(CRASH_HEIGHT)
// Wait until this node proposes a value
.on_event(|event, state| match event {
Event::ProposedValue(value) => {
info!("Proposer proposed block: {:?}", value.value);
state.first_proposed_value = Some(value);
Ok(HandlerResult::ContinueTest)
}
_ => Ok(HandlerResult::WaitForNextEvent),
})
// Crash right after
.crash()
// Restart after 5 seconds
.restart_after(Duration::from_secs(5))
// Check that we replay messages from the WAL
.expect_wal_replay(CRASH_HEIGHT)
// Wait until it proposes a value again, while replaying WAL
// Check that it is the same value as the first time
.on_proposed_value(|_value, _state| Ok(HandlerResult::ContinueTest))
.wait_until(CRASH_HEIGHT + 2)
.success();

test.build()
.run_with_params(
Duration::from_secs(60),
TestParams {
enable_sync: false,
..params
},
)
.await
}
Loading