diff --git a/code/crates/core-driver/src/driver.rs b/code/crates/core-driver/src/driver.rs index 869bb37a7..badb2f081 100644 --- a/code/crates/core-driver/src/driver.rs +++ b/code/crates/core-driver/src/driver.rs @@ -241,7 +241,7 @@ where } } - /// Get a commit certificate for the given round and value id. + /// Get a commit certificate for the given round and value id, if any. pub fn commit_certificate( &self, round: Round, @@ -252,6 +252,16 @@ where .find(|c| &c.value_id == value_id && c.round == round && c.height == self.height()) } + /// Get a commit certificate for the given round, if any. + pub fn commit_certificate_for_round( + &self, + round: Round, + ) -> Option<&CommitCertificate> { + self.commit_certificates + .iter() + .find(|c| c.round == round && c.height == self.height()) + } + /// Return the commit certificates, if any. pub fn commit_certificates(&self) -> &[CommitCertificate] { self.commit_certificates.as_ref() diff --git a/code/crates/core-driver/src/mux.rs b/code/crates/core-driver/src/mux.rs index a9f61d172..a8bf48d39 100644 --- a/code/crates/core-driver/src/mux.rs +++ b/code/crates/core-driver/src/mux.rs @@ -214,7 +214,9 @@ where // Store the certificate self.commit_certificates.push(certificate); - if let Some((signed_proposal, validity)) = + if certificate_round > self.round() { + return Some(RoundInput::SkipRound(certificate_round)); + } else if let Some((signed_proposal, validity)) = self.proposal_and_validity_for_round_and_value(certificate_round, certificate_value_id) { if validity.is_valid() { @@ -399,6 +401,10 @@ where for threshold in find_non_value_threshold(&self.vote_keeper, round) { result.push(self.multiplex_vote_threshold(threshold, round)) } + // This is equivalent of having a VKOutput::PrecommitAny + if self.commit_certificate_for_round(round).is_some() { + result.push(self.multiplex_vote_threshold(VKOutput::PrecommitAny, round)); + } result } } diff --git a/code/crates/core-driver/tests/it/extra.rs b/code/crates/core-driver/tests/it/extra.rs index 21ec981a6..79d6f14a8 100644 --- a/code/crates/core-driver/tests/it/extra.rs +++ b/code/crates/core-driver/tests/it/extra.rs @@ -2977,7 +2977,7 @@ fn polka_nil_and_prevote_step_precommit_nil() { prevote_nil_output(Round::new(0), &my_addr), precommit_nil_output(Round::new(0), &my_addr), start_precommit_timer_output(Round::new(0)), - start_precommit_timer_output(Round::new(0)), + start_precommit_timer_output(Round::new(0)), // :'( ], expected_round: Round::new(0), new_state: precommit_state(Round::new(0)), @@ -3646,6 +3646,264 @@ fn sync_decision_certificate_then_proposal() { run_steps(&mut driver, steps); } +#[test] +fn round_1_decision_during_round_0_with_votes() { + let value = Value::new(9999); + + let [(v1, _sk1), (v2, _sk2), (v3, sk3)] = make_validators([2, 3, 2]); + let (_my_sk, my_addr) = (sk3.clone(), v3.address); + + let height = Height::new(1); + let ctx = TestContext::new(); + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); + + let proposal = Proposal::new( + Height::new(1), + Round::new(1), + value.clone(), + Round::Nil, + v1.address, + ); + + let mut driver = Driver::new(ctx, height, vs, my_addr, Default::default()); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are not the proposer", + input: new_round_input(Round::new(0), v1.address), + expected_outputs: vec![start_propose_timer_output(Round::new(0))], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "v1 precommits a proposal in round 1", + input: precommit_input(Round::new(1), value.clone(), &v1.address), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "Receive proposal from round 1", + input: proposal_input_from_proposal(proposal.clone(), Validity::Valid), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "v2 precommits the same round 1 proposal, move to round 1", + input: precommit_input(Round::new(1), value.clone(), &v2.address), + expected_outputs: vec![new_round_output(Round::new(1))], + expected_round: Round::new(1), + new_state: new_round(Round::new(1)), + }, + TestStep { + desc: "Start round 1 and decide immediately", + input: new_round_input(Round::new(1), v2.address), + expected_outputs: vec![ + start_propose_timer_output(Round::new(1)), + decide_output(Round::new(1), proposal), + ], + expected_round: Round::new(1), + new_state: decided_state(Round::new(1), Round::new(1), value), + }, + ]; + + run_steps(&mut driver, steps); +} + +#[test] +fn round_1_decision_during_round_0_via_certificate() { + let value = Value::new(9999); + + let [(v1, _sk1), (v2, _sk2), (v3, sk3)] = make_validators([2, 3, 2]); + let (_my_sk, my_addr) = (sk3.clone(), v3.address); + + let height = Height::new(1); + let ctx = TestContext::new(); + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); + + let proposal = Proposal::new( + Height::new(1), + Round::new(1), + value.clone(), + Round::Nil, + v1.address, + ); + + let mut driver = Driver::new(ctx, height, vs, my_addr, Default::default()); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are not the proposer", + input: new_round_input(Round::new(0), v1.address), + expected_outputs: vec![start_propose_timer_output(Round::new(0))], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "Receive proposal from round 1", + input: proposal_input_from_proposal(proposal.clone(), Validity::Valid), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "we get a commit certificate for v at round 1, move to round 1", + input: commit_certificate_input_at( + Round::new(1), + value.clone(), + &[v1.address, v2.address], + ), + expected_outputs: vec![new_round_output(Round::new(1))], + expected_round: Round::new(1), + new_state: new_round(Round::new(1)), + }, + TestStep { + desc: "Start round 1 and decide immediately", + input: new_round_input(Round::new(1), v2.address), + expected_outputs: vec![ + start_propose_timer_output(Round::new(1)), + decide_output(Round::new(1), proposal), + ], + expected_round: Round::new(1), + new_state: decided_state(Round::new(1), Round::new(1), value), + }, + ]; + + run_steps(&mut driver, steps); +} + +#[test] +fn round_1_invalid_decision_during_round_0_with_votes() { + let value = Value::new(9999); + + let [(v1, _sk1), (v2, _sk2), (v3, sk3)] = make_validators([2, 3, 2]); + let (_my_sk, my_addr) = (sk3.clone(), v3.address); + + let height = Height::new(1); + let ctx = TestContext::new(); + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); + + let proposal = Proposal::new( + Height::new(1), + Round::new(1), + value.clone(), + Round::Nil, + v1.address, + ); + + let mut driver = Driver::new(ctx, height, vs, my_addr, Default::default()); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are not the proposer", + input: new_round_input(Round::new(0), v1.address), + expected_outputs: vec![start_propose_timer_output(Round::new(0))], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "v1 precommits a proposal in round 1", + input: precommit_input(Round::new(1), value.clone(), &v1.address), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "Receive proposal from round 1, which is invalid", + input: proposal_input_from_proposal(proposal.clone(), Validity::Invalid), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "v2 precommits the same round 1 proposal, move to round 1", + input: precommit_input(Round::new(1), value.clone(), &v2.address), + expected_outputs: vec![new_round_output(Round::new(1))], + expected_round: Round::new(1), + new_state: new_round(Round::new(1)), + }, + TestStep { + desc: "Start round 1 but does not decide invalid proposal", + input: new_round_input(Round::new(1), v2.address), + expected_outputs: vec![ + start_propose_timer_output(Round::new(1)), + prevote_nil_output(Round::new(1), &my_addr), + start_precommit_timer_output(Round::new(1)), + start_precommit_timer_output(Round::new(1)), // :'( + ], + expected_round: Round::new(1), + new_state: prevote_state(Round::new(1)), + }, + ]; + + run_steps(&mut driver, steps); +} + +#[test] +fn round_1_invalid_decision_during_round_0_via_certificate() { + let value = Value::new(9999); + + let [(v1, _sk1), (v2, _sk2), (v3, sk3)] = make_validators([2, 3, 2]); + let (_my_sk, my_addr) = (sk3.clone(), v3.address); + + let height = Height::new(1); + let ctx = TestContext::new(); + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); + + let proposal = Proposal::new( + Height::new(1), + Round::new(1), + value.clone(), + Round::Nil, + v1.address, + ); + + let mut driver = Driver::new(ctx, height, vs, my_addr, Default::default()); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are not the proposer", + input: new_round_input(Round::new(0), v1.address), + expected_outputs: vec![start_propose_timer_output(Round::new(0))], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "Receive proposal from round 1, which is invalid", + input: proposal_input_from_proposal(proposal.clone(), Validity::Invalid), + expected_outputs: vec![], + expected_round: Round::new(0), + new_state: propose_state(Round::new(0)), + }, + TestStep { + desc: "we get a commit certificate for v at round 1, move to round 1", + input: commit_certificate_input_at( + Round::new(1), + value.clone(), + &[v1.address, v2.address], + ), + expected_outputs: vec![new_round_output(Round::new(1))], + expected_round: Round::new(1), + new_state: new_round(Round::new(1)), + }, + TestStep { + desc: "Start round 1 but does not decide invalid proposal", + input: new_round_input(Round::new(1), v2.address), + expected_outputs: vec![ + start_propose_timer_output(Round::new(1)), + prevote_nil_output(Round::new(1), &my_addr), + start_precommit_timer_output(Round::new(1)), + start_precommit_timer_output(Round::new(1)), // :'( + ], + expected_round: Round::new(1), + new_state: prevote_state(Round::new(1)), + }, + ]; + + run_steps(&mut driver, steps); +} + fn run_steps(driver: &mut Driver, steps: Vec) { let mut last_step = Step::Unstarted; for step in steps { diff --git a/code/crates/core-votekeeper/tests/vote_keeper.rs b/code/crates/core-votekeeper/tests/vote_keeper.rs index 2196a144b..13730c4ba 100644 --- a/code/crates/core-votekeeper/tests/vote_keeper.rs +++ b/code/crates/core-votekeeper/tests/vote_keeper.rs @@ -1,6 +1,7 @@ -use malachitebft_core_types::{NilOrVal, Round, SignedVote}; +use malachitebft_core_types::{NilOrVal, Round, SignedVote, VoteType}; use informalsystems_malachitebft_core_votekeeper::keeper::{Output, VoteKeeper}; +use informalsystems_malachitebft_core_votekeeper::Threshold; use malachitebft_test::{ Address, Height, PrivateKey, Signature, TestContext, Validator, ValidatorSet, ValueId, Vote, @@ -249,6 +250,30 @@ fn no_skip_round_full_quorum_with_same_val() { assert_eq!(msg, None); } +#[test] +fn skip_round_and_precommit_value_future_round() { + let ([addr1, addr2, ..], mut keeper) = setup([2, 3, 2]); + + let id = ValueId::new(1); + let val = NilOrVal::Val(id); + let height = Height::new(1); + let cur_round = Round::new(0); + let fut_round = Round::new(1); + + let vote = new_signed_precommit(height, fut_round, val, addr1); + let msg = keeper.apply_vote(vote, cur_round); + assert_eq!(msg, None); + + let vote = new_signed_precommit(height, fut_round, val, addr2); + let msg = keeper.apply_vote(vote, cur_round); + assert_eq!(msg, Some(Output::SkipRound(fut_round))); + + // A PrecommitValue(id) could be produced + assert!(keeper.is_threshold_met(&fut_round, VoteType::Precommit, Threshold::Value(id))); + // FIXME: should we instead expect it to be produced? + // assert_eq!(msg, Some(Output::PrecommitValue(id))); +} + #[test] fn same_votes() { let ([addr1, ..], mut keeper) = setup([1, 1]);