Skip to content

Commit

Permalink
Add test step for NewRound, fix precommit for nil quorum to emit Prec…
Browse files Browse the repository at this point in the history
…ommitAny
  • Loading branch information
ancazamfir committed Nov 4, 2023
1 parent 90bd7f3 commit dc5a1a5
Show file tree
Hide file tree
Showing 4 changed files with 41 additions and 28 deletions.
2 changes: 1 addition & 1 deletion Code/consensus/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ where
private_key: Secret<PrivateKey<Ctx>>,
address: Ctx::Address,
validator_set: Ctx::ValidatorSet,
round: Round,
pub round: Round,
votes: VoteKeeper<Ctx>,
round_states: BTreeMap<Round, RoundState<Ctx>>,
}
Expand Down
63 changes: 38 additions & 25 deletions Code/test/tests/consensus_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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 {
Expand All @@ -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),
Expand All @@ -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 {
Expand All @@ -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),
)),
Expand All @@ -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),
Expand All @@ -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 {
Expand All @@ -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),
)),
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion Code/test/tests/vote_keeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion Code/vote/src/keeper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
}
}
Expand Down

0 comments on commit dc5a1a5

Please sign in to comment.