diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index e446b918e..116bdf095 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -1,5 +1,6 @@ use std::collections::BTreeMap; +use malachite_round::state_machine::RoundContext; use secrecy::{ExposeSecret, Secret}; use malachite_common::signature::Keypair; @@ -93,10 +94,9 @@ where RoundMessage::NewRound(round) => { // TODO: check if we are the proposer - self.round_states.insert( - round, - RoundState::new(self.height.clone(), self.address.clone()).new_round(round), - ); + // XXX: Check if there is an existing state? + self.round_states + .insert(round, RoundState::default().new_round(round)); None } @@ -160,7 +160,7 @@ where } // Check that the proposal is for the current height and round - if round_state.height != proposal.height() || proposal.round() != self.round { + if self.height != proposal.height() || self.round != proposal.round() { return None; } @@ -232,13 +232,12 @@ where /// Apply the event, update the state. fn apply_event(&mut self, round: Round, event: RoundEvent) -> Option> { // Get the round state, or create a new one - let round_state = self - .round_states - .remove(&round) - .unwrap_or_else(|| RoundState::new(self.height.clone(), self.address.clone())); + let round_state = self.round_states.remove(&round).unwrap_or_default(); + + let round_ctx = RoundContext::new(round, &self.height, &self.address); // Apply the event to the round state machine - let transition = round_state.apply_event(round, event); + let transition = round_state.apply_event(&round_ctx, event); // Update state self.round_states.insert(round, transition.next_state); diff --git a/Code/round/src/state.rs b/Code/round/src/state.rs index 3b460f523..a813d85f7 100644 --- a/Code/round/src/state.rs +++ b/Code/round/src/state.rs @@ -1,4 +1,5 @@ use crate::events::Event; +use crate::state_machine::RoundContext; use crate::transition::Transition; use malachite_common::{Context, Round}; @@ -32,8 +33,6 @@ pub struct State where Ctx: Context, { - pub address: Ctx::Address, - pub height: Ctx::Height, pub round: Round, pub step: Step, pub proposal: Option, @@ -47,8 +46,6 @@ where { fn clone(&self) -> Self { Self { - address: self.address.clone(), - height: self.height.clone(), round: self.round, step: self.step, proposal: self.proposal.clone(), @@ -62,10 +59,8 @@ impl State where Ctx: Context, { - pub fn new(height: Ctx::Height, address: Ctx::Address) -> Self { + pub fn new() -> Self { Self { - address, - height, round: Round::INITIAL, step: Step::NewRound, proposal: None, @@ -114,7 +109,16 @@ where } } - pub fn apply_event(self, round: Round, event: Event) -> Transition { - crate::state_machine::apply_event(self, round, event) + pub fn apply_event(self, ctx: &RoundContext, event: Event) -> Transition { + crate::state_machine::apply_event(self, ctx, event) + } +} + +impl Default for State +where + Ctx: Context, +{ + fn default() -> Self { + Self::new() } } diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index 2dead4a08..1241d6a1f 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -5,6 +5,28 @@ use crate::message::Message; use crate::state::{State, Step}; use crate::transition::Transition; +pub struct RoundContext<'a, Ctx> +where + Ctx: Context, +{ + pub round: Round, + pub height: &'a Ctx::Height, + pub address: &'a Ctx::Address, +} + +impl<'a, Ctx> RoundContext<'a, Ctx> +where + Ctx: Context, +{ + pub fn new(round: Round, height: &'a Ctx::Height, address: &'a Ctx::Address) -> Self { + Self { + round, + height, + address, + } + } +} + /// Check that a proposal has a valid Proof-Of-Lock round fn is_valid_pol_round(state: &State, pol_round: Round) -> bool where @@ -21,15 +43,21 @@ where /// Valid transitions result in at least a change to the state and/or an output message. /// /// Commented numbers refer to line numbers in the spec paper. -pub fn apply_event(mut state: State, round: Round, event: Event) -> Transition +pub fn apply_event( + mut state: State, + ctx: &RoundContext, + event: Event, +) -> Transition where Ctx: Context, { - let this_round = state.round == round; + let this_round = state.round == ctx.round; match (state.step, event) { // From NewRound. Event must be for current round. - (Step::NewRound, Event::NewRoundProposer(value)) if this_round => propose(state, value), // L11/L14 + (Step::NewRound, Event::NewRoundProposer(value)) if this_round => { + propose(state, ctx.height, value) // L11/L14 + } (Step::NewRound, Event::NewRound) if this_round => schedule_timeout_propose(state), // L11/L20 // From Propose. Event must be for current round. @@ -44,9 +72,9 @@ where .map_or(true, |locked| &locked.value == proposal.value()) { state.proposal = Some(proposal.clone()); - prevote(state, proposal.round(), proposal.value().id()) + prevote(state, ctx.address, proposal.round(), proposal.value().id()) } else { - prevote_nil(state) + prevote_nil(state, ctx.address) } } @@ -62,33 +90,35 @@ where if proposal.value().is_valid() && (locked.round <= proposal.pol_round() || &locked.value == proposal.value()) { - prevote(state, proposal.round(), proposal.value().id()) + prevote(state, ctx.address, proposal.round(), proposal.value().id()) } else { - prevote_nil(state) + prevote_nil(state, ctx.address) } } - (Step::Propose, Event::ProposalInvalid) if this_round => prevote_nil(state), // L22/L25, L28/L31 - (Step::Propose, Event::TimeoutPropose) if this_round => prevote_nil(state), // L57 + (Step::Propose, Event::ProposalInvalid) if this_round => prevote_nil(state, ctx.address), // L22/L25, L28/L31 + (Step::Propose, Event::TimeoutPropose) if this_round => prevote_nil(state, ctx.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), // L44 - (Step::Prevote, Event::PolkaValue(value_id)) if this_round => precommit(state, value_id), // L36/L37 - NOTE: only once? - (Step::Prevote, Event::TimeoutPrevote) if this_round => precommit_nil(state), // L61 + (Step::Prevote, Event::PolkaNil) if this_round => precommit_nil(state, ctx), // L44 + (Step::Prevote, Event::PolkaValue(value_id)) if this_round => { + precommit(state, ctx.address, value_id) // L36/L37 - NOTE: only once? + } + (Step::Prevote, Event::TimeoutPrevote) if this_round => precommit_nil(state, ctx), // L61 // From Precommit. Event must be for current round. (Step::Precommit, Event::PolkaValue(value_id)) if this_round => { - set_valid_value(state, value_id) - } // L36/L42 - NOTE: only once? + set_valid_value(state, value_id) // L36/L42 - NOTE: only once? + } // From Commit. No more state transitions. (Step::Commit, _) => Transition::invalid(state), // 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, round.increment()), // L65 - (_, Event::RoundSkip) if state.round < round => round_skip(state, round), // L55 - (_, Event::PrecommitValue(value_id)) => commit(state, round, value_id), // L49 + (_, Event::TimeoutPrecommit) if this_round => round_skip(state, ctx.round.increment()), // L65 + (_, Event::RoundSkip) if state.round < ctx.round => round_skip(state, ctx.round), // L55 + (_, Event::PrecommitValue(value_id)) => commit(state, ctx.round, value_id), // L49 // Invalid transition. _ => Transition::invalid(state), @@ -103,7 +133,7 @@ where /// otherwise propose the given value. /// /// Ref: L11/L14 -pub fn propose(state: State, value: Ctx::Value) -> Transition +pub fn propose(state: State, height: &Ctx::Height, value: Ctx::Value) -> Transition where Ctx: Context, { @@ -112,7 +142,7 @@ where None => (value, Round::Nil), }; - let proposal = Message::proposal(state.height.clone(), state.round, value, pol_round); + let proposal = Message::proposal(height.clone(), state.round, value, pol_round); Transition::to(state.next_step()).with_message(proposal) } @@ -124,7 +154,12 @@ where /// unless we are locked on something else at a higher round. /// /// Ref: L22/L28 -pub fn prevote(state: State, vr: Round, proposed: ValueId) -> Transition +pub fn prevote( + state: State, + address: &Ctx::Address, + vr: Round, + proposed: ValueId, +) -> Transition where Ctx: Context, { @@ -135,18 +170,18 @@ where None => Some(proposed), // not locked, prevote the value }; - let message = Message::prevote(state.round, value, state.address.clone()); + let message = Message::prevote(state.round, value, address.clone()); Transition::to(state.next_step()).with_message(message) } /// Received a complete proposal for an empty or invalid value, or timed out; prevote nil. /// /// Ref: L22/L25, L28/L31, L57 -pub fn prevote_nil(state: State) -> Transition +pub fn prevote_nil(state: State, address: &Ctx::Address) -> Transition where Ctx: Context, { - let message = Message::prevote(state.round, None, state.address.clone()); + let message = Message::prevote(state.round, None, address.clone()); Transition::to(state.next_step()).with_message(message) } @@ -160,11 +195,15 @@ where /// /// NOTE: Only one of this and set_valid_value should be called once in a round /// How do we enforce this? -pub fn precommit(state: State, value_id: ValueId) -> Transition +pub fn precommit( + state: State, + address: &Ctx::Address, + value_id: ValueId, +) -> Transition where Ctx: Context, { - let message = Message::precommit(state.round, Some(value_id), state.address.clone()); + let message = Message::precommit(state.round, Some(value_id), address.clone()); let Some(value) = state .proposal @@ -183,11 +222,11 @@ where /// Received a polka for nil or timed out of prevote; precommit nil. /// /// Ref: L44, L61 -pub fn precommit_nil(state: State) -> Transition +pub fn precommit_nil(state: State, ctx: &RoundContext) -> Transition where Ctx: Context, { - let message = Message::precommit(state.round, None, state.address.clone()); + let message = Message::precommit(state.round, None, ctx.address.clone()); Transition::to(state.next_step()).with_message(message) } diff --git a/Code/test/tests/consensus_executor.rs b/Code/test/tests/consensus_executor.rs index 799705e19..57e7fe3bf 100644 --- a/Code/test/tests/consensus_executor.rs +++ b/Code/test/tests/consensus_executor.rs @@ -57,8 +57,6 @@ fn executor_steps_proposer() { input_event: Some(Event::NewRound(Round::new(0))), expected_output: Some(Message::Propose(proposal.clone())), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Propose, proposal: None, @@ -73,8 +71,6 @@ fn executor_steps_proposer() { Vote::new_prevote(Round::new(0), Some(value_id), my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -87,8 +83,6 @@ fn executor_steps_proposer() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -103,8 +97,6 @@ fn executor_steps_proposer() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -121,8 +113,6 @@ fn executor_steps_proposer() { Vote::new_precommit(Round::new(0), Some(value_id), my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -141,8 +131,6 @@ fn executor_steps_proposer() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -163,8 +151,6 @@ fn executor_steps_proposer() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -185,8 +171,6 @@ fn executor_steps_proposer() { )), expected_output: Some(Message::Decide(Round::new(0), value.clone())), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Commit, proposal: Some(proposal.clone()), @@ -254,8 +238,6 @@ fn executor_steps_not_proposer() { input_event: Some(Event::NewRound(Round::new(0))), expected_output: Some(Message::SetTimeout(Timeout::propose(Round::new(0)))), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Propose, proposal: None, @@ -270,8 +252,6 @@ fn executor_steps_not_proposer() { Vote::new_prevote(Round::new(0), Some(value_id), my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -284,8 +264,6 @@ fn executor_steps_not_proposer() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -300,8 +278,6 @@ fn executor_steps_not_proposer() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: Some(proposal.clone()), @@ -318,8 +294,6 @@ fn executor_steps_not_proposer() { Vote::new_precommit(Round::new(0), Some(value_id), my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -338,8 +312,6 @@ fn executor_steps_not_proposer() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -360,8 +332,6 @@ fn executor_steps_not_proposer() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: Some(proposal.clone()), @@ -382,8 +352,6 @@ fn executor_steps_not_proposer() { )), expected_output: Some(Message::Decide(Round::new(0), value.clone())), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Commit, proposal: Some(proposal.clone()), @@ -450,8 +418,6 @@ fn executor_steps_not_proposer_timeout() { input_event: Some(Event::NewRound(Round::new(0))), expected_output: Some(Message::SetTimeout(Timeout::propose(Round::new(0)))), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Propose, proposal: None, @@ -467,8 +433,6 @@ fn executor_steps_not_proposer_timeout() { Vote::new_prevote(Round::new(0), None, my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: None, @@ -482,8 +446,6 @@ fn executor_steps_not_proposer_timeout() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: None, @@ -499,8 +461,6 @@ fn executor_steps_not_proposer_timeout() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Prevote, proposal: None, @@ -518,8 +478,6 @@ fn executor_steps_not_proposer_timeout() { Vote::new_precommit(Round::new(0), None, my_addr).signed(&my_sk), )), new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: None, @@ -533,8 +491,6 @@ fn executor_steps_not_proposer_timeout() { input_event: None, expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: None, @@ -550,8 +506,6 @@ fn executor_steps_not_proposer_timeout() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: None, @@ -567,8 +521,6 @@ fn executor_steps_not_proposer_timeout() { )), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(0), step: Step::Precommit, proposal: None, @@ -582,8 +534,6 @@ fn executor_steps_not_proposer_timeout() { input_event: Some(Event::Timeout(Timeout::precommit(Round::new(0)))), expected_output: None, new_state: State { - address: my_addr, - height: Height::new(1), round: Round::new(1), step: Step::NewRound, proposal: None, diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index 52be337b7..12a8d32d4 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -4,16 +4,19 @@ 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; +use malachite_round::state_machine::{apply_event, RoundContext}; const ADDRESS: Address = Address::new([42; 20]); #[test] fn test_propose() { let value = Value::new(42); - let mut state: State = State::new(Height::new(10), ADDRESS); + let height = Height::new(10); - let transition = apply_event(state.clone(), Round::new(0), Event::NewRoundProposer(value)); + let mut state: State = State::default(); + let ctx = RoundContext::new(Round::new(0), &height, &ADDRESS); + + let transition = apply_event(state.clone(), &ctx, Event::NewRoundProposer(value)); state.step = Step::Propose; assert_eq!(transition.next_state, state); @@ -27,9 +30,12 @@ fn test_propose() { #[test] fn test_prevote() { let value = Value::new(42); - let state: State = State::new(Height::new(1), ADDRESS).new_round(Round::new(1)); + let height = Height::new(1); + + let state: State = State::default().new_round(Round::new(1)); + let ctx = RoundContext::new(Round::new(1), &height, &ADDRESS); - let transition = apply_event(state, Round::new(1), Event::NewRound); + let transition = apply_event(state, &ctx, Event::NewRound); assert_eq!(transition.next_state.step, Step::Propose); assert_eq!( @@ -44,7 +50,7 @@ fn test_prevote() { let transition = apply_event( state, - Round::new(1), + &ctx, Event::Proposal(Proposal::new( Height::new(1), Round::new(1),