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(code): Vote extensions v2 #775

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
20 changes: 20 additions & 0 deletions code/crates/app-channel/src/connector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,26 @@ where
reply_to.send(rx.await?)?;
}

HostMsg::ExtendVote {
height,
round,
value_id,
reply_to,
} => {
let (reply, rx) = oneshot::channel();

self.sender
.send(AppMsg::ExtendVote {
height,
round,
value_id,
reply,
})
.await?;

reply_to.send(rx.await?)?;
}

HostMsg::RestreamValue {
height,
round,
Expand Down
10 changes: 9 additions & 1 deletion code/crates/app-channel/src/msgs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use tokio::sync::oneshot;
use malachitebft_engine::consensus::Msg as ConsensusActorMsg;
use malachitebft_engine::network::Msg as NetworkActorMsg;

use crate::app::types::core::{CommitCertificate, Context, Round, ValueId};
use crate::app::types::core::{CommitCertificate, Context, Extension, Round, ValueId};
use crate::app::types::streaming::StreamMessage;
use crate::app::types::sync::RawDecidedValue;
use crate::app::types::{LocallyProposedValue, PeerId, ProposedValue};
Expand Down Expand Up @@ -60,6 +60,14 @@ pub enum AppMsg<Ctx: Context> {
reply: Reply<LocallyProposedValue<Ctx>>,
},

// TODO
ExtendVote {
height: Ctx::Height,
round: Round,
value_id: ValueId<Ctx>,
reply: Reply<Option<Extension>>,
},

/// Requests the application to re-stream a proposal that it has already seen.
///
/// The application MUST re-publish again all the proposal parts pertaining
Expand Down
17 changes: 17 additions & 0 deletions code/crates/core-consensus/src/effect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,9 @@ where
/// Resume with: [`resume::Continue`]
GetValue(Ctx::Height, Round, Timeout, resume::Continue),

/// TODO
ExtendVote(Ctx::Height, Round, ValueId<Ctx>, resume::VoteExtension),

/// Requests the application to re-stream a proposal that it has already seen.
///
/// The application MUST re-publish again to its pwers all
Expand Down Expand Up @@ -209,6 +212,9 @@ where
/// Resume execution with the signed proposal
SignedProposal(SignedMessage<Ctx, Ctx::Proposal>),

/// TODO
VoteExtension(Option<SignedExtension<Ctx>>),

/// Resume execution with the result of the verification of the [`CommitCertificate`]
CertificateValidity(Result<(), CertificateError<Ctx>>),
}
Expand Down Expand Up @@ -271,6 +277,17 @@ pub mod resume {
}
}

#[derive(Debug, Default)]
pub struct VoteExtension;

impl<Ctx: Context> Resumable<Ctx> for VoteExtension {
type Value = Option<SignedExtension<Ctx>>;

fn resume_with(self, value: Self::Value) -> Resume<Ctx> {
Resume::VoteExtension(value)
}
}

#[derive(Debug, Default)]
pub struct CertificateValidity;

Expand Down
53 changes: 17 additions & 36 deletions code/crates/core-consensus/src/full_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ use std::collections::BTreeMap;
use derive_where::derive_where;
use tracing::debug;

use malachitebft_core_types::{
Context, Height, Proposal, Round, SignedExtension, SignedProposal, Validity, Value,
};
use malachitebft_core_types::{Context, Height, Proposal, Round, SignedProposal, Validity, Value};

use crate::ProposedValue;

Expand All @@ -18,22 +16,18 @@ pub struct FullProposal<Ctx: Context> {
pub validity: Validity,
/// Proposal consensus message
pub proposal: SignedProposal<Ctx>,
/// Extension
pub extension: Option<SignedExtension<Ctx>>,
}

