Skip to content

Commit

Permalink
feat(core)!: use newtype sequencer block hashes
Browse files Browse the repository at this point in the history
  • Loading branch information
SuperFluffy committed Dec 19, 2024
1 parent 0332384 commit 3179682
Show file tree
Hide file tree
Showing 13 changed files with 172 additions and 58 deletions.
14 changes: 8 additions & 6 deletions crates/astria-conductor/src/celestia/block_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,10 @@ mod tests {
generated::astria::sequencerblock::v1::SequencerBlockHeader as RawSequencerBlockHeader,
primitive::v1::RollupId,
sequencerblock::v1::{
block::SequencerBlockHeader,
block::{
self,
SequencerBlockHeader,
},
celestia::UncheckedSubmittedMetadata,
},
};
Expand All @@ -237,7 +240,6 @@ mod tests {
block::Commit,
validator,
validator::Info as Validator,
Hash,
},
tendermint_proto,
tendermint_rpc::endpoint::validators,
Expand Down Expand Up @@ -345,7 +347,7 @@ mod tests {
let header = SequencerBlockHeader::try_from_raw(header).unwrap();

let sequencer_blob = UncheckedSubmittedMetadata {
block_hash: [0u8; 32],
block_hash: block::Hash::new([0u8; 32]),
header,
rollup_ids: vec![],
rollup_transactions_proof,
Expand Down Expand Up @@ -390,7 +392,7 @@ mod tests {
let header = SequencerBlockHeader::try_from_raw(header).unwrap();

let sequencer_blob = UncheckedSubmittedMetadata {
block_hash: [0u8; 32],
block_hash: block::Hash::new([0u8; 32]),
header,
rollup_ids: vec![rollup_id],
rollup_transactions_proof,
Expand Down Expand Up @@ -510,12 +512,12 @@ mod tests {
round: 0u16.into(),
block_id: tendermint::block::Id {
hash: "74BD4E7F7EF902A84D55589F2AA60B332F1C2F34DDE7652C80BFEB8E7471B1DA"
.parse::<Hash>()
.parse::<tendermint::Hash>()
.unwrap(),
part_set_header: tendermint::block::parts::Header::new(
1,
"7632FFB5D84C3A64279BC9EA86992418ED23832C66E0C3504B7025A9AF42C8C4"
.parse::<Hash>()
.parse::<tendermint::Hash>()
.unwrap(),
)
.unwrap(),
Expand Down
13 changes: 10 additions & 3 deletions crates/astria-conductor/src/celestia/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ use std::{

use astria_core::{
primitive::v1::RollupId,
sequencerblock::v1::block::SequencerBlockHeader,
sequencerblock::v1::block::{
self,
SequencerBlockHeader,
},
};
use astria_eyre::eyre::{
self,
Expand Down Expand Up @@ -103,12 +106,16 @@ use crate::{
#[derive(Clone, Debug)]
pub(crate) struct ReconstructedBlock {
pub(crate) celestia_height: u64,
pub(crate) block_hash: [u8; 32],
pub(crate) block_hash: block::Hash,
pub(crate) header: SequencerBlockHeader,
pub(crate) transactions: Vec<Bytes>,
}

impl ReconstructedBlock {
pub(crate) fn block_hash(&self) -> &block::Hash {
&self.block_hash
}

pub(crate) fn sequencer_height(&self) -> SequencerHeight {
self.header.height()
}
Expand Down Expand Up @@ -458,7 +465,7 @@ impl RunningReader {
error = %eyre::Report::new(e),
source_celestia_height = celestia_height,
sequencer_height,
block_hash = %base64(&block_hash),
block_hash = %block_hash,
"failed pushing reconstructed block into sequential cache; dropping it",
);
}
Expand Down
8 changes: 4 additions & 4 deletions crates/astria-conductor/src/celestia/reconstruct.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@ use std::collections::HashMap;
use astria_core::{
primitive::v1::RollupId,
sequencerblock::v1::{
block,
celestia::UncheckedSubmittedMetadata,
SubmittedMetadata,
SubmittedRollupData,
},
};
use telemetry::display::base64;
use tracing::{
info,
warn,
Expand Down Expand Up @@ -72,7 +72,7 @@ pub(super) fn reconstruct_blocks_from_verified_blobs(
"no sequencer header blob matching the rollup blob's block hash found"
};
info!(
block_hash = %base64(&rollup.sequencer_block_hash()),
block_hash = %rollup.sequencer_block_hash(),
reason,
"dropping rollup blob",
);
Expand All @@ -83,7 +83,7 @@ pub(super) fn reconstruct_blocks_from_verified_blobs(
for header_blob in header_blobs.into_values() {
if header_blob.contains_rollup_id(rollup_id) {
warn!(
block_hash = %base64(header_blob.block_hash()),
block_hash = %header_blob.block_hash(),
"sequencer header blob contains the target rollup ID, but no matching rollup blob was found; dropping it",
);
} else {
Expand All @@ -99,7 +99,7 @@ pub(super) fn reconstruct_blocks_from_verified_blobs(
}

fn remove_header_blob_matching_rollup_blob(
headers: &mut HashMap<[u8; 32], SubmittedMetadata>,
headers: &mut HashMap<block::Hash, SubmittedMetadata>,
rollup: &SubmittedRollupData,
) -> Option<SubmittedMetadata> {
// chaining methods and returning () to use the ? operator and to not bind the value
Expand Down
3 changes: 2 additions & 1 deletion crates/astria-conductor/src/celestia/reporting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ impl<'a> Serialize for ReportReconstructedBlock<'a> {
];
let mut state = serializer.serialize_struct("ReconstructedBlockInfo", FIELDS.len())?;
state.serialize_field(FIELDS[0], &self.0.celestia_height)?;
state.serialize_field(FIELDS[1], &base64(&self.0.block_hash))?;
// TODO: use the block hash's Display impl for this
state.serialize_field(FIELDS[1], &base64(&*self.0.block_hash))?;
state.serialize_field(FIELDS[2], &self.0.transactions.len())?;
state.serialize_field(FIELDS[3], &self.0.celestia_height)?;
state.end()
Expand Down
19 changes: 11 additions & 8 deletions crates/astria-conductor/src/celestia/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::{
};

use astria_core::sequencerblock::v1::{
block,
SubmittedMetadata,
SubmittedRollupData,
};
Expand All @@ -25,7 +26,6 @@ use sequencer_client::{
Client as _,
HttpClient as SequencerClient,
};
use telemetry::display::base64;
use tokio_util::task::JoinMap;
use tower::{
util::BoxService,
Expand Down Expand Up @@ -58,7 +58,7 @@ use crate::executor::{

pub(super) struct VerifiedBlobs {
celestia_height: u64,
header_blobs: HashMap<[u8; 32], SubmittedMetadata>,
header_blobs: HashMap<block::Hash, SubmittedMetadata>,
rollup_blobs: Vec<SubmittedRollupData>,
}

Expand All @@ -75,7 +75,7 @@ impl VerifiedBlobs {
self,
) -> (
u64,
HashMap<[u8; 32], SubmittedMetadata>,
HashMap<block::Hash, SubmittedMetadata>,
Vec<SubmittedRollupData>,
) {
(self.celestia_height, self.header_blobs, self.rollup_blobs)
Expand All @@ -88,7 +88,7 @@ impl VerifiedBlobs {
#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
struct VerificationTaskKey {
index: usize,
block_hash: [u8; 32],
block_hash: block::Hash,
sequencer_height: SequencerHeight,
}

Expand Down Expand Up @@ -142,7 +142,7 @@ pub(super) async fn verify_metadata(
.get(dropped_entry.block_hash())
.expect("must exist; just inserted an item under the same key");
info!(
block_hash = %base64(&dropped_entry.block_hash()),
block_hash = %dropped_entry.block_hash(),
dropped_blob.sequencer_height = dropped_entry.height().value(),
accepted_blob.sequencer_height = accepted_entry.height().value(),
"two Sequencer header blobs were well formed and validated against \
Expand All @@ -154,7 +154,7 @@ pub(super) async fn verify_metadata(
Ok(None) => {}
Err(error) => {
info!(
block_hash = %base64(&key.block_hash),
block_hash = %key.block_hash,
sequencer_height = %key.sequencer_height,
%error,
"verification of sequencer blob was cancelled abruptly; dropping it"
Expand Down Expand Up @@ -552,10 +552,13 @@ fn ensure_chain_ids_match(in_commit: &str, in_header: &str) -> eyre::Result<()>
Ok(())
}

fn ensure_block_hashes_match(in_commit: &[u8], in_header: &[u8]) -> eyre::Result<()> {
fn ensure_block_hashes_match(in_commit: &[u8], in_header: &block::Hash) -> eyre::Result<()> {
use base64::prelude::*;
// NOTE: we still re-encode the block hash using base64 instead of its display impl
// to ensure that the formatting of the two byte slices doesn't accidentally go
// out of whack should the display impl change.
ensure!(
in_commit == in_header,
in_commit == in_header.as_ref(),
"expected block hash `{}` (from commit), but found `{}` in retrieved metadata",
BASE64_STANDARD.encode(in_commit),
BASE64_STANDARD.encode(in_header),
Expand Down
13 changes: 7 additions & 6 deletions crates/astria-conductor/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use astria_core::{
},
primitive::v1::RollupId,
sequencerblock::v1::block::{
self,
FilteredSequencerBlock,
FilteredSequencerBlockParts,
},
Expand Down Expand Up @@ -285,7 +286,7 @@ impl Executor {
{
debug_span!("conductor::Executor::run_until_stopped").in_scope(||debug!(
block.height = %block.sequencer_height(),
block.hash = %telemetry::display::base64(&block.block_hash),
block.hash = %block.block_hash(),
"received block from celestia reader",
));
if let Err(error) = self.execute_firm(block).await {
Expand All @@ -298,7 +299,7 @@ impl Executor {
{
debug_span!("conductor::Executor::run_until_stopped").in_scope(||debug!(
block.height = %block.height(),
block.hash = %telemetry::display::base64(&block.block_hash()),
block.hash = %block.block_hash(),
"received block from sequencer reader",
));
if let Err(error) = self.execute_soft(block).await {
Expand Down Expand Up @@ -388,7 +389,7 @@ impl Executor {
}

#[instrument(skip_all, fields(
block.hash = %telemetry::display::base64(&block.block_hash()),
block.hash = %block.block_hash(),
block.height = block.height().value(),
err,
))]
Expand Down Expand Up @@ -452,7 +453,7 @@ impl Executor {
}

#[instrument(skip_all, fields(
block.hash = %telemetry::display::base64(&block.block_hash),
block.hash = %block.block_hash(),
block.height = block.sequencer_height().value(),
err,
))]
Expand Down Expand Up @@ -532,7 +533,7 @@ impl Executor {
/// This function is called via [`Executor::execute_firm`] or [`Executor::execute_soft`],
/// and should not be called directly.
#[instrument(skip_all, fields(
block.hash = %telemetry::display::base64(&block.hash),
block.hash = %block.hash,
block.height = block.height.value(),
block.num_of_transactions = block.transactions.len(),
rollup.parent_hash = %telemetry::display::base64(&parent_hash),
Expand Down Expand Up @@ -690,7 +691,7 @@ enum Update {

#[derive(Debug)]
struct ExecutableBlock {
hash: [u8; 32],
hash: block::Hash,
height: SequencerHeight,
timestamp: pbjson_types::Timestamp,
transactions: Vec<Bytes>,
Expand Down
4 changes: 2 additions & 2 deletions crates/astria-conductor/tests/blackbox/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ pub fn make_commit(height: u32) -> tendermint::block::Commit {
height: height.into(),
round: 0u16.into(),
block_id: Some(tendermint::block::Id {
hash: tendermint::Hash::Sha256(block_hash),
hash: tendermint::Hash::Sha256(block_hash.get()),
part_set_header: tendermint::block::parts::Header::default(),
}),
timestamp: Some(timestamp),
Expand All @@ -657,7 +657,7 @@ pub fn make_commit(height: u32) -> tendermint::block::Commit {
height: height.into(),
round: 0u16.into(),
block_id: tendermint::block::Id {
hash: tendermint::Hash::Sha256(block_hash),
hash: tendermint::Hash::Sha256(block_hash.get()),
part_set_header: tendermint::block::parts::Header::default(),
},
signatures: vec![tendermint::block::CommitSig::BlockIdFlagCommit {
Expand Down
2 changes: 2 additions & 0 deletions crates/astria-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
to `astria_core::generated::astria`
[#1825](https://github.com/astriaorg/astria/pull/1825).
- Update `idna` dependency to resolve cargo audit warning [#1869](https://github.com/astriaorg/astria/pull/1869).
- Replaced all instances of `[u8; 32]` by newtype `astria_core::sequencerblock::v1::block::Hash` where appropraite

Check failure on line 30 in crates/astria-core/CHANGELOG.md

View workflow job for this annotation

GitHub Actions / markdown

Line length

crates/astria-core/CHANGELOG.md:30:81 MD013/line-length Line length [Expected: 80; Actual: 114] https://github.com/DavidAnson/markdownlint/blob/v0.29.0/doc/md013.md
[#1884](https://github.com/astriaorg/astria/pull/1884).

### Removed

Expand Down
Loading

0 comments on commit 3179682

Please sign in to comment.