From dc5a1a55b3f961f8e4be85367d20b873e21fbcae Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Fri, 3 Nov 2023 20:11:13 -0400 Subject: [PATCH] Add test step for NewRound, fix precommit for nil quorum to emit PrecommitAny --- Code/consensus/src/executor.rs | 2 +- Code/test/tests/consensus_executor.rs | 63 ++++++++++++++++----------- Code/test/tests/vote_keeper.rs | 2 +- Code/vote/src/keeper.rs | 2 +- 4 files changed, 41 insertions(+), 28 deletions(-) diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index c8dd81eca..a4e337046 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -36,7 +36,7 @@ where private_key: Secret>, address: Ctx::Address, validator_set: Ctx::ValidatorSet, - round: Round, + pub round: Round, votes: VoteKeeper, round_states: BTreeMap>, } diff --git a/Code/test/tests/consensus_executor.rs b/Code/test/tests/consensus_executor.rs index 217c52804..821eb0fb7 100644 --- a/Code/test/tests/consensus_executor.rs +++ b/Code/test/tests/consensus_executor.rs @@ -388,7 +388,7 @@ fn executor_steps_not_proposer() { } #[test] -fn executor_steps_not_proposer_timeout() { +fn executor_steps_not_proposer_timeout_multiple_rounds() { let value = TestContext::DUMMY_VALUE; // TODO: get value from external source let value_id = value.id(); @@ -403,19 +403,19 @@ fn executor_steps_not_proposer_timeout() { let addr3 = Address::from_public_key(&sk3.public_key()); let v1 = Validator::new(sk1.public_key(), 1); - let v2 = Validator::new(sk2.public_key(), 1); - let v3 = Validator::new(sk3.public_key(), 3); + let v2 = Validator::new(sk2.public_key(), 3); + let v3 = Validator::new(sk3.public_key(), 1); - // Proposer is v1, so we are not the proposer - let (my_sk, my_addr) = (sk2, addr2); + // Proposer is v1, so we, v3, are not the proposer + let (my_sk, my_addr) = (sk3, addr3); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); let mut executor = Executor::new(Height::new(1), vs, my_sk.clone(), my_addr); let steps = vec![ - // Start round 0, we are not the proposer + // Start round 0, we, v3, are not the proposer TestStep { - desc: "Start round 0, we are not the proposer", + desc: "Start round 0, we, v3, 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 { @@ -426,9 +426,9 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // Receive a propose timeout, prevote for nil (v1) + // Receive a propose timeout, prevote for nil (from v3) TestStep { - desc: "Receive a propose timeout, prevote for nil (v1)", + desc: "Receive a propose timeout, prevote for nil (v3)", input_event: Some(Event::TimeoutElapsed(Timeout::propose(Round::new(0)))), expected_output: Some(Message::Vote( Vote::new_prevote(Round::new(0), None, my_addr).signed(&my_sk), @@ -441,9 +441,9 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // Receive our own prevote v1 + // Receive our own prevote v3 TestStep { - desc: "Receive our own prevote v1", + desc: "Receive our own prevote v3", input_event: None, expected_output: None, new_state: State { @@ -454,9 +454,9 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // v2 prevotes for its own proposal + // v1 prevotes for its own proposal TestStep { - desc: "v2 prevotes for its own proposal", + desc: "v1 prevotes for its own proposal", input_event: Some(Event::Vote( Vote::new_prevote(Round::new(0), Some(value_id), addr1).signed(&sk1), )), @@ -469,11 +469,11 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // v3 prevotes for nil, it gets +2/3 prevotes, precommit for it (v1) + // v2 prevotes for nil, we get +2/3 nil prevotes and precommit for nil TestStep { - desc: "v3 prevotes for nil, it gets +2/3 prevotes, precommit for it (v1)", + desc: "v2 prevotes for nil, we get +2/3 prevotes, precommit for nil", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), None, addr3).signed(&sk3), + Vote::new_prevote(Round::new(0), None, addr2).signed(&sk2), )), expected_output: Some(Message::Vote( Vote::new_precommit(Round::new(0), None, my_addr).signed(&my_sk), @@ -486,9 +486,9 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // v1 receives its own precommit + // v3 receives its own precommit TestStep { - desc: "v1 receives its own precommit", + desc: "v3 receives its own precommit", input_event: None, expected_output: None, new_state: State { @@ -499,9 +499,9 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // v2 precommits its proposal + // v1 precommits its proposal TestStep { - desc: "v2 precommits its proposal", + desc: "v1 precommits its proposal", input_event: Some(Event::Vote( Vote::new_precommit(Round::new(0), Some(value_id), addr1).signed(&sk1), )), @@ -514,13 +514,13 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, - // v3 precommits for nil + // v2 precommits for nil TestStep { - desc: "v3 precommits for nil", + desc: "v2 precommits for nil", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), None, addr3).signed(&sk3), + Vote::new_precommit(Round::new(0), None, addr2).signed(&sk2), )), - expected_output: None, + expected_output: Some(Message::ScheduleTimeout(Timeout::precommit(Round::new(0)))), new_state: State { round: Round::new(0), step: Step::Precommit, @@ -542,6 +542,18 @@ fn executor_steps_not_proposer_timeout() { valid: None, }, }, + TestStep { + desc: "Start round 1, we are not the proposer", + input_event: Some(Event::NewRound(Round::new(1))), + expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(1)))), + new_state: State { + round: Round::new(1), + step: Step::Propose, + proposal: None, + locked: None, + valid: None, + }, + }, ]; let mut previous_message = None; @@ -556,7 +568,8 @@ fn executor_steps_not_proposer_timeout() { let output = executor.execute(execute_message); assert_eq!(output, step.expected_output, "expected output message"); - let new_state = executor.round_state(Round::new(0)).unwrap(); + // TODO - add expected executor round to test and assert before the new_state check below + let new_state = executor.round_state(executor.round).unwrap(); assert_eq!(new_state, &step.new_state, "new state"); previous_message = output.and_then(to_input_msg); diff --git a/Code/test/tests/vote_keeper.rs b/Code/test/tests/vote_keeper.rs index 77eeae569..2eed7ad6a 100644 --- a/Code/test/tests/vote_keeper.rs +++ b/Code/test/tests/vote_keeper.rs @@ -34,7 +34,7 @@ fn precommit_apply_nil() { assert_eq!(msg, None); let msg = keeper.apply_vote(vote, 1); - assert_eq!(msg, None); + assert_eq!(msg, Some(Message::PrecommitAny)); } #[test] diff --git a/Code/vote/src/keeper.rs b/Code/vote/src/keeper.rs index b346f0632..1240dcc70 100644 --- a/Code/vote/src/keeper.rs +++ b/Code/vote/src/keeper.rs @@ -89,7 +89,7 @@ where (VoteType::Prevote, Threshold::Value(v)) => Some(Message::PolkaValue(v)), (VoteType::Precommit, Threshold::Any) => Some(Message::PrecommitAny), - (VoteType::Precommit, Threshold::Nil) => None, + (VoteType::Precommit, Threshold::Nil) => Some(Message::PrecommitAny), (VoteType::Precommit, Threshold::Value(v)) => Some(Message::PrecommitValue(v)), } }