Skip to content

Commit

Permalink
moved protocol_version to ConsensusMessage
Browse files Browse the repository at this point in the history
  • Loading branch information
pompon0 committed Oct 26, 2023
1 parent a6daec2 commit 41a4e1a
Show file tree
Hide file tree
Showing 16 changed files with 82 additions and 62 deletions.
5 changes: 4 additions & 1 deletion node/actors/consensus/src/leader/replica_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,10 @@ impl StateMachine {
message: consensus
.secret_key
.sign_msg(validator::ConsensusMsg::LeaderCommit(
validator::LeaderCommit { justification },
validator::LeaderCommit {
protocol_version: validator::CURRENT_VERSION,
justification,
},
)),
recipient: Target::Broadcast,
};
Expand Down
1 change: 1 addition & 0 deletions node/actors/consensus/src/leader/replica_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ impl StateMachine {
.secret_key
.sign_msg(validator::ConsensusMsg::LeaderPrepare(
validator::LeaderPrepare {
protocol_version: validator::CURRENT_VERSION,
view: self.view,
proposal,
proposal_payload: payload,
Expand Down
1 change: 1 addition & 0 deletions node/actors/consensus/src/leader/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ async fn replica_commit() {
.secret_key
.sign_msg(validator::ConsensusMsg::ReplicaCommit(
validator::ReplicaCommit {
protocol_version: validator::CURRENT_VERSION,
view: consensus.leader.view,
proposal: rng.gen(),
},
Expand Down
8 changes: 8 additions & 0 deletions node/actors/consensus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,14 @@ impl Consensus {

match input {
Some(InputMessage::Network(req)) => {
if req.msg.msg.protocol_version() != validator::CURRENT_VERSION {
tracing::warn!(
"bad protocol version (expected: {:?}, received: {:?})",
validator::CURRENT_VERSION,
req.msg.msg.protocol_version()
);
continue;
}
match &req.msg.msg {
validator::ConsensusMsg::ReplicaPrepare(_)
| validator::ConsensusMsg::ReplicaCommit(_) => {
Expand Down
13 changes: 0 additions & 13 deletions node/actors/consensus/src/replica/leader_commit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,6 @@ use tracing::instrument;
#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
pub(crate) enum Error {
#[error("bad protocol version (expected: {expected:?}, received: {received:?})")]
BadProtocolVersion {
expected: validator::ProtocolVersion,
received: validator::ProtocolVersion,
},
#[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?})]")]
InvalidLeader {
correct_leader: validator::PublicKey,
Expand Down Expand Up @@ -48,14 +43,6 @@ impl StateMachine {
let author = &signed_message.key;
let view = message.justification.message.view;

// Check protocol version.
if message.justification.message.proposal.protocol_version != validator::CURRENT_VERSION {
return Err(Error::BadProtocolVersion {
expected: validator::CURRENT_VERSION,
received: message.justification.message.proposal.protocol_version,
});
}

// Check that it comes from the correct leader.
if author != &consensus.view_leader(view) {
return Err(Error::InvalidLeader {
Expand Down
14 changes: 1 addition & 13 deletions node/actors/consensus/src/replica/leader_prepare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@ use tracing::instrument;
#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
pub(crate) enum Error {
#[error("bad protocol version (expected: {expected:?}, received: {received:?})")]
BadProtocolVersion {
expected: validator::ProtocolVersion,
received: validator::ProtocolVersion,
},
#[error("invalid leader (correct leader: {correct_leader:?}, received leader: {received_leader:?}])")]
InvalidLeader {
correct_leader: validator::PublicKey,
Expand Down Expand Up @@ -87,14 +82,6 @@ impl StateMachine {
let author = &signed_message.key;
let view = message.view;

// Check protocol version.
if message.proposal.protocol_version != validator::CURRENT_VERSION {
return Err(Error::BadProtocolVersion {
expected: validator::CURRENT_VERSION,
received: message.proposal.protocol_version,
});
}

// Check that it comes from the correct leader.
if author != &consensus.view_leader(view) {
return Err(Error::InvalidLeader {
Expand Down Expand Up @@ -227,6 +214,7 @@ impl StateMachine {

// Create our commit vote.
let commit_vote = validator::ReplicaCommit {
protocol_version: validator::CURRENT_VERSION,
view,
proposal: message.proposal,
};
Expand Down
1 change: 1 addition & 0 deletions node/actors/consensus/src/replica/new_view.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ impl StateMachine {
.secret_key
.sign_msg(validator::ConsensusMsg::ReplicaPrepare(
validator::ReplicaPrepare {
protocol_version: validator::CURRENT_VERSION,
view: next_view,
high_vote: self.high_vote,
high_qc: self.high_qc.clone(),
Expand Down
1 change: 1 addition & 0 deletions node/actors/consensus/src/replica/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ async fn start_new_view_not_leader() {
.secret_key
.sign_msg(validator::ConsensusMsg::ReplicaPrepare(
validator::ReplicaPrepare {
protocol_version: validator::CURRENT_VERSION,
view: consensus.replica.view,
high_vote: consensus.replica.high_vote,
high_qc: consensus.replica.high_qc.clone(),
Expand Down
20 changes: 13 additions & 7 deletions node/actors/consensus/src/testonly/fuzz.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,38 +35,45 @@ impl Fuzz for validator::ConsensusMsg {

impl Fuzz for validator::ReplicaPrepare {
fn mutate(&mut self, rng: &mut impl Rng) {
match rng.gen_range(0..2) {
match rng.gen_range(0..4) {
0 => self.view = rng.gen(),
1 => self.high_vote.mutate(rng),
2 => self.high_qc.mutate(rng),
3 => self.protocol_version = rng.gen(),
_ => unreachable!(),
}
}
}

impl Fuzz for validator::ReplicaCommit {
fn mutate(&mut self, rng: &mut impl Rng) {
match rng.gen_range(0..2) {
match rng.gen_range(0..3) {
0 => self.view = rng.gen(),
1 => self.proposal.mutate(rng),
2 => self.protocol_version = rng.gen(),
_ => unreachable!(),
}
}
}

impl Fuzz for validator::LeaderPrepare {
fn mutate(&mut self, rng: &mut impl Rng) {
match rng.gen_range(0..2) {
match rng.gen_range(0..3) {
0 => self.proposal.mutate(rng),
1 => self.justification.mutate(rng),
2 => self.protocol_version = rng.gen(),
_ => unreachable!(),
}
}
}

impl Fuzz for validator::LeaderCommit {
fn mutate(&mut self, rng: &mut impl Rng) {
self.justification.mutate(rng);
match rng.gen_range(0..2) {
0 => self.justification.mutate(rng),
1 => self.protocol_version = rng.gen(),
_ => unreachable!(),
}
}
}

Expand Down Expand Up @@ -157,11 +164,10 @@ impl Fuzz for validator::Payload {

impl Fuzz for validator::BlockHeader {
fn mutate(&mut self, rng: &mut impl Rng) {
match rng.gen_range(0..4) {
match rng.gen_range(0..3) {
0 => self.parent = rng.gen(),
1 => self.number = rng.gen(),
2 => self.protocol_version = rng.gen(),
3 => self.payload = rng.gen(),
2 => self.payload = rng.gen(),
_ => unreachable!(),
}
}
Expand Down
1 change: 1 addition & 0 deletions node/actors/consensus/src/testonly/make.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ pub fn make_genesis(
.iter()
.map(|sk| {
sk.sign_msg(validator::ReplicaCommit {
protocol_version: validator::CURRENT_VERSION,
view: validator::ViewNumber(0),
proposal: header,
})
Expand Down
1 change: 1 addition & 0 deletions node/actors/sync_blocks/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ impl TestValidators {

fn certify_block(&self, proposal: &BlockHeader) -> CommitQC {
let message_to_sign = validator::ReplicaCommit {
protocol_version: validator::CURRENT_VERSION,
view: validator::ViewNumber(proposal.number.0),
proposal: *proposal,
};
Expand Down
10 changes: 8 additions & 2 deletions node/libs/roles/src/validator/conv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,13 @@ impl ProtoFmt for BlockHeader {
type Proto = proto::BlockHeader;
fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self {
protocol_version: ProtocolVersion(r.protocol_version.context("protocol_version")?),
parent: read_required(&r.parent).context("parent")?,
number: BlockNumber(r.number.context("number")?),
payload: read_required(&r.payload).context("payload")?,
})
}
fn build(&self) -> Self::Proto {
Self::Proto {
protocol_version: Some(self.protocol_version.0),
parent: Some(self.parent.build()),
number: Some(self.number.0),
payload: Some(self.payload.build()),
Expand Down Expand Up @@ -109,6 +107,7 @@ impl ProtoFmt for ReplicaPrepare {

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self {
protocol_version: ProtocolVersion(r.protocol_version.context("protocol_version")?),
view: ViewNumber(*required(&r.view).context("view")?),
high_vote: read_required(&r.high_vote).context("high_vote")?,
high_qc: read_required(&r.high_qc).context("high_qc")?,
Expand All @@ -117,6 +116,7 @@ impl ProtoFmt for ReplicaPrepare {

fn build(&self) -> Self::Proto {
Self::Proto {
protocol_version: Some(self.protocol_version.0),
view: Some(self.view.0),
high_vote: Some(self.high_vote.build()),
high_qc: Some(self.high_qc.build()),
Expand All @@ -129,13 +129,15 @@ impl ProtoFmt for ReplicaCommit {

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self {
protocol_version: ProtocolVersion(r.protocol_version.context("protocol_version")?),
view: ViewNumber(*required(&r.view).context("view")?),
proposal: read_required(&r.proposal).context("proposal")?,
})
}

fn build(&self) -> Self::Proto {
Self::Proto {
protocol_version: Some(self.protocol_version.0),
view: Some(self.view.0),
proposal: Some(self.proposal.build()),
}
Expand All @@ -147,6 +149,7 @@ impl ProtoFmt for LeaderPrepare {

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self {
protocol_version: ProtocolVersion(r.protocol_version.context("protocol_version")?),
view: ViewNumber(*required(&r.view).context("view")?),
proposal: read_required(&r.proposal).context("proposal")?,
proposal_payload: r.proposal_payload.as_ref().map(|p| Payload(p.clone())),
Expand All @@ -156,6 +159,7 @@ impl ProtoFmt for LeaderPrepare {

fn build(&self) -> Self::Proto {
Self::Proto {
protocol_version: Some(self.protocol_version.0),
view: Some(self.view.0),
proposal: Some(self.proposal.build()),
proposal_payload: self.proposal_payload.as_ref().map(|p| p.0.clone()),
Expand All @@ -169,12 +173,14 @@ impl ProtoFmt for LeaderCommit {

fn read(r: &Self::Proto) -> anyhow::Result<Self> {
Ok(Self {
protocol_version: ProtocolVersion(r.protocol_version.context("protocol_version")?),
justification: read_required(&r.justification).context("justification")?,
})
}

fn build(&self) -> Self::Proto {
Self::Proto {
protocol_version: Some(self.protocol_version.0),
justification: Some(self.justification.build()),
}
}
Expand Down
20 changes: 0 additions & 20 deletions node/libs/roles/src/validator/messages/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,22 +4,6 @@ use super::CommitQC;
use crypto::{sha256, ByteFmt, Text, TextFmt};
use std::fmt;

/// Version of the consensus algorithm used by the leader in a given view.
/// Currently it is stored in the BlockHeader (TODO: consider whether it should be present in every
/// consensus message).
/// Currently it also determines the format and semantics of the block payload
/// (TODO: consider whether the payload should be versioned separately).
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct ProtocolVersion(pub(crate) u32);

/// We use a hardcoded protocol version for now.
/// Eventually validators should determine which version to use for which block by observing the relevant L1 contract.
///
/// The validator binary has to support the current and next protocol version whenever
/// a protocol version update is needed (so that it can dynamically switch from producing
/// blocks for version X to version X+1).
pub const CURRENT_VERSION: ProtocolVersion = ProtocolVersion(0);

/// Payload of the block. Consensus algorithm does not interpret the payload
/// (except for imposing a size limit for the payload). Proposing a payload
/// for a new block and interpreting the payload of the finalized blocks
Expand Down Expand Up @@ -102,8 +86,6 @@ impl fmt::Debug for BlockHeaderHash {
/// A block header.
#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct BlockHeader {
/// Protocol version according to which this block should be interpreted.
pub protocol_version: ProtocolVersion,
/// Hash of the parent block.
pub parent: BlockHeaderHash,
/// Number of the block.
Expand All @@ -121,7 +103,6 @@ impl BlockHeader {
/// Creates a genesis block.
pub fn genesis(payload: PayloadHash) -> Self {
Self {
protocol_version: CURRENT_VERSION,
parent: BlockHeaderHash(sha256::Sha256::default()),
number: BlockNumber(0),
payload,
Expand All @@ -131,7 +112,6 @@ impl BlockHeader {
/// Creates a child block for the given parent.
pub fn new(parent: &BlockHeader, payload: PayloadHash) -> Self {
Self {
protocol_version: CURRENT_VERSION,
parent: parent.hash(),
number: parent.number.next(),
payload,
Expand Down
Loading

0 comments on commit 41a4e1a

Please sign in to comment.