Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: implemented BlockHeader (BFT-357) #12

Merged
merged 23 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions .github/workflows/protobuf_conformance.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ jobs:
conformance:
runs-on: [ubuntu-22.04-github-hosted-16core]
steps:
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
path: "this"
- uses: actions/checkout@v3
- uses: actions/checkout@v4
with:
repository: "protocolbuffers/protobuf"
ref: "main"
ref: "v24.4"
path: "protobuf"
- uses: mozilla-actions/[email protected]
- name: build test
Expand Down
1 change: 1 addition & 0 deletions node/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions node/Cranky.toml
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ warn = [
allow = [
# Produces too many false positives.
"clippy::redundant_locals",
"clippy::needless_pass_by_ref_mut",
]
55 changes: 0 additions & 55 deletions node/actors/consensus/src/leader/error.rs

This file was deleted.

1 change: 0 additions & 1 deletion node/actors/consensus/src/leader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
//! and aggregates replica messages. It mainly acts as a central point of communication for the replicas. Note that
//! our consensus node will perform both the replica and leader roles simultaneously.

mod error;
mod replica_commit;
mod replica_prepare;
mod state_machine;
Expand Down
50 changes: 38 additions & 12 deletions node/actors/consensus/src/leader/replica_commit.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,35 @@
use super::StateMachine;
use crate::{inner::ConsensusInner, leader::error::Error, metrics};
use crate::{inner::ConsensusInner, metrics};
use concurrency::{ctx, metrics::LatencyHistogramExt as _};
use network::io::{ConsensusInputMessage, Target};
use roles::validator;
use tracing::instrument;

#[derive(thiserror::Error, Debug)]
#[allow(clippy::missing_docs_in_private_items)]
pub(crate) enum Error {
#[error("unexpected proposal")]
UnexpectedProposal,
#[error("past view/phase (current view: {current_view:?}, current phase: {current_phase:?})")]
Old {
current_view: validator::ViewNumber,
current_phase: validator::Phase,
},
#[error("we are not a leader for this message's view")]
NotLeaderInView,
#[error("duplicate message from a replica (existing message: {existing_message:?}")]
DuplicateMessage {
existing_message: validator::ReplicaCommit,
},
#[error("number of received messages is below threshold. waiting for more (received: {num_messages:?}, threshold: {threshold:?}")]
NumReceivedBelowThreshold {
num_messages: usize,
threshold: usize,
},
#[error("invalid signature: {0:#}")]
InvalidSignature(#[source] crypto::bls12_381::Error),
}

impl StateMachine {
#[instrument(level = "trace", ret)]
pub(crate) fn process_replica_commit(
Expand All @@ -21,15 +46,15 @@ impl StateMachine {

// If the message is from the "past", we discard it.
if (message.view, validator::Phase::Commit) < (self.view, self.phase) {
return Err(Error::ReplicaCommitOld {
return Err(Error::Old {
current_view: self.view,
current_phase: self.phase,
});
}

// If the message is for a view when we are not a leader, we discard it.
if consensus.view_leader(message.view) != consensus.secret_key.public() {
return Err(Error::ReplicaCommitWhenNotLeaderInView);
return Err(Error::NotLeaderInView);
}

// If we already have a message from the same validator and for the same view, we discard it.
Expand All @@ -38,24 +63,22 @@ impl StateMachine {
.get(&message.view)
.and_then(|x| x.get(author))
{
return Err(Error::ReplicaCommitExists {
existing_message: format!("{:?}", existing_message),
return Err(Error::DuplicateMessage {
existing_message: existing_message.msg,
});
}

// ----------- Checking the signed part of the message --------------

// Check the signature on the message.
signed_message
.verify()
.map_err(Error::ReplicaCommitInvalidSignature)?;
signed_message.verify().map_err(Error::InvalidSignature)?;

// ----------- Checking the contents of the message --------------

// We only accept replica commit messages for proposals that we have cached. That's so
// we don't need to store replica commit messages for different proposals.
if self.block_proposal_cache != Some(message) {
return Err(Error::ReplicaCommitMissingProposal);
if self.block_proposal_cache != Some(message.proposal) {
return Err(Error::UnexpectedProposal);
}

// ----------- All checks finished. Now we process the message. --------------
Expand All @@ -70,7 +93,7 @@ impl StateMachine {
let num_messages = self.commit_message_cache.get(&message.view).unwrap().len();

if num_messages < consensus.threshold() {
return Err(Error::ReplicaCommitNumReceivedBelowThreshold {
return Err(Error::NumReceivedBelowThreshold {
num_messages,
threshold: consensus.threshold(),
});
Expand Down Expand Up @@ -110,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
Loading
Loading