From 47584fe78d28f5b68ae911a8c52234809025e6b1 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 31 Oct 2023 10:53:48 +0100 Subject: [PATCH 1/5] Move address from SignedVote into Vote --- Code/common/src/context.rs | 12 ++++++++++-- Code/common/src/signed_vote.rs | 15 +++++++-------- Code/common/src/vote.rs | 5 ++++- Code/consensus/src/executor.rs | 12 ++++-------- Code/round/src/message.rs | 8 ++++---- Code/test/src/consensus.rs | 8 ++++---- Code/test/src/vote.rs | 18 ++++++++++++------ Code/test/tests/vote_keeper.rs | 16 +++++++++------- 8 files changed, 54 insertions(+), 40 deletions(-) diff --git a/Code/common/src/context.rs b/Code/common/src/context.rs index a02625313..8469c6365 100644 --- a/Code/common/src/context.rs +++ b/Code/common/src/context.rs @@ -39,9 +39,17 @@ where /// Build a new prevote vote by the validator with the given address, /// for the value identified by the given value id, at the given round. - fn new_prevote(round: Round, value_id: Option>) -> Self::Vote; + fn new_prevote( + round: Round, + value_id: Option>, + address: Self::Address, + ) -> Self::Vote; /// Build a new precommit vote by the validator with the given address, /// for the value identified by the given value id, at the given round. - fn new_precommit(round: Round, value_id: Option>) -> Self::Vote; + fn new_precommit( + round: Round, + value_id: Option>, + address: Self::Address, + ) -> Self::Vote; } diff --git a/Code/common/src/signed_vote.rs b/Code/common/src/signed_vote.rs index 48bd6f0d4..29777599b 100644 --- a/Code/common/src/signed_vote.rs +++ b/Code/common/src/signed_vote.rs @@ -1,4 +1,4 @@ -use crate::{Context, Signature}; +use crate::{Context, Signature, Vote}; // TODO: Do we need to abstract over `SignedVote` as well? @@ -8,7 +8,6 @@ where Ctx: Context, { pub vote: Ctx::Vote, - pub address: Ctx::Address, pub signature: Signature, } @@ -16,11 +15,11 @@ impl SignedVote where Ctx: Context, { - pub fn new(vote: Ctx::Vote, address: Ctx::Address, signature: Signature) -> Self { - Self { - vote, - address, - signature, - } + pub fn new(vote: Ctx::Vote, signature: Signature) -> Self { + Self { vote, signature } + } + + pub fn validator_address(&self) -> &Ctx::Address { + self.vote.validator_address() } } diff --git a/Code/common/src/vote.rs b/Code/common/src/vote.rs index 069d04a18..a67190a26 100644 --- a/Code/common/src/vote.rs +++ b/Code/common/src/vote.rs @@ -18,7 +18,7 @@ pub enum VoteType { /// include information about the validator signing it. pub trait Vote where - Self: Clone + Debug + PartialEq + Eq, + Self: Clone + Debug + Eq, Ctx: Context, { /// The round for which the vote is for. @@ -32,4 +32,7 @@ where /// The type of vote. fn vote_type(&self) -> VoteType; + + /// Address of the validator who issued this vote + fn validator_address(&self) -> &Ctx::Address; } diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index e31b12a8b..5f263af02 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -102,14 +102,8 @@ where } RoundMessage::Vote(vote) => { - let address = self - .validator_set - .get_by_public_key(&self.key.expose_secret().verifying_key())? - .address() - .clone(); - let signature = Ctx::sign_vote(&vote, self.key.expose_secret()); - let signed_vote = SignedVote::new(vote, address, signature); + let signed_vote = SignedVote::new(vote, signature); Some(Message::Vote(signed_vote)) } @@ -194,7 +188,9 @@ where fn apply_vote(&mut self, signed_vote: SignedVote) -> Option> { // TODO: How to handle missing validator? - let validator = self.validator_set.get_by_address(&signed_vote.address)?; + let validator = self + .validator_set + .get_by_address(signed_vote.validator_address())?; if !Ctx::verify_signed_vote(&signed_vote, validator.public_key()) { // TODO: How to handle invalid votes? diff --git a/Code/round/src/message.rs b/Code/round/src/message.rs index 832fd05fe..dd94fe83b 100644 --- a/Code/round/src/message.rs +++ b/Code/round/src/message.rs @@ -42,12 +42,12 @@ where Message::Proposal(Ctx::new_proposal(height, round, value, pol_round)) } - pub fn prevote(round: Round, value_id: Option>) -> Self { - Message::Vote(Ctx::new_prevote(round, value_id)) + pub fn prevote(round: Round, value_id: Option>, address: Ctx::Address) -> Self { + Message::Vote(Ctx::new_prevote(round, value_id, address)) } - pub fn precommit(round: Round, value_id: Option>) -> Self { - Message::Vote(Ctx::new_precommit(round, value_id)) + pub fn precommit(round: Round, value_id: Option>, address: Ctx::Address) -> Self { + Message::Vote(Ctx::new_precommit(round, value_id, address)) } pub fn timeout(round: Round, step: TimeoutStep) -> Self { diff --git a/Code/test/src/consensus.rs b/Code/test/src/consensus.rs index aac73c2e2..3c4ef0537 100644 --- a/Code/test/src/consensus.rs +++ b/Code/test/src/consensus.rs @@ -40,11 +40,11 @@ impl Context for TestConsensus { Proposal::new(height, round, value, pol_round) } - fn new_prevote(round: Round, value_id: Option) -> Vote { - Vote::new_prevote(round, value_id) + fn new_prevote(round: Round, value_id: Option, address: Address) -> Vote { + Vote::new_prevote(round, value_id, address) } - fn new_precommit(round: Round, value_id: Option) -> Vote { - Vote::new_precommit(round, value_id) + fn new_precommit(round: Round, value_id: Option, address: Address) -> Vote { + Vote::new_precommit(round, value_id, address) } } diff --git a/Code/test/src/vote.rs b/Code/test/src/vote.rs index 5601b8a7e..98550b321 100644 --- a/Code/test/src/vote.rs +++ b/Code/test/src/vote.rs @@ -1,6 +1,7 @@ -use malachite_common::{Round, SignedVote, VoteType}; use signature::Signer; +use malachite_common::{Round, SignedVote, VoteType}; + use crate::{Address, PrivateKey, TestConsensus, ValueId}; /// A vote for a value in a round @@ -9,26 +10,29 @@ pub struct Vote { pub typ: VoteType, pub round: Round, pub value: Option, + pub validator_address: Address, } impl Vote { - pub fn new_prevote(round: Round, value: Option) -> Self { + pub fn new_prevote(round: Round, value: Option, validator_address: Address) -> Self { Self { typ: VoteType::Prevote, round, value, + validator_address, } } - pub fn new_precommit(round: Round, value: Option) -> Self { + pub fn new_precommit(round: Round, value: Option, address: Address) -> Self { Self { typ: VoteType::Precommit, round, value, + validator_address: address, } } - // TODO: Use the canonical vote + // TODO: Use a canonical vote pub fn to_bytes(&self) -> Vec { let vtpe = match self.typ { VoteType::Prevote => 0, @@ -47,12 +51,10 @@ impl Vote { } pub fn signed(self, private_key: &PrivateKey) -> SignedVote { - let address = Address::from_public_key(&private_key.public_key()); let signature = private_key.sign(&self.to_bytes()); SignedVote { vote: self, - address, signature, } } @@ -74,4 +76,8 @@ impl malachite_common::Vote for Vote { fn vote_type(&self) -> VoteType { self.typ } + + fn validator_address(&self) -> &Address { + &self.validator_address + } } diff --git a/Code/test/tests/vote_keeper.rs b/Code/test/tests/vote_keeper.rs index c8947c030..04e36fc97 100644 --- a/Code/test/tests/vote_keeper.rs +++ b/Code/test/tests/vote_keeper.rs @@ -1,13 +1,15 @@ use malachite_common::Round; use malachite_vote::keeper::{Message, VoteKeeper}; -use malachite_test::{Height, TestConsensus, ValueId, Vote}; +use malachite_test::{Address, Height, TestConsensus, ValueId, Vote}; + +const ADDRESS: Address = Address::new([42; 20]); #[test] fn prevote_apply_nil() { let mut keeper: VoteKeeper = VoteKeeper::new(Height::new(1), Round::INITIAL, 3); - let vote = Vote::new_prevote(Round::new(0), None); + let vote = Vote::new_prevote(Round::new(0), None, ADDRESS); let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); @@ -23,7 +25,7 @@ fn prevote_apply_nil() { fn precommit_apply_nil() { let mut keeper: VoteKeeper = VoteKeeper::new(Height::new(1), Round::INITIAL, 3); - let vote = Vote::new_precommit(Round::new(0), None); + let vote = Vote::new_precommit(Round::new(0), None, ADDRESS); let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); @@ -41,7 +43,7 @@ fn prevote_apply_single_value() { let v = ValueId::new(1); let val = Some(v); - let vote = Vote::new_prevote(Round::new(0), val); + let vote = Vote::new_prevote(Round::new(0), val, ADDRESS); let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); @@ -49,7 +51,7 @@ fn prevote_apply_single_value() { let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); - let vote_nil = Vote::new_prevote(Round::new(0), None); + let vote_nil = Vote::new_prevote(Round::new(0), None, ADDRESS); let msg = keeper.apply_vote(vote_nil, 1); assert_eq!(msg, Some(Message::PolkaAny)); @@ -63,7 +65,7 @@ fn precommit_apply_single_value() { let v = ValueId::new(1); let val = Some(v); - let vote = Vote::new_precommit(Round::new(0), val); + let vote = Vote::new_precommit(Round::new(0), val, ADDRESS); let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); @@ -71,7 +73,7 @@ fn precommit_apply_single_value() { let msg = keeper.apply_vote(vote.clone(), 1); assert_eq!(msg, None); - let vote_nil = Vote::new_precommit(Round::new(0), None); + let vote_nil = Vote::new_precommit(Round::new(0), None, ADDRESS); let msg = keeper.apply_vote(vote_nil, 1); assert_eq!(msg, Some(Message::PrecommitAny)); From a0e2535d1db71ff2d5869c81c01762a54ef4a6d4 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 31 Oct 2023 11:04:14 +0100 Subject: [PATCH 2/5] Rename a few fields for better clarity --- Code/consensus/src/executor.rs | 10 +++++----- Code/round/src/state_machine.rs | 10 +++++----- Code/test/tests/round.rs | 8 ++++---- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index 5f263af02..3f84b16ca 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -32,7 +32,7 @@ where Ctx: Context, { height: Ctx::Height, - key: Secret>, + private_key: Secret>, validator_set: Ctx::ValidatorSet, round: Round, votes: VoteKeeper, @@ -67,7 +67,7 @@ where Self { height, - key: Secret::new(key), + private_key: Secret::new(key), validator_set, round: Round::INITIAL, votes, @@ -102,7 +102,7 @@ where } RoundMessage::Vote(vote) => { - let signature = Ctx::sign_vote(&vote, self.key.expose_secret()); + let signature = Ctx::sign_vote(&vote, self.private_key.expose_secret()); let signed_vote = SignedVote::new(vote, signature); Some(Message::Vote(signed_vote)) @@ -129,7 +129,7 @@ where fn apply_new_round(&mut self, round: Round) -> Option> { let proposer = self.validator_set.get_proposer(); - let event = if proposer.public_key() == &self.key.expose_secret().verifying_key() { + let event = if proposer.public_key() == &self.private_key.expose_secret().verifying_key() { let value = self.get_value(); RoundEvent::NewRoundProposer(value) } else { @@ -236,7 +236,7 @@ where let transition = round_state.apply_event(round, event); // Update state - self.round_states.insert(round, transition.state); + self.round_states.insert(round, transition.next_state); // Return message, if any transition.message diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index d1bc77944..e1f82b0ab 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -9,7 +9,7 @@ pub struct Transition where Ctx: Context, { - pub state: State, + pub next_state: State, pub message: Option>, pub valid: bool, } @@ -18,17 +18,17 @@ impl Transition where Ctx: Context, { - pub fn to(state: State) -> Self { + pub fn to(next_state: State) -> Self { Self { - state, + next_state, message: None, valid: true, } } - pub fn invalid(state: State) -> Self { + pub fn invalid(next_state: State) -> Self { Self { - state, + next_state, message: None, valid: false, } diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index 44757b921..aceb30861 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -14,7 +14,7 @@ fn test_propose() { let transition = apply_event(state.clone(), Round::new(0), Event::NewRoundProposer(value)); state.step = Step::Propose; - assert_eq!(transition.state, state); + assert_eq!(transition.next_state, state); assert_eq!( transition.message.unwrap(), @@ -29,7 +29,7 @@ fn test_prevote() { let transition = apply_event(state, Round::new(1), Event::NewRound); - assert_eq!(transition.state.step, Step::Propose); + assert_eq!(transition.next_state.step, Step::Propose); assert_eq!( transition.message.unwrap(), Message::Timeout(Timeout { @@ -38,7 +38,7 @@ fn test_prevote() { }) ); - let state = transition.state; + let state = transition.next_state; let transition = apply_event( state, @@ -51,7 +51,7 @@ fn test_prevote() { )), ); - assert_eq!(transition.state.step, Step::Prevote); + assert_eq!(transition.next_state.step, Step::Prevote); assert_eq!( transition.message.unwrap(), Message::prevote(Round::new(1), Some(value.id()),) From ee55d302a3f4f47aa6024dcdbe90e185d30317f8 Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 31 Oct 2023 11:23:41 +0100 Subject: [PATCH 3/5] Supply validator address to the state machine --- Code/consensus/src/executor.rs | 15 +++-- Code/round/src/state.rs | 5 +- Code/round/src/state_machine.rs | 8 +-- Code/test/tests/consensus_executor.rs | 95 +++++++++++++++++++-------- Code/test/tests/round.rs | 10 +-- Code/test/tests/round_votes.rs | 16 +++-- 6 files changed, 100 insertions(+), 49 deletions(-) diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index 3f84b16ca..e446b918e 100644 --- a/Code/consensus/src/executor.rs +++ b/Code/consensus/src/executor.rs @@ -33,6 +33,7 @@ where { height: Ctx::Height, private_key: Secret>, + address: Ctx::Address, validator_set: Ctx::ValidatorSet, round: Round, votes: VoteKeeper, @@ -57,7 +58,8 @@ where pub fn new( height: Ctx::Height, validator_set: Ctx::ValidatorSet, - key: PrivateKey, + private_key: PrivateKey, + address: Ctx::Address, ) -> Self { let votes = VoteKeeper::new( height.clone(), @@ -67,7 +69,8 @@ where Self { height, - private_key: Secret::new(key), + private_key: Secret::new(private_key), + address, validator_set, round: Round::INITIAL, votes, @@ -90,8 +93,10 @@ where RoundMessage::NewRound(round) => { // TODO: check if we are the proposer - self.round_states - .insert(round, RoundState::new(self.height.clone()).new_round(round)); + self.round_states.insert( + round, + RoundState::new(self.height.clone(), self.address.clone()).new_round(round), + ); None } @@ -230,7 +235,7 @@ where let round_state = self .round_states .remove(&round) - .unwrap_or_else(|| RoundState::new(self.height.clone())); + .unwrap_or_else(|| RoundState::new(self.height.clone(), self.address.clone())); // Apply the event to the round state machine let transition = round_state.apply_event(round, event); diff --git a/Code/round/src/state.rs b/Code/round/src/state.rs index 81ecd0391..3c42e6d92 100644 --- a/Code/round/src/state.rs +++ b/Code/round/src/state.rs @@ -32,6 +32,7 @@ pub struct State where Ctx: Context, { + pub address: Ctx::Address, pub height: Ctx::Height, pub round: Round, pub step: Step, @@ -46,6 +47,7 @@ where { fn clone(&self) -> Self { Self { + address: self.address.clone(), height: self.height.clone(), round: self.round, step: self.step, @@ -60,8 +62,9 @@ impl State where Ctx: Context, { - pub fn new(height: Ctx::Height) -> Self { + pub fn new(height: Ctx::Height, address: Ctx::Address) -> Self { Self { + address, height, round: Round::INITIAL, step: Step::NewRound, diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index e1f82b0ab..c2ce3803e 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -170,7 +170,7 @@ where None => Some(proposed), // not locked, prevote the value }; - let message = Message::prevote(state.round, value); + let message = Message::prevote(state.round, value, state.address.clone()); Transition::to(state.next_step()).with_message(message) } @@ -181,7 +181,7 @@ pub fn prevote_nil(state: State) -> Transition where Ctx: Context, { - let message = Message::prevote(state.round, None); + let message = Message::prevote(state.round, None, state.address.clone()); Transition::to(state.next_step()).with_message(message) } @@ -199,7 +199,7 @@ pub fn precommit(state: State, value_id: ValueId) -> Transition(state: State) -> Transition where Ctx: Context, { - let message = Message::precommit(state.round, None); + let message = Message::precommit(state.round, None, state.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 c416ef802..799705e19 100644 --- a/Code/test/tests/consensus_executor.rs +++ b/Code/test/tests/consensus_executor.rs @@ -2,7 +2,9 @@ use malachite_common::{Context, Round, Timeout}; use malachite_consensus::executor::{Event, Executor, Message}; use malachite_round::state::{RoundValue, State, Step}; -use malachite_test::{Height, PrivateKey, Proposal, TestConsensus, Validator, ValidatorSet, Vote}; +use malachite_test::{ + Address, Height, PrivateKey, Proposal, TestConsensus, Validator, ValidatorSet, Vote, +}; use rand::rngs::StdRng; use rand::SeedableRng; @@ -33,15 +35,19 @@ fn executor_steps_proposer() { let sk2 = PrivateKey::generate(&mut rng); let sk3 = PrivateKey::generate(&mut rng); + let addr1 = Address::from_public_key(&sk1.public_key()); + let addr2 = Address::from_public_key(&sk2.public_key()); + let addr3 = Address::from_public_key(&sk3.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 = sk1; + let (my_sk, my_addr) = (sk1, addr1); let vs = ValidatorSet::new(vec![v1, v2.clone(), v3.clone()]); - let mut executor = Executor::new(Height::new(1), vs, my_sk.clone()); + let mut executor = Executor::new(Height::new(1), vs, my_sk.clone(), my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value.clone(), Round::new(-1)); @@ -51,6 +57,7 @@ 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, @@ -63,9 +70,10 @@ fn executor_steps_proposer() { desc: "Receive our own proposal, prevote for it (v1)", input_event: None, expected_output: Some(Message::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&my_sk), + 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, @@ -79,6 +87,7 @@ 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, @@ -90,10 +99,11 @@ fn executor_steps_proposer() { TestStep { desc: "v2 prevotes for our proposal", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&sk2), + Vote::new_prevote(Round::new(0), Some(value_id), addr2).signed(&sk2), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Prevote, @@ -105,12 +115,13 @@ fn executor_steps_proposer() { TestStep { desc: "v3 prevotes for our proposal, we get +2/3 prevotes, precommit for it (v1)", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&sk3), + Vote::new_prevote(Round::new(0), Some(value_id), addr3).signed(&sk3), )), expected_output: Some(Message::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&my_sk), + 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, @@ -130,6 +141,7 @@ 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, @@ -147,10 +159,11 @@ fn executor_steps_proposer() { TestStep { desc: "v2 precommits for our proposal", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&sk2), + Vote::new_precommit(Round::new(0), Some(value_id), addr2).signed(&sk2), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Precommit, @@ -168,10 +181,11 @@ fn executor_steps_proposer() { TestStep { desc: "v3 precommits for our proposal, we get +2/3 precommits, decide it (v1)", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&sk2), + Vote::new_precommit(Round::new(0), Some(value_id), addr2).signed(&sk2), )), 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, @@ -218,15 +232,19 @@ fn executor_steps_not_proposer() { let sk2 = PrivateKey::generate(&mut rng); let sk3 = PrivateKey::generate(&mut rng); + let addr1 = Address::from_public_key(&sk1.public_key()); + let addr2 = Address::from_public_key(&sk2.public_key()); + let addr3 = Address::from_public_key(&sk3.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); // Proposer is v1, so we are not the proposer - let my_sk = &sk2; + let (my_sk, my_addr) = (sk2, addr2); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut executor = Executor::new(Height::new(1), vs, my_sk.clone()); + let mut executor = Executor::new(Height::new(1), vs, my_sk.clone(), my_addr); let proposal = Proposal::new(Height::new(1), Round::new(0), value.clone(), Round::new(-1)); @@ -236,6 +254,7 @@ 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, @@ -248,9 +267,10 @@ fn executor_steps_not_proposer() { desc: "Receive a proposal, prevote for it (v2)", input_event: Some(Event::Proposal(proposal.clone())), expected_output: Some(Message::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(my_sk), + 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, @@ -260,10 +280,11 @@ fn executor_steps_not_proposer() { }, }, TestStep { - desc: "Receive our own prevote", + desc: "Receive our own prevote (v2)", input_event: None, expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Prevote, @@ -273,12 +294,13 @@ fn executor_steps_not_proposer() { }, }, TestStep { - desc: "v2 prevotes for its own proposal", + desc: "v1 prevotes for its own proposal", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&sk2), + Vote::new_prevote(Round::new(0), Some(value_id), addr1).signed(&sk1), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Prevote, @@ -288,14 +310,15 @@ fn executor_steps_not_proposer() { }, }, TestStep { - desc: "v3 prevotes for v2's proposal, it gets +2/3 prevotes, precommit for it (v2)", + desc: "v3 prevotes for v1's proposal, it gets +2/3 prevotes, precommit for it (v2)", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&sk3), + Vote::new_prevote(Round::new(0), Some(value_id), addr3).signed(&sk3), )), expected_output: Some(Message::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(my_sk), + 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, @@ -315,6 +338,7 @@ 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, @@ -332,10 +356,11 @@ fn executor_steps_not_proposer() { TestStep { desc: "v1 precommits its proposal", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&sk1), + Vote::new_precommit(Round::new(0), Some(value_id), addr1).signed(&sk1), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Precommit, @@ -353,10 +378,11 @@ fn executor_steps_not_proposer() { TestStep { desc: "v3 precommits for v1's proposal, it gets +2/3 precommits, decide it", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&sk3), + Vote::new_precommit(Round::new(0), Some(value_id), addr3).signed(&sk3), )), 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, @@ -403,15 +429,19 @@ fn executor_steps_not_proposer_timeout() { let sk2 = PrivateKey::generate(&mut rng); let sk3 = PrivateKey::generate(&mut rng); + let addr1 = Address::from_public_key(&sk1.public_key()); + let addr2 = Address::from_public_key(&sk2.public_key()); + let addr3 = Address::from_public_key(&sk3.public_key()); + let v1 = Validator::new(sk1.public_key(), 1); let v2 = Validator::new(sk2.public_key(), 1); let v3 = Validator::new(sk3.public_key(), 3); // Proposer is v1, so we are not the proposer - let my_sk = sk2; + let (my_sk, my_addr) = (sk2, addr2); let vs = ValidatorSet::new(vec![v1.clone(), v2.clone(), v3.clone()]); - let mut executor = Executor::new(Height::new(1), vs, my_sk.clone()); + let mut executor = Executor::new(Height::new(1), vs, my_sk.clone(), my_addr); let steps = vec![ // Start round 0, we are not the proposer @@ -420,6 +450,7 @@ 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, @@ -433,9 +464,10 @@ fn executor_steps_not_proposer_timeout() { desc: "Receive a propose timeout, prevote for nil (v1)", input_event: Some(Event::Timeout(Timeout::propose(Round::new(0)))), expected_output: Some(Message::Vote( - Vote::new_prevote(Round::new(0), None).signed(&my_sk), + 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, @@ -450,6 +482,7 @@ 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, @@ -462,10 +495,11 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "v2 prevotes for its own proposal", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), Some(value_id)).signed(&sk1), + Vote::new_prevote(Round::new(0), Some(value_id), addr1).signed(&sk1), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Prevote, @@ -478,12 +512,13 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "v3 prevotes for nil, it gets +2/3 prevotes, precommit for it (v1)", input_event: Some(Event::Vote( - Vote::new_prevote(Round::new(0), None).signed(&sk3), + Vote::new_prevote(Round::new(0), None, addr3).signed(&sk3), )), expected_output: Some(Message::Vote( - Vote::new_precommit(Round::new(0), None).signed(&my_sk), + 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, @@ -498,6 +533,7 @@ 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, @@ -510,10 +546,11 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "v2 precommits its proposal", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), Some(value_id)).signed(&sk1), + Vote::new_precommit(Round::new(0), Some(value_id), addr1).signed(&sk1), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Precommit, @@ -526,10 +563,11 @@ fn executor_steps_not_proposer_timeout() { TestStep { desc: "v3 precommits for nil", input_event: Some(Event::Vote( - Vote::new_precommit(Round::new(0), None).signed(&sk3), + Vote::new_precommit(Round::new(0), None, addr3).signed(&sk3), )), expected_output: None, new_state: State { + address: my_addr, height: Height::new(1), round: Round::new(0), step: Step::Precommit, @@ -544,6 +582,7 @@ 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, diff --git a/Code/test/tests/round.rs b/Code/test/tests/round.rs index aceb30861..52be337b7 100644 --- a/Code/test/tests/round.rs +++ b/Code/test/tests/round.rs @@ -1,4 +1,4 @@ -use malachite_test::{Height, Proposal, TestConsensus, Value}; +use malachite_test::{Address, Height, Proposal, TestConsensus, Value}; use malachite_common::{Round, Timeout, TimeoutStep}; use malachite_round::events::Event; @@ -6,10 +6,12 @@ use malachite_round::message::Message; use malachite_round::state::{State, Step}; use malachite_round::state_machine::apply_event; +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)); + let mut state: State = State::new(Height::new(10), ADDRESS); let transition = apply_event(state.clone(), Round::new(0), Event::NewRoundProposer(value)); @@ -25,7 +27,7 @@ fn test_propose() { #[test] fn test_prevote() { let value = Value::new(42); - let state: State = State::new(Height::new(1)).new_round(Round::new(1)); + let state: State = State::new(Height::new(1), ADDRESS).new_round(Round::new(1)); let transition = apply_event(state, Round::new(1), Event::NewRound); @@ -54,6 +56,6 @@ fn test_prevote() { assert_eq!(transition.next_state.step, Step::Prevote); assert_eq!( transition.message.unwrap(), - Message::prevote(Round::new(1), Some(value.id()),) + Message::prevote(Round::new(1), Some(value.id()), ADDRESS) ); } diff --git a/Code/test/tests/round_votes.rs b/Code/test/tests/round_votes.rs index fcd164fcd..b3bd4bf72 100644 --- a/Code/test/tests/round_votes.rs +++ b/Code/test/tests/round_votes.rs @@ -2,7 +2,9 @@ use malachite_common::Round; use malachite_vote::count::Threshold; use malachite_vote::RoundVotes; -use malachite_test::{Height, TestConsensus, ValueId, Vote}; +use malachite_test::{Address, Height, TestConsensus, ValueId, Vote}; + +const ADDRESS: Address = Address::new([42; 20]); #[test] fn add_votes_nil() { @@ -12,7 +14,7 @@ fn add_votes_nil() { RoundVotes::new(Height::new(1), Round::new(0), total); // add a vote for nil. nothing changes. - let vote = Vote::new_prevote(Round::new(0), None); + let vote = Vote::new_prevote(Round::new(0), None, ADDRESS); let thresh = round_votes.add_vote(vote.clone(), 1); assert_eq!(thresh, Threshold::Init); @@ -36,7 +38,7 @@ fn add_votes_single_value() { RoundVotes::new(Height::new(1), Round::new(0), total); // add a vote. nothing changes. - let vote = Vote::new_prevote(Round::new(0), val); + let vote = Vote::new_prevote(Round::new(0), val, ADDRESS); let thresh = round_votes.add_vote(vote.clone(), weight); assert_eq!(thresh, Threshold::Init); @@ -45,7 +47,7 @@ fn add_votes_single_value() { assert_eq!(thresh, Threshold::Init); // add a vote for nil, get Thresh::Any - let vote_nil = Vote::new_prevote(Round::new(0), None); + let vote_nil = Vote::new_prevote(Round::new(0), None, ADDRESS); let thresh = round_votes.add_vote(vote_nil, weight); assert_eq!(thresh, Threshold::Any); @@ -66,17 +68,17 @@ fn add_votes_multi_values() { RoundVotes::new(Height::new(1), Round::new(0), total); // add a vote for v1. nothing changes. - let vote1 = Vote::new_precommit(Round::new(0), val1); + let vote1 = Vote::new_precommit(Round::new(0), val1, ADDRESS); let thresh = round_votes.add_vote(vote1.clone(), 1); assert_eq!(thresh, Threshold::Init); // add a vote for v2. nothing changes. - let vote2 = Vote::new_precommit(Round::new(0), val2); + let vote2 = Vote::new_precommit(Round::new(0), val2, ADDRESS); let thresh = round_votes.add_vote(vote2.clone(), 1); assert_eq!(thresh, Threshold::Init); // add a vote for nil. nothing changes. - let vote_nil = Vote::new_precommit(Round::new(0), None); + let vote_nil = Vote::new_precommit(Round::new(0), None, ADDRESS); let thresh = round_votes.add_vote(vote_nil.clone(), 1); assert_eq!(thresh, Threshold::Init); From 26a55730467f4a9f3e654f571424ce40297c6a8e Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 31 Oct 2023 11:41:50 +0100 Subject: [PATCH 4/5] Move `Transition` struct into its own module --- Code/round/src/lib.rs | 1 + Code/round/src/state.rs | 2 +- Code/round/src/state_machine.rs | 37 +----------------------------- Code/round/src/transition.rs | 40 +++++++++++++++++++++++++++++++++ 4 files changed, 43 insertions(+), 37 deletions(-) create mode 100644 Code/round/src/transition.rs diff --git a/Code/round/src/lib.rs b/Code/round/src/lib.rs index 79dddd8ae..34ae70a59 100644 --- a/Code/round/src/lib.rs +++ b/Code/round/src/lib.rs @@ -14,3 +14,4 @@ pub mod events; pub mod message; pub mod state; pub mod state_machine; +pub mod transition; diff --git a/Code/round/src/state.rs b/Code/round/src/state.rs index 3c42e6d92..3b460f523 100644 --- a/Code/round/src/state.rs +++ b/Code/round/src/state.rs @@ -1,5 +1,5 @@ use crate::events::Event; -use crate::state_machine::Transition; +use crate::transition::Transition; use malachite_common::{Context, Round}; diff --git a/Code/round/src/state_machine.rs b/Code/round/src/state_machine.rs index c2ce3803e..2dead4a08 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -3,42 +3,7 @@ use malachite_common::{Context, Proposal, Round, TimeoutStep, Value, ValueId}; use crate::events::Event; use crate::message::Message; use crate::state::{State, Step}; - -#[derive(Clone, Debug, PartialEq, Eq)] -pub struct Transition -where - Ctx: Context, -{ - pub next_state: State, - pub message: Option>, - pub valid: bool, -} - -impl Transition -where - Ctx: Context, -{ - pub fn to(next_state: State) -> Self { - Self { - next_state, - message: None, - valid: true, - } - } - - pub fn invalid(next_state: State) -> Self { - Self { - next_state, - message: None, - valid: false, - } - } - - pub fn with_message(mut self, message: Message) -> Self { - self.message = Some(message); - self - } -} +use crate::transition::Transition; /// Check that a proposal has a valid Proof-Of-Lock round fn is_valid_pol_round(state: &State, pol_round: Round) -> bool diff --git a/Code/round/src/transition.rs b/Code/round/src/transition.rs new file mode 100644 index 000000000..33bed24fb --- /dev/null +++ b/Code/round/src/transition.rs @@ -0,0 +1,40 @@ +use malachite_common::Context; + +use crate::message::Message; +use crate::state::State; + +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Transition +where + Ctx: Context, +{ + pub next_state: State, + pub message: Option>, + pub valid: bool, +} + +impl Transition +where + Ctx: Context, +{ + pub fn to(next_state: State) -> Self { + Self { + next_state, + message: None, + valid: true, + } + } + + pub fn invalid(next_state: State) -> Self { + Self { + next_state, + message: None, + valid: false, + } + } + + pub fn with_message(mut self, message: Message) -> Self { + self.message = Some(message); + self + } +} From cb93ff746680030e82a1224b7556f2b96e6d181c Mon Sep 17 00:00:00 2001 From: Romain Ruetschi Date: Tue, 31 Oct 2023 13:46:20 +0100 Subject: [PATCH 5/5] Extract immutable data required to run the round state machine into a `RoundData` struct (#37) --- Code/consensus/src/executor.rs | 19 +++-- Code/round/src/state.rs | 22 +++--- Code/round/src/state_machine.rs | 99 +++++++++++++++++++-------- Code/test/tests/consensus_executor.rs | 50 -------------- Code/test/tests/round.rs | 18 +++-- 5 files changed, 106 insertions(+), 102 deletions(-) diff --git a/Code/consensus/src/executor.rs b/Code/consensus/src/executor.rs index e446b918e..c9d56ad9d 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::RoundData; 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 data = RoundData::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(&data, 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..743ea684c 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::RoundData; 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, data: &RoundData, event: Event) -> Transition { + crate::state_machine::apply_event(self, data, 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..cb7a08fe7 100644 --- a/Code/round/src/state_machine.rs +++ b/Code/round/src/state_machine.rs @@ -5,6 +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> +where + Ctx: Context, +{ + pub round: Round, + pub height: &'a Ctx::Height, + pub address: &'a Ctx::Address, +} + +impl<'a, Ctx> RoundData<'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 +49,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, + data: &RoundData, + event: Event, +) -> Transition where Ctx: Context, { - let this_round = state.round == round; + let this_round = state.round == data.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, data.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 +78,9 @@ where .map_or(true, |locked| &locked.value == proposal.value()) { state.proposal = Some(proposal.clone()); - prevote(state, proposal.round(), proposal.value().id()) + prevote(state, data.address, proposal.round(), proposal.value().id()) } else { - prevote_nil(state) + prevote_nil(state, data.address) } } @@ -62,33 +96,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, data.address, proposal.round(), proposal.value().id()) } else { - prevote_nil(state) + prevote_nil(state, data.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, data.address), // L22/L25, L28/L31 + (Step::Propose, Event::TimeoutPropose) if this_round => prevote_nil(state, data.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, data.address), // L44 + (Step::Prevote, Event::PolkaValue(value_id)) if this_round => { + precommit(state, data.address, value_id) // L36/L37 - NOTE: only once? + } + (Step::Prevote, Event::TimeoutPrevote) if this_round => precommit_nil(state, data.address), // 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, data.round.increment()), // L65 + (_, Event::RoundSkip) if state.round < data.round => round_skip(state, data.round), // L55 + (_, Event::PrecommitValue(value_id)) => commit(state, data.round, value_id), // L49 // Invalid transition. _ => Transition::invalid(state), @@ -103,7 +139,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 +148,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 +160,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 +176,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 +201,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 +228,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, address: &Ctx::Address) -> Transition where Ctx: Context, { - let message = Message::precommit(state.round, None, state.address.clone()); + let message = Message::precommit(state.round, None, 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..60d738d94 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, RoundData}; 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 data = RoundData::new(Round::new(0), &height, &ADDRESS); + + let transition = apply_event(state.clone(), &data, 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 data = RoundData::new(Round::new(1), &height, &ADDRESS); - let transition = apply_event(state, Round::new(1), Event::NewRound); + let transition = apply_event(state, &data, 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), + &data, Event::Proposal(Proposal::new( Height::new(1), Round::new(1),