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

Introduce and use TimeoutCertificate #2845

Merged
merged 6 commits into from
Nov 11, 2024
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
26 changes: 25 additions & 1 deletion linera-chain/src/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@

use std::fmt::Debug;

use linera_base::crypto::BcsHashable;
use linera_base::{crypto::BcsHashable, data_types::BlockHeight, identifiers::ChainId};
use linera_execution::committee::Epoch;
use serde::{Deserialize, Serialize};

use crate::data_types::{ExecutedBlock, HashedCertificateValue};
Expand Down Expand Up @@ -79,3 +80,26 @@ impl ConfirmedBlock {
self.executed_block
}
}

#[derive(Debug, PartialEq, Eq, Hash, Clone, Deserialize, Serialize)]
pub struct Timeout {
pub chain_id: ChainId,
pub height: BlockHeight,
pub epoch: Epoch,
}

impl Timeout {
pub fn new(chain_id: ChainId, height: BlockHeight, epoch: Epoch) -> Self {
Self {
chain_id,
height,
epoch,
}
}
}

impl From<Timeout> for HashedCertificateValue {
fn from(value: Timeout) -> Self {
HashedCertificateValue::new_timeout(value.chain_id, value.height, value.epoch)
}
}
114 changes: 105 additions & 9 deletions linera-chain/src/certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ use linera_base::{
identifiers::BlobId,
};
use linera_execution::committee::{Committee, ValidatorName};
use serde::{ser::Serializer, Deserialize, Deserializer, Serialize};
use serde::{
ser::{Serialize, SerializeStruct, Serializer},
Deserialize, Deserializer,
};

use crate::{
block::{ConfirmedBlock, ValidatedBlock},
block::{ConfirmedBlock, Timeout, ValidatedBlock},
data_types::{
Certificate, CertificateValue, ExecutedBlock, HashedCertificateValue, LiteCertificate,
LiteValue,
Expand All @@ -30,6 +33,9 @@ pub type ValidatedBlockCertificate = GenericCertificate<ValidatedBlock>;
/// Certificate for a [`ConfirmedBlock`] instance.
pub type ConfirmedBlockCertificate = GenericCertificate<ConfirmedBlock>;

/// Certificate for a Timeout instance.
pub type TimeoutCertificate = GenericCertificate<Timeout>;

/// Generic type representing a certificate for `value` of type `T`.
pub struct GenericCertificate<T> {
value: Hashed<T>,
Expand Down Expand Up @@ -69,13 +75,33 @@ impl<T: Debug> Debug for GenericCertificate<T> {
}
}

// NOTE: For backwards compatiblity reasons we serialize the new `Certificate` type as the old
// one. Otherwise we would be breaking the RPC API schemas. We can't implement generic serialization
// for `Certificate<T>` since only specific `T`s have corresponding `CertificateValue` variants.
impl Serialize for ValidatedBlockCertificate {
afck marked this conversation as resolved.
Show resolved Hide resolved
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
let cert = Certificate::from(self.clone());
cert.serialize(serializer)
let ValidatedBlockCertificate {
value,
round,
signatures,
} = self;
let mut state = serializer.serialize_struct("ValidatedBlockCertificate", 4)?;
state.serialize_field("value", value.inner())?;
state.serialize_field("round", &round)?;
state.serialize_field("signatures", &signatures)?;
state.end()
}
}

impl Serialize for TimeoutCertificate {
fn serialize<S: Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
afck marked this conversation as resolved.
Show resolved Hide resolved
let TimeoutCertificate {
value,
round,
signatures,
} = self;
let mut state = serializer.serialize_struct("TimeoutCertificate", 4)?;
state.serialize_field("value", value.inner())?;
state.serialize_field("round", &round)?;
state.serialize_field("signatures", &signatures)?;
state.end()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does that really BCS-serialize to the same bytes as a Certificate with Timeout value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it matter? I carry-on the hash and I map to/from the old ConfirmedValue types.

Copy link
Contributor

@afck afck Nov 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it will only matter in the future, when we use this directly to compute the hash.

Or at least it will matter then that ConfirmedBlockCertificate serializes to something different than ValidatedBlockCertificate (in BCS).

}
}

Expand All @@ -84,7 +110,42 @@ impl<'de> Deserialize<'de> for ValidatedBlockCertificate {
where
D: Deserializer<'de>,
{
Certificate::deserialize(deserializer).map(ValidatedBlockCertificate::from)
#[derive(Deserialize)]
#[serde(rename = "ValidatedBlockCertificate")]
struct Inner {
value: ValidatedBlock,
round: Round,
signatures: Vec<(ValidatorName, Signature)>,
}
let inner = Inner::deserialize(deserializer)?;
let validated_hashed: HashedCertificateValue = inner.value.clone().into();
Ok(Self {
value: Hashed::unchecked_new(inner.value, validated_hashed.hash()),
round: inner.round,
signatures: inner.signatures,
})
}
}

