Skip to content

Commit

Permalink
refactor(consensus): make BlockHash optional
Browse files Browse the repository at this point in the history
  • Loading branch information
asmaastarkware committed Jul 25, 2024
1 parent db74524 commit e213d84
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 47 deletions.
2 changes: 1 addition & 1 deletion crates/papyrus_protobuf/src/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub enum VoteType {
pub struct Vote {
pub vote_type: VoteType,
pub height: u64,
pub block_hash: BlockHash,
pub block_hash: Option<BlockHash>,
pub voter: ContractAddress,
}

Expand Down
4 changes: 2 additions & 2 deletions crates/papyrus_protobuf/src/converters/consensus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl TryFrom<protobuf::Vote> for Vote {
.block_hash
.ok_or(ProtobufConversionError::MissingField { field_description: "block_hash" })?
.try_into()?;
let block_hash = BlockHash(block_hash);
let block_hash = Some(BlockHash(block_hash));
let voter = value
.voter
.ok_or(ProtobufConversionError::MissingField { field_description: "voter" })?
Expand All @@ -98,7 +98,7 @@ impl From<Vote> for protobuf::Vote {
protobuf::Vote {
vote_type: vote_type as i32,
height: value.height,
block_hash: Some(value.block_hash.0.into()),
block_hash: value.block_hash.map(|hash| hash.0.into()),
voter: Some(value.voter.into()),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ impl<BlockT: ConsensusBlock> SingleHeightConsensus<BlockT> {
"block signature doesn't match expected block hash".into(),
));
}
let sm_proposal = StateMachineEvent::Proposal(block.id(), ROUND_ZERO);
let sm_proposal = StateMachineEvent::Proposal(Some(block.id()), ROUND_ZERO);
// TODO(matan): Handle multiple rounds.
self.proposals.insert(ROUND_ZERO, block);
let sm_events = self.state_machine.handle_event(sm_proposal);
Expand Down Expand Up @@ -250,7 +250,7 @@ impl<BlockT: ConsensusBlock> SingleHeightConsensus<BlockT> {
async fn handle_state_machine_vote<ContextT: ConsensusContext<Block = BlockT>>(
&mut self,
context: &mut ContextT,
block_hash: BlockHash,
block_hash: Option<BlockHash>,
round: Round,
vote_type: VoteType,
) -> Result<Option<Decision<BlockT>>, ConsensusError> {
Expand Down Expand Up @@ -281,7 +281,7 @@ impl<BlockT: ConsensusBlock> SingleHeightConsensus<BlockT> {
.iter()
.filter_map(|v| {
let vote = self.precommits.get(&(round, *v))?;
if vote.block_hash != block_hash {
if vote.block_hash != Some(block_hash) {
return None;
}
Some(vote.clone())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@ use super::SingleHeightConsensus;
use crate::test_utils::{MockTestContext, TestBlock};
use crate::types::{ConsensusBlock, ProposalInit, ValidatorId};

fn prevote(block_hash: BlockHash, height: u64, voter: ValidatorId) -> ConsensusMessage {
fn prevote(block_hash: Option<BlockHash>, height: u64, voter: ValidatorId) -> ConsensusMessage {
ConsensusMessage::Vote(Vote { vote_type: VoteType::Prevote, height, block_hash, voter })
}

fn precommit(block_hash: BlockHash, height: u64, voter: ValidatorId) -> ConsensusMessage {
fn precommit(block_hash: Option<BlockHash>, height: u64, voter: ValidatorId) -> ConsensusMessage {
ConsensusMessage::Vote(Vote { vote_type: VoteType::Precommit, height, block_hash, voter })
}

Expand Down Expand Up @@ -51,30 +51,30 @@ async fn proposer() {
});
context
.expect_broadcast()
.withf(move |msg: &ConsensusMessage| msg == &prevote(block_id, 0, node_id))
.withf(move |msg: &ConsensusMessage| msg == &prevote(Some(block_id), 0, node_id))
.returning(move |_| Ok(()));
// Sends proposal and prevote.
assert!(matches!(shc.start(&mut context).await, Ok(None)));

assert_eq!(
shc.handle_message(&mut context, prevote(block.id(), 0, 2_u32.into())).await,
shc.handle_message(&mut context, prevote(Some(block.id()), 0, 2_u32.into())).await,
Ok(None)
);
// 3 of 4 Prevotes is enough to send a Precommit.
context
.expect_broadcast()
.withf(move |msg: &ConsensusMessage| msg == &precommit(block_id, 0, node_id))
.withf(move |msg: &ConsensusMessage| msg == &precommit(Some(block_id), 0, node_id))
.returning(move |_| Ok(()));
assert_eq!(
shc.handle_message(&mut context, prevote(block.id(), 0, 3_u32.into())).await,
shc.handle_message(&mut context, prevote(Some(block.id()), 0, 3_u32.into())).await,
Ok(None)
);

let precommits = vec![
precommit(block.id(), 0, 1_u32.into()),
precommit(BlockHash(Felt::TWO), 0, 4_u32.into()), // Ignores since disagrees.
precommit(block.id(), 0, 2_u32.into()),
precommit(block.id(), 0, 3_u32.into()),
precommit(Some(block.id()), 0, 1_u32.into()),
precommit(Some(BlockHash(Felt::TWO)), 0, 4_u32.into()), // Ignores since disagrees.
precommit(Some(block.id()), 0, 2_u32.into()),
precommit(Some(block.id()), 0, 3_u32.into()),
];
assert_eq!(shc.handle_message(&mut context, precommits[1].clone()).await, Ok(None));
assert_eq!(shc.handle_message(&mut context, precommits[2].clone()).await, Ok(None));
Expand Down Expand Up @@ -121,7 +121,7 @@ async fn validator() {
});
context
.expect_broadcast()
.withf(move |msg: &ConsensusMessage| msg == &prevote(block_id, 0, node_id))
.withf(move |msg: &ConsensusMessage| msg == &prevote(Some(block_id), 0, node_id))
.returning(move |_| Ok(()));
let res = shc
.handle_proposal(
Expand All @@ -134,23 +134,23 @@ async fn validator() {
assert_eq!(res, Ok(None));

assert_eq!(
shc.handle_message(&mut context, prevote(block.id(), 0, 2_u32.into())).await,
shc.handle_message(&mut context, prevote(Some(block.id()), 0, 2_u32.into())).await,
Ok(None)
);
// 3 of 4 Prevotes is enough to send a Precommit.
context
.expect_broadcast()
.withf(move |msg: &ConsensusMessage| msg == &precommit(block_id, 0, node_id))
.withf(move |msg: &ConsensusMessage| msg == &precommit(Some(block_id), 0, node_id))
.returning(move |_| Ok(()));
assert_eq!(
shc.handle_message(&mut context, prevote(block.id(), 0, 3_u32.into())).await,
shc.handle_message(&mut context, prevote(Some(block.id()), 0, 3_u32.into())).await,
Ok(None)
);

let precommits = vec![
precommit(block.id(), 0, 2_u32.into()),
precommit(block.id(), 0, 3_u32.into()),
precommit(block.id(), 0, node_id),
precommit(Some(block.id()), 0, 2_u32.into()),
precommit(Some(block.id()), 0, 3_u32.into()),
precommit(Some(block.id()), 0, node_id),
];
assert_eq!(shc.handle_message(&mut context, precommits[0].clone()).await, Ok(None));
let decision = shc.handle_message(&mut context, precommits[1].clone()).await.unwrap().unwrap();
Expand Down
51 changes: 32 additions & 19 deletions crates/sequencing/papyrus_consensus/src/state_machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,11 @@ pub enum StateMachineEvent {
/// machine, and the same round sent out.
GetProposal(Option<BlockHash>, Round),
/// Consensus message, can be both sent from and to the state machine.
Proposal(BlockHash, Round),
Proposal(Option<BlockHash>, Round),
/// Consensus message, can be both sent from and to the state machine.
Prevote(BlockHash, Round),
Prevote(Option<BlockHash>, Round),
/// Consensus message, can be both sent from and to the state machine.
Precommit(BlockHash, Round),
Precommit(Option<BlockHash>, Round),
/// The state machine returns this event to the caller when a decision is reached. Not
/// expected as an inbound message. We presume that the caller is able to recover the set of
/// precommits which led to this decision from the information returned here.
Expand All @@ -44,17 +44,16 @@ pub enum Step {
/// State Machine. Major assumptions:
/// 1. SHC handles replays and conflicts.
/// 2. SM must handle "out of order" messages (E.g. vote arrives before proposal).
/// 3. Only valid proposals (e.g. no NIL)
/// 4. No network failures - together with 3 this means we only support round 0.
/// 3. No network failures.
pub struct StateMachine {
id: ValidatorId,
round: Round,
step: Step,
quorum: u32,
proposals: HashMap<Round, BlockHash>,
proposals: HashMap<Round, Option<BlockHash>>,
// {round: {block_hash: vote_count}
prevotes: HashMap<Round, HashMap<BlockHash, u32>>,
precommits: HashMap<Round, HashMap<BlockHash, u32>>,
prevotes: HashMap<Round, HashMap<Option<BlockHash>, u32>>,
precommits: HashMap<Round, HashMap<Option<BlockHash>, u32>>,
// When true, the state machine will wait for a GetProposal event, buffering all other input
// events in `events_queue`.
awaiting_get_proposal: bool,
Expand Down Expand Up @@ -184,16 +183,14 @@ impl StateMachine {
assert!(self.awaiting_get_proposal);
assert_eq!(round, self.round);
self.awaiting_get_proposal = false;
let Some(hash) = block_hash else {
panic!("SHC should pass a valid block hash");
};
VecDeque::from([StateMachineEvent::Proposal(hash, round)])
assert!(block_hash.is_some(), "SHC should pass a valid block hash");
VecDeque::from([StateMachineEvent::Proposal(block_hash, round)])
}

// A proposal from a peer (or self) node.
fn handle_proposal(
&mut self,
block_hash: BlockHash,
block_hash: Option<BlockHash>,
round: u32,
) -> VecDeque<StateMachineEvent> {
let old = self.proposals.insert(round, block_hash);
Expand All @@ -208,7 +205,11 @@ impl StateMachine {
}

// A prevote from a peer (or self) node.
fn handle_prevote(&mut self, block_hash: BlockHash, round: u32) -> VecDeque<StateMachineEvent> {
fn handle_prevote(
&mut self,
block_hash: Option<BlockHash>,
round: u32,
) -> VecDeque<StateMachineEvent> {
let prevote_count = self.prevotes.entry(round).or_default().entry(block_hash).or_insert(0);
// TODO(matan): Use variable weight.
*prevote_count += 1;
Expand All @@ -222,7 +223,7 @@ impl StateMachine {
// A precommit from a peer (or self) node.
fn handle_precommit(
&mut self,
block_hash: BlockHash,
block_hash: Option<BlockHash>,
round: u32,
) -> VecDeque<StateMachineEvent> {
let precommit_count =
Expand Down Expand Up @@ -277,20 +278,32 @@ impl StateMachine {
if *count < self.quorum {
return VecDeque::new();
}
VecDeque::from([StateMachineEvent::Decision(*block_hash, round)])
if let Some(block_hash) = block_hash {
VecDeque::from([StateMachineEvent::Decision(*block_hash, round)])
} else {
self.advance_to_round(round + 1)
}
}

fn send_precommit(&mut self, block_hash: BlockHash, round: u32) -> VecDeque<StateMachineEvent> {
fn send_precommit(
&mut self,
block_hash: Option<BlockHash>,
round: u32,
) -> VecDeque<StateMachineEvent> {
let mut output = VecDeque::from([StateMachineEvent::Precommit(block_hash, round)]);
output.append(&mut self.advance_to_step(Step::Precommit));
output
}

fn advance_to_round(&mut self, _round: Round) -> VecDeque<StateMachineEvent> {
todo!()
}
}

fn leading_vote(
votes: &HashMap<u32, HashMap<BlockHash, u32>>,
votes: &HashMap<u32, HashMap<Option<BlockHash>, u32>>,
round: u32,
) -> Option<(&BlockHash, &u32)> {
) -> Option<(&Option<BlockHash>, &u32)> {
// We don't care which value is chosen in the case of a tie, since consensus requires 2/3+1.
votes.get(&round)?.iter().max_by(|a, b| a.1.cmp(b.1))
}
15 changes: 10 additions & 5 deletions crates/sequencing/papyrus_consensus/src/state_machine_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ lazy_static! {
static ref PROPOSER_ID: ValidatorId = 0_u32.into();
}

const BLOCK_HASH: BlockHash = BlockHash(Felt::ONE);
const BLOCK_HASH: Option<BlockHash> = Some(BlockHash(Felt::ONE));
const ROUND: Round = 0;

#[test_case(true; "proposer")]
Expand All @@ -24,8 +24,7 @@ fn events_arrive_in_ideal_order(is_proposer: bool) {
let mut events = state_machine.start(&leader_fn);
if is_proposer {
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::GetProposal(None, ROUND));
events =
state_machine.handle_event(StateMachineEvent::GetProposal(Some(BLOCK_HASH), ROUND));
events = state_machine.handle_event(StateMachineEvent::GetProposal(BLOCK_HASH, ROUND));
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::Proposal(BLOCK_HASH, ROUND));
} else {
assert!(events.is_empty(), "{:?}", events);
Expand All @@ -45,7 +44,10 @@ fn events_arrive_in_ideal_order(is_proposer: bool) {
assert!(events.is_empty(), "{:?}", events);

events = state_machine.handle_event(StateMachineEvent::Precommit(BLOCK_HASH, ROUND));
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::Decision(BLOCK_HASH, ROUND));
assert_eq!(
events.pop_front().unwrap(),
StateMachineEvent::Decision(BLOCK_HASH.unwrap(), ROUND)
);
assert!(events.is_empty(), "{:?}", events);
}

Expand All @@ -70,7 +72,10 @@ fn validator_receives_votes_first() {
events = state_machine.handle_event(StateMachineEvent::Proposal(BLOCK_HASH, ROUND));
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::Prevote(BLOCK_HASH, ROUND));
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::Precommit(BLOCK_HASH, ROUND));
assert_eq!(events.pop_front().unwrap(), StateMachineEvent::Decision(BLOCK_HASH, ROUND));
assert_eq!(
events.pop_front().unwrap(),
StateMachineEvent::Decision(BLOCK_HASH.unwrap(), ROUND)
);
assert!(events.is_empty(), "{:?}", events);
}
// TODO(Asmaa): Add this test when we support NIL votes.
Expand Down

0 comments on commit e213d84

Please sign in to comment.