Create a BlockId type for safe conversion to Message (#719)

* Create a BlockId type for safe conversion to Message

* fmt

* Clippy

* Remove direct dep to fuel-crypto, as it's re-exported by fuel-vm

* Allow Into impl for clippy

* Fix lint name

fuel-block-producer/src/block_producer.rs

@@ -236,7 +236,7 @@ impl Producer {
             // TODO: this should use a proper BMT MMR
             let hash = previous_block.id();
             let prev_root = ephemeral_merkle_root(
-                vec![*previous_block.header.prev_root(), hash].iter(),
+                vec![*previous_block.header.prev_root(), hash.into()].iter(),
             );
 
             Ok(PreviousBlockInfo {

fuel-core-interfaces/src/executor.rs

@@ -15,6 +15,7 @@ use crate::{
     },
     db::KvStoreError,
     model::{
+        BlockId,
         FuelBlock,
         PartialFuelBlock,
     },
@@ -136,7 +137,7 @@ pub enum Error {
 
 impl ExecutionBlock {
     /// Get the hash of the full [`FuelBlock`] if validating.
-    pub fn id(&self) -> Option<Bytes32> {
+    pub fn id(&self) -> Option<BlockId> {
         match self {
             ExecutionTypes::Production(_) => None,
             ExecutionTypes::Validation(v) => Some(v.id()),

fuel-core-interfaces/src/model.rs

@@ -11,6 +11,7 @@ use crate::common::fuel_types::{
     Bytes32,
 };
 pub use block::{
+    BlockId,
     ConsensusType,
     FuelApplicationHeader,
     FuelBlock,

fuel-core-interfaces/src/model/block.rs

@@ -16,14 +16,52 @@ use chrono::{
 };
 use core::ops::Deref;
 use fuel_vm::{
+    fuel_crypto,
     fuel_merkle,
     fuel_types::MessageId,
     prelude::Signature,
 };
+use std::fmt;
 
 #[cfg(any(test, feature = "test-helpers"))]
 use chrono::TimeZone;
 
+/// A cryptographically secure hash, identifying a block.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord, Hash, Default)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
+#[cfg_attr(feature = "serde", serde(transparent))]
+#[repr(transparent)]
+pub struct BlockId(Bytes32);
+
+impl BlockId {
+    /// Converts the hash into a message having the same bytes.
+    pub fn into_message(self) -> fuel_crypto::Message {
+        // This is safe because BlockId is a cryptographically secure hash.
+        unsafe { fuel_crypto::Message::from_bytes_unchecked(*self.0) }
+        // Without this, the signature would be using a hash of the id making it more
+        // difficult to verify.
+    }
+}
+
+impl From<Bytes32> for BlockId {
+    fn from(bytes: Bytes32) -> Self {
+        Self(bytes)
+    }
+}
+
+#[allow(clippy::from_over_into)] // Avoids circular dependency
+impl Into<Bytes32> for BlockId {
+    fn into(self) -> Bytes32 {
+        self.0
+    }
+}
+
+impl fmt::LowerHex for BlockId {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        fmt::LowerHex::fmt(&self.0, f)
+    }
+}
+
 #[derive(Clone, Debug)]
 #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))]
@@ -120,7 +158,7 @@ pub struct GeneratedConsensusFields {
 /// Extra data that is not actually part of the header.
 pub struct HeaderMetadata {
     /// Hash of the header.
-    id: Bytes32,
+    id: BlockId,
 }
 
 #[derive(Debug, Clone, Copy, PartialEq, Eq)]
@@ -182,13 +220,13 @@ impl FuelBlockHeader {
     }
 
     /// Get the hash of the fuel header.
-    pub fn hash(&self) -> Bytes32 {
+    pub fn hash(&self) -> BlockId {
         // This internally hashes the hash of the application header.
         self.consensus.hash()
     }
 
     /// Get the cached fuel header hash.
-    pub fn id(&self) -> Bytes32 {
+    pub fn id(&self) -> BlockId {
         if let Some(ref metadata) = self.metadata {
             metadata.id
         } else {
@@ -299,14 +337,14 @@ impl FuelApplicationHeader<GeneratedApplicationFields> {
 
 impl FuelConsensusHeader<GeneratedConsensusFields> {
     /// Hash the consensus header.
-    fn hash(&self) -> Bytes32 {
+    fn hash(&self) -> BlockId {
         // Order matters and is the same as the spec.
         let mut hasher = Hasher::default();
         hasher.input(self.prev_root.as_ref());
         hasher.input(&self.height.to_bytes()[..]);
         hasher.input(self.time.timestamp_millis().to_be_bytes());
         hasher.input(self.application_hash.as_ref());
-        hasher.digest()
+        BlockId(hasher.digest())
     }
 }
 
@@ -367,7 +405,7 @@ pub struct FuelBlockDb {
 
 impl FuelBlockDb {
     /// Hash of the header.
-    pub fn id(&self) -> Bytes32 {
+    pub fn id(&self) -> BlockId {
         self.header.id()
     }
 
@@ -458,7 +496,7 @@ impl FuelBlock {
     }
 
     /// Get the hash of the header.
-    pub fn id(&self) -> Bytes32 {
+    pub fn id(&self) -> BlockId {
         self.header.id()
     }
 

fuel-core-interfaces/src/poa_coordinator.rs

@@ -1,8 +1,8 @@
 use anyhow::Result;
-use fuel_vm::fuel_tx::Bytes32;
 
 use crate::model::{
     BlockHeight,
+    BlockId,
     FuelBlockConsensus,
 };
 
@@ -12,7 +12,7 @@ pub trait BlockDb: Send + Sync {
     // Returns error if already sealed
     fn seal_block(
         &mut self,
-        block_id: Bytes32,
+        block_id: BlockId,
         consensus: FuelBlockConsensus,
     ) -> Result<()>;
 }

fuel-core/src/database.rs

@@ -34,6 +34,7 @@ use fuel_core_interfaces::{
     },
     model::{
         BlockHeight,
+        BlockId,
         ConsensusType,
         FuelBlockDb,
         SealedFuelBlock,
@@ -313,18 +314,18 @@ impl BlockDb for Database {
 
     fn seal_block(
         &mut self,
-        block_id: Bytes32,
+        block_id: BlockId,
         consensus: FuelBlockConsensus,
     ) -> anyhow::Result<()> {
         if self
             .storage::<SealedBlockConsensus>()
-            .contains_key(&block_id)?
+            .contains_key(&block_id.into())?
         {
-            return Err(anyhow!("block {} is already sealed", &block_id))
+            return Err(anyhow!("block {:x} is already sealed", &block_id))
         }
 
         self.storage::<SealedBlockConsensus>()
-            .insert(&block_id, &consensus)
+            .insert(&block_id.into(), &consensus)
             .map(|_| ())
             .map_err(Into::into)
     }
@@ -366,11 +367,7 @@ impl InterpreterStorage for Database {
                 // FIXME: Get producer address from block signature.
                 // block_id -> Signature
                 let signature = Signature::default();
-                let message = unsafe {
-                    fuel_core_interfaces::common::fuel_crypto::Message::from_bytes_unchecked(
-                    block.header.id().into(),
-                )
-                };
+                let message = block.header.id().into_message();
                 // TODO: throw an error if public key isn't recoverable
                 //  when implementing signing (https://github.com/FuelLabs/fuel-core/issues/668)
                 let public_key = signature.recover(&message).unwrap_or_default();

fuel-core/src/executor.rs

@@ -60,6 +60,7 @@ use fuel_core_interfaces::{
         TransactionValidityError,
     },
     model::{
+        BlockId,
         DaBlockHeight,
         Message,
         PartialFuelBlock,
@@ -213,7 +214,7 @@ impl Executor {
         block_db_transaction
             .deref_mut()
             .storage::<FuelBlocks>()
-            .insert(&finalized_block_id, &block.to_db_block())?;
+            .insert(&finalized_block_id.into(), &block.to_db_block())?;
 
         // Commit the database transaction.
         block_db_transaction.commit()?;
@@ -880,7 +881,7 @@ impl Executor {
 
     fn persist_transaction_status(
         &self,
-        finalized_block_id: Bytes32,
+        finalized_block_id: BlockId,
         tx_status: &mut [(Bytes32, TransactionStatus)],
         db: &Database,
     ) -> Result<(), Error> {

fuel-core/src/schema/block.rs

@@ -38,6 +38,7 @@ use fuel_core_interfaces::{
     common::{
         fuel_storage::StorageAsRef,
         fuel_types,
+        prelude::Bytes32,
     },
     db::Transactions,
     executor::{
@@ -62,7 +63,8 @@ pub struct Block(pub(crate) FuelBlockDb);
 #[Object]
 impl Block {
     async fn id(&self) -> BlockId {
-        self.0.id().into()
+        let bytes: Bytes32 = self.0.id().into();
+        bytes.into()
     }
 
     async fn height(&self) -> U64 {

fuel-core/src/schema/tx/types.rs

@@ -39,7 +39,6 @@ use fuel_core_interfaces::{
     common::{
         fuel_storage::StorageAsRef,
         fuel_tx,
-        fuel_types,
         fuel_types::bytes::SerializableVec,
         fuel_vm::prelude::ProgramState as VmProgramState,
     },
@@ -113,7 +112,7 @@ impl SubmittedStatus {
 }
 
 pub struct SuccessStatus {
-    block_id: fuel_types::Bytes32,
+    block_id: fuel_core_interfaces::model::BlockId,
     time: DateTime<Utc>,
     result: VmProgramState,
 }
@@ -124,7 +123,7 @@ impl SuccessStatus {
         let db = ctx.data_unchecked::<Database>();
         let block = db
             .storage::<FuelBlocks>()
-            .get(&self.block_id)?
+            .get(&self.block_id.into())?
             .ok_or(KvStoreError::NotFound)?
             .into_owned();
         let block = Block(block);
@@ -141,7 +140,7 @@ impl SuccessStatus {
 }
 
 pub struct FailureStatus {
-    block_id: fuel_types::Bytes32,
+    block_id: fuel_core_interfaces::model::BlockId,
     time: DateTime<Utc>,
     reason: String,
     state: Option<VmProgramState>,
@@ -153,7 +152,7 @@ impl FailureStatus {
         let db = ctx.data_unchecked::<Database>();
         let block = db
             .storage::<FuelBlocks>()
-            .get(&self.block_id)?
+            .get(&self.block_id.into())?
             .ok_or(KvStoreError::NotFound)?
             .into_owned();
         let block = Block(block);

fuel-core/src/tx_pool.rs

@@ -2,9 +2,9 @@ use chrono::{
     DateTime,
     Utc,
 };
-use fuel_core_interfaces::common::{
-    fuel_tx::Bytes32,
-    fuel_vm::prelude::ProgramState,
+use fuel_core_interfaces::{
+    common::fuel_vm::prelude::ProgramState,
+    model::BlockId,
 };
 use serde::{
     Deserialize,
@@ -17,12 +17,12 @@ pub enum TransactionStatus {
         time: DateTime<Utc>,
     },
     Success {
-        block_id: Bytes32,
+        block_id: BlockId,
         time: DateTime<Utc>,
         result: ProgramState,
     },
     Failed {
-        block_id: Bytes32,
+        block_id: BlockId,
         time: DateTime<Utc>,
         reason: String,
         result: Option<ProgramState>,

fuel-poa-coordinator/src/service.rs

@@ -14,7 +14,6 @@ use fuel_core_interfaces::{
     block_importer::ImportBlockBroadcast,
     block_producer::BlockProducer,
     common::{
-        fuel_crypto,
         prelude::{
             SecretKey,
             Signature,
@@ -320,11 +319,9 @@ where
     fn seal_block(&mut self, block: &FuelBlock) -> anyhow::Result<()> {
         if let Some(key) = &self.signing_key {
             let block_hash = block.id();
-            // This is safe because block.id() is a cryptographically secure hash.
             // Without this, the signature would be using a hash of the id making it more
             // difficult to verify.
-            let message =
-                unsafe { fuel_crypto::Message::from_bytes_unchecked(*block_hash) };
+            let message = block_hash.into_message();
 
             // The length of the secret is checked
             let signing_key =

fuel-poa-coordinator/tests/test_trigger.rs

@@ -15,6 +15,7 @@ use fuel_core_interfaces::{
     },
     model::{
         BlockHeight,
+        BlockId,
         FuelBlock,
         FuelBlockConsensus,
         FuelConsensusHeader,
@@ -165,7 +166,7 @@ pub struct MockDatabase {
 #[derive(Default)]
 pub struct MockDatabaseInner {
     height: u32,
-    consensus: HashMap<Bytes32, FuelBlockConsensus>,
+    consensus: HashMap<BlockId, FuelBlockConsensus>,
 }
 
 impl MockDatabase {
@@ -181,7 +182,7 @@ impl BlockDb for MockDatabase {
 
     fn seal_block(
         &mut self,
-        block_id: Bytes32,
+        block_id: BlockId,
         consensus: FuelBlockConsensus,
     ) -> anyhow::Result<()> {
         if self.inner.read().consensus.contains_key(&block_id) {
@@ -516,7 +517,7 @@ async fn instant_trigger_produces_block_instantly() -> anyhow::Result<()> {
                 }
                 .public_key();
 
-                let message = unsafe { Message::from_bytes_unchecked((*id).into()) };
+                let message = id.into_message();
 
                 poa.signature
                     .verify(&pk, &message)