From 8398069e05baab4919ca3dcd3e0aa4ecfda7b241 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 14 Nov 2023 17:49:05 +0100 Subject: [PATCH 1/3] Simplify code to keep only one round state --- Code/driver/src/driver.rs | 79 ++++++++++++++------------------- Code/driver/src/event.rs | 1 + Code/round/src/state.rs | 16 ++----- Code/round/src/state_machine.rs | 3 +- Code/test/tests/driver.rs | 63 +++++++++++++------------- Code/test/tests/round.rs | 8 +++- 6 files changed, 80 insertions(+), 90 deletions(-) diff --git a/Code/driver/src/driver.rs b/Code/driver/src/driver.rs index d7e044fe4..6863b0c2e 100644 --- a/Code/driver/src/driver.rs +++ b/Code/driver/src/driver.rs @@ -1,5 +1,3 @@ -use alloc::collections::BTreeMap; - use malachite_round::state_machine::RoundData; use malachite_common::{ @@ -32,13 +30,11 @@ where pub env: Env, pub proposer_selector: PSel, - pub height: Ctx::Height, pub address: Ctx::Address, pub validator_set: Ctx::ValidatorSet, - pub round: Round, pub votes: VoteKeeper, - pub round_states: BTreeMap>, + pub round_state: RoundState, } impl Driver @@ -51,7 +47,6 @@ where ctx: Ctx, env: Env, proposer_selector: PSel, - height: Ctx::Height, validator_set: Ctx::ValidatorSet, address: Ctx::Address, ) -> Self { @@ -61,17 +56,17 @@ where ctx, env, proposer_selector, - height, address, validator_set, - round: Round::NIL, votes, - round_states: BTreeMap::new(), + round_state: RoundState::default(), } } - async fn get_value(&self, round: Round) -> Option { - self.env.get_value(self.height.clone(), round).await + async fn get_value(&self) -> Option { + self.env + .get_value(self.round_state.height.clone(), self.round_state.round) + .await } pub async fn execute(&mut self, msg: Event) -> Result>, Error> { @@ -81,11 +76,7 @@ where }; let msg = match round_msg { - RoundMessage::NewRound(round) => { - // XXX: Check if there is an existing state? - assert!(self.round < round); - Message::NewRound(round) - } + RoundMessage::NewRound(round) => Message::NewRound(round), RoundMessage::Proposal(proposal) => { // sign the proposal @@ -108,21 +99,32 @@ where Ok(Some(msg)) } - async fn apply(&mut self, msg: Event) -> Result>, Error> { - match msg { - Event::NewRound(round) => self.apply_new_round(round).await, + async fn apply(&mut self, event: Event) -> Result>, Error> { + match event { + Event::StartHeight(height) => self.apply_new_round(height, Round::new(0)).await, + + Event::NewRound(round) => { + self.apply_new_round(self.round_state.height.clone(), round) + .await + } + Event::Proposal(proposal, validity) => { Ok(self.apply_proposal(proposal, validity).await) } + Event::Vote(signed_vote) => self.apply_vote(signed_vote), + Event::TimeoutElapsed(timeout) => Ok(self.apply_timeout(timeout)), } } async fn apply_new_round( &mut self, + height: Ctx::Height, round: Round, ) -> Result>, Error> { + self.round_state = RoundState::new(height, round); + let proposer_address = self .proposer_selector .select_proposer(round, &self.validator_set); @@ -136,7 +138,7 @@ where // We are the proposer // TODO: Schedule propose timeout - let Some(value) = self.get_value(round).await else { + let Some(value) = self.get_value().await else { return Err(Error::NoValueToPropose); }; @@ -145,13 +147,6 @@ where RoundEvent::NewRound }; - assert!(self.round < round); - self.round_states.insert( - round, - RoundState::default().new_round(self.height.clone(), round), - ); - self.round = round; - Ok(self.apply_event(round, event)) } @@ -161,23 +156,24 @@ where validity: Validity, ) -> Option> { // Check that there is an ongoing round - let Some(round_state) = self.round_states.get(&self.round) else { - // TODO: Add logging + if self.round_state.round == Round::NIL { return None; - }; + } // Only process the proposal if there is no other proposal - if round_state.proposal.is_some() { + if self.round_state.proposal.is_some() { return None; } // Check that the proposal is for the current height and round - if self.height != proposal.height() || self.round != proposal.round() { + if self.round_state.height != proposal.height() + || self.round_state.round != proposal.round() + { return None; } // TODO: Document - if proposal.pol_round().is_defined() && proposal.pol_round() >= round_state.round { + if proposal.pol_round().is_defined() && proposal.pol_round() >= self.round_state.round { return None; } @@ -268,20 +264,17 @@ 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_default(); - - let data = RoundData::new(round, &self.height, &self.address); + let data = RoundData::new(round, &self.round_state.height, &self.address); // Multiplex the event with the round state. let mux_event = match event { - RoundEvent::PolkaValue(value_id) => match round_state.proposal { + RoundEvent::PolkaValue(value_id) => match self.round_state.proposal { Some(ref proposal) if proposal.value().id() == value_id => { RoundEvent::ProposalAndPolkaCurrent(proposal.clone()) } _ => RoundEvent::PolkaAny, }, - RoundEvent::PrecommitValue(value_id) => match round_state.proposal { + RoundEvent::PrecommitValue(value_id) => match self.round_state.proposal { Some(ref proposal) if proposal.value().id() == value_id => { RoundEvent::ProposalAndPrecommitValue(proposal.clone()) } @@ -292,16 +285,12 @@ where }; // Apply the event to the round state machine - let transition = round_state.apply_event(&data, mux_event); + let transition = self.round_state.clone().apply_event(&data, mux_event); // Update state - self.round_states.insert(round, transition.next_state); + self.round_state = transition.next_state; // Return message, if any transition.message } - - pub fn round_state(&self, round: Round) -> Option<&RoundState> { - self.round_states.get(&round) - } } diff --git a/Code/driver/src/event.rs b/Code/driver/src/event.rs index d75dfab5c..ab616ea1e 100644 --- a/Code/driver/src/event.rs +++ b/Code/driver/src/event.rs @@ -8,6 +8,7 @@ pub enum Event where Ctx: Context, { + StartHeight(Ctx::Height), NewRound(Round), Proposal(Ctx::Proposal, Validity), Vote(SignedVote), diff --git a/Code/round/src/state.rs b/Code/round/src/state.rs index 50d6879b7..c7edc6e5e 100644 --- a/Code/round/src/state.rs +++ b/Code/round/src/state.rs @@ -46,10 +46,10 @@ impl State where Ctx: Context, { - pub fn new() -> Self { + pub fn new(height: Ctx::Height, round: Round) -> Self { Self { - height: Ctx::Height::default(), - round: Round::INITIAL, + height, + round, step: Step::NewRound, proposal: None, locked: None, @@ -57,14 +57,6 @@ where } } - pub fn new_round(self, height: Ctx::Height, round: Round) -> Self { - Self { - height, - round, - step: Step::NewRound, - ..self - } - } pub fn with_step(self, step: Step) -> Self { Self { step, ..self } } @@ -97,7 +89,7 @@ where Ctx: Context, { fn default() -> Self { - Self::new() + Self::new(Ctx::Height::default(), Round::NIL) } } diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index cf51be128..1dce8a5dd 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -331,8 +331,7 @@ pub fn round_skip(state: State, round: Round) -> Transition where Ctx: Context, { - Transition::to(state.clone().new_round(state.height.clone(), round)) - .with_message(Message::NewRound(round)) + Transition::to(State::new(state.height.clone(), round)).with_message(Message::NewRound(round)) } /// We received +2/3 precommits for a value - commit and decide that value! diff --git a/Code/test/tests/driver.rs b/Code/test/tests/driver.rs index 81588032b..7d079bb38 100644 --- a/Code/test/tests/driver.rs +++ b/Code/test/tests/driver.rs @@ -85,14 +85,14 @@ fn driver_steps_proposer() { let ctx = TestContext::new(my_sk.clone()); let vs = ValidatorSet::new(vec![v1, v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, 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", - input_event: Some(Event::NewRound(Round::new(0))), + input_event: Some(Event::StartHeight(Height::new(1))), expected_output: Some(Message::Propose(proposal.clone())), expected_round: Round::new(0), new_state: State { @@ -258,10 +258,12 @@ fn driver_steps_proposer() { let output = block_on(driver.execute(execute_message)).expect("execute succeeded"); assert_eq!(output, step.expected_output, "expected output message"); - assert_eq!(driver.round, step.expected_round, "expected round"); + assert_eq!( + driver.round_state.round, step.expected_round, + "expected round" + ); - let new_state = driver.round_state(Round::new(0)).unwrap(); - assert_eq!(new_state, &step.new_state, "expected state"); + assert_eq!(driver.round_state, step.new_state, "expected state"); previous_message = output.and_then(to_input_msg); } @@ -294,14 +296,14 @@ fn driver_steps_not_proposer_valid() { let ctx = TestContext::new(my_sk.clone()); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, 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 not the proposer", - input_event: Some(Event::NewRound(Round::new(0))), + input_event: Some(Event::StartHeight(Height::new(1))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -467,10 +469,12 @@ fn driver_steps_not_proposer_valid() { let output = block_on(driver.execute(execute_message)).expect("execute succeeded"); assert_eq!(output, step.expected_output, "expected output message"); - assert_eq!(driver.round, step.expected_round, "expected round"); + assert_eq!( + driver.round_state.round, step.expected_round, + "expected round" + ); - let new_state = driver.round_state(Round::new(0)).unwrap(); - assert_eq!(new_state, &step.new_state, "expected state"); + assert_eq!(driver.round_state, step.new_state, "expected state"); previous_message = output.and_then(to_input_msg); } @@ -503,14 +507,14 @@ fn driver_steps_not_proposer_invalid() { let ctx = TestContext::new(my_sk.clone()); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, 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 not the proposer", - input_event: Some(Event::NewRound(Round::new(0))), + input_event: Some(Event::StartHeight(Height::new(1))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -614,10 +618,12 @@ fn driver_steps_not_proposer_invalid() { let output = block_on(driver.execute(execute_message)).expect("execute succeeded"); assert_eq!(output, step.expected_output, "expected output"); - assert_eq!(driver.round, step.expected_round, "expected round"); + assert_eq!( + driver.round_state.round, step.expected_round, + "expected round" + ); - let new_state = driver.round_state(driver.round).unwrap(); - assert_eq!(new_state, &step.new_state, "expected state"); + assert_eq!(driver.round_state, step.new_state, "expected state"); previous_message = output.and_then(to_input_msg); } @@ -650,13 +656,13 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { let ctx = TestContext::new(my_sk.clone()); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut driver = Driver::new(ctx, env, sel, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, sel, vs, my_addr); let steps = vec![ // Start round 0, we, v3, are not the proposer TestStep { desc: "Start round 0, we, v3, are not the proposer", - input_event: Some(Event::NewRound(Round::new(0))), + input_event: Some(Event::StartHeight(Height::new(1))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -830,10 +836,7 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { let output = block_on(driver.execute(execute_message)).expect("execute succeeded"); assert_eq!(output, step.expected_output, "expected output message"); - assert_eq!(driver.round, step.expected_round, "expected round"); - - let new_state = driver.round_state(driver.round).unwrap(); - assert_eq!(new_state, &step.new_state, "new state"); + assert_eq!(driver.round_state, step.new_state, "new state"); previous_message = output.and_then(to_input_msg); } @@ -861,9 +864,9 @@ 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, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, sel, vs, my_addr); - let output = block_on(driver.execute(Event::NewRound(Round::new(0)))); + let output = block_on(driver.execute(Event::StartHeight(Height::new(1)))); assert_eq!(output, Err(Error::NoValueToPropose)); } @@ -892,9 +895,9 @@ 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, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, sel, vs, my_addr); - let output = block_on(driver.execute(Event::NewRound(Round::new(0)))); + let output = block_on(driver.execute(Event::StartHeight(Height::new(1)))); assert_eq!(output, Err(Error::ProposerNotFound(v1.address))); } @@ -922,10 +925,10 @@ 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, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, sel, vs, my_addr); - // Start new round - block_on(driver.execute(Event::NewRound(Round::new(0)))).expect("execute succeeded"); + // Start new height + block_on(driver.execute(Event::StartHeight(Height::new(1)))).expect("execute succeeded"); // v2 prevotes for some proposal, we cannot find it in the validator set => error let output = block_on(driver.execute(Event::Vote( @@ -957,10 +960,10 @@ 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, Height::new(1), vs, my_addr); + let mut driver = Driver::new(ctx, env, sel, vs, my_addr); // Start new round - block_on(driver.execute(Event::NewRound(Round::new(0)))).expect("execute succeeded"); + block_on(driver.execute(Event::StartHeight(Height::new(1)))).expect("execute succeeded"); // v2 prevotes for some proposal, with an invalid signature, // ie. signed by v1 instead of v2, just a way of forging an invalid signature diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index 34b02d566..18cf67924 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -36,8 +36,14 @@ fn test_propose() { fn test_prevote() { let value = Value::new(42); let height = Height::new(1); + let round = Round::new(1); + + let state: State = State { + height, + round, + ..Default::default() + }; - let state: State = State::default().new_round(height, Round::new(1)); let data = RoundData::new(Round::new(1), &height, &ADDRESS); let transition = apply_event(state, &data, Event::NewRound); From a9ea037544336b9a57501c7b8eff965afcce1d00 Mon Sep 17 00:00:00 2001 From: Anca Zamfir Date: Tue, 14 Nov 2023 18:14:32 +0100 Subject: [PATCH 2/3] Replace StartHeight with NewRound with height and round zero --- Code/driver/src/driver.rs | 11 ++++------- Code/driver/src/event.rs | 3 +-- Code/driver/src/message.rs | 10 ++++++---- Code/test/tests/driver.rs | 24 +++++++++++++----------- 4 files changed, 24 insertions(+), 24 deletions(-) diff --git a/Code/driver/src/driver.rs b/Code/driver/src/driver.rs index 6863b0c2e..e25c26d98 100644 --- a/Code/driver/src/driver.rs +++ b/Code/driver/src/driver.rs @@ -76,7 +76,9 @@ where }; let msg = match round_msg { - RoundMessage::NewRound(round) => Message::NewRound(round), + RoundMessage::NewRound(round) => { + Message::NewRound(self.round_state.height.clone(), round) + } RoundMessage::Proposal(proposal) => { // sign the proposal @@ -101,12 +103,7 @@ where async fn apply(&mut self, event: Event) -> Result>, Error> { match event { - Event::StartHeight(height) => self.apply_new_round(height, Round::new(0)).await, - - Event::NewRound(round) => { - self.apply_new_round(self.round_state.height.clone(), round) - .await - } + Event::NewRound(height, round) => self.apply_new_round(height, round).await, Event::Proposal(proposal, validity) => { Ok(self.apply_proposal(proposal, validity).await) diff --git a/Code/driver/src/event.rs b/Code/driver/src/event.rs index ab616ea1e..d0a3381e1 100644 --- a/Code/driver/src/event.rs +++ b/Code/driver/src/event.rs @@ -8,8 +8,7 @@ pub enum Event where Ctx: Context, { - StartHeight(Ctx::Height), - NewRound(Round), + NewRound(Ctx::Height, Round), Proposal(Ctx::Proposal, Validity), Vote(SignedVote), TimeoutElapsed(Timeout), diff --git a/Code/driver/src/message.rs b/Code/driver/src/message.rs index 89a56333b..b5f4d7073 100644 --- a/Code/driver/src/message.rs +++ b/Code/driver/src/message.rs @@ -11,7 +11,7 @@ where Vote(SignedVote), Decide(Round, Ctx::Value), ScheduleTimeout(Timeout), - NewRound(Round), + NewRound(Ctx::Height, Round), } // NOTE: We have to derive these instances manually, otherwise @@ -26,7 +26,7 @@ impl Clone for Message { 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(round) => Message::NewRound(*round), + Message::NewRound(height, round) => Message::NewRound(height.clone(), *round), } } } @@ -39,7 +39,7 @@ impl fmt::Debug for Message { 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(round) => write!(f, "NewRound({:?})", round), + Message::NewRound(height, round) => write!(f, "NewRound({:?}, {:?})", height, round), } } } @@ -60,7 +60,9 @@ impl PartialEq for Message { (Message::ScheduleTimeout(timeout), Message::ScheduleTimeout(other_timeout)) => { timeout == other_timeout } - (Message::NewRound(round), Message::NewRound(other_round)) => round == other_round, + (Message::NewRound(height, round), Message::NewRound(other_height, other_round)) => { + height == other_height && round == other_round + } _ => false, } } diff --git a/Code/test/tests/driver.rs b/Code/test/tests/driver.rs index 7d079bb38..93d4006ba 100644 --- a/Code/test/tests/driver.rs +++ b/Code/test/tests/driver.rs @@ -25,7 +25,7 @@ fn to_input_msg(output: Message) -> Option> { Message::Vote(v) => Some(Event::Vote(v)), Message::Decide(_, _) => None, Message::ScheduleTimeout(_) => None, - Message::NewRound(round) => Some(Event::NewRound(round)), + Message::NewRound(height, round) => Some(Event::NewRound(height, round)), } } @@ -92,7 +92,7 @@ fn driver_steps_proposer() { let steps = vec![ TestStep { desc: "Start round 0, we are proposer, propose value", - input_event: Some(Event::StartHeight(Height::new(1))), + input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), expected_output: Some(Message::Propose(proposal.clone())), expected_round: Round::new(0), new_state: State { @@ -303,7 +303,7 @@ fn driver_steps_not_proposer_valid() { let steps = vec![ TestStep { desc: "Start round 0, we are not the proposer", - input_event: Some(Event::StartHeight(Height::new(1))), + input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -514,7 +514,7 @@ fn driver_steps_not_proposer_invalid() { let steps = vec![ TestStep { desc: "Start round 0, we are not the proposer", - input_event: Some(Event::StartHeight(Height::new(1))), + input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -662,7 +662,7 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { // Start round 0, we, v3, are not the proposer TestStep { desc: "Start round 0, we, v3, are not the proposer", - input_event: Some(Event::StartHeight(Height::new(1))), + input_event: Some(Event::NewRound(Height::new(1), Round::new(0))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(0)))), expected_round: Round::new(0), new_state: State { @@ -797,7 +797,7 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { TestStep { desc: "we receive a precommit timeout, start a new round", input_event: Some(Event::TimeoutElapsed(Timeout::precommit(Round::new(0)))), - expected_output: Some(Message::NewRound(Round::new(1))), + expected_output: Some(Message::NewRound(Height::new(1), Round::new(1))), expected_round: Round::new(0), new_state: State { height: Height::new(1), @@ -810,7 +810,7 @@ fn driver_steps_not_proposer_timeout_multiple_rounds() { }, TestStep { desc: "Start round 1, we are not the proposer", - input_event: Some(Event::NewRound(Round::new(1))), + input_event: Some(Event::NewRound(Height::new(1), Round::new(1))), expected_output: Some(Message::ScheduleTimeout(Timeout::propose(Round::new(1)))), expected_round: Round::new(1), new_state: State { @@ -866,7 +866,7 @@ fn driver_steps_no_value_to_propose() { let mut driver = Driver::new(ctx, env, sel, vs, my_addr); - let output = block_on(driver.execute(Event::StartHeight(Height::new(1)))); + let output = block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))); assert_eq!(output, Err(Error::NoValueToPropose)); } @@ -897,7 +897,7 @@ fn driver_steps_proposer_not_found() { let mut driver = Driver::new(ctx, env, sel, vs, my_addr); - let output = block_on(driver.execute(Event::StartHeight(Height::new(1)))); + let output = block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))); assert_eq!(output, Err(Error::ProposerNotFound(v1.address))); } @@ -928,7 +928,8 @@ fn driver_steps_validator_not_found() { let mut driver = Driver::new(ctx, env, sel, vs, my_addr); // Start new height - block_on(driver.execute(Event::StartHeight(Height::new(1)))).expect("execute succeeded"); + block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))) + .expect("execute succeeded"); // v2 prevotes for some proposal, we cannot find it in the validator set => error let output = block_on(driver.execute(Event::Vote( @@ -963,7 +964,8 @@ fn driver_steps_invalid_signature() { let mut driver = Driver::new(ctx, env, sel, vs, my_addr); // Start new round - block_on(driver.execute(Event::StartHeight(Height::new(1)))).expect("execute succeeded"); + block_on(driver.execute(Event::NewRound(Height::new(1), Round::new(0)))) + .expect("execute succeeded"); // v2 prevotes for some proposal, with an invalid signature, // ie. signed by v1 instead of v2, just a way of forging an invalid signature From cbb27a65d3e44e90f9ad8b30862039b1b2c8cd95 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Wed, 15 Nov 2023 16:24:17 +0100 Subject: [PATCH 3/3] chore: Avoid cloning the whole round state on new round --- Code/driver/src/driver.rs | 10 ++++++---- Code/round/src/state_machine.rs | 6 +++--- Code/test/tests/round.rs | 4 ++-- 3 files changed, 11 insertions(+), 9 deletions(-) diff --git a/Code/driver/src/driver.rs b/Code/driver/src/driver.rs index e25c26d98..574dd903f 100644 --- a/Code/driver/src/driver.rs +++ b/Code/driver/src/driver.rs @@ -261,17 +261,19 @@ where /// Apply the event, update the state. fn apply_event(&mut self, round: Round, event: RoundEvent) -> Option> { - let data = RoundData::new(round, &self.round_state.height, &self.address); + let round_state = core::mem::take(&mut self.round_state); + + let data = RoundData::new(round, round_state.height.clone(), &self.address); // Multiplex the event with the round state. let mux_event = match event { - RoundEvent::PolkaValue(value_id) => match self.round_state.proposal { + RoundEvent::PolkaValue(value_id) => match round_state.proposal { Some(ref proposal) if proposal.value().id() == value_id => { RoundEvent::ProposalAndPolkaCurrent(proposal.clone()) } _ => RoundEvent::PolkaAny, }, - RoundEvent::PrecommitValue(value_id) => match self.round_state.proposal { + RoundEvent::PrecommitValue(value_id) => match round_state.proposal { Some(ref proposal) if proposal.value().id() == value_id => { RoundEvent::ProposalAndPrecommitValue(proposal.clone()) } @@ -282,7 +284,7 @@ where }; // Apply the event to the round state machine - let transition = self.round_state.clone().apply_event(&data, mux_event); + let transition = round_state.apply_event(&data, mux_event); // Update state self.round_state = transition.next_state; diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index 1dce8a5dd..62c257fe6 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -16,7 +16,7 @@ where Ctx: Context, { pub round: Round, - pub height: &'a Ctx::Height, + pub height: Ctx::Height, pub address: &'a Ctx::Address, } @@ -24,7 +24,7 @@ impl<'a, Ctx> RoundData<'a, Ctx> where Ctx: Context, { - pub fn new(round: Round, height: &'a Ctx::Height, address: &'a Ctx::Address) -> Self { + pub fn new(round: Round, height: Ctx::Height, address: &'a Ctx::Address) -> Self { Self { round, height, @@ -62,7 +62,7 @@ where 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 + propose(state, &data.height, value) // L11/L14 } (Step::NewRound, Event::NewRound) if this_round => schedule_timeout_propose(state), // L11/L20 diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index 18cf67924..cd6d2b5b9 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -19,7 +19,7 @@ fn test_propose() { ..Default::default() }; - let data = RoundData::new(round, &height, &ADDRESS); + let data = RoundData::new(round, height, &ADDRESS); let transition = apply_event(state.clone(), &data, Event::NewRoundProposer(value)); @@ -44,7 +44,7 @@ fn test_prevote() { ..Default::default() }; - let data = RoundData::new(Round::new(1), &height, &ADDRESS); + let data = RoundData::new(Round::new(1), height, &ADDRESS); let transition = apply_event(state, &data, Event::NewRound);