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

Create a BlockId type for safe conversion to Message #719

Merged
merged 6 commits into from
Oct 25, 2022
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
2 changes: 1 addition & 1 deletion fuel-block-producer/src/block_producer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 2 additions & 1 deletion fuel-core-interfaces/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::{
},
db::KvStoreError,
model::{
BlockId,
FuelBlock,
PartialFuelBlock,
},
Expand Down Expand Up @@ -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()),
Expand Down
1 change: 1 addition & 0 deletions fuel-core-interfaces/src/model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use crate::common::fuel_types::{
Bytes32,
};
pub use block::{
BlockId,
ConsensusType,
FuelApplicationHeader,
FuelBlock,
Expand Down
52 changes: 45 additions & 7 deletions fuel-core-interfaces/src/model/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))]
Expand Down Expand Up @@ -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)]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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()
}

Expand Down Expand Up @@ -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()
}

Expand Down
4 changes: 2 additions & 2 deletions fuel-core-interfaces/src/poa_coordinator.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use anyhow::Result;
use fuel_vm::fuel_tx::Bytes32;

use crate::model::{
BlockHeight,
BlockId,
FuelBlockConsensus,
};

Expand All @@ -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<()>;
}
Expand Down
15 changes: 6 additions & 9 deletions fuel-core/src/database.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use fuel_core_interfaces::{
},
model::{
BlockHeight,
BlockId,
ConsensusType,
FuelBlockDb,
SealedFuelBlock,
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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();
Expand Down
5 changes: 3 additions & 2 deletions fuel-core/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ use fuel_core_interfaces::{
TransactionValidityError,
},
model::{
BlockId,
DaBlockHeight,
Message,
PartialFuelBlock,
Expand Down Expand Up @@ -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()?;
Expand Down Expand Up @@ -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> {
Expand Down
4 changes: 3 additions & 1 deletion fuel-core/src/schema/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ use fuel_core_interfaces::{
common::{
fuel_storage::StorageAsRef,
fuel_types,
prelude::Bytes32,
},
db::Transactions,
executor::{
Expand All @@ -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 {
Expand Down
9 changes: 4 additions & 5 deletions fuel-core/src/schema/tx/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down Expand Up @@ -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,
}
Expand All @@ -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);
Expand All @@ -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>,
Expand All @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions fuel-core/src/tx_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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>,
Expand Down
5 changes: 1 addition & 4 deletions fuel-poa-coordinator/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ use fuel_core_interfaces::{
block_importer::ImportBlockBroadcast,
block_producer::BlockProducer,
common::{
fuel_crypto,
prelude::{
SecretKey,
Signature,
Expand Down Expand Up @@ -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 =
Expand Down
7 changes: 4 additions & 3 deletions fuel-poa-coordinator/tests/test_trigger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use fuel_core_interfaces::{
},
model::{
BlockHeight,
BlockId,
FuelBlock,
FuelBlockConsensus,
FuelConsensusHeader,
Expand Down Expand Up @@ -165,7 +166,7 @@ pub struct MockDatabase {
#[derive(Default)]
pub struct MockDatabaseInner {
height: u32,
consensus: HashMap<Bytes32, FuelBlockConsensus>,
consensus: HashMap<BlockId, FuelBlockConsensus>,
}

impl MockDatabase {
Expand All @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
Loading