impl<Ctx: Context> FullProposal<Ctx> {
pub fn new(
builder_value: Ctx::Value,
validity: Validity,
proposal: SignedProposal<Ctx>,
extension: Option<SignedExtension<Ctx>>,
) -> Self {
Self {
builder_value,
validity,
proposal,
extension,
}
}
}
Expand All @@ -48,7 +42,7 @@ enum Entry<Ctx: Context> {
ProposalOnly(SignedProposal<Ctx>),

/// Only the value has been received.
ValueOnly(Ctx::Value, Validity, Option<SignedExtension<Ctx>>),
ValueOnly(Ctx::Value, Validity),

// This is a placeholder for converting a partial
// entry (`ProposalOnly` or `ValueOnly`) to a full entry (`Full`).
Expand All @@ -58,13 +52,8 @@ enum Entry<Ctx: Context> {
}

impl<Ctx: Context> Entry<Ctx> {
fn full(
value: Ctx::Value,
validity: Validity,
proposal: SignedProposal<Ctx>,
extension: Option<SignedExtension<Ctx>>,
) -> Self {
Entry::Full(FullProposal::new(value, validity, proposal, extension))
fn full(value: Ctx::Value, validity: Validity, proposal: SignedProposal<Ctx>) -> Self {
Entry::Full(FullProposal::new(value, validity, proposal))
}
}

Expand Down Expand Up @@ -176,7 +165,7 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
height: &Ctx::Height,
round: Round,
value: &'a Ctx::Value,
) -> Option<(&'a Ctx::Value, Validity, Option<SignedExtension<Ctx>>)> {
) -> Option<(&'a Ctx::Value, Validity)> {
let entries = self
.keeper
.get(&(*height, round))
Expand All @@ -185,10 +174,10 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
for entry in entries {
match entry {
Entry::Full(p) if p.proposal.value().id() == value.id() => {
return Some((value, p.validity, p.extension.clone()));
return Some((value, p.validity));
}
Entry::ValueOnly(v, validity, extension) if v.id() == value.id() => {
return Some((value, *validity, extension.clone()));
Entry::ValueOnly(v, validity) if v.id() == value.id() => {
return Some((value, *validity));
}
_ => continue,
}
Expand Down Expand Up @@ -216,12 +205,9 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
None => Entry::ProposalOnly(new_proposal),

// There is a value, create a full entry
Some((v, validity, extension)) => Entry::Full(FullProposal::new(
v.clone(),
validity,
new_proposal,
extension,
)),
Some((v, validity)) => {
Entry::Full(FullProposal::new(v.clone(), validity, new_proposal))
}
}
}

Expand Down Expand Up @@ -249,11 +235,11 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
return;
}
}
Entry::ValueOnly(value, _validity, _extension) => {
Entry::ValueOnly(value, _validity) => {
if value == new_proposal.value() {
// Found a matching value. Add the proposal
replace_with!(entry, Entry::ValueOnly(value, validity, extension) => {
Entry::full(value, validity, new_proposal, extension.clone())
replace_with!(entry, Entry::ValueOnly(value, validity) => {
Entry::full(value, validity, new_proposal)
});

return;
Expand Down Expand Up @@ -292,11 +278,7 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
None => {
// First time we see something (a proposed value) for this height and round
// Create a full proposal with just the proposal
let entry = Entry::ValueOnly(
new_value.value.clone(),
new_value.validity,
new_value.extension.clone(),
);
let entry = Entry::ValueOnly(new_value.value.clone(), new_value.validity);
self.keeper.insert(key, vec![entry]);
}
Some(entries) => {
Expand All @@ -309,7 +291,7 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
if proposal.value().id() == new_value.value.id() {
// Found a matching proposal. Change the entry at index i
replace_with!(entry, Entry::ProposalOnly(proposal) => {
Entry::full(new_value.value.clone(), new_value.validity, proposal, new_value.extension.clone())
Entry::full(new_value.value.clone(), new_value.validity, proposal)
});

return;
Expand Down Expand Up @@ -338,7 +320,6 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
entries.push(Entry::ValueOnly(
new_value.value.clone(),
new_value.validity,
new_value.extension.clone(),
));
}
}
Expand All @@ -363,7 +344,7 @@ impl<Ctx: Context> FullProposalKeeper<Ctx> {
{
// Found a matching proposal. Change the entry at index i
replace_with!(entry, Entry::ProposalOnly(proposal) => {
Entry::full(new_value.value.clone(), new_value.validity, proposal, new_value.extension.clone())
Entry::full(new_value.value.clone(), new_value.validity, proposal)
});
}
}
Expand Down
29 changes: 14 additions & 15 deletions code/crates/core-consensus/src/handle/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ where

// Only sign and publish if we're in the validator set
if state.is_validator() {
let extended_vote = extend_vote(vote, state);
let extended_vote = extend_vote(co, vote).await?;
let signed_vote = sign_vote(co, extended_vote).await?;

on_vote(co, state, metrics, signed_vote.clone()).await?;
Expand Down Expand Up @@ -303,26 +303,25 @@ where
}
}

fn extend_vote<Ctx: Context>(vote: Ctx::Vote, state: &mut State<Ctx>) -> Ctx::Vote {
async fn extend_vote<Ctx: Context>(co: &Co<Ctx>, vote: Ctx::Vote) -> Result<Ctx::Vote, Error<Ctx>> {
let VoteType::Precommit = vote.vote_type() else {
return vote;
return Ok(vote);
};

let NilOrVal::Val(val_id) = vote.value() else {
return vote;
let NilOrVal::Val(value_id) = vote.value().as_ref().cloned() else {
return Ok(vote);
};

let Some(full_proposal) = state.full_proposal_keeper.full_proposal_at_round_and_value(
&vote.height(),
vote.round(),
val_id,
) else {
return vote;
};
let extension = perform!(
co,


Effect::ExtendVote(vote.height(), vote.round(), value_id, Default::default()),
Resume::VoteExtension(extension) => extension);

if let Some(extension) = &full_proposal.extension {
vote.extend(extension.clone())
if let Some(extension) = extension {
Ok(vote.extend(extension))
} else {
vote
Ok(vote)
}
}
1 change: 0 additions & 1 deletion code/crates/core-consensus/src/handle/proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,6 @@ where
proposer: signed_proposal.validator_address().clone(),
value: signed_proposal.value().clone(),
validity: Validity::Valid,
extension: Default::default(),
};

state.store_value(&new_value);
Expand Down
2 changes: 0 additions & 2 deletions code/crates/core-consensus/src/handle/propose.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ where
round,
valid_round,
value,
extension,
} = value;

if state.driver.height() != height {
Expand Down Expand Up @@ -47,7 +46,6 @@ where
proposer: state.address().clone(),
value: value.clone(),
validity: Validity::Valid,
extension,
});

apply_driver_input(co, state, metrics, DriverInput::ProposeValue(round, value)).await
Expand Down
5 changes: 1 addition & 4 deletions code/crates/core-consensus/src/types.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
use derive_where::derive_where;

use malachitebft_core_types::{
Context, Proposal, Round, Signature, SignedExtension, SignedProposal, SignedVote, Validity,
Vote,
Context, Proposal, Round, Signature, SignedProposal, SignedVote, Validity, Vote,
};

pub use malachitebft_peer::PeerId;
Expand Down Expand Up @@ -46,7 +45,6 @@ pub struct ValueToPropose<Ctx: Context> {
pub round: Round,
pub valid_round: Round,
pub value: Ctx::Value,
pub extension: Option<SignedExtension<Ctx>>,
}

/// A value proposed by a validator
Expand All @@ -58,7 +56,6 @@ pub struct ProposedValue<Ctx: Context> {
pub proposer: Ctx::Address,
pub value: Ctx::Value,
pub validity: Validity,
pub extension: Option<SignedExtension<Ctx>>,
}

/// The possible messages used to deliver proposals
Expand Down
2 changes: 0 additions & 2 deletions code/crates/core-consensus/tests/full_proposal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ fn value(
proposer,
value: Value::new(value),
validity,
extension: Default::default(),
}
}

Expand All @@ -74,7 +73,6 @@ fn val_msg(proposer: Address, round: u32, value: u64, validity: Validity) -> Inp
value: Value::new(value),
validity,
proposer,
extension: Default::default(),
},
ValueOrigin::Consensus,
)
Expand Down
13 changes: 12 additions & 1 deletion code/crates/core-types/src/signing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use alloc::vec::Vec;
use core::fmt::{Debug, Display};

use crate::{
CertificateError, CommitCertificate, CommitSignature, Context, PublicKey, Signature,
CertificateError, CommitCertificate, CommitSignature, Context, Extension, PublicKey, Signature,
SignedMessage, ThresholdParams, VotingPower,
};

Expand Down Expand Up @@ -86,6 +86,17 @@ where
public_key: &PublicKey<Ctx>,
) -> bool;

/// Sign the given vote extension with our private key.
fn sign_vote_extension(&self, extension: Extension) -> SignedMessage<Ctx, Extension>;

/// Verify the given vote extension's signature using the given public key.
fn verify_signed_vote_extension(
&self,
extension: &Extension,
signature: &Signature<Ctx>,
public_key: &PublicKey<Ctx>,
) -> bool;

/// Verify a commit signature in a certificate against the public key of its validator.
///
/// ## Return
Expand Down
Loading
Loading