Skip to content

Commit

Permalink
[checkpoints] Added a previous hash to the checkpoint (MystenLabs#2433)
Browse files Browse the repository at this point in the history 
* Added a previous hash to the checkpoint
* Fix comment

Co-authored-by: George Danezis <[email protected]>
  • Loading branch information
@gdanezis
gdanezis and George Danezis authored Jun 7, 2022
1 parent d719946 commit 8668510
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 26 deletions.
10 changes: 5 additions & 5 deletions crates/sui-core/src/authority_active/checkpoint_driver/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use sui_types::{
messages::{CertifiedTransaction, ConfirmationTransaction, TransactionInfoRequest},
messages_checkpoint::{
AuthenticatedCheckpoint, AuthorityCheckpointInfo, CertifiedCheckpoint, CheckpointContents,
CheckpointFragment, CheckpointRequest, CheckpointResponse, CheckpointSequenceNumber,
SignedCheckpoint, SignedCheckpointProposal,
CheckpointDigest, CheckpointFragment, CheckpointRequest, CheckpointResponse,
CheckpointSequenceNumber, SignedCheckpoint, SignedCheckpointProposal,
},
};
use tokio::time::timeout;
Expand Down Expand Up @@ -329,7 +329,7 @@ where
// Attempt to construct a newer checkpoint from signed summaries.
#[allow(clippy::type_complexity)]
let mut partial_checkpoints: BTreeMap<
(CheckpointSequenceNumber, [u8; 32]),
(CheckpointSequenceNumber, CheckpointDigest),
Vec<(AuthorityName, SignedCheckpoint)>,
> = BTreeMap::new();
final_state
Expand Down Expand Up @@ -553,8 +553,8 @@ where
info: _info,
detail: Some(contents),
}) => {
// TODO: check here that the digest of contents matches
if contents.digest() != checkpoint.checkpoint.digest {
// Check here that the digest of contents matches
if contents.digest() != checkpoint.checkpoint.content_digest {
// A byzantine authority!
// TODO: Report Byzantine authority
warn!("Sync Error: Incorrect contents returned");
Expand Down
54 changes: 45 additions & 9 deletions crates/sui-core/src/checkpoints/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use sui_types::{
messages::CertifiedTransaction,
messages_checkpoint::{
AuthenticatedCheckpoint, AuthorityCheckpointInfo, CertifiedCheckpoint, CheckpointContents,
CheckpointFragment, CheckpointRequest, CheckpointResponse, CheckpointSequenceNumber,
CheckpointSummary, SignedCheckpoint, SignedCheckpointProposal,
CheckpointDigest, CheckpointFragment, CheckpointRequest, CheckpointResponse,
CheckpointSequenceNumber, CheckpointSummary, SignedCheckpoint, SignedCheckpointProposal,
},
};
use typed_store::{
Expand Down Expand Up @@ -138,6 +138,26 @@ pub struct CheckpointStore {
}

impl CheckpointStore {
fn get_prev_checkpoint_digest(
&mut self,
checkpoint_sequence: CheckpointSequenceNumber,
) -> Result<Option<CheckpointDigest>, SuiError> {
// Extract the previous checkpoint digest if there is one.
Ok(if checkpoint_sequence > 0 {
self.checkpoints
.get(&(checkpoint_sequence - 1))?
.map(|prev_checkpoint| match prev_checkpoint {
AuthenticatedCheckpoint::Certified(cert) => cert.checkpoint.digest(),
AuthenticatedCheckpoint::Signed(signed) => signed.checkpoint.digest(),
_ => {
unreachable!();
}
})
} else {
None
})
}

// Manage persistent local variables

/// Loads the locals from the store -- do this at init
Expand All @@ -154,19 +174,20 @@ impl CheckpointStore {

// Recreate the proposal
if locals.proposal_next_transaction.is_some() {
let checkpoint = locals.next_checkpoint;
let checkpoint_sequence = locals.next_checkpoint;
let transactions = self
.extra_transactions
.iter()
.filter(|(_, seq)| seq < locals.proposal_next_transaction.as_ref().unwrap())
.map(|(digest, _)| digest);

let transactions = CheckpointContents::new(transactions);
let previous_digest = self.get_prev_checkpoint_digest(checkpoint_sequence)?;
let proposal = SignedCheckpointProposal(SignedCheckpoint::new(
checkpoint,
checkpoint_sequence,
self.name,
&*self.secret,
&transactions,
previous_digest,
));

let proposal_and_transactions = CheckpointProposal::new(proposal, transactions);
Expand Down Expand Up @@ -644,7 +665,11 @@ impl CheckpointStore {
.map_err(|e| FragmentInternalError::Error(e.into()))?;

// Now create the new checkpoint and move all locals forward.
let summary = CheckpointSummary::new(next_sequence_number, &contents);
let previous_digest = self
.get_prev_checkpoint_digest(next_sequence_number)
.map_err(FragmentInternalError::Error)?;
let summary =
CheckpointSummary::new(next_sequence_number, &contents, previous_digest);
self.handle_internal_set_checkpoint(summary, &contents)
.map_err(FragmentInternalError::Error)?;
return Ok(true);
Expand Down Expand Up @@ -693,7 +718,14 @@ impl CheckpointStore {
.map_err(|e| FragmentInternalError::Error(e.into()))?;

let contents = CheckpointContents::new(contents.into_iter());
let summary = CheckpointSummary::new(next_sequence_number, &contents);
let previous_digest = self
.get_prev_checkpoint_digest(next_sequence_number)
.map_err(FragmentInternalError::Error)?;
let summary = CheckpointSummary::new(
next_sequence_number,
&contents,
previous_digest,
);
self.handle_internal_set_checkpoint(summary, &contents)
.map_err(FragmentInternalError::Error)?;
return Ok(true);
Expand Down Expand Up @@ -880,15 +912,19 @@ impl CheckpointStore {

// Include the sequence number of all extra transactions not already in a
// checkpoint. And make a list of the transactions.
let sequence_number = self.next_checkpoint();
let checkpoint_sequence = self.next_checkpoint();
let next_local_tx_sequence = self.extra_transactions.values().max().unwrap() + 1;

// Extract the previous checkpoint digest if there is one.
let previous_digest = self.get_prev_checkpoint_digest(checkpoint_sequence)?;

let transactions = CheckpointContents::new(self.extra_transactions.keys());
let proposal = SignedCheckpointProposal(SignedCheckpoint::new(
sequence_number,
checkpoint_sequence,
self.name,
&*self.secret,
&transactions,
previous_digest,
));

let proposal_and_transactions = CheckpointProposal::new(proposal, transactions);
Expand Down
12 changes: 9 additions & 3 deletions crates/sui-core/src/checkpoints/tests/checkpoint_tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ fn latest_proposal() {
.collect();

let transactions = CheckpointContents::new(ckp_items.clone().into_iter());
let summary = CheckpointSummary::new(0, &transactions);
let summary = CheckpointSummary::new(0, &transactions, None);

cps1.handle_internal_set_checkpoint(summary.clone(), &transactions)
.unwrap();
Expand Down Expand Up @@ -529,7 +529,7 @@ fn set_get_checkpoint() {
.cloned();

let transactions = CheckpointContents::new(ckp_items);
let summary = CheckpointSummary::new(0, &transactions);
let summary = CheckpointSummary::new(0, &transactions, None);

cps1.handle_internal_set_checkpoint(summary.clone(), &transactions)
.unwrap();
Expand Down Expand Up @@ -673,7 +673,13 @@ fn checkpoint_integration() {
.chain(some_fresh_transactions.iter().cloned().map(|(_, d)| d))
.collect();
let transactions = CheckpointContents::new(unprocessed.clone().into_iter());
let summary = CheckpointSummary::new(cps.get_locals().next_checkpoint, &transactions);
let next_checkpoint = cps.get_locals().next_checkpoint;
let summary = CheckpointSummary::new(
next_checkpoint,
&transactions,
cps.get_prev_checkpoint_digest(next_checkpoint)
.expect("previous checkpoint should exist"),
);

cps.handle_internal_set_checkpoint(summary.clone(), &transactions)
.unwrap();
Expand Down
26 changes: 17 additions & 9 deletions crates/sui-types/src/messages_checkpoint.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,26 +183,29 @@ pub type CheckpointDigest = [u8; 32];
pub struct CheckpointSummary {
pub sequence_number: CheckpointSequenceNumber,
pub waypoint: Box<Waypoint>, // Bigger structure, can live on heap.
pub digest: CheckpointDigest,
pub content_digest: CheckpointDigest,
pub previous_digest: Option<CheckpointDigest>,
// TODO: add digest of previous checkpoint summary
}

impl CheckpointSummary {
pub fn new(
sequence_number: CheckpointSequenceNumber,
transactions: &CheckpointContents,
previous_digest: Option<CheckpointDigest>,
) -> CheckpointSummary {
let mut waypoint = Box::new(Waypoint::default());
transactions.transactions.iter().for_each(|tx| {
waypoint.insert(tx);
});

let proposal_digest = transactions.digest();
let content_digest = transactions.digest();

Self {
sequence_number,
waypoint,
digest: proposal_digest,
content_digest,
previous_digest,
}
}

Expand Down Expand Up @@ -232,8 +235,9 @@ impl SignedCheckpoint {
authority: AuthorityName,
signer: &dyn signature::Signer<AuthoritySignature>,
transactions: &CheckpointContents,
previous_digest: Option<CheckpointDigest>,
) -> SignedCheckpoint {
let checkpoint = CheckpointSummary::new(sequence_number, transactions);
let checkpoint = CheckpointSummary::new(sequence_number, transactions, previous_digest);
SignedCheckpoint::new_from_summary(checkpoint, authority, signer)
}

Expand All @@ -260,7 +264,11 @@ impl SignedCheckpoint {
// Check that the digest and transactions are correctly signed
pub fn verify_with_transactions(&self, contents: &CheckpointContents) -> Result<(), SuiError> {
self.verify()?;
let recomputed = CheckpointSummary::new(*self.checkpoint.sequence_number(), contents);
let recomputed = CheckpointSummary::new(
*self.checkpoint.sequence_number(),
contents,
self.checkpoint.previous_digest,
);

fp_ensure!(
recomputed == self.checkpoint,
Expand Down Expand Up @@ -382,7 +390,7 @@ impl CertifiedCheckpoint {
) -> Result<(), SuiError> {
self.verify(committee)?;
fp_ensure!(
contents.digest() == self.checkpoint.digest,
contents.digest() == self.checkpoint.content_digest,
SuiError::from("Transaction digest mismatch")
);
Ok(())
Expand Down Expand Up @@ -482,7 +490,7 @@ mod tests {
let set = [ExecutionDigests::random()];
let set = CheckpointContents::new(set.iter().cloned());

let mut proposal = SignedCheckpoint::new(1, *name, &authority_key[0], &set);
let mut proposal = SignedCheckpoint::new(1, *name, &authority_key[0], &set, None);

// Signature is correct on proposal, and with same transactions
assert!(proposal.verify().is_ok());
Expand Down Expand Up @@ -512,7 +520,7 @@ mod tests {
.map(|k| {
let name = k.public_key_bytes();

SignedCheckpoint::new(1, *name, k, &set)
SignedCheckpoint::new(1, *name, k, &set, None)
})
.collect();

Expand All @@ -532,7 +540,7 @@ mod tests {
let set: BTreeSet<_> = [ExecutionDigests::random()].into_iter().collect();
let set = CheckpointContents::new(set.iter().cloned());

SignedCheckpoint::new(1, *name, k, &set)
SignedCheckpoint::new(1, *name, k, &set, None)
})
.collect();

Expand Down

0 comments on commit 8668510

Please sign in to comment.