Merge branch 'featue/block-verifier' into featue/connect-block-importer

crates/services/consensus_module/Cargo.toml

@@ -1,17 +1,20 @@
 [package]
-name = "fuel-core-consensus-module"
-version = "0.15.1"
 authors = ["Fuel Labs <contact@fuel.sh>"]
+description = "The common code for fuel core consensuses."
 edition = "2021"
 homepage = "https://fuel.network/"
-keywords = ["blockchain", "fuel", "consensus"]
+keywords = ["blockchain", "consensus", "fuel"]
 license = "BUSL-1.1"
+name = "fuel-core-consensus-module"
 repository = "https://github.com/FuelLabs/fuel-core"
-description = "The common code for fuel core consensuses."
+version = "0.15.1"
 
 [dependencies]
 anyhow = "1.0"
 fuel-core-chain-config = { version = "0.15.1", path = "../../chain-config" }
 fuel-core-poa = { version = "0.15.1", path = "poa" }
 fuel-core-types = { version = "0.15.1", path = "../../types" }
 
+[dev-dependencies]
+fuel-core-types = { path = "../../types", features = ["test-helpers"] }
+test-case = "2.2"

crates/services/consensus_module/poa/Cargo.toml

@@ -25,4 +25,5 @@ fuel-core-storage = { path = "../../../storage", features = ["test-helpers"] }
 fuel-core-types = { path = "../../../types", features = ["test-helpers"] }
 mockall = "0.11"
 rand = "0.8"
+test-case = "2.2"
 tokio = { version = "1.21", features = ["full", "test-util"] }

crates/services/consensus_module/poa/src/service.rs

@@ -116,6 +116,7 @@ pub struct Task<T, B, I> {
     trigger: Trigger,
     // TODO: Consider that the creation of the block takes some time, and maybe we need to
     //  patch the timer to generate the block earlier.
+    //  https://github.com/FuelLabs/fuel-core/issues/918
     /// Deadline clock, used by the triggers
     timer: DeadlineClock,
 }

crates/services/consensus_module/poa/src/verifier.rs

@@ -12,6 +12,9 @@ use fuel_core_types::{
 };
 use std::ops::Add;
 
