feat(core)!: use newtype sequencer block hashes

crates/astria-conductor/src/celestia/block_verifier.rs

@@ -1,4 +1,4 @@
 use std::collections::HashMap;
 
 use astria_core::crypto::{
     Signature,
@@ -224,7 +224,10 @@
         generated::astria::sequencerblock::v1::SequencerBlockHeader as RawSequencerBlockHeader,
         primitive::v1::RollupId,
         sequencerblock::v1::{
-            block::SequencerBlockHeader,
+            block::{
+                self,
+                SequencerBlockHeader,
+            },
             celestia::UncheckedSubmittedMetadata,
         },
     };
@@ -237,7 +240,6 @@
             block::Commit,
             validator,
             validator::Info as Validator,
-            Hash,
         },
         tendermint_proto,
         tendermint_rpc::endpoint::validators,
@@ -345,7 +347,7 @@
         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,
@@ -390,7 +392,7 @@
         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,
@@ -510,12 +512,12 @@
             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(),

crates/astria-conductor/src/celestia/mod.rs

@@ -1,4 +1,4 @@
 use std::{
     cmp::max,
     sync::Arc,
     time::Duration,
@@ -6,7 +6,10 @@
 
 use astria_core::{
     primitive::v1::RollupId,
-    sequencerblock::v1::block::SequencerBlockHeader,
+    sequencerblock::v1::block::{
+        self,
+        SequencerBlockHeader,
+    },
 };
 use astria_eyre::eyre::{
     self,
@@ -103,12 +106,16 @@
 #[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()
     }
@@ -458,7 +465,7 @@
                     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",
                 );
             }

crates/astria-conductor/src/celestia/reconstruct.rs

@@ -1,14 +1,14 @@
 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,
@@ -72,7 +72,7 @@
                 "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",
             );
@@ -83,7 +83,7 @@
     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 {
@@ -99,7 +99,7 @@
 }
 
 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

crates/astria-conductor/src/celestia/reporting.rs

@@ -1,4 +1,4 @@
 //! Various newtype-wrappers to emit serde-serialized tracing event fields.
 use serde::ser::{
     Serialize,
     SerializeSeq,
@@ -53,7 +53,8 @@
         ];
         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()

crates/astria-conductor/src/celestia/verify.rs

@@ -1,10 +1,11 @@
 use std::{
     collections::HashMap,
     sync::Arc,
     time::Duration,
 };
 
 use astria_core::sequencerblock::v1::{
+    block,
     SubmittedMetadata,
     SubmittedRollupData,
 };
@@ -25,7 +26,6 @@
     Client as _,
     HttpClient as SequencerClient,
 };
-use telemetry::display::base64;
 use tokio_util::task::JoinMap;
 use tower::{
     util::BoxService,
@@ -58,7 +58,7 @@
 
 pub(super) struct VerifiedBlobs {
     celestia_height: u64,
-    header_blobs: HashMap<[u8; 32], SubmittedMetadata>,
+    header_blobs: HashMap<block::Hash, SubmittedMetadata>,
     rollup_blobs: Vec<SubmittedRollupData>,
 }
 
@@ -75,7 +75,7 @@
         self,
     ) -> (
         u64,
-        HashMap<[u8; 32], SubmittedMetadata>,
+        HashMap<block::Hash, SubmittedMetadata>,
         Vec<SubmittedRollupData>,
     ) {
         (self.celestia_height, self.header_blobs, self.rollup_blobs)
@@ -88,7 +88,7 @@
 #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
 struct VerificationTaskKey {
     index: usize,
-    block_hash: [u8; 32],
+    block_hash: block::Hash,
     sequencer_height: SequencerHeight,
 }
 
@@ -142,7 +142,7 @@
                         .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 \
@@ -154,7 +154,7 @@
             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"
@@ -552,10 +552,13 @@
     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),

crates/astria-conductor/src/executor/mod.rs

@@ -1,4 +1,4 @@
 use std::collections::HashMap;
 
 use astria_core::{
     execution::v1::{
@@ -7,6 +7,7 @@
     },
     primitive::v1::RollupId,
     sequencerblock::v1::block::{
+        self,
         FilteredSequencerBlock,
         FilteredSequencerBlockParts,
     },
@@ -285,7 +286,7 @@
                 {
                     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 {
@@ -298,7 +299,7 @@
                 {
                     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 {
@@ -388,7 +389,7 @@
     }
 
     #[instrument(skip_all, fields(
-        block.hash = %telemetry::display::base64(&block.block_hash()),
+        block.hash = %block.block_hash(),
         block.height = block.height().value(),
         err,
     ))]
@@ -452,7 +453,7 @@
     }
 
     #[instrument(skip_all, fields(
-        block.hash = %telemetry::display::base64(&block.block_hash),
+        block.hash = %block.block_hash(),
         block.height = block.sequencer_height().value(),
         err,
     ))]
@@ -532,7 +533,7 @@
     /// 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),
@@ -690,7 +691,7 @@
 
 #[derive(Debug)]
 struct ExecutableBlock {
-    hash: [u8; 32],
+    hash: block::Hash,
     height: SequencerHeight,
     timestamp: pbjson_types::Timestamp,
     transactions: Vec<Bytes>,

crates/astria-conductor/tests/blackbox/helpers/mod.rs

@@ -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),
@@ -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 {

crates/astria-core/CHANGELOG.md

@@ -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 appropriate [#1884](https://github.com/astriaorg/astria/pull/1884).
 
 ### Removed