From 5e5d175376c092b14fcd6b72e6b60771a7498b15 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 27 Oct 2023 16:34:33 +0200 Subject: [PATCH] Use different types for the executors inputs and outputs (#17) --- Code/consensus/Cargo.toml | 4 +- Code/consensus/src/executor.rs | 30 ++++++---- Code/round/src/message.rs | 5 +- Code/test/Cargo.toml | 5 +- Code/test/tests/consensus_executor.rs | 86 +++++++++++++++------------ 5 files changed, 74 insertions(+), 56 deletions(-) diff --git a/Code/consensus/Cargo.toml b/Code/consensus/Cargo.toml index 90fda0325..0ab2d7f27 100644 --- a/Code/consensus/Cargo.toml +++ b/Code/consensus/Cargo.toml @@ -6,5 +6,5 @@ publish = false [dependencies] malachite-common = { version = "0.1.0", path = "../common" } -malachite-round = { version = "0.1.0", path = "../round" } -malachite-vote = { version = "0.1.0", path = "../vote" } +malachite-round = { version = "0.1.0", path = "../round" } +malachite-vote = { version = "0.1.0", path = "../vote" } diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index 814158cbf..348359608 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -34,6 +34,17 @@ where Timeout(Timeout), } +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum Output +where + C: Consensus, +{ + Propose(C::Proposal), + Vote(C::Vote), + Decide(Round, C::Value), + SetTimeout(Timeout), +} + impl Executor where C: Consensus, @@ -60,7 +71,7 @@ where C::DUMMY_VALUE } - pub fn execute(&mut self, msg: Message) -> Option> { + pub fn execute(&mut self, msg: Message) -> Option> { let round_msg = match self.apply(msg) { Some(msg) => msg, None => return None, @@ -76,9 +87,9 @@ where None } - RoundMessage::Proposal(p) => { + RoundMessage::Proposal(proposal) => { // sign the proposal - Some(Message::Proposal(p)) + Some(Output::Propose(proposal)) } RoundMessage::Vote(mut v) => { @@ -93,17 +104,14 @@ where v.set_address(address); - Some(Message::Vote(v)) + Some(Output::Vote(v)) } - RoundMessage::Timeout(_) => { - // schedule the timeout - None - } + RoundMessage::Timeout(timeout) => Some(Output::SetTimeout(timeout)), - RoundMessage::Decision(_) => { - // update the state - None + RoundMessage::Decision(value) => { + // TODO: update the state + Some(Output::Decide(value.round, value.value)) } } } diff --git a/Code/round/src/message.rs b/Code/round/src/message.rs index 2fb422cf1..2f45c79c0 100644 --- a/Code/round/src/message.rs +++ b/Code/round/src/message.rs @@ -3,7 +3,10 @@ use malachite_common::{Consensus, Round, Timeout, TimeoutStep, ValueId}; use crate::state::RoundValue; #[derive(Debug, PartialEq, Eq)] -pub enum Message { +pub enum Message +where + C: Consensus, +{ NewRound(Round), // Move to the new round. Proposal(C::Proposal), // Broadcast the proposal. Vote(C::Vote), // Broadcast the vote. diff --git a/Code/test/Cargo.toml b/Code/test/Cargo.toml index 43da1d089..d8234efa7 100644 --- a/Code/test/Cargo.toml +++ b/Code/test/Cargo.toml @@ -1,9 +1,8 @@ [package] -name = "malachite-test" +name = "malachite-test" version = "0.1.0" edition = "2021" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html +publish = false [dependencies] malachite-common = { version = "0.1.0", path = "../common" } diff --git a/Code/test/tests/consensus_executor.rs b/Code/test/tests/consensus_executor.rs index fc26fe778..0f4ae491f 100644 --- a/Code/test/tests/consensus_executor.rs +++ b/Code/test/tests/consensus_executor.rs @@ -1,5 +1,5 @@ use malachite_common::{Consensus, Round, Timeout}; -use malachite_consensus::executor::{Executor, Message}; +use malachite_consensus::executor::{Executor, Message, Output}; use malachite_round::state::{RoundValue, State, Step}; use malachite_test::{Height, Proposal, PublicKey, TestConsensus, Validator, ValidatorSet, Vote}; @@ -7,10 +7,19 @@ use malachite_test::{Height, Proposal, PublicKey, TestConsensus, Validator, Vali struct TestStep { desc: &'static str, input_message: Option>, - expected_output_message: Option>, + expected_output: Option>, new_state: State, } +fn to_input_msg(output: Output) -> Option> { + match output { + Output::Propose(p) => Some(Message::Proposal(p)), + Output::Vote(v) => Some(Message::Vote(v)), + Output::Decide(_, _) => None, + Output::SetTimeout(_) => None, + } +} + #[test] fn executor_steps_proposer() { let value = TestConsensus::DUMMY_VALUE; // TODO: get value from external source @@ -32,7 +41,7 @@ fn executor_steps_proposer() { TestStep { desc: "Start round 0, we are proposer, propose value", input_message: Some(Message::NewRound(Round::new(0))), - expected_output_message: Some(Message::Proposal(proposal.clone())), + expected_output: Some(Output::Propose(proposal.clone())), new_state: State { height: Height::new(1), round: Round::new(0), @@ -46,7 +55,7 @@ fn executor_steps_proposer() { TestStep { desc: "Receive our own proposal, prevote for it (v1)", input_message: None, - expected_output_message: Some(Message::Vote(Vote::new_prevote( + expected_output: Some(Output::Vote(Vote::new_prevote( Round::new(0), Some(value_id), my_address, @@ -64,7 +73,7 @@ fn executor_steps_proposer() { TestStep { desc: "Receive our own prevote v1", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -82,7 +91,7 @@ fn executor_steps_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -100,7 +109,7 @@ fn executor_steps_proposer() { Some(value_id), v3.address, ))), - expected_output_message: Some(Message::Vote(Vote::new_precommit( + expected_output: Some(Output::Vote(Vote::new_precommit( Round::new(0), Some(value_id), my_address, @@ -124,7 +133,7 @@ fn executor_steps_proposer() { TestStep { desc: "v1 receives its own precommit", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -148,7 +157,7 @@ fn executor_steps_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -172,7 +181,7 @@ fn executor_steps_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: Some(Output::Decide(Round::new(0), value.clone())), new_state: State { height: Height::new(1), round: Round::new(0), @@ -197,13 +206,13 @@ fn executor_steps_proposer() { .input_message .unwrap_or_else(|| previous_message.unwrap()); - let message = executor.execute(execute_message); - assert_eq!(message, step.expected_output_message); + let output = executor.execute(execute_message); + assert_eq!(output, step.expected_output); let new_state = executor.round_state(Round::new(0)).unwrap(); assert_eq!(new_state, &step.new_state); - previous_message = message; + previous_message = output.and_then(to_input_msg); } } @@ -230,7 +239,7 @@ fn executor_steps_not_proposer() { TestStep { desc: "Start round 0, we are not the proposer", input_message: Some(Message::NewRound(Round::new(0))), - expected_output_message: None, + expected_output: Some(Output::SetTimeout(Timeout::propose(Round::new(0)))), new_state: State { height: Height::new(1), round: Round::new(0), @@ -244,7 +253,7 @@ fn executor_steps_not_proposer() { TestStep { desc: "Receive a proposal, prevote for it (v1)", input_message: Some(Message::Proposal(proposal.clone())), - expected_output_message: Some(Message::Vote(Vote::new_prevote( + expected_output: Some(Output::Vote(Vote::new_prevote( Round::new(0), Some(value_id), my_address, @@ -262,7 +271,7 @@ fn executor_steps_not_proposer() { TestStep { desc: "Receive our own prevote v1", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -280,7 +289,7 @@ fn executor_steps_not_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -298,7 +307,7 @@ fn executor_steps_not_proposer() { Some(value_id), v3.address, ))), - expected_output_message: Some(Message::Vote(Vote::new_precommit( + expected_output: Some(Output::Vote(Vote::new_precommit( Round::new(0), Some(value_id), my_address, @@ -322,7 +331,7 @@ fn executor_steps_not_proposer() { TestStep { desc: "v1 receives its own precommit", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -346,7 +355,7 @@ fn executor_steps_not_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -370,7 +379,7 @@ fn executor_steps_not_proposer() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: Some(Output::Decide(Round::new(0), value.clone())), new_state: State { height: Height::new(1), round: Round::new(0), @@ -391,17 +400,19 @@ fn executor_steps_not_proposer() { let mut previous_message = None; for step in steps { + println!("Step: {}", step.desc); + let execute_message = step .input_message .unwrap_or_else(|| previous_message.unwrap()); - let message = executor.execute(execute_message); - assert_eq!(message, step.expected_output_message); + let output = executor.execute(execute_message); + assert_eq!(output, step.expected_output); let new_state = executor.round_state(Round::new(0)).unwrap(); assert_eq!(new_state, &step.new_state); - previous_message = message; + previous_message = output.and_then(to_input_msg); } } @@ -426,7 +437,7 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "Start round 0, we are not the proposer", input_message: Some(Message::NewRound(Round::new(0))), - expected_output_message: None, + expected_output: Some(Output::SetTimeout(Timeout::propose(Round::new(0)))), new_state: State { height: Height::new(1), round: Round::new(0), @@ -440,7 +451,7 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "Receive a propose timeout, prevote for nil (v1)", input_message: Some(Message::Timeout(Timeout::propose(Round::new(0)))), - expected_output_message: Some(Message::Vote(Vote::new_prevote( + expected_output: Some(Output::Vote(Vote::new_prevote( Round::new(0), None, my_address, @@ -458,7 +469,7 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "Receive our own prevote v1", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -476,7 +487,7 @@ fn executor_steps_not_proposer_timeout() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -494,7 +505,7 @@ fn executor_steps_not_proposer_timeout() { None, v3.address, ))), - expected_output_message: Some(Message::Vote(Vote::new_precommit( + expected_output: Some(Output::Vote(Vote::new_precommit( Round::new(0), None, my_address, @@ -512,7 +523,7 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "v1 receives its own precommit", input_message: None, - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -530,7 +541,7 @@ fn executor_steps_not_proposer_timeout() { Some(value_id), v2.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -548,7 +559,7 @@ fn executor_steps_not_proposer_timeout() { None, v3.address, ))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(0), @@ -562,7 +573,7 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "we receive a precommit timeout, start a new round", input_message: Some(Message::Timeout(Timeout::precommit(Round::new(0)))), - expected_output_message: None, + expected_output: None, new_state: State { height: Height::new(1), round: Round::new(1), @@ -583,15 +594,12 @@ fn executor_steps_not_proposer_timeout() { .input_message .unwrap_or_else(|| previous_message.unwrap()); - let message = executor.execute(execute_message); - assert_eq!( - message, step.expected_output_message, - "expected output message" - ); + 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(); assert_eq!(new_state, &step.new_state, "new state"); - previous_message = message; + previous_message = output.and_then(to_input_msg); } }