+#[cfg(test)]
+mod tests;
+
 /// The config of the block verifier.
 pub struct Config {
     /// If the manual block is enabled, skip verification of some fields.
@@ -20,11 +23,14 @@ pub struct Config {
     /// during the generation of the block. That means if the node(block producer) was
     /// offline for a while, it would not use the expected block time after
     /// restarting(it should generate blocks with old timestamps).
+    ///
+    /// https://github.com/FuelLabs/fuel-core/issues/918
     pub perform_strict_time_rules: bool,
     /// The configuration of the network.
     pub production: PoABlockProduction,
 }
 
+#[cfg_attr(test, mockall::automock)]
 /// The port for the database.
 pub trait Database {
     /// Gets the block header at `height`.
@@ -62,6 +68,7 @@ pub fn verify_poa_block_fields<D: Database>(
 
     // Skip the verification of the time if it is possible to produce blocks manually.
     if !config.enabled_manual_blocks {
+        // See comment of the `Config.perform_strict_time_rules`
         if config.perform_strict_time_rules {
             match config.production {
                 PoABlockProduction::Instant => {
@@ -77,18 +84,19 @@ pub fn verify_poa_block_fields<D: Database>(
 
                     // If the `average_block_time` is 15 seconds, the block should be in the range
                     // [15..15 + 15/2] -> [15..23]
+                    // https://github.com/FuelLabs/fuel-core/issues/918
                     let average_block_time = average_block_time;
                     let half_average_block_time = average_block_time / 2;
-                    let lower_bound = previous_block_time.add(average_block_time).0;
+                    let lower_bound = previous_block_time.add(average_block_time);
                     let upper_bound = previous_block_time
-                        .add(average_block_time + half_average_block_time)
-                        .0;
-                    let block_time = Tai64N::from(header.time()).0;
+                        .add(average_block_time + half_average_block_time);
+                    let block_time = Tai64N::from(header.time());
                     ensure!(
                         lower_bound <= block_time && block_time <= upper_bound,
-                        "The `time` of the next should be more than {:?} but less than {:?}",
+                        "The `time` of the next should be more than {:?} but less than {:?} but got {:?}",
                         lower_bound,
                         upper_bound,
+                        block_time,
                     );
                 }
                 PoABlockProduction::Hybrid {
@@ -98,16 +106,17 @@ pub fn verify_poa_block_fields<D: Database>(
                 } => {
                     let previous_block_time = Tai64N::from(prev_header.time());
                     // The block should be in the range
-                    // [min..max + (max - min)/2]
-                    let half = (max_block_time + min_block_time) / 2;
-                    let lower_bound = previous_block_time.add(min_block_time).0;
-                    let upper_bound = previous_block_time.add(max_block_time + half).0;
-                    let block_time = Tai64N::from(header.time()).0;
+                    // [min..max]
+                    // https://github.com/FuelLabs/fuel-core/issues/918
+                    let lower_bound = previous_block_time.add(min_block_time);
+                    let upper_bound = previous_block_time.add(max_block_time);
+                    let block_time = Tai64N::from(header.time());
                     ensure!(
                         lower_bound <= block_time && block_time <= upper_bound,
-                        "The `time` of the next should be more than {:?} but less than {:?}",
+                        "The `time` of the next should be more than {:?} but less than {:?} but got {:?}",
                         lower_bound,
                         upper_bound,
+                        block_time,
                     );
                 }
             };

crates/services/consensus_module/poa/src/verifier/tests.rs

@@ -0,0 +1,195 @@
+use super::*;
+use fuel_core_types::{
+    blockchain::header::{
+        ApplicationHeader,
+        ConsensusHeader,
+        GeneratedApplicationFields,
+        GeneratedConsensusFields,
+    },
+    tai64::Tai64,
+};
+use std::time::Duration;
+use test_case::test_case;
+
+struct Input {
+    c: Config,
+    block_header_merkle_root: [u8; 32],
+    prev_header_time: Tai64,
+    prev_header_da_height: u64,
+    ch: ConsensusHeader<GeneratedConsensusFields>,
+    ah: ApplicationHeader<GeneratedApplicationFields>,
+}
+
+fn app_hash(da_height: u64) -> Bytes32 {
+    ApplicationHeader {
+        da_height: da_height.into(),
+        ..Default::default()
+    }
+    .hash()
+}
+
+fn correct() -> Input {
+    Input {
+        c: Config {
+            enabled_manual_blocks: false,
+            perform_strict_time_rules: false,
+            production: PoABlockProduction::Instant,
+        },
+        block_header_merkle_root: [2u8; 32],
+        prev_header_time: Tai64(2),
+        prev_header_da_height: 2,
+        ch: ConsensusHeader {
+            prev_root: [2u8; 32].into(),
+            height: 2u32.into(),
+            time: Tai64(2),
+            generated: GeneratedConsensusFields {
+                application_hash: app_hash(2),
+            },
+        },
+        ah: ApplicationHeader {
+            da_height: 2u64.into(),
+            ..Default::default()
+        },
+    }
+}
+
+#[test_case(correct() => matches Ok(_) ; "Correct block")]
+#[test_case(
+    {
+        let mut i = correct();
+        i.ch.height = 0u32.into();
+        i
+    } => matches Err(_) ; "Height 0"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.ch.prev_root = [3u8; 32].into();
+        i
+    } => matches Err(_) ; "Prev root mis-match"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.ah.da_height = 1u64.into();
+        i
+    } => matches Err(_) ; "da height lower then prev header"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.ch.generated.application_hash = [0u8; 32].into();
+        i
+    } => matches Err(_) ; "application hash mis-match"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.ch.time = Tai64(1);
+        i
+    } => matches Err(_) ; "time before prev header"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i
+    } => matches Ok(_) ; "Strict rules with correct block"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.ch.time = Tai64(1);
+        i
+    } => matches Err(_) ; "time before prev header with strict rules"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) };
+        i.ch.time = Tai64(7);
+        i
+    } => matches Ok(_) ; "interval time ok"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) };
+        i.ch.time = Tai64(5);
+        i
+    } => matches Err(_) ; "interval time too early"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Interval { block_time: Duration::from_secs(4) };
+        i.ch.time = Tai64(9);
+        i
+    } => matches Err(_) ; "interval time too late"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Hybrid {
+            min_block_time: Duration::from_secs(4),
+            max_tx_idle_time: Duration::from_secs(5),
+            max_block_time: Duration::from_secs(6),
+        };
+        i.ch.time = Tai64(7);
+        i
+    } => matches Ok(_) ; "hybrid time ok"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Hybrid {
+            min_block_time: Duration::from_secs(4),
+            max_tx_idle_time: Duration::from_secs(5),
+            max_block_time: Duration::from_secs(6),
+        };
+        i.ch.time = Tai64(5);
+        i
+    } => matches Err(_) ; "hybrid time too early"
+)]
+#[test_case(
+    {
+        let mut i = correct();
+        i.c.perform_strict_time_rules = true;
+        i.c.production = PoABlockProduction::Hybrid {
+            min_block_time: Duration::from_secs(4),
+            max_tx_idle_time: Duration::from_secs(5),
+            max_block_time: Duration::from_secs(6),
+        };
+        i.ch.time = Tai64(14);
+        i
+    } => matches Err(_) ; "hybrid time too late"
+)]
+fn test_verify_genesis_block_fields(input: Input) -> anyhow::Result<()> {
+    let Input {
+        c,
+        block_header_merkle_root,
+        prev_header_time,
+        prev_header_da_height,
+        ch,
+        ah,
+    } = input;
+    let mut d = MockDatabase::default();
+    d.expect_block_header_merkle_root()
+        .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();
+        Ok(h)
+    });
+    let mut b = Block::default();
+    b.header_mut().consensus = ch;
+    b.header_mut().application = ah;
+    verify_poa_block_fields(&c, &d, &b)
+}