From d257e9a4c73b6a39b69b27fd68f9097c07b86ac1 Mon Sep 17 00:00:00 2001 From: Turner Date: Tue, 23 Jan 2024 16:26:40 -0800 Subject: [PATCH 1/5] Make block header a versionable enum --- crates/fuel-core/src/schema/dap.rs | 2 +- .../consensus_module/poa/src/verifier.rs | 2 +- crates/services/sync/src/import.rs | 2 +- .../services/sync/src/import/test_helpers.rs | 4 +- crates/types/src/blockchain/block.rs | 5 +- crates/types/src/blockchain/header.rs | 96 +++++++++++++++---- 6 files changed, 88 insertions(+), 23 deletions(-) diff --git a/crates/fuel-core/src/schema/dap.rs b/crates/fuel-core/src/schema/dap.rs index 832d92a1339..0232fa06f23 100644 --- a/crates/fuel-core/src/schema/dap.rs +++ b/crates/fuel-core/src/schema/dap.rs @@ -163,7 +163,7 @@ impl ConcreteStorage { let vm_database = VmStorage::new( storage.as_ref().clone(), - &block.header().consensus, + &block.header().consensus(), // TODO: Use a real coinbase address Default::default(), ); diff --git a/crates/services/consensus_module/poa/src/verifier.rs b/crates/services/consensus_module/poa/src/verifier.rs index 738c04f7681..9da54343d47 100644 --- a/crates/services/consensus_module/poa/src/verifier.rs +++ b/crates/services/consensus_module/poa/src/verifier.rs @@ -62,7 +62,7 @@ pub fn verify_block_fields( ); ensure!( - header.consensus.application_hash == header.application.hash(), + header.consensus().application_hash == header.application().hash(), "The application hash mismatch." ); diff --git a/crates/services/sync/src/import.rs b/crates/services/sync/src/import.rs index c02f1d920a5..bc21bc489a2 100644 --- a/crates/services/sync/src/import.rs +++ b/crates/services/sync/src/import.rs @@ -552,7 +552,7 @@ where skip_all, fields( height = **block.entity.header().height(), - id = %block.entity.header().consensus.generated.application_hash + id = %block.entity.header().consensus().generated.application_hash ), err )] diff --git a/crates/services/sync/src/import/test_helpers.rs b/crates/services/sync/src/import/test_helpers.rs index 5a9d37ad477..583357262c4 100644 --- a/crates/services/sync/src/import/test_helpers.rs +++ b/crates/services/sync/src/import/test_helpers.rs @@ -42,11 +42,11 @@ pub fn random_peer() -> PeerId { pub fn empty_header>(i: I) -> SealedBlockHeader { let mut header = BlockHeader::default(); let height = i.into(); - header.consensus.height = height; + header.consensus_mut().height = height; let transaction_tree = fuel_core_types::fuel_merkle::binary::root_calculator::MerkleRootCalculator::new( ); - header.application.generated.transactions_root = transaction_tree.root().into(); + header.application_mut().generated.transactions_root = transaction_tree.root().into(); let consensus = Consensus::default(); Sealed { diff --git a/crates/types/src/blockchain/block.rs b/crates/types/src/blockchain/block.rs index 521ad5516e2..92a0212e6e0 100644 --- a/crates/types/src/blockchain/block.rs +++ b/crates/types/src/blockchain/block.rs @@ -14,6 +14,7 @@ use super::{ }, }; use crate::{ + blockchain::header::BlockHeaderV1, fuel_tx::{ Transaction, TxId, @@ -227,7 +228,7 @@ impl From for PartialFuelBlock { match block { Block::V1(BlockV1 { header: - BlockHeader { + BlockHeader::V1(BlockHeaderV1 { application: ApplicationHeader { da_height, .. }, consensus: ConsensusHeader { @@ -237,7 +238,7 @@ impl From for PartialFuelBlock { .. }, .. - }, + }), transactions, }) => Self { header: PartialBlockHeader { diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index 284fcebcb45..4e48095bc9c 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -20,12 +20,22 @@ use crate::{ }; use tai64::Tai64; +/// Version-able block header type +#[derive(Clone, Debug, derivative::Derivative)] +#[derivative(PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[non_exhaustive] +pub enum BlockHeader { + /// V1 BlockHeader + V1(BlockHeaderV1), +} + /// A fuel block header that has all the fields generated because it /// has been executed. #[derive(Clone, Debug, derivative::Derivative)] #[derivative(PartialEq, Eq)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct BlockHeader { +pub struct BlockHeaderV1 { /// The application header. pub application: ApplicationHeader, /// The consensus header. @@ -37,6 +47,58 @@ pub struct BlockHeader { metadata: Option, } +impl From for BlockHeader { + fn from(v1: BlockHeaderV1) -> Self { + BlockHeader::V1(v1) + } +} + +impl BlockHeader { + /// Getter for consensus portion of header + pub fn consensus(&self) -> &ConsensusHeader { + match self { + BlockHeader::V1(v1) => &v1.consensus, + } + } + + /// Getter for application portion of header + pub fn application(&self) -> &ApplicationHeader { + match self { + BlockHeader::V1(v1) => &v1.application, + } + } + + /// Getter for metadata portion of header + pub fn metadata(&self) -> &Option { + match self { + BlockHeader::V1(v1) => &v1.metadata, + } + } + + /// Mutable getter for consensus portion of header + pub fn consensus_mut(&mut self) -> &mut ConsensusHeader { + match self { + BlockHeader::V1(v1) => &mut v1.consensus, + } + } + + /// Mutable getter for application portion of header + pub fn application_mut( + &mut self, + ) -> &mut ApplicationHeader { + match self { + BlockHeader::V1(v1) => &mut v1.application, + } + } + + /// Mutable getter for metadata portion of header + pub fn metadata_mut(&mut self) -> &mut Option { + match self { + BlockHeader::V1(v1) => &mut v1.metadata, + } + } +} + #[derive(Clone, Debug)] #[cfg_attr(any(test, feature = "test-helpers"), derive(Default))] /// A partially complete fuel block header that doesn't not @@ -118,11 +180,12 @@ pub struct BlockHeaderMetadata { #[cfg(any(test, feature = "test-helpers"))] impl Default for BlockHeader { fn default() -> Self { - let mut default = Self { + let mut default: BlockHeader = BlockHeaderV1 { application: Default::default(), consensus: Default::default(), metadata: None, - }; + } + .into(); default.recalculate_metadata(); default } @@ -134,8 +197,8 @@ impl BlockHeader { /// The method should be used only for tests. pub fn new_block(height: BlockHeight, time: Tai64) -> Self { let mut default = Self::default(); - default.consensus.height = height; - default.consensus.time = time; + default.consensus_mut().height = height; + default.consensus_mut().time = time; default.recalculate_metadata(); default } @@ -189,9 +252,9 @@ impl PartialBlockHeader { impl BlockHeader { /// Re-generate the header metadata. pub fn recalculate_metadata(&mut self) { - let application_hash = self.application.hash(); - self.consensus.generated.application_hash = application_hash; - self.metadata = Some(BlockHeaderMetadata { id: self.hash() }); + let application_hash = self.application().hash(); + self.consensus_mut().generated.application_hash = application_hash; + *self.metadata_mut() = Some(BlockHeaderMetadata { id: self.hash() }); } /// Get the hash of the fuel header. @@ -201,14 +264,14 @@ impl BlockHeader { // and can't change its final hash on the fly. // // This assertion is a double-checks that this behavior is not changed. - debug_assert_eq!(self.consensus.application_hash, self.application.hash()); + debug_assert_eq!(self.consensus().application_hash, self.application().hash()); // This internally hashes the hash of the application header. - self.consensus.hash() + self.consensus().hash() } /// Get the cached fuel header hash. pub fn id(&self) -> BlockId { - if let Some(ref metadata) = self.metadata { + if let Some(ref metadata) = self.metadata() { metadata.id } else { self.hash() @@ -220,7 +283,7 @@ impl BlockHeader { // Generate the transaction merkle root. let transactions_root = generate_txns_root(transactions); - transactions_root == self.application.transactions_root + transactions_root == self.application().transactions_root } } @@ -263,7 +326,7 @@ impl PartialBlockHeader { }, }; - let mut header = BlockHeader { + let mut header: BlockHeader = BlockHeaderV1 { application, consensus: ConsensusHeader { prev_root: self.consensus.prev_root, @@ -275,7 +338,8 @@ impl PartialBlockHeader { }, }, metadata: None, - }; + } + .into(); // Cache the hash. header.recalculate_metadata(); @@ -340,7 +404,7 @@ impl core::ops::Deref for BlockHeader { type Target = ApplicationHeader; fn deref(&self) -> &Self::Target { - &self.application + &self.application() } } @@ -370,7 +434,7 @@ impl core::ops::Deref for ConsensusHeader { impl core::convert::AsRef> for BlockHeader { fn as_ref(&self) -> &ConsensusHeader { - &self.consensus + &self.consensus() } } From 725f9f6e42cea54292b71e147bb0a7bc32fc7ee0 Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 24 Jan 2024 10:11:54 -0800 Subject: [PATCH 2/5] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b791cb34ec..9e5c88ae765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,7 @@ Description of the upcoming release here. - [#1601](https://github.com/FuelLabs/fuel-core/pull/1601): Fix formatting in docs and check that `cargo doc` passes in the CI. #### Breaking +- [#1616](https://github.com/FuelLabs/fuel-core/pull/1616) Make `BlockHeader` type a version-able enum - [#1596](https://github.com/FuelLabs/fuel-core/pull/1596) Make `Consensus` type a version-able enum - [#1593](https://github.com/FuelLabs/fuel-core/pull/1593) Make `Block` type a version-able enum - [#1576](https://github.com/FuelLabs/fuel-core/pull/1576): The change moves the implementation of the storage traits for required tables from `fuel-core` to `fuel-core-storage` crate. The change also adds a more flexible configuration of the encoding/decoding per the table and allows the implementation of specific behaviors for the table in a much easier way. It unifies the encoding between database, SMTs, and iteration, preventing mismatching bytes representation on the Rust type system level. Plus, it increases the re-usage of the code by applying the same blueprint to other tables. From f76202b6949988faac9f7fd373bc2397912777de Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 24 Jan 2024 10:38:12 -0800 Subject: [PATCH 3/5] Fix ci_checks --- crates/fuel-core/src/executor.rs | 10 ++++-- crates/fuel-core/src/schema/dap.rs | 2 +- crates/fuel-core/src/service/adapters/p2p.rs | 2 +- .../service_test/manually_produce_tests.rs | 2 +- .../poa/src/verifier/tests.rs | 8 ++--- .../src/block_verifier/tests.rs | 36 +++++++++---------- crates/services/importer/src/importer/test.rs | 6 ++-- crates/services/p2p/src/p2p_service.rs | 6 ++-- crates/types/src/blockchain/header.rs | 4 +-- 9 files changed, 40 insertions(+), 36 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 8b74df131b1..9b4a77d0905 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -371,7 +371,7 @@ mod tests { let invalid_duplicate_tx = script.clone().into(); let mut block = Block::default(); - block.header_mut().consensus.height = 1.into(); + block.header_mut().consensus_mut().height = 1.into(); *block.transactions_mut() = vec![script.into(), invalid_duplicate_tx]; block.header_mut().recalculate_metadata(); @@ -439,7 +439,7 @@ mod tests { .max_fee(); let mut block = Block::default(); - block.header_mut().consensus.height = 2.into(); + block.header_mut().consensus_mut().height = 2.into(); *block.transactions_mut() = vec![script.into()]; block.header_mut().recalculate_metadata(); @@ -1174,7 +1174,11 @@ mod tests { .unwrap(); // randomize transaction commitment - block.header_mut().application.generated.transactions_root = rng.gen(); + block + .header_mut() + .application_mut() + .generated + .transactions_root = rng.gen(); block.header_mut().recalculate_metadata(); let verify_result = verifier diff --git a/crates/fuel-core/src/schema/dap.rs b/crates/fuel-core/src/schema/dap.rs index 0232fa06f23..fc3df100e27 100644 --- a/crates/fuel-core/src/schema/dap.rs +++ b/crates/fuel-core/src/schema/dap.rs @@ -163,7 +163,7 @@ impl ConcreteStorage { let vm_database = VmStorage::new( storage.as_ref().clone(), - &block.header().consensus(), + block.header().consensus(), // TODO: Use a real coinbase address Default::default(), ); diff --git a/crates/fuel-core/src/service/adapters/p2p.rs b/crates/fuel-core/src/service/adapters/p2p.rs index 35dbac0f918..dc4e78bffbe 100644 --- a/crates/fuel-core/src/service/adapters/p2p.rs +++ b/crates/fuel-core/src/service/adapters/p2p.rs @@ -55,7 +55,7 @@ impl BlockHeightImporter for BlockImporterAdapter { Box::pin( BroadcastStream::new(self.block_importer.subscribe()) .filter_map(|result| result.ok()) - .map(|result| result.sealed_block.entity.header().consensus.height), + .map(|result| result.sealed_block.entity.header().consensus().height), ) } } diff --git a/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs b/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs index 3699fffb39b..f3a1d1a0c16 100644 --- a/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs +++ b/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs @@ -85,7 +85,7 @@ async fn can_manually_produce_block( .expect_produce_and_execute_block() .returning(|_, time, _, _| { let mut block = Block::default(); - block.header_mut().consensus.time = time; + block.header_mut().consensus_mut().time = time; block.header_mut().recalculate_metadata(); Ok(UncommittedResult::new( ExecutionResult { diff --git a/crates/services/consensus_module/poa/src/verifier/tests.rs b/crates/services/consensus_module/poa/src/verifier/tests.rs index 74887b4eed8..2308833f516 100644 --- a/crates/services/consensus_module/poa/src/verifier/tests.rs +++ b/crates/services/consensus_module/poa/src/verifier/tests.rs @@ -97,12 +97,12 @@ fn test_verify_genesis_block_fields(input: Input) -> anyhow::Result<()> { .returning(move |_| Ok(block_header_merkle_root.into())); d.expect_block_header().returning(move |_| { let mut h = BlockHeader::default(); - h.consensus.time = prev_header_time; - h.application.da_height = prev_header_da_height.into(); + h.consensus_mut().time = prev_header_time; + h.application_mut().da_height = prev_header_da_height.into(); Ok(h) }); let mut b = Block::default(); - b.header_mut().consensus = ch; - b.header_mut().application = ah; + *b.header_mut().consensus_mut() = ch; + *b.header_mut().application_mut() = ah; verify_block_fields(&d, &b) } diff --git a/crates/services/consensus_module/src/block_verifier/tests.rs b/crates/services/consensus_module/src/block_verifier/tests.rs index 2c7bc090597..7c9367e1909 100644 --- a/crates/services/consensus_module/src/block_verifier/tests.rs +++ b/crates/services/consensus_module/src/block_verifier/tests.rs @@ -4,9 +4,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::zeroed(); - h.consensus.time = Tai64::UNIX_EPOCH; - h.consensus.height = 0u32.into(); + h.consensus_mut().prev_root = Bytes32::zeroed(); + h.consensus_mut().time = Tai64::UNIX_EPOCH; + h.consensus_mut().height = 0u32.into(); h }, 0 => matches Ok(_) ; "Correct header at `0`" @@ -14,9 +14,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::zeroed(); - h.consensus.time = Tai64::UNIX_EPOCH; - h.consensus.height = 113u32.into(); + h.consensus_mut().prev_root = Bytes32::zeroed(); + h.consensus_mut().time = Tai64::UNIX_EPOCH; + h.consensus_mut().height = 113u32.into(); h }, 113 => matches Ok(_) ; "Correct header at `113`" @@ -24,9 +24,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::zeroed(); - h.consensus.time = Tai64::UNIX_EPOCH; - h.consensus.height = 0u32.into(); + h.consensus_mut().prev_root = Bytes32::zeroed(); + h.consensus_mut().time = Tai64::UNIX_EPOCH; + h.consensus_mut().height = 0u32.into(); h }, 10 => matches Err(_) ; "wrong expected height" @@ -34,9 +34,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::zeroed(); - h.consensus.time = Tai64::UNIX_EPOCH; - h.consensus.height = 5u32.into(); + h.consensus_mut().prev_root = Bytes32::zeroed(); + h.consensus_mut().time = Tai64::UNIX_EPOCH; + h.consensus_mut().height = 5u32.into(); h }, 0 => matches Err(_) ; "wrong header height" @@ -44,9 +44,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::zeroed(); - h.consensus.time = Tai64(0); - h.consensus.height = 0u32.into(); + h.consensus_mut().prev_root = Bytes32::zeroed(); + h.consensus_mut().time = Tai64(0); + h.consensus_mut().height = 0u32.into(); h }, 0 => matches Err(_) ; "wrong time" @@ -54,9 +54,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus.prev_root = Bytes32::from([1u8; 32]); - h.consensus.time = Tai64::UNIX_EPOCH; - h.consensus.height = 0u32.into(); + h.consensus_mut().prev_root = Bytes32::from([1u8; 32]); + h.consensus_mut().time = Tai64::UNIX_EPOCH; + h.consensus_mut().height = 0u32.into(); h }, 0 => matches Err(_) ; "wrong root" diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index 717271093fd..4141b145250 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -88,7 +88,7 @@ struct MockExecutionResult { fn genesis(height: u32) -> SealedBlock { let mut block = Block::default(); - block.header_mut().consensus.height = height.into(); + block.header_mut().consensus_mut().height = height.into(); block.header_mut().recalculate_metadata(); SealedBlock { @@ -99,7 +99,7 @@ fn genesis(height: u32) -> SealedBlock { fn poa_block(height: u32) -> SealedBlock { let mut block = Block::default(); - block.header_mut().consensus.height = height.into(); + block.header_mut().consensus_mut().height = height.into(); block.header_mut().recalculate_metadata(); SealedBlock { @@ -514,7 +514,7 @@ where // We tested commit part in the `commit_result_and_execute_and_commit_poa` so setup the // databases to always pass the committing part. - let expected_height: u32 = sealed_block.entity.header().consensus.height.into(); + let expected_height: u32 = sealed_block.entity.header().consensus().height.into(); let previous_height = expected_height.checked_sub(1).unwrap_or_default(); let execute_and_commit_result = execute_and_commit_assert( sealed_block, diff --git a/crates/services/p2p/src/p2p_service.rs b/crates/services/p2p/src/p2p_service.rs index 9f606582385..6b16819adc0 100644 --- a/crates/services/p2p/src/p2p_service.rs +++ b/crates/services/p2p/src/p2p_service.rs @@ -1463,7 +1463,7 @@ mod tests { let mut blocks = Vec::new(); for i in range { let mut header: BlockHeader = Default::default(); - header.consensus.height = i.into(); + header.consensus_mut().height = i.into(); let sealed_block = SealedBlockHeader { entity: header, @@ -1476,8 +1476,8 @@ mod tests { // Metadata gets skipped during serialization, so this is the fuzzy way to compare blocks fn eq_except_metadata(a: &SealedBlockHeader, b: &SealedBlockHeader) -> bool { - a.entity.application == b.entity.application - && a.entity.consensus == b.entity.consensus + a.entity.application() == b.entity.application() + && a.entity.consensus() == b.entity.consensus() } async fn request_response_works_with(request_msg: RequestMessage) { diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index 4e48095bc9c..01a413a0fa9 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -404,7 +404,7 @@ impl core::ops::Deref for BlockHeader { type Target = ApplicationHeader; fn deref(&self) -> &Self::Target { - &self.application() + self.application() } } @@ -434,7 +434,7 @@ impl core::ops::Deref for ConsensusHeader { impl core::convert::AsRef> for BlockHeader { fn as_ref(&self) -> &ConsensusHeader { - &self.consensus() + self.consensus() } } From 65d20a2a63c48cfebe7968c2e3e98e94a4eacabc Mon Sep 17 00:00:00 2001 From: Turner Date: Wed, 24 Jan 2024 16:57:44 -0800 Subject: [PATCH 4/5] Make fewer things public --- crates/fuel-core/src/executor.rs | 10 +--- .../service_test/manually_produce_tests.rs | 2 +- .../poa/src/verifier/tests.rs | 8 +-- .../src/block_verifier/tests.rs | 36 +++++------ crates/services/importer/src/importer/test.rs | 4 +- crates/services/p2p/src/p2p_service.rs | 2 +- .../services/sync/src/import/test_helpers.rs | 5 +- crates/types/src/blockchain/header.rs | 59 ++++++++++++++++--- 8 files changed, 82 insertions(+), 44 deletions(-) diff --git a/crates/fuel-core/src/executor.rs b/crates/fuel-core/src/executor.rs index 9b4a77d0905..8caa1fba087 100644 --- a/crates/fuel-core/src/executor.rs +++ b/crates/fuel-core/src/executor.rs @@ -371,7 +371,7 @@ mod tests { let invalid_duplicate_tx = script.clone().into(); let mut block = Block::default(); - block.header_mut().consensus_mut().height = 1.into(); + block.header_mut().set_block_height(1.into()); *block.transactions_mut() = vec![script.into(), invalid_duplicate_tx]; block.header_mut().recalculate_metadata(); @@ -439,7 +439,7 @@ mod tests { .max_fee(); let mut block = Block::default(); - block.header_mut().consensus_mut().height = 2.into(); + block.header_mut().set_block_height(2.into()); *block.transactions_mut() = vec![script.into()]; block.header_mut().recalculate_metadata(); @@ -1174,11 +1174,7 @@ mod tests { .unwrap(); // randomize transaction commitment - block - .header_mut() - .application_mut() - .generated - .transactions_root = rng.gen(); + block.header_mut().set_transaction_root(rng.gen()); block.header_mut().recalculate_metadata(); let verify_result = verifier diff --git a/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs b/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs index f3a1d1a0c16..761d5fcbd99 100644 --- a/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs +++ b/crates/services/consensus_module/poa/src/service_test/manually_produce_tests.rs @@ -85,7 +85,7 @@ async fn can_manually_produce_block( .expect_produce_and_execute_block() .returning(|_, time, _, _| { let mut block = Block::default(); - block.header_mut().consensus_mut().time = time; + block.header_mut().set_time(time); block.header_mut().recalculate_metadata(); Ok(UncommittedResult::new( ExecutionResult { diff --git a/crates/services/consensus_module/poa/src/verifier/tests.rs b/crates/services/consensus_module/poa/src/verifier/tests.rs index 2308833f516..0183ebc6b59 100644 --- a/crates/services/consensus_module/poa/src/verifier/tests.rs +++ b/crates/services/consensus_module/poa/src/verifier/tests.rs @@ -97,12 +97,12 @@ fn test_verify_genesis_block_fields(input: Input) -> anyhow::Result<()> { .returning(move |_| Ok(block_header_merkle_root.into())); d.expect_block_header().returning(move |_| { let mut h = BlockHeader::default(); - h.consensus_mut().time = prev_header_time; - h.application_mut().da_height = prev_header_da_height.into(); + h.set_time(prev_header_time); + h.set_da_height(prev_header_da_height.into()); Ok(h) }); let mut b = Block::default(); - *b.header_mut().consensus_mut() = ch; - *b.header_mut().application_mut() = ah; + b.header_mut().set_consensus_header(ch); + b.header_mut().set_application_header(ah); verify_block_fields(&d, &b) } diff --git a/crates/services/consensus_module/src/block_verifier/tests.rs b/crates/services/consensus_module/src/block_verifier/tests.rs index 7c9367e1909..1fb2bbe2426 100644 --- a/crates/services/consensus_module/src/block_verifier/tests.rs +++ b/crates/services/consensus_module/src/block_verifier/tests.rs @@ -4,9 +4,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::zeroed(); - h.consensus_mut().time = Tai64::UNIX_EPOCH; - h.consensus_mut().height = 0u32.into(); + h.set_previous_root(Bytes32::zeroed()); + h.set_time(Tai64::UNIX_EPOCH); + h.set_block_height(0u32.into()); h }, 0 => matches Ok(_) ; "Correct header at `0`" @@ -14,9 +14,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::zeroed(); - h.consensus_mut().time = Tai64::UNIX_EPOCH; - h.consensus_mut().height = 113u32.into(); + h.set_previous_root(Bytes32::zeroed()); + h.set_time(Tai64::UNIX_EPOCH); + h.set_block_height(113u32.into()); h }, 113 => matches Ok(_) ; "Correct header at `113`" @@ -24,9 +24,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::zeroed(); - h.consensus_mut().time = Tai64::UNIX_EPOCH; - h.consensus_mut().height = 0u32.into(); + h.set_previous_root(Bytes32::zeroed()); + h.set_time(Tai64::UNIX_EPOCH); + h.set_block_height(0u32.into()); h }, 10 => matches Err(_) ; "wrong expected height" @@ -34,9 +34,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::zeroed(); - h.consensus_mut().time = Tai64::UNIX_EPOCH; - h.consensus_mut().height = 5u32.into(); + h.set_previous_root(Bytes32::zeroed()); + h.set_time(Tai64::UNIX_EPOCH); + h.set_block_height(5u32.into()); h }, 0 => matches Err(_) ; "wrong header height" @@ -44,9 +44,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::zeroed(); - h.consensus_mut().time = Tai64(0); - h.consensus_mut().height = 0u32.into(); + h.set_previous_root(Bytes32::zeroed()); + h.set_time(Tai64(0)); + h.set_block_height(0u32.into()); h }, 0 => matches Err(_) ; "wrong time" @@ -54,9 +54,9 @@ use test_case::test_case; #[test_case( { let mut h = BlockHeader::default(); - h.consensus_mut().prev_root = Bytes32::from([1u8; 32]); - h.consensus_mut().time = Tai64::UNIX_EPOCH; - h.consensus_mut().height = 0u32.into(); + h.set_previous_root(Bytes32::from([1u8; 32])); + h.set_time(Tai64::UNIX_EPOCH); + h.set_block_height(0u32.into()); h }, 0 => matches Err(_) ; "wrong root" diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index 4141b145250..5821ceaf298 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -88,7 +88,7 @@ struct MockExecutionResult { fn genesis(height: u32) -> SealedBlock { let mut block = Block::default(); - block.header_mut().consensus_mut().height = height.into(); + block.header_mut().set_block_height(height.into()); block.header_mut().recalculate_metadata(); SealedBlock { @@ -99,7 +99,7 @@ fn genesis(height: u32) -> SealedBlock { fn poa_block(height: u32) -> SealedBlock { let mut block = Block::default(); - block.header_mut().consensus_mut().height = height.into(); + block.header_mut().set_block_height(height.into()); block.header_mut().recalculate_metadata(); SealedBlock { diff --git a/crates/services/p2p/src/p2p_service.rs b/crates/services/p2p/src/p2p_service.rs index 6b16819adc0..4e926fb59e0 100644 --- a/crates/services/p2p/src/p2p_service.rs +++ b/crates/services/p2p/src/p2p_service.rs @@ -1463,7 +1463,7 @@ mod tests { let mut blocks = Vec::new(); for i in range { let mut header: BlockHeader = Default::default(); - header.consensus_mut().height = i.into(); + header.set_block_height(i.into()); let sealed_block = SealedBlockHeader { entity: header, diff --git a/crates/services/sync/src/import/test_helpers.rs b/crates/services/sync/src/import/test_helpers.rs index 583357262c4..2321a6562b0 100644 --- a/crates/services/sync/src/import/test_helpers.rs +++ b/crates/services/sync/src/import/test_helpers.rs @@ -42,11 +42,12 @@ pub fn random_peer() -> PeerId { pub fn empty_header>(i: I) -> SealedBlockHeader { let mut header = BlockHeader::default(); let height = i.into(); - header.consensus_mut().height = height; + header.set_block_height(height); let transaction_tree = fuel_core_types::fuel_merkle::binary::root_calculator::MerkleRootCalculator::new( ); - header.application_mut().generated.transactions_root = transaction_tree.root().into(); + let root = transaction_tree.root().into(); + header.set_transaction_root(root); let consensus = Consensus::default(); Sealed { diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index 01a413a0fa9..50f31ab8581 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -69,34 +69,70 @@ impl BlockHeader { } /// Getter for metadata portion of header - pub fn metadata(&self) -> &Option { + fn metadata(&self) -> &Option { match self { BlockHeader::V1(v1) => &v1.metadata, } } /// Mutable getter for consensus portion of header - pub fn consensus_mut(&mut self) -> &mut ConsensusHeader { + fn consensus_mut(&mut self) -> &mut ConsensusHeader { match self { BlockHeader::V1(v1) => &mut v1.consensus, } } - /// Mutable getter for application portion of header - pub fn application_mut( + /// Set the entire consensus header + pub fn set_consensus_header( &mut self, - ) -> &mut ApplicationHeader { + consensus: ConsensusHeader, + ) { + match self { + BlockHeader::V1(v1) => v1.consensus = consensus, + } + } + + /// Mutable getter for application portion of header + fn application_mut(&mut self) -> &mut ApplicationHeader { match self { BlockHeader::V1(v1) => &mut v1.application, } } - /// Mutable getter for metadata portion of header - pub fn metadata_mut(&mut self) -> &mut Option { + /// Set the entire application header + pub fn set_application_header( + &mut self, + application: ApplicationHeader, + ) { match self { - BlockHeader::V1(v1) => &mut v1.metadata, + BlockHeader::V1(v1) => v1.application = application, } } + + /// Set the block height for the header + pub fn set_block_height(&mut self, height: BlockHeight) { + self.consensus_mut().height = height; + } + + /// Set the previous root for the header + pub fn set_previous_root(&mut self, root: Bytes32) { + self.consensus_mut().prev_root = root; + } + + /// Set the time for the header + pub fn set_time(&mut self, time: Tai64) { + self.consensus_mut().time = time; + } + + /// Set the transaction root for the header + pub fn set_transaction_root(&mut self, root: Bytes32) { + self.application_mut().generated.transactions_root = root; + } + + /// Set the DA height for the header + pub fn set_da_height(&mut self, da_height: DaBlockHeight) { + self.application_mut().da_height = da_height; + } } #[derive(Clone, Debug)] @@ -254,7 +290,12 @@ impl BlockHeader { pub fn recalculate_metadata(&mut self) { let application_hash = self.application().hash(); self.consensus_mut().generated.application_hash = application_hash; - *self.metadata_mut() = Some(BlockHeaderMetadata { id: self.hash() }); + let id = self.hash(); + match self { + BlockHeader::V1(v1) => { + v1.metadata = Some(BlockHeaderMetadata { id }); + } + } } /// Get the hash of the fuel header. From 09a2539758fd6c9d0401dbe8dd0b2294f5a94ce2 Mon Sep 17 00:00:00 2001 From: Turner Date: Thu, 25 Jan 2024 13:42:46 -0800 Subject: [PATCH 5/5] Remove nested access to fields --- crates/fuel-core/src/service/adapters/p2p.rs | 2 +- crates/services/consensus_module/poa/src/verifier.rs | 2 +- crates/services/importer/src/importer/test.rs | 2 +- crates/types/src/blockchain/header.rs | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/fuel-core/src/service/adapters/p2p.rs b/crates/fuel-core/src/service/adapters/p2p.rs index dc4e78bffbe..4101108c8a0 100644 --- a/crates/fuel-core/src/service/adapters/p2p.rs +++ b/crates/fuel-core/src/service/adapters/p2p.rs @@ -55,7 +55,7 @@ impl BlockHeightImporter for BlockImporterAdapter { Box::pin( BroadcastStream::new(self.block_importer.subscribe()) .filter_map(|result| result.ok()) - .map(|result| result.sealed_block.entity.header().consensus().height), + .map(|result| *result.sealed_block.entity.header().height()), ) } } diff --git a/crates/services/consensus_module/poa/src/verifier.rs b/crates/services/consensus_module/poa/src/verifier.rs index 9da54343d47..62a8a505e01 100644 --- a/crates/services/consensus_module/poa/src/verifier.rs +++ b/crates/services/consensus_module/poa/src/verifier.rs @@ -62,7 +62,7 @@ pub fn verify_block_fields( ); ensure!( - header.consensus().application_hash == header.application().hash(), + header.application_hash() == &header.application().hash(), "The application hash mismatch." ); diff --git a/crates/services/importer/src/importer/test.rs b/crates/services/importer/src/importer/test.rs index 5821ceaf298..595d80159b6 100644 --- a/crates/services/importer/src/importer/test.rs +++ b/crates/services/importer/src/importer/test.rs @@ -514,7 +514,7 @@ where // We tested commit part in the `commit_result_and_execute_and_commit_poa` so setup the // databases to always pass the committing part. - let expected_height: u32 = sealed_block.entity.header().consensus().height.into(); + let expected_height: u32 = (*sealed_block.entity.header().height()).into(); let previous_height = expected_height.checked_sub(1).unwrap_or_default(); let execute_and_commit_result = execute_and_commit_assert( sealed_block, diff --git a/crates/types/src/blockchain/header.rs b/crates/types/src/blockchain/header.rs index 50f31ab8581..1635380bad3 100644 --- a/crates/types/src/blockchain/header.rs +++ b/crates/types/src/blockchain/header.rs @@ -305,7 +305,7 @@ impl BlockHeader { // and can't change its final hash on the fly. // // This assertion is a double-checks that this behavior is not changed. - debug_assert_eq!(self.consensus().application_hash, self.application().hash()); + debug_assert_eq!(self.application_hash(), &self.application().hash()); // This internally hashes the hash of the application header. self.consensus().hash() }