impl<'de> Deserialize<'de> for TimeoutCertificate {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: Deserializer<'de>,
{
#[derive(Deserialize)]
#[serde(rename = "TimeoutCertificate")]
struct Inner {
value: Timeout,
round: Round,
signatures: Vec<(ValidatorName, Signature)>,
}
let inner = Inner::deserialize(deserializer)?;
let timeout_hashed: HashedCertificateValue = inner.value.clone().into();
Ok(Self {
value: Hashed::unchecked_new(inner.value, timeout_hashed.hash()),
round: inner.round,
signatures: inner.signatures,
})
}
}

Expand Down Expand Up @@ -129,7 +190,23 @@ impl From<ValidatedBlockCertificate> for Certificate {
}
}

// TODO(#2842): In practice, it should be HashedCertificateValue = Hashed<CertificateValue>
impl From<TimeoutCertificate> for Certificate {
fn from(cert: TimeoutCertificate) -> Certificate {
let TimeoutCertificate {
value,
round,
signatures,
} = cert;
let timeout = value.into_inner();
Certificate::new(
HashedCertificateValue::new_timeout(timeout.chain_id, timeout.height, timeout.epoch),
round,
signatures,
)
}
}

// In practice, it should be HashedCertificateValue = Hashed<CertificateValue>
afck marked this conversation as resolved.
Show resolved Hide resolved
// but [`HashedCertificateValue`] is used in too many places to change it now.
/// Wrapper type around hashed instance of `T` type.
pub struct Hashed<T> {
Expand Down Expand Up @@ -319,3 +396,22 @@ impl TryFrom<Certificate> for ConfirmedBlockCertificate {
}
}
}

impl From<Certificate> for TimeoutCertificate {
fn from(cert: Certificate) -> Self {
let signatures = cert.signatures().clone();
let hash = cert.value.hash();
match cert.value.into_inner() {
CertificateValue::Timeout {
chain_id,
epoch,
height,
} => Self {
value: Hashed::unchecked_new(Timeout::new(chain_id, height, epoch), hash),
round: cert.round,
signatures,
},
_ => panic!("Expected a timeout certificate"),
}
}
}
22 changes: 10 additions & 12 deletions linera-chain/src/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,13 @@ use linera_execution::committee::Epoch;
use rand_chacha::{rand_core::SeedableRng, ChaCha8Rng};
use rand_distr::{Distribution, WeightedAliasIndex};
use serde::{Deserialize, Serialize};
use tracing::error;

use crate::{
data_types::{
Block, BlockExecutionOutcome, BlockProposal, Certificate, CertificateValue,
HashedCertificateValue, LiteVote, ProposalContent, Vote,
Block, BlockExecutionOutcome, BlockProposal, CertificateValue, HashedCertificateValue,
LiteVote, ProposalContent, Vote,
},
types::{ConfirmedBlockCertificate, ValidatedBlockCertificate},
types::{ConfirmedBlockCertificate, TimeoutCertificate, ValidatedBlockCertificate},
ChainError,
};

