Skip to content

Commit

Permalink
Fix BlockHeader hash (#1868)
Browse files Browse the repository at this point in the history
- Add `event_inbox_root` to hash calcuation
- Use `to_be_bytes` instead of `to_bytes` to avoid unnecessary padding
of `u32` to `u64`.

## Checklist
- [ ] Breaking changes are clearly marked as such in the PR description
and changelog
- [ ] New behavior is reflected in tests
- [x] [The specification](https://github.com/FuelLabs/fuel-specs/)
matches the implemented behavior (link update PR if changes are needed)

### Before requesting review
- [x] I have reviewed the code myself
- [ ] I have created follow-up issues caused by this PR and linked them
here

### After merging, notify other teams

[Add or remove entries as needed]

- [ ] [Rust SDK](https://github.com/FuelLabs/fuels-rs/)
- [ ] [Sway compiler](https://github.com/FuelLabs/sway/)
- [ ] [Platform
documentation](https://github.com/FuelLabs/devrel-requests/issues/new?assignees=&labels=new+request&projects=&template=NEW-REQUEST.yml&title=%5BRequest%5D%3A+)
(for out-of-organization contributors, the person merging the PR will do
this)
- [ ] Someone else?

---------

Co-authored-by: Green Baneling <XgreenX9999@gmail.com>
  • Loading branch information
MitchTurner and xgreenx committed Apr 29, 2024
1 parent 6f5c4fc commit 5eca451
Show file tree
Hide file tree
Showing 22 changed files with 115 additions and 42 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

### Fixed

#### Breaking

- [#1868](https://github.com/FuelLabs/fuel-core/pull/1868): Include the `event_inbox_root` in the header hash. Changed types of the `transactions_count` to `u16` and `message_receipt_count` to `u32` instead of `u64`. Updated the application hash root calculation to not pad numbers.
- [#1866](https://github.com/FuelLabs/fuel-core/pull/1866): Fixed a runtime panic that occurred when restarting a node. The panic happens when the relayer database is already populated, and the relayer attempts an empty commit during start up. This invalid commit is removed in this PR.
- [#1871](https://github.com/FuelLabs/fuel-core/pull/1871): Fixed `block` endpoint to return fetch the blocks from both databases after regenesis.
- [#1856](https://github.com/FuelLabs/fuel-core/pull/1856): Replaced instances of `Union` with `Enum` for GraphQL definitions of `ConsensusParametersVersion` and related types. This is needed because `Union` does not support multiple `Version`s inside discriminants or empty variants.
Expand Down
Binary file not shown.
Binary file modified bin/fuel-core/chainspec/testnet/state_transition_bytecode.wasm
Binary file not shown.
2 changes: 1 addition & 1 deletion bin/fuel-core/src/cli/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ impl Command {
}

pub fn get_service(command: Command) -> anyhow::Result<FuelService> {
#[cfg(any(feature = "rocks-db", feature = "rocksdb-production"))]
#[cfg(any(feature = "rocksdb", feature = "rocksdb-production"))]
if command.db_prune && command.database_path.exists() {
fuel_core::combined_database::CombinedDatabase::prune(&command.database_path)?;
}
Expand Down
4 changes: 3 additions & 1 deletion crates/chain-config/src/config/randomize.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,9 @@ impl Randomize for CompressedBlock {
let tx_ids = vec![tx1.id(&ChainId::default()), tx2.id(&ChainId::default())];

Self::test(
PartialBlockHeader::default().generate(&[tx1, tx2], &[], rng.gen()),
PartialBlockHeader::default()
.generate(&[tx1, tx2], &[], rng.gen())
.expect("The header is valid"),
tx_ids,
)
}
Expand Down
4 changes: 2 additions & 2 deletions crates/client/assets/schema.sdl
Original file line number Diff line number Diff line change
Expand Up @@ -494,11 +494,11 @@ type Header {
"""
Number of transactions in this block.
"""
transactionsCount: U64!
transactionsCount: U16!
"""
Number of message receipts in this block.
"""
messageReceiptCount: U64!
messageReceiptCount: U32!
"""
Merkle root of transactions.
"""
Expand Down
5 changes: 3 additions & 2 deletions crates/client/src/client/schema/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use crate::client::schema::{
PageInfo,
Signature,
Tai64Timestamp,
U16,
U32,
U64,
};
Expand Down Expand Up @@ -121,8 +122,8 @@ pub struct Header {
pub da_height: U64,
pub consensus_parameters_version: U32,
pub state_transition_bytecode_version: U32,
pub transactions_count: U64,
pub message_receipt_count: U64,
pub transactions_count: U16,
pub message_receipt_count: U32,
pub transactions_root: Bytes32,
pub message_outbox_root: Bytes32,
pub event_inbox_root: Bytes32,
Expand Down
4 changes: 2 additions & 2 deletions crates/client/src/client/types/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,8 @@ pub struct Header {
pub da_height: u64,
pub consensus_parameters_version: u32,
pub state_transition_bytecode_version: u32,
pub transactions_count: u64,
pub message_receipt_count: u64,
pub transactions_count: u16,
pub message_receipt_count: u32,
pub transactions_root: MerkleRoot,
pub message_outbox_root: MerkleRoot,
pub event_inbox_root: MerkleRoot,
Expand Down
2 changes: 1 addition & 1 deletion crates/fuel-core/src/database/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ mod tests {
},
};
let block = PartialFuelBlock::new(header, vec![]);
block.generate(&[], Default::default())
block.generate(&[], Default::default()).unwrap()
})
.collect::<Vec<_>>();

Expand Down
6 changes: 4 additions & 2 deletions crates/fuel-core/src/query/message/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,8 @@ async fn can_build_message_proof() {
generated: Default::default(),
},
}
.generate(&[], &[], Default::default());
.generate(&[], &[], Default::default())
.unwrap();
let commit_block = CompressedBlock::test(commit_block_header, vec![]);
let message_block_header = PartialBlockHeader {
application: ApplicationHeader {
Expand All @@ -157,7 +158,8 @@ async fn can_build_message_proof() {
generated: Default::default(),
},
}
.generate(&[], &message_ids, Default::default());
.generate(&[], &message_ids, Default::default())
.unwrap();
let message_block = CompressedBlock::test(message_block_header, TXNS.to_vec());

let block_proof = MerkleProof {
Expand Down
5 changes: 3 additions & 2 deletions crates/fuel-core/src/schema/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use crate::{
scalars::{
BlockId,
Signature,
U16,
U32,
U64,
},
Expand Down Expand Up @@ -148,12 +149,12 @@ impl Header {
}

/// Number of transactions in this block.
async fn transactions_count(&self) -> U64 {
async fn transactions_count(&self) -> U16 {
self.0.transactions_count.into()
}

/// Number of message receipts in this block.
async fn message_receipt_count(&self) -> U64 {
async fn message_receipt_count(&self) -> U32 {
self.0.message_receipt_count.into()
}

Expand Down
1 change: 1 addition & 0 deletions crates/fuel-core/src/service/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@ pub fn create_genesis_block(config: &Config) -> Block {
message_ids,
events,
)
.expect("The block is valid; qed")
}

#[cfg(test)]
Expand Down
4 changes: 3 additions & 1 deletion crates/services/consensus_module/poa/src/verifier/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@ fn correct() -> Input {
..Default::default()
},
};
let block_header = partial_header.generate(&txs, &[], Default::default());
let block_header = partial_header
.generate(&txs, &[], Default::default())
.unwrap();

Input {
block_header_merkle_root: [2u8; 32],
Expand Down
8 changes: 6 additions & 2 deletions crates/services/executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,9 @@ where

// Now that the transactions have been executed, generate the full header.

let block = block.generate(&message_ids[..], event_inbox_root);
let block = block
.generate(&message_ids[..], event_inbox_root)
.map_err(ExecutorError::BlockHeaderError)?;

let finalized_block_id = block.id();

Expand Down Expand Up @@ -802,7 +804,9 @@ where
}
})?;

let new_block = new_partial_block.generate(&message_ids[..], *event_inbox_root);
let new_block = new_partial_block
.generate(&message_ids[..], *event_inbox_root)
.map_err(ExecutorError::BlockHeaderError)?;
if new_block.header() != old_block.header() {
Err(ExecutorError::BlockMismatch)
} else {
Expand Down
4 changes: 4 additions & 0 deletions crates/services/producer/src/block_producer/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ mod produce_and_execute_block_txpool {
transactions: vec![],
}
.generate(&[], Default::default())
.unwrap()
.compress(&Default::default());

let db = MockDb {
Expand Down Expand Up @@ -165,6 +166,7 @@ mod produce_and_execute_block_txpool {
transactions: vec![],
}
.generate(&[], Default::default())
.unwrap()
.compress(&Default::default());

// Given
Expand Down Expand Up @@ -217,6 +219,7 @@ mod produce_and_execute_block_txpool {
transactions: vec![],
}
.generate(&[], Default::default())
.unwrap()
.compress(&Default::default());

// Given
Expand Down Expand Up @@ -698,6 +701,7 @@ impl TestContextBuilder {
transactions: vec![],
}
.generate(&[], Default::default())
.unwrap()
.compress(&Default::default());

let db = MockDb {
Expand Down
1 change: 1 addition & 0 deletions crates/services/producer/src/mocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ fn to_block(component: &Components<Vec<ArcPoolTx>>) -> Block {
&[],
Default::default(),
)
.unwrap()
}

impl Executor<Vec<ArcPoolTx>> for MockExecutor {
Expand Down
1 change: 1 addition & 0 deletions crates/services/upgradable-executor/src/executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -733,6 +733,7 @@ mod test {
.into()],
)
.generate(&[], Bytes32::zeroed())
.unwrap()
}

#[cfg(not(feature = "wasm-executor"))]
Expand Down
2 changes: 1 addition & 1 deletion crates/storage/src/structured_storage/blocks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ mod tests {
},
};
let block = PartialFuelBlock::new(header, vec![]);
block.generate(&[], Default::default())
block.generate(&[], Default::default()).unwrap()
})
.collect::<Vec<_>>();

Expand Down
17 changes: 12 additions & 5 deletions crates/types/src/blockchain/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ use super::{
},
};
use crate::{
blockchain::header::BlockHeaderV1,
blockchain::header::{
BlockHeaderError,
BlockHeaderV1,
},
fuel_tx::{
Transaction,
TxId,
Expand Down Expand Up @@ -88,12 +91,16 @@ impl Block<Transaction> {
transactions: Vec<Transaction>,
outbox_message_ids: &[MessageId],
event_inbox_root: Bytes32,
) -> Self {
) -> Result<Self, BlockHeaderError> {
let inner = BlockV1 {
header: header.generate(&transactions, outbox_message_ids, event_inbox_root),
header: header.generate(
&transactions,
outbox_message_ids,
event_inbox_root,
)?,
transactions,
};
Block::V1(inner)
Ok(Block::V1(inner))
}

/// Try creating a new full fuel block from a [`BlockHeader`] and
Expand Down Expand Up @@ -224,7 +231,7 @@ impl PartialFuelBlock {
self,
outbox_message_ids: &[MessageId],
event_inbox_root: Bytes32,
) -> Block {
) -> Result<Block, BlockHeaderError> {
Block::new(
self.header,
self.transactions,
Expand Down
75 changes: 57 additions & 18 deletions crates/types/src/blockchain/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,9 @@ pub struct ApplicationHeader<Generated> {
/// These are generated once the full block has been run.
pub struct GeneratedApplicationFields {
/// Number of transactions in this block.
pub transactions_count: u64,
pub transactions_count: u16,
/// Number of message receipts in this block.
pub message_receipt_count: u64,
pub message_receipt_count: u32,
/// Merkle root of transactions.
pub transactions_root: Bytes32,
/// Merkle root of message receipts in this block.
Expand Down Expand Up @@ -373,10 +373,21 @@ impl BlockHeader {
let transactions_root = generate_txns_root(transactions);

transactions_root == self.application().transactions_root
&& transactions.len() as u64 == self.application().transactions_count
&& transactions.len() == self.application().transactions_count as usize
}
}

/// Error type for generating a full [`BlockHeader`] from a [`PartialBlockHeader`].
#[derive(derive_more::Display, Debug, Copy, Clone, PartialEq, Eq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
#[allow(missing_docs)]
pub enum BlockHeaderError {
#[display(fmt = "The block header has too many transactions to fit into the `u16`")]
TooManyTransactions,
#[display(fmt = "The block header has too many messages to fit into the `u32`")]
TooManyMessages,
}

impl PartialBlockHeader {
/// Generate all fields to create a full [`BlockHeader`]
/// after running the transactions.
Expand All @@ -395,7 +406,7 @@ impl PartialBlockHeader {
transactions: &[Transaction],
outbox_message_ids: &[MessageId],
event_inbox_root: Bytes32,
) -> BlockHeader {
) -> Result<BlockHeader, BlockHeaderError> {
// Generate the transaction merkle root.
let transactions_root = generate_txns_root(transactions);

Expand All @@ -416,8 +427,10 @@ impl PartialBlockHeader {
.application
.state_transition_bytecode_version,
generated: GeneratedApplicationFields {
transactions_count: transactions.len() as u64,
message_receipt_count: outbox_message_ids.len() as u64,
transactions_count: u16::try_from(transactions.len())
.map_err(|_| BlockHeaderError::TooManyTransactions)?,
message_receipt_count: u32::try_from(outbox_message_ids.len())
.map_err(|_| BlockHeaderError::TooManyMessages)?,
transactions_root,
message_outbox_root,
event_inbox_root,
Expand All @@ -441,7 +454,7 @@ impl PartialBlockHeader {

// Cache the hash.
header.recalculate_metadata();
header
Ok(header)
}
}

Expand All @@ -461,13 +474,30 @@ impl ApplicationHeader<GeneratedApplicationFields> {
pub fn hash(&self) -> Bytes32 {
// Order matters and is the same as the spec.
let mut hasher = crate::fuel_crypto::Hasher::default();
hasher.input(self.da_height.to_bytes().as_slice());
hasher.input(self.transactions_count.to_be_bytes());
hasher.input(self.message_receipt_count.to_be_bytes());
hasher.input(self.transactions_root.as_ref());
hasher.input(self.message_outbox_root.as_ref());
hasher.input(self.consensus_parameters_version.to_bytes().as_slice());
hasher.input(self.state_transition_bytecode_version.to_bytes().as_slice());
let Self {
da_height,
consensus_parameters_version,
state_transition_bytecode_version,
generated:
GeneratedApplicationFields {
transactions_count,
message_receipt_count,
transactions_root,
message_outbox_root,
event_inbox_root,
},
} = self;

hasher.input(da_height.to_be_bytes());
hasher.input(consensus_parameters_version.to_be_bytes());
hasher.input(state_transition_bytecode_version.to_be_bytes());

hasher.input(transactions_count.to_be_bytes());
hasher.input(message_receipt_count.to_be_bytes());
hasher.input(transactions_root.as_ref());
hasher.input(message_outbox_root.as_ref());
hasher.input(event_inbox_root.as_ref());

hasher.digest()
}
}
Expand All @@ -477,10 +507,19 @@ impl ConsensusHeader<GeneratedConsensusFields> {
pub fn hash(&self) -> BlockId {
// Order matters and is the same as the spec.
let mut hasher = crate::fuel_crypto::Hasher::default();
hasher.input(self.prev_root.as_ref());
hasher.input(&self.height.to_bytes()[..]);
hasher.input(self.time.0.to_be_bytes());
hasher.input(self.application_hash.as_ref());
let Self {
prev_root,
height,
time,
generated: GeneratedConsensusFields { application_hash },
} = self;

hasher.input(prev_root.as_ref());
hasher.input(height.to_be_bytes());
hasher.input(time.0.to_be_bytes());

hasher.input(application_hash.as_ref());

BlockId::from(hasher.digest())
}
}
Expand Down
Loading

0 comments on commit 5eca451

Please sign in to comment.