From 9ad79aa3eacfcaefe149d6c69274fbc1a75677e4 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 7 Nov 2023 16:59:05 +0100 Subject: [PATCH] Add codepath and test for invalid proposal --- Code/driver/src/driver.rs | 21 ++++-- Code/test/tests/driver.rs | 145 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 154 insertions(+), 12 deletions(-) diff --git a/Code/driver/src/driver.rs b/Code/driver/src/driver.rs index fad28b96e..7720b219b 100644 --- a/Code/driver/src/driver.rs +++ b/Code/driver/src/driver.rs @@ -131,10 +131,6 @@ where } fn apply_proposal(&mut self, proposal: Ctx::Proposal) -> Option> { - if !self.validate_proposal(&proposal) { - todo!("invalid proposal"); - } - // Check that there is an ongoing round let Some(round_state) = self.round_states.get(&self.round) else { // TODO: Add logging @@ -157,12 +153,20 @@ where } // TODO: Verify proposal signature (make some of these checks part of message validation) + + let is_valid = self.validate_proposal(&proposal); + match proposal.pol_round() { Round::Nil => { // Is it possible to get +2/3 prevotes before the proposal? // Do we wait for our own prevote to check the threshold? let round = proposal.round(); - let event = RoundEvent::Proposal(proposal); + let event = if is_valid { + RoundEvent::Proposal(proposal) + } else { + RoundEvent::ProposalInvalid + }; + self.apply_event(round, event) } Round::Some(_) @@ -173,7 +177,12 @@ where ) => { let round = proposal.round(); - let event = RoundEvent::Proposal(proposal); + let event = if is_valid { + RoundEvent::Proposal(proposal) + } else { + RoundEvent::ProposalInvalid + }; + self.apply_event(round, event) } _ => None, diff --git a/Code/test/tests/driver.rs b/Code/test/tests/driver.rs index 144484b58..4a233bc6e 100644 --- a/Code/test/tests/driver.rs +++ b/Code/test/tests/driver.rs @@ -27,9 +27,11 @@ fn to_input_msg(output: Message) -> Option> { #[test] fn driver_steps_proposer() { - let value = TestContext::DUMMY_VALUE; // TODO: get value from external source + let value = TestContext::DUMMY_VALUE; let value_id = value.id(); + let client = TestClient::new(value.clone(), |_| true); + let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -47,7 +49,6 @@ fn driver_steps_proposer() { let (my_sk, my_addr) = (sk1, addr1); let vs = ValidatorSet::new(vec![v1, v2.clone(), v3.clone()]); - let client = TestClient::new(value.clone(), |_| true); let mut driver = Driver::new(client, Height::new(1), vs, my_sk.clone(), my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value.clone(), Round::new(-1)); @@ -207,10 +208,12 @@ fn driver_steps_proposer() { } #[test] -fn driver_steps_not_proposer() { - let value = TestContext::DUMMY_VALUE; // TODO: get value from external source +fn driver_steps_not_proposer_valid() { + let value = TestContext::DUMMY_VALUE; let value_id = value.id(); + let client = TestClient::new(value.clone(), |_| true); + let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -229,7 +232,6 @@ fn driver_steps_not_proposer() { let (my_sk, my_addr) = (sk2, addr2); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let client = TestClient::new(value.clone(), |_| true); let mut driver = Driver::new(client, Height::new(1), vs, my_sk.clone(), my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value.clone(), Round::new(-1)); @@ -388,9 +390,140 @@ fn driver_steps_not_proposer() { } } +#[test] +fn driver_steps_not_proposer_invalid() { + let value = TestContext::DUMMY_VALUE; + let value_id = value.id(); + + let client = TestClient::new(value.clone(), |_| false); + + let mut rng = StdRng::seed_from_u64(0x42); + + let sk1 = PrivateKey::generate(&mut rng); + let sk2 = PrivateKey::generate(&mut rng); + let sk3 = PrivateKey::generate(&mut rng); + + let addr1 = Address::from_public_key(&sk1.public_key()); + let addr2 = Address::from_public_key(&sk2.public_key()); + let addr3 = Address::from_public_key(&sk3.public_key()); + + let v1 = Validator::new(sk1.public_key(), 1); + let v2 = Validator::new(sk2.public_key(), 2); + let v3 = Validator::new(sk3.public_key(), 3); + + // Proposer is v1, so we are not the proposer + let (my_sk, my_addr) = (sk2, addr2); + + let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); + let mut driver = Driver::new(client, Height::new(1), vs, my_sk.clone(), my_addr); + + let proposal = Proposal::new(Height::new(1), Round::new(0), value.clone(), Round::new(-1)); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are not the proposer", + input_event: Some(Event::NewRound(Round::new(0))), + expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), + new_state: State { + round: Round::new(0), + step: Step::Propose, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "Receive an invalid proposal, prevote for nil (v2)", + input_event: Some(Event::Proposal(proposal.clone())), + expected_output: Some(Message::Vote( + Vote::new_prevote(Round::new(0), None, my_addr).signed(&my_sk), + )), + new_state: State { + round: Round::new(0), + step: Step::Prevote, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "Receive our own prevote (v2)", + input_event: None, + expected_output: None, + new_state: State { + round: Round::new(0), + step: Step::Prevote, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "v1 prevotes for its own proposal", + input_event: Some(Event::Vote( + Vote::new_prevote(Round::new(0), Some(value_id), addr1).signed(&sk1), + )), + expected_output: None, + new_state: State { + round: Round::new(0), + step: Step::Prevote, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "v3 prevotes for v1's proposal, we have polka for any, schedule prevote timeout (v2)", + input_event: Some(Event::Vote( + Vote::new_prevote(Round::new(0), Some(value_id), addr3).signed(&sk3), + )), + expected_output: Some(Message::ScheduleTimeout(Timeout::prevote(Round::new(0)))), + new_state: State { + round: Round::new(0), + step: Step::Prevote, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "prevote timeout elapses, we precommit for nil (v2)", + input_event: Some(Event::TimeoutElapsed(Timeout::prevote(Round::new(0)))), + expected_output: Some(Message::Vote( + Vote::new_precommit(Round::new(0), None, my_addr).signed(&my_sk), + )), + new_state: State { + round: Round::new(0), + step: Step::Precommit, + proposal: None, + locked: None, + valid: None, + }, + }, + ]; + + let mut previous_message = None; + + for step in steps { + println!("Step: {}", step.desc); + + let execute_message = step + .input_event + .unwrap_or_else(|| previous_message.unwrap()); + + let output = driver.execute(execute_message); + assert_eq!(output, step.expected_output); + + let new_state = driver.round_state(Round::new(0)).unwrap(); + assert_eq!(new_state, &step.new_state); + + previous_message = output.and_then(to_input_msg); + } +} + #[test] fn driver_steps_not_proposer_timeout_multiple_rounds() { - let value = TestContext::DUMMY_VALUE; // TODO: get value from external source + let value = TestContext::DUMMY_VALUE; let value_id = value.id(); let mut rng = StdRng::seed_from_u64(0x42);