From a755b7b0da0f3b146b0be2299bcd5dae3d2d904f Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Fri, 24 Nov 2023 15:21:34 +0100 Subject: [PATCH] feat(driver): Turn `get_value` into an asynchronous step within the state machine (#65) * feat(driver): Turn `get_value` into an asynchronous step within the state machine * chore(driver): Remove unused error variant `NoValueToPropose`, remove `Env` trait * Use valid value if present else get_value_and_schedule_timeout * Remove redundant height from `RoundData` * Include proposer for the current round in the `RoundData` * Rename `RoundData` to `Info` * Replace nil proposal with propose timeout * Differentiate between propose timeout when we are proposer or not * Check that proposer is in validator set in `Driver::get_proposer` * Remove `NewRoundProposer` event in favor of check within the state machine (#79) --------- Co-authored-by: Anca Zamfir Co-authored-by: Anca Zamfir --- Code/driver/src/driver.rs | 98 ++++++++--------- Code/driver/src/env.rs | 19 ---- Code/driver/src/error.rs | 5 - Code/driver/src/event.rs | 10 ++ Code/driver/src/lib.rs | 2 - Code/driver/src/message.rs | 34 +++++- Code/driver/src/proposer.rs | 10 +- Code/round/src/events.rs | 6 +- Code/round/src/message.rs | 25 ++++- Code/round/src/state.rs | 9 +- Code/round/src/state_machine.rs | 116 ++++++++++++++------ Code/test/src/env.rs | 25 ----- Code/test/src/lib.rs | 2 - Code/test/tests/driver.rs | 188 +++++++++++++++++++++++--------- Code/test/tests/round.rs | 20 +++- 15 files changed, 353 insertions(+), 216 deletions(-) delete mode 100644 Code/driver/src/env.rs delete mode 100644 Code/test/src/env.rs diff --git a/Code/driver/src/driver.rs b/Code/driver/src/driver.rs index 7b7cbc484..0a6419d99 100644 --- a/Code/driver/src/driver.rs +++ b/Code/driver/src/driver.rs @@ -1,4 +1,4 @@ -use malachite_round::state_machine::RoundData; +use malachite_round::state_machine::Info; use malachite_common::{ Context, Proposal, Round, SignedVote, Timeout, TimeoutStep, Validator, ValidatorSet, Value, @@ -12,7 +12,6 @@ use malachite_vote::keeper::VoteKeeper; use malachite_vote::Threshold; use malachite_vote::ThresholdParams; -use crate::env::Env as DriverEnv; use crate::event::Event; use crate::message::Message; use crate::Error; @@ -21,14 +20,12 @@ use crate::Validity; /// Driver for the state machine of the Malachite consensus engine at a given height. #[derive(Clone, Debug)] -pub struct Driver +pub struct Driver where Ctx: Context, - Env: DriverEnv, PSel: ProposerSelector, { pub ctx: Ctx, - pub env: Env, pub proposer_selector: PSel, pub address: Ctx::Address, @@ -38,15 +35,13 @@ where pub round_state: RoundState, } -impl Driver +impl Driver where Ctx: Context, - Env: DriverEnv, PSel: ProposerSelector, { pub fn new( ctx: Ctx, - env: Env, proposer_selector: PSel, validator_set: Ctx::ValidatorSet, address: Ctx::Address, @@ -58,7 +53,6 @@ where Self { ctx, - env, proposer_selector, address, validator_set, @@ -75,10 +69,17 @@ where self.round_state.round } - async fn get_value(&self) -> Option { - self.env - .get_value(self.height().clone(), self.round()) - .await + pub fn get_proposer(&self, round: Round) -> Result<&Ctx::Validator, Error> { + let address = self + .proposer_selector + .select_proposer(round, &self.validator_set); + + let proposer = self + .validator_set + .get_by_address(&address) + .ok_or_else(|| Error::ProposerNotFound(address))?; + + Ok(proposer) } pub async fn execute(&mut self, msg: Event) -> Result>, Error> { @@ -102,6 +103,10 @@ where RoundMessage::ScheduleTimeout(timeout) => Message::ScheduleTimeout(timeout), + RoundMessage::GetValueAndScheduleTimeout(round, timeout) => { + Message::GetValueAndScheduleTimeout(round, timeout) + } + RoundMessage::Decision(value) => { // TODO: update the state Message::Decide(value.round, value.value) @@ -114,14 +119,10 @@ where async fn apply(&mut self, event: Event) -> Result>, Error> { match event { Event::NewRound(height, round) => self.apply_new_round(height, round).await, - - Event::Proposal(proposal, validity) => { - Ok(self.apply_proposal(proposal, validity).await) - } - + Event::ProposeValue(round, value) => self.apply_propose_value(round, value).await, + Event::Proposal(proposal, validity) => self.apply_proposal(proposal, validity).await, Event::Vote(signed_vote) => self.apply_vote(signed_vote), - - Event::TimeoutElapsed(timeout) => Ok(self.apply_timeout(timeout)), + Event::TimeoutElapsed(timeout) => self.apply_timeout(timeout), } } @@ -132,56 +133,42 @@ where ) -> Result>, Error> { self.round_state = RoundState::new(height, round); - let proposer_address = self - .proposer_selector - .select_proposer(round, &self.validator_set); - - let proposer = self - .validator_set - .get_by_address(&proposer_address) - .ok_or_else(|| Error::ProposerNotFound(proposer_address.clone()))?; - - let event = if proposer.address() == &self.address { - // We are the proposer - // TODO: Schedule propose timeout - - let Some(value) = self.get_value().await else { - return Err(Error::NoValueToPropose); - }; - - RoundEvent::NewRoundProposer(value) - } else { - RoundEvent::NewRound - }; + self.apply_event(round, RoundEvent::NewRound) + } - Ok(self.apply_event(round, event)) + async fn apply_propose_value( + &mut self, + round: Round, + value: Ctx::Value, + ) -> Result>, Error> { + self.apply_event(round, RoundEvent::ProposeValue(value)) } async fn apply_proposal( &mut self, proposal: Ctx::Proposal, validity: Validity, - ) -> Option> { + ) -> Result>, Error> { // Check that there is an ongoing round if self.round_state.round == Round::NIL { - return None; + return Ok(None); } // Only process the proposal if there is no other proposal if self.round_state.proposal.is_some() { - return None; + return Ok(None); } // Check that the proposal is for the current height and round if self.round_state.height != proposal.height() || self.round_state.round != proposal.round() { - return None; + return Ok(None); } // TODO: Document if proposal.pol_round().is_defined() && proposal.pol_round() >= self.round_state.round { - return None; + return Ok(None); } // TODO: Verify proposal signature (make some of these checks part of message validation) @@ -215,7 +202,7 @@ where self.apply_event(round, event) } - _ => None, + _ => Ok(None), } } @@ -257,10 +244,10 @@ where VoteMessage::SkipRound(r) => RoundEvent::SkipRound(r), }; - Ok(self.apply_event(vote_round, round_event)) + self.apply_event(vote_round, round_event) } - fn apply_timeout(&mut self, timeout: Timeout) -> Option> { + fn apply_timeout(&mut self, timeout: Timeout) -> Result>, Error> { let event = match timeout.step { TimeoutStep::Propose => RoundEvent::TimeoutPropose, TimeoutStep::Prevote => RoundEvent::TimeoutPrevote, @@ -271,10 +258,15 @@ where } /// Apply the event, update the state. - fn apply_event(&mut self, round: Round, event: RoundEvent) -> Option> { + fn apply_event( + &mut self, + event_round: Round, + event: RoundEvent, + ) -> Result>, Error> { let round_state = core::mem::take(&mut self.round_state); + let proposer = self.get_proposer(round_state.round)?; - let data = RoundData::new(round, round_state.height.clone(), &self.address); + let data = Info::new(event_round, &self.address, proposer.address()); // Multiplex the event with the round state. let mux_event = match event { @@ -301,6 +293,6 @@ where self.round_state = transition.next_state; // Return message, if any - transition.message + Ok(transition.message) } } diff --git a/Code/driver/src/env.rs b/Code/driver/src/env.rs deleted file mode 100644 index 2915da387..000000000 --- a/Code/driver/src/env.rs +++ /dev/null @@ -1,19 +0,0 @@ -use alloc::boxed::Box; - -use async_trait::async_trait; - -use malachite_common::{Context, Round}; - -/// Environment for use by the [`Driver`](crate::Driver) to ask -/// for a value to propose and validate proposals. -#[async_trait] -pub trait Env -where - Ctx: Context, -{ - /// Get the value to propose for the given height and round. - /// - /// If `None` is returned, the driver will understand this - /// as an error and will not propose a value. - async fn get_value(&self, height: Ctx::Height, round: Round) -> Option; -} diff --git a/Code/driver/src/error.rs b/Code/driver/src/error.rs index 85c2e77aa..c659e4455 100644 --- a/Code/driver/src/error.rs +++ b/Code/driver/src/error.rs @@ -7,9 +7,6 @@ pub enum Error where Ctx: Context, { - /// No value to propose - NoValueToPropose, - /// Proposer not found ProposerNotFound(Ctx::Address), @@ -27,7 +24,6 @@ where #[cfg_attr(coverage_nightly, coverage(off))] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Error::NoValueToPropose => write!(f, "No value to propose"), Error::ProposerNotFound(addr) => write!(f, "Proposer not found: {addr}"), Error::ValidatorNotFound(addr) => write!(f, "Validator not found: {addr}"), Error::InvalidVoteSignature(vote, validator) => write!( @@ -46,7 +42,6 @@ where #[cfg_attr(coverage_nightly, coverage(off))] fn eq(&self, other: &Self) -> bool { match (self, other) { - (Error::NoValueToPropose, Error::NoValueToPropose) => true, (Error::ProposerNotFound(addr1), Error::ProposerNotFound(addr2)) => addr1 == addr2, (Error::ValidatorNotFound(addr1), Error::ValidatorNotFound(addr2)) => addr1 == addr2, ( diff --git a/Code/driver/src/event.rs b/Code/driver/src/event.rs index d0a3381e1..b2cf5f726 100644 --- a/Code/driver/src/event.rs +++ b/Code/driver/src/event.rs @@ -8,8 +8,18 @@ pub enum Event where Ctx: Context, { + /// Start a new round NewRound(Ctx::Height, Round), + + /// Propose a value for the given round + ProposeValue(Round, Ctx::Value), + + /// Receive a proposal, of the given validity Proposal(Ctx::Proposal, Validity), + + /// Receive a signed vote Vote(SignedVote), + + /// Receive a timeout TimeoutElapsed(Timeout), } diff --git a/Code/driver/src/lib.rs b/Code/driver/src/lib.rs index 62401f55e..6fba7865f 100644 --- a/Code/driver/src/lib.rs +++ b/Code/driver/src/lib.rs @@ -15,7 +15,6 @@ extern crate alloc; mod driver; -mod env; mod error; mod event; mod message; @@ -23,7 +22,6 @@ mod proposer; mod util; pub use driver::Driver; -pub use env::Env; pub use error::Error; pub use event::Event; pub use message::Message; diff --git a/Code/driver/src/message.rs b/Code/driver/src/message.rs index b5f4d7073..52b354d80 100644 --- a/Code/driver/src/message.rs +++ b/Code/driver/src/message.rs @@ -7,11 +7,23 @@ pub enum Message where Ctx: Context, { + /// Start a new round + NewRound(Ctx::Height, Round), + + /// Broadcast a proposal Propose(Ctx::Proposal), + + /// Broadcast a vote for a value Vote(SignedVote), + + /// Decide on a value Decide(Round, Ctx::Value), + + /// Schedule a timeout ScheduleTimeout(Timeout), - NewRound(Ctx::Height, Round), + + /// Ask for a value to propose and schedule a timeout + GetValueAndScheduleTimeout(Round, Timeout), } // NOTE: We have to derive these instances manually, otherwise @@ -22,11 +34,14 @@ impl Clone for Message { #[cfg_attr(coverage_nightly, coverage(off))] fn clone(&self) -> Self { match self { + Message::NewRound(height, round) => Message::NewRound(height.clone(), *round), Message::Propose(proposal) => Message::Propose(proposal.clone()), Message::Vote(signed_vote) => Message::Vote(signed_vote.clone()), Message::Decide(round, value) => Message::Decide(*round, value.clone()), Message::ScheduleTimeout(timeout) => Message::ScheduleTimeout(*timeout), - Message::NewRound(height, round) => Message::NewRound(height.clone(), *round), + Message::GetValueAndScheduleTimeout(round, timeout) => { + Message::GetValueAndScheduleTimeout(*round, *timeout) + } } } } @@ -35,11 +50,14 @@ impl fmt::Debug for Message { #[cfg_attr(coverage_nightly, coverage(off))] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { + Message::NewRound(height, round) => write!(f, "NewRound({:?}, {:?})", height, round), Message::Propose(proposal) => write!(f, "Propose({:?})", proposal), Message::Vote(signed_vote) => write!(f, "Vote({:?})", signed_vote), Message::Decide(round, value) => write!(f, "Decide({:?}, {:?})", round, value), Message::ScheduleTimeout(timeout) => write!(f, "ScheduleTimeout({:?})", timeout), - Message::NewRound(height, round) => write!(f, "NewRound({:?}, {:?})", height, round), + Message::GetValueAndScheduleTimeout(round, timeout) => { + write!(f, "GetValueAndScheduleTimeout({:?}, {:?})", round, timeout) + } } } } @@ -48,6 +66,9 @@ impl PartialEq for Message { #[cfg_attr(coverage_nightly, coverage(off))] fn eq(&self, other: &Self) -> bool { match (self, other) { + (Message::NewRound(height, round), Message::NewRound(other_height, other_round)) => { + height == other_height && round == other_round + } (Message::Propose(proposal), Message::Propose(other_proposal)) => { proposal == other_proposal } @@ -60,9 +81,10 @@ impl PartialEq for Message { (Message::ScheduleTimeout(timeout), Message::ScheduleTimeout(other_timeout)) => { timeout == other_timeout } - (Message::NewRound(height, round), Message::NewRound(other_height, other_round)) => { - height == other_height && round == other_round - } + ( + Message::GetValueAndScheduleTimeout(round, timeout), + Message::GetValueAndScheduleTimeout(other_round, other_timeout), + ) => round == other_round && timeout == other_timeout, _ => false, } } diff --git a/Code/driver/src/proposer.rs b/Code/driver/src/proposer.rs index 47a0b4a75..22bf99cd6 100644 --- a/Code/driver/src/proposer.rs +++ b/Code/driver/src/proposer.rs @@ -4,5 +4,13 @@ pub trait ProposerSelector where Ctx: Context, { - fn select_proposer(&mut self, round: Round, validator_set: &Ctx::ValidatorSet) -> Ctx::Address; + /// Select a proposer from the given validator set for the given round. + /// + /// This function is called at the beginning of each round to select the proposer for that + /// round. The proposer is responsible for proposing a value for the round. + /// + /// # Important + /// This function must be deterministic! + /// For a given round and validator set, it must always return the same proposer. + fn select_proposer(&self, round: Round, validator_set: &Ctx::ValidatorSet) -> Ctx::Address; } diff --git a/Code/round/src/events.rs b/Code/round/src/events.rs index a0e1e4bba..77c6fff40 100644 --- a/Code/round/src/events.rs +++ b/Code/round/src/events.rs @@ -5,9 +5,9 @@ pub enum Event where Ctx: Context, { - NewRound, // Start a new round, not as proposer.L20 - NewRoundProposer(Ctx::Value), // Start a new round and propose the Value.L14 - Proposal(Ctx::Proposal), // Receive a proposal. L22 + L23 (valid) + NewRound, // Start a new round, either as proposer or not. L14/L20 + ProposeValue(Ctx::Value), // Propose a value.L14 + Proposal(Ctx::Proposal), // Receive a proposal. L22 + L23 (valid) ProposalAndPolkaPrevious(Ctx::Proposal), // Recieved a proposal and a polka value from a previous round. L28 + L29 (valid) ProposalInvalid, // Receive an invalid proposal. L26 + L32 (invalid) PolkaValue(ValueId), // Receive +2/3 prevotes for valueId. L44 diff --git a/Code/round/src/message.rs b/Code/round/src/message.rs index 877677f9e..fc5d8f8f5 100644 --- a/Code/round/src/message.rs +++ b/Code/round/src/message.rs @@ -8,11 +8,12 @@ pub enum Message where Ctx: Context, { - NewRound(Round), // Move to the new round. - Proposal(Ctx::Proposal), // Broadcast the proposal. - Vote(Ctx::Vote), // Broadcast the vote. - ScheduleTimeout(Timeout), // Schedule the timeout. - Decision(RoundValue), // Decide the value. + NewRound(Round), // Move to the new round. + Proposal(Ctx::Proposal), // Broadcast the proposal. + Vote(Ctx::Vote), // Broadcast the vote. + ScheduleTimeout(Timeout), // Schedule the timeout. + GetValueAndScheduleTimeout(Round, Timeout), // Ask for a value and schedule a timeout. + Decision(RoundValue), // Decide the value. } impl Message { @@ -47,6 +48,10 @@ impl Message { Message::ScheduleTimeout(Timeout { round, step }) } + pub fn get_value_and_schedule_timeout(round: Round, step: TimeoutStep) -> Self { + Message::GetValueAndScheduleTimeout(round, Timeout { round, step }) + } + pub fn decision(round: Round, value: Ctx::Value) -> Self { Message::Decision(RoundValue { round, value }) } @@ -64,6 +69,9 @@ impl Clone for Message { Message::Proposal(proposal) => Message::Proposal(proposal.clone()), Message::Vote(vote) => Message::Vote(vote.clone()), Message::ScheduleTimeout(timeout) => Message::ScheduleTimeout(*timeout), + Message::GetValueAndScheduleTimeout(round, timeout) => { + Message::GetValueAndScheduleTimeout(*round, *timeout) + } Message::Decision(round_value) => Message::Decision(round_value.clone()), } } @@ -77,6 +85,9 @@ impl fmt::Debug for Message { Message::Proposal(proposal) => write!(f, "Proposal({:?})", proposal), Message::Vote(vote) => write!(f, "Vote({:?})", vote), Message::ScheduleTimeout(timeout) => write!(f, "ScheduleTimeout({:?})", timeout), + Message::GetValueAndScheduleTimeout(round, timeout) => { + write!(f, "GetValueAndScheduleTimeout({:?}, {:?})", round, timeout) + } Message::Decision(round_value) => write!(f, "Decision({:?})", round_value), } } @@ -94,6 +105,10 @@ impl PartialEq for Message { (Message::ScheduleTimeout(timeout), Message::ScheduleTimeout(other_timeout)) => { timeout == other_timeout } + ( + Message::GetValueAndScheduleTimeout(round, timeout), + Message::GetValueAndScheduleTimeout(other_round, other_timeout), + ) => round == other_round && timeout == other_timeout, (Message::Decision(round_value), Message::Decision(other_round_value)) => { round_value == other_round_value } diff --git a/Code/round/src/state.rs b/Code/round/src/state.rs index c7edc6e5e..e2fb6f56e 100644 --- a/Code/round/src/state.rs +++ b/Code/round/src/state.rs @@ -1,7 +1,7 @@ use core::fmt; use crate::events::Event; -use crate::state_machine::RoundData; +use crate::state_machine::Info; use crate::transition::Transition; use malachite_common::{Context, Round}; @@ -36,6 +36,7 @@ where { pub height: Ctx::Height, pub round: Round, + pub step: Step, pub proposal: Option, pub locked: Option>, @@ -75,7 +76,7 @@ where } } - pub fn apply_event(self, data: &RoundData, event: Event) -> Transition { + pub fn apply_event(self, data: &Info, event: Event) -> Transition { crate::state_machine::apply_event(self, data, event) } } @@ -117,6 +118,7 @@ where #[cfg_attr(coverage_nightly, coverage(off))] fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.debug_struct("State") + .field("height", &self.round) .field("round", &self.round) .field("step", &self.step) .field("proposal", &self.proposal) @@ -132,7 +134,8 @@ where { #[cfg_attr(coverage_nightly, coverage(off))] fn eq(&self, other: &Self) -> bool { - self.round == other.round + self.height == other.height + && self.round == other.round && self.step == other.step && self.proposal == other.proposal && self.locked == other.locked diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index 62c257fe6..aa5bb1ec8 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -5,32 +5,34 @@ use crate::message::Message; use crate::state::{State, Step}; use crate::transition::Transition; -/// Immutable data about the current round, -/// height and address of the node. -/// -/// Because this data is immutable for a given round, -/// it is purposefully not included in the state, -/// but rather passed in as a reference. -pub struct RoundData<'a, Ctx> +/// Immutable information about the event and our node: +/// - Address of our node +/// - Proposer for the round we are at +/// - Round for which the event is for, can be different than the round we are at +pub struct Info<'a, Ctx> where Ctx: Context, { - pub round: Round, - pub height: Ctx::Height, + pub event_round: Round, pub address: &'a Ctx::Address, + pub proposer: &'a Ctx::Address, } -impl<'a, Ctx> RoundData<'a, Ctx> +impl<'a, Ctx> Info<'a, Ctx> where Ctx: Context, { - pub fn new(round: Round, height: Ctx::Height, address: &'a Ctx::Address) -> Self { + pub fn new(event_round: Round, address: &'a Ctx::Address, proposer: &'a Ctx::Address) -> Self { Self { - round, - height, + event_round, address, + proposer, } } + + pub fn is_proposer(&self) -> bool { + self.address == self.proposer + } } /// Check that a proposal has a valid Proof-Of-Lock round @@ -51,22 +53,30 @@ where /// Commented numbers refer to line numbers in the spec paper. pub fn apply_event( mut state: State, - data: &RoundData, + info: &Info, event: Event, ) -> Transition where Ctx: Context, { - let this_round = state.round == data.round; + let this_round = state.round == info.event_round; match (state.step, event) { // From NewRound. Event must be for current round. - (Step::NewRound, Event::NewRoundProposer(value)) if this_round => { - propose(state, &data.height, value) // L11/L14 + + // We are the proposer + (Step::NewRound, Event::NewRound) if this_round && info.is_proposer() => { + propose_valid_or_get_value(state) // L18 } + // We are not the proposer (Step::NewRound, Event::NewRound) if this_round => schedule_timeout_propose(state), // L11/L20 // From Propose. Event must be for current round. + (Step::Propose, Event::ProposeValue(value)) if this_round => { + debug_assert!(info.is_proposer()); + propose(state, value) // L11/L14 + } + (Step::Propose, Event::Proposal(proposal)) if this_round && proposal.pol_round().is_nil() => { @@ -77,9 +87,9 @@ where .map_or(true, |locked| &locked.value == proposal.value()) { state.proposal = Some(proposal.clone()); - prevote(state, data.address, &proposal) + prevote(state, info.address, &proposal) } else { - prevote_nil(state, data.address) + prevote_nil(state, info.address) } } @@ -88,25 +98,33 @@ where { // L28 let Some(locked) = state.locked.as_ref() else { - return prevote_nil(state, data.address); + return prevote_nil(state, info.address); }; if locked.round <= proposal.pol_round() || &locked.value == proposal.value() { - prevote(state, data.address, &proposal) + prevote(state, info.address, &proposal) } else { - prevote_nil(state, data.address) + prevote_nil(state, info.address) } } - (Step::Propose, Event::ProposalInvalid) if this_round => prevote_nil(state, data.address), // L22/L25, L28/L31 - (Step::Propose, Event::TimeoutPropose) if this_round => prevote_nil(state, data.address), // L57 + + (Step::Propose, Event::ProposalInvalid) if this_round => prevote_nil(state, info.address), // L22/L25, L28/L31 + + // We are the proposer. + (Step::Propose, Event::TimeoutPropose) if this_round && info.is_proposer() => { + // TOOD: Do we need to do something else here? + prevote_nil(state, info.address) // L57 + } + // We are not the proposer. + (Step::Propose, Event::TimeoutPropose) if this_round => prevote_nil(state, info.address), // L57 // From Prevote. Event must be for current round. (Step::Prevote, Event::PolkaAny) if this_round => schedule_timeout_prevote(state), // L34 - (Step::Prevote, Event::PolkaNil) if this_round => precommit_nil(state, data.address), // L44 + (Step::Prevote, Event::PolkaNil) if this_round => precommit_nil(state, info.address), // L44 (Step::Prevote, Event::ProposalAndPolkaCurrent(proposal)) if this_round => { - precommit(state, data.address, proposal) // L36/L37 - NOTE: only once? + precommit(state, info.address, proposal) // L36/L37 - NOTE: only once? } - (Step::Prevote, Event::TimeoutPrevote) if this_round => precommit_nil(state, data.address), // L61 + (Step::Prevote, Event::TimeoutPrevote) if this_round => precommit_nil(state, info.address), // L61 // From Precommit. Event must be for current round. (Step::Precommit, Event::ProposalAndPolkaCurrent(proposal)) if this_round => { @@ -118,9 +136,13 @@ where // From all (except Commit). Various round guards. (_, Event::PrecommitAny) if this_round => schedule_timeout_precommit(state), // L47 - (_, Event::TimeoutPrecommit) if this_round => round_skip(state, data.round.increment()), // L65 + (_, Event::TimeoutPrecommit) if this_round => { + round_skip(state, info.event_round.increment()) + } // L65 (_, Event::SkipRound(round)) if state.round < round => round_skip(state, round), // L55 - (_, Event::ProposalAndPrecommitValue(proposal)) => commit(state, data.round, proposal), // L49 + (_, Event::ProposalAndPrecommitValue(proposal)) => { + commit(state, info.event_round, proposal) + } // L49 // Invalid transition. _ => Transition::invalid(state), @@ -131,20 +153,42 @@ where // Propose //--------------------------------------------------------------------- +/// We are the proposer. Propose the valid value if present, otherwise schedule timeout propose +/// and ask for a value. +/// +/// Ref: L15-L18 +pub fn propose_valid_or_get_value(state: State) -> Transition +where + Ctx: Context, +{ + match &state.valid { + Some(round_value) => { + let pol_round = round_value.round; + let proposal = Message::proposal( + state.height.clone(), + state.round, + round_value.value.clone(), + pol_round, + ); + Transition::to(state.with_step(Step::Propose)).with_message(proposal) + } + None => { + let timeout = + Message::get_value_and_schedule_timeout(state.round, TimeoutStep::Propose); + Transition::to(state.with_step(Step::Propose)).with_message(timeout) + } + } +} + /// We are the proposer; propose the valid value if it exists, /// otherwise propose the given value. /// /// Ref: L11/L14 -pub fn propose(state: State, height: &Ctx::Height, value: Ctx::Value) -> Transition +pub fn propose(state: State, value: Ctx::Value) -> Transition where Ctx: Context, { - let (value, pol_round) = match &state.valid { - Some(round_value) => (round_value.value.clone(), round_value.round), - None => (value, Round::Nil), - }; - - let proposal = Message::proposal(height.clone(), state.round, value, pol_round); + let proposal = Message::proposal(state.height.clone(), state.round, value, Round::Nil); Transition::to(state.with_step(Step::Propose)).with_message(proposal) } diff --git a/Code/test/src/env.rs b/Code/test/src/env.rs deleted file mode 100644 index af79543a4..000000000 --- a/Code/test/src/env.rs +++ /dev/null @@ -1,25 +0,0 @@ -use async_trait::async_trait; - -use malachite_common::Round; -use malachite_driver::Env; - -use crate::{Height, TestContext, Value}; - -pub struct TestEnv { - get_value: Box Option + Send + Sync>, -} - -impl TestEnv { - pub fn new(get_value: impl Fn(Height, Round) -> Option + Send + Sync + 'static) -> Self { - Self { - get_value: Box::new(get_value), - } - } -} - -#[async_trait] -impl Env for TestEnv { - async fn get_value(&self, height: Height, round: Round) -> Option { - (self.get_value)(height, round) - } -} diff --git a/Code/test/src/lib.rs b/Code/test/src/lib.rs index 8e09b3a31..181ccd7ce 100644 --- a/Code/test/src/lib.rs +++ b/Code/test/src/lib.rs @@ -3,7 +3,6 @@ #![cfg_attr(coverage_nightly, feature(coverage_attribute))] mod context; -mod env; mod height; mod proposal; mod signing; @@ -12,7 +11,6 @@ mod value; mod vote; pub use crate::context::*; -pub use crate::env::*; pub use crate::height::*; pub use crate::proposal::*; pub use crate::signing::*; diff --git a/Code/test/tests/driver.rs b/Code/test/tests/driver.rs index 2e031e0a0..40396e986 100644 --- a/Code/test/tests/driver.rs +++ b/Code/test/tests/driver.rs @@ -2,12 +2,11 @@ use futures::executor::block_on; use rand::rngs::StdRng; use rand::SeedableRng; -use malachite_common::{Round, Timeout}; +use malachite_common::{Round, Timeout, TimeoutStep}; use malachite_driver::{Driver, Error, Event, Message, ProposerSelector, Validity}; use malachite_round::state::{RoundValue, State, Step}; use malachite_test::{ - Address, Height, PrivateKey, Proposal, TestContext, TestEnv, Validator, ValidatorSet, Value, - Vote, + Address, Height, PrivateKey, Proposal, TestContext, Validator, ValidatorSet, Value, Vote, }; struct TestStep { @@ -20,25 +19,23 @@ struct TestStep { fn to_input_msg(output: Message) -> Option> { match output { + Message::NewRound(height, round) => Some(Event::NewRound(height, round)), // Let's consider our own proposal to always be valid Message::Propose(p) => Some(Event::Proposal(p, Validity::Valid)), Message::Vote(v) => Some(Event::Vote(v)), Message::Decide(_, _) => None, Message::ScheduleTimeout(_) => None, - Message::NewRound(height, round) => Some(Event::NewRound(height, round)), + Message::GetValueAndScheduleTimeout(_, _) => None, } } #[derive(Copy, Clone, Debug, Default)] -pub struct RotateProposer { - proposer_index: usize, -} +pub struct RotateProposer; impl ProposerSelector for RotateProposer { - fn select_proposer(&mut self, _round: Round, validator_set: &ValidatorSet) -> Address { - let proposer = &validator_set.validators[self.proposer_index]; - self.proposer_index = (self.proposer_index + 1) % validator_set.validators.len(); - proposer.address + fn select_proposer(&self, round: Round, validator_set: &ValidatorSet) -> Address { + let proposer_index = round.as_i64() as usize % validator_set.validators.len(); + validator_set.validators[proposer_index].address } } @@ -54,7 +51,7 @@ impl FixedProposer { } impl ProposerSelector for FixedProposer { - fn select_proposer(&mut self, _round: Round, _validator_set: &ValidatorSet) -> Address { + fn select_proposer(&self, _round: Round, _validator_set: &ValidatorSet) -> Address { self.proposer } } @@ -63,9 +60,6 @@ impl ProposerSelector for FixedProposer { fn driver_steps_proposer() { let value = Value::new(9999); - let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -83,16 +77,34 @@ fn driver_steps_proposer() { let (my_sk, my_addr) = (sk1, addr1); let ctx = TestContext::new(my_sk.clone()); - + let sel = FixedProposer::new(my_addr); let vs = ValidatorSet::new(vec![v1, v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + + let mut driver = Driver::new(ctx, sel, vs, my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value, Round::new(-1)); let steps = vec![ TestStep { - desc: "Start round 0, we are proposer, propose value", + desc: "Start round 0, we are proposer, ask for a value to propose", input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), + expected_output: Some(Message::GetValueAndScheduleTimeout( + Round::new(0), + Timeout::new(Round::new(0), TimeoutStep::Propose), + )), + expected_round: Round::new(0), + new_state: State { + height: Height::new(1), + round: Round::new(0), + step: Step::Propose, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "Feed a value to propose, propose that value", + input_event: Some(Event::ProposeValue(Round::new(0), value)), expected_output: Some(Message::Propose(proposal.clone())), expected_round: Round::new(0), new_state: State { @@ -269,13 +281,91 @@ fn driver_steps_proposer() { } } +#[test] +fn driver_steps_proposer_timeout_get_value() { + 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 v1 = Validator::new(sk1.public_key(), 1); + let v2 = Validator::new(sk2.public_key(), 2); + let v3 = Validator::new(sk3.public_key(), 3); + + let (my_sk, my_addr) = (sk1, addr1); + + let ctx = TestContext::new(my_sk.clone()); + let sel = FixedProposer::new(my_addr); + let vs = ValidatorSet::new(vec![v1, v2.clone(), v3.clone()]); + + let mut driver = Driver::new(ctx, sel, vs, my_addr); + + let steps = vec![ + TestStep { + desc: "Start round 0, we are proposer, ask for a value to propose", + input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), + expected_output: Some(Message::GetValueAndScheduleTimeout( + Round::new(0), + Timeout::new(Round::new(0), TimeoutStep::Propose), + )), + expected_round: Round::new(0), + new_state: State { + height: Height::new(1), + round: Round::new(0), + step: Step::Propose, + proposal: None, + locked: None, + valid: None, + }, + }, + TestStep { + desc: "Receive a propose timeout", + input_event: Some(Event::TimeoutElapsed(Timeout::propose(Round::new(0)))), + expected_output: Some(Message::Vote( + Vote::new_prevote(Height::new(1), Round::new(0), None, my_addr).signed(&my_sk), + )), + expected_round: Round::new(0), + new_state: State { + round: Round::new(0), + height: Height::new(1), + step: Step::Prevote, + 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 = block_on(driver.execute(execute_message)).expect("execute succeeded"); + assert_eq!(output, step.expected_output, "expected output message"); + + assert_eq!( + driver.round_state.round, step.expected_round, + "expected round" + ); + + assert_eq!(driver.round_state, step.new_state, "expected state"); + + previous_message = output.and_then(to_input_msg); + } +} + #[test] fn driver_steps_not_proposer_valid() { let value = Value::new(9999); - let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -294,9 +384,10 @@ fn driver_steps_not_proposer_valid() { let (my_sk, my_addr) = (sk2, addr2); let ctx = TestContext::new(my_sk.clone()); - + let sel = FixedProposer::new(addr1); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + + let mut driver = Driver::new(ctx, sel, vs, my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value, Round::new(-1)); @@ -484,9 +575,6 @@ fn driver_steps_not_proposer_valid() { fn driver_steps_not_proposer_invalid() { let value = Value::new(9999); - let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -505,9 +593,10 @@ fn driver_steps_not_proposer_invalid() { let (my_sk, my_addr) = (sk2, addr2); let ctx = TestContext::new(my_sk.clone()); - + let sel = FixedProposer::new(addr1); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + + let mut driver = Driver::new(ctx, sel, vs, my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value, Round::new(-1)); @@ -633,9 +722,6 @@ fn driver_steps_not_proposer_invalid() { fn driver_steps_not_proposer_timeout_multiple_rounds() { let value = Value::new(9999); - let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -654,9 +740,10 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { let (my_sk, my_addr) = (sk3, addr3); let ctx = TestContext::new(my_sk.clone()); - + let sel = FixedProposer::new(addr1); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + + let mut driver = Driver::new(ctx, sel, vs, my_addr); let steps = vec![ // Start round 0, we, v3, are not the proposer @@ -845,7 +932,6 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { #[test] fn driver_steps_no_value_to_propose() { // No value to propose - let env = TestEnv::new(|_, _| None); let mut rng = StdRng::seed_from_u64(0x42); @@ -864,18 +950,22 @@ fn driver_steps_no_value_to_propose() { let sel = FixedProposer::new(v1.address); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); - let output = block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))); - assert_eq!(output, Err(Error::NoValueToPropose)); + let output = block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))) + .expect("execute succeeded"); + + assert_eq!( + output, + Some(Message::GetValueAndScheduleTimeout( + Round::new(0), + Timeout::propose(Round::new(0)) + )) + ); } #[test] fn driver_steps_proposer_not_found() { - let value = Value::new(9999); - - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -895,7 +985,7 @@ fn driver_steps_proposer_not_found() { let sel = FixedProposer::new(v1.address); let vs = ValidatorSet::new(vec![v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); let output = block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))); assert_eq!(output, Err(Error::ProposerNotFound(v1.address))); @@ -905,8 +995,6 @@ fn driver_steps_proposer_not_found() { fn driver_steps_validator_not_found() { let value = Value::new(9999); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -925,7 +1013,7 @@ fn driver_steps_validator_not_found() { // We omit v2 from the validator set let vs = ValidatorSet::new(vec![v1.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); // Start new height block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))) @@ -943,8 +1031,6 @@ fn driver_steps_validator_not_found() { fn driver_steps_invalid_signature() { let value = Value::new(9999); - let env = TestEnv::new(move |_, _| Some(value)); - let mut rng = StdRng::seed_from_u64(0x42); let sk1 = PrivateKey::generate(&mut rng); @@ -961,7 +1047,7 @@ fn driver_steps_invalid_signature() { let sel = FixedProposer::new(v1.address); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); // Start new round block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))) @@ -981,7 +1067,6 @@ fn driver_steps_skip_round_skip_threshold() { let value = Value::new(9999); let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); let mut rng = StdRng::seed_from_u64(0x42); @@ -1004,7 +1089,7 @@ fn driver_steps_skip_round_skip_threshold() { let height = Height::new(1); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); let steps = vec![ // Start round 0, we, v3, are not the proposer @@ -1114,7 +1199,6 @@ fn driver_steps_skip_round_quorum_threshold() { let value = Value::new(9999); let sel = RotateProposer::default(); - let env = TestEnv::new(move |_, _| Some(value)); let mut rng = StdRng::seed_from_u64(0x42); @@ -1137,7 +1221,7 @@ fn driver_steps_skip_round_quorum_threshold() { let height = Height::new(1); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, vs, my_addr); + let mut driver = Driver::new(ctx, sel, vs, my_addr); let steps = vec![ // Start round 0, we, v3, are not the proposer diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index cd6d2b5b9..0952ff747 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -4,9 +4,10 @@ use malachite_common::{Round, Timeout, TimeoutStep}; use malachite_round::events::Event; use malachite_round::message::Message; use malachite_round::state::{State, Step}; -use malachite_round::state_machine::{apply_event, RoundData}; +use malachite_round::state_machine::{apply_event, Info}; const ADDRESS: Address = Address::new([42; 20]); +const OTHER_ADDRESS: Address = Address::new([21; 20]); #[test] fn test_propose() { @@ -15,17 +16,27 @@ fn test_propose() { let round = Round::new(0); let mut state: State = State { + height, round, ..Default::default() }; - let data = RoundData::new(round, height, &ADDRESS); + // We are the proposer + let data = Info::new(round, &ADDRESS, &ADDRESS); - let transition = apply_event(state.clone(), &data, Event::NewRoundProposer(value)); + let transition = apply_event(state.clone(), &data, Event::NewRound); state.step = Step::Propose; assert_eq!(transition.next_state, state); + assert_eq!( + transition.message.unwrap(), + Message::get_value_and_schedule_timeout(round, TimeoutStep::Propose) + ); + let transition = apply_event(transition.next_state, &data, Event::ProposeValue(value)); + + state.step = Step::Propose; + assert_eq!(transition.next_state, state); assert_eq!( transition.message.unwrap(), Message::proposal(Height::new(10), Round::new(0), Value::new(42), Round::Nil) @@ -44,7 +55,8 @@ fn test_prevote() { ..Default::default() }; - let data = RoundData::new(Round::new(1), height, &ADDRESS); + // We are not the proposer + let data = Info::new(Round::new(1), &ADDRESS, &OTHER_ADDRESS); let transition = apply_event(state, &data, Event::NewRound);