Expand Down Expand Up @@ -117,7 +116,7 @@ pub struct ChainManager {
/// validator).
pub locked: Option<ValidatedBlockCertificate>,
/// Latest leader timeout certificate we have received.
pub timeout: Option<Certificate>,
pub timeout: Option<TimeoutCertificate>,
/// Latest vote we have cast, to validate or confirm.
pub pending: Option<Vote>,
/// Latest timeout vote we cast.
Expand Down Expand Up @@ -470,12 +469,11 @@ impl ChainManager {

/// Updates the round number and timer if the timeout certificate is from a higher round than
/// any known certificate.
pub fn handle_timeout_certificate(&mut self, certificate: Certificate, local_time: Timestamp) {
if !certificate.value().is_timeout() {
// Unreachable: This is only called with timeout certificates.
error!("Unexpected certificate; expected leader timeout");
return;
}
pub fn handle_timeout_certificate(
&mut self,
certificate: TimeoutCertificate,
local_time: Timestamp,
) {
let round = certificate.round;
if let Some(known_certificate) = &self.timeout {
if known_certificate.round >= round {
Expand Down Expand Up @@ -565,7 +563,7 @@ pub struct ChainManagerInfo {
/// validator).
pub requested_locked: Option<Box<ValidatedBlockCertificate>>,
/// Latest timeout certificate we have seen.
pub timeout: Option<Box<Certificate>>,
pub timeout: Option<Box<TimeoutCertificate>>,
/// Latest vote we cast (either to validate or to confirm a block).
pub pending: Option<LiteVote>,
/// Latest timeout vote we cast.
Expand Down
10 changes: 5 additions & 5 deletions linera-core/src/chain_worker/actor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,9 @@ use linera_base::{
};
use linera_chain::{
data_types::{
Block, BlockProposal, Certificate, ExecutedBlock, HashedCertificateValue, MessageBundle,
Origin, Target,
Block, BlockProposal, ExecutedBlock, HashedCertificateValue, MessageBundle, Origin, Target,
},
types::{ConfirmedBlockCertificate, ValidatedBlockCertificate},
types::{ConfirmedBlockCertificate, TimeoutCertificate, ValidatedBlockCertificate},
ChainStateView,
};
use linera_execution::{
Expand All @@ -45,7 +44,8 @@ where
#[cfg(with_testing)]
ReadCertificate {
height: BlockHeight,
callback: oneshot::Sender<Result<Option<Certificate>, WorkerError>>,
callback:
oneshot::Sender<Result<Option<linera_chain::data_types::Certificate>, WorkerError>>,
},

/// Search for a bundle in one of the chain's inboxes.
Expand Down Expand Up @@ -84,7 +84,7 @@ where

/// Process a leader timeout issued for this multi-owner chain.
ProcessTimeout {
certificate: Certificate,
certificate: TimeoutCertificate,
callback: oneshot::Sender<Result<(ChainInfoResponse, NetworkActions), WorkerError>>,
},

Expand Down
38 changes: 15 additions & 23 deletions linera-core/src/chain_worker/state/attempted_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,10 @@ use linera_base::{
};
use linera_chain::{
data_types::{
BlockExecutionOutcome, BlockProposal, Certificate, CertificateValue, MessageBundle, Origin,
Target,
BlockExecutionOutcome, BlockProposal, Certificate, MessageBundle, Origin, Target,
},
manager,
types::{ConfirmedBlockCertificate, ValidatedBlockCertificate},
types::{ConfirmedBlockCertificate, TimeoutCertificate, ValidatedBlockCertificate},
ChainStateView,
};
use linera_execution::{
Expand Down Expand Up @@ -68,17 +67,8 @@ where
/// Processes a leader timeout issued for this multi-owner chain.
pub(super) async fn process_timeout(
&mut self,
certificate: Certificate,
certificate: TimeoutCertificate,
) -> Result<(ChainInfoResponse, NetworkActions), WorkerError> {
let (chain_id, height, epoch) = match certificate.value() {
CertificateValue::Timeout {
chain_id,
height,
epoch,
..
} => (*chain_id, *height, *epoch),
_ => panic!("Expecting a leader timeout certificate"),
};
// Check that the chain is active and ready for this timeout.
// Verify the certificate. Returns a catch-all error to make client code more robust.
self.state.ensure_is_active()?;
Expand All @@ -90,11 +80,11 @@ where
.current_committee()
.expect("chain is active");
ensure!(
epoch == chain_epoch,
certificate.inner().epoch == chain_epoch,
WorkerError::InvalidEpoch {
chain_id,
chain_id: certificate.inner().chain_id,
chain_epoch,
epoch
epoch: certificate.inner().epoch
}
);
certificate.check(committee)?;
Expand All @@ -104,27 +94,29 @@ where
.chain
.tip_state
.get()
.already_validated_block(height)?
.already_validated_block(certificate.inner().height)?
{
return Ok((
ChainInfoResponse::new(&self.state.chain, self.state.config.key_pair()),
actions,
));
}
let old_round = self.state.chain.manager.get().current_round;
let timeout_chainid = certificate.inner().chain_id;
let timeout_height = certificate.inner().height;
self.state
.chain
.manager
.get_mut()
.handle_timeout_certificate(
certificate.clone(),
self.state.storage.clock().current_time(),
);
.handle_timeout_certificate(certificate, self.state.storage.clock().current_time());
let round = self.state.chain.manager.get().current_round;
if round > old_round {
actions.notifications.push(Notification {
chain_id,
reason: Reason::NewRound { height, round },
chain_id: timeout_chainid,
reason: Reason::NewRound {
height: timeout_height,
round,
},
})
}
let info = ChainInfoResponse::new(&self.state.chain, self.state.config.key_pair());
Expand Down
10 changes: 5 additions & 5 deletions linera-core/src/chain_worker/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@ use linera_base::{
};
use linera_chain::{
data_types::{
Block, BlockProposal, Certificate, ExecutedBlock, HashedCertificateValue, Medium,
MessageBundle, Origin, Target,
Block, BlockProposal, ExecutedBlock, HashedCertificateValue, Medium, MessageBundle, Origin,
Target,
},
types::{ConfirmedBlockCertificate, ValidatedBlockCertificate},
types::{ConfirmedBlockCertificate, TimeoutCertificate, ValidatedBlockCertificate},
ChainError, ChainStateView,
};
use linera_execution::{
Expand Down Expand Up @@ -130,7 +130,7 @@ where
pub(super) async fn read_certificate(
&mut self,
height: BlockHeight,
) -> Result<Option<Certificate>, WorkerError> {
) -> Result<Option<linera_chain::data_types::Certificate>, WorkerError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should double-check, but I believe this will always be a ConfirmedBlockCertificate read from storage?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct (and one of the benefits of this refactoring)

ChainWorkerStateWithTemporaryChanges::new(self)
.await
.read_certificate(height)
Expand Down Expand Up @@ -188,7 +188,7 @@ where
/// Processes a leader timeout issued for this multi-owner chain.
pub(super) async fn process_timeout(
&mut self,
certificate: Certificate,
certificate: TimeoutCertificate,
) -> Result<(ChainInfoResponse, NetworkActions), WorkerError> {
ChainWorkerStateWithAttemptedChanges::new(self)
.await
Expand Down
Loading
Loading