Browse Source

validator: strictly upper bound proposals timestamps

skoupidi 2 months ago
parent
commit
ceeb7c9c09

+ 1 - 1
bin/darkfid/src/proto/protocol_proposal.rs

@@ -217,7 +217,7 @@ async fn handle_receive_proposal(
         }
 
         // Append proposal
-        match validator.append_proposal(&proposal.0).await {
+        match validator.append_proposal(&proposal.0, None).await {
             Ok(()) => {
                 // Signal handler to broadcast the valid proposal to rest nodes
                 handler.send_action(channel, ProtocolGenericAction::Broadcast).await;

+ 1 - 1
bin/darkfid/src/registry/mod.rs

@@ -185,7 +185,7 @@ impl DarkfiMinersRegistryState {
         block: BlockInfo,
     ) -> Result<()> {
         let proposal = Proposal::new(block);
-        validator.append_proposal(&proposal).await?;
+        validator.append_proposal(&proposal, None).await?;
 
         info!(
             target: "darkfid::registry::mod::DarkfiMinersRegistry::submit",

+ 74 - 16
bin/darkfid/src/task/sync.rs

@@ -19,13 +19,18 @@
 use std::collections::HashMap;
 
 use darkfi::{
-    blockchain::HeaderHash, net::ChannelPtr, rpc::jsonrpc::JsonSubscriber, system::sleep,
-    util::encoding::base64, validator::consensus::Proposal, Error, Result,
+    blockchain::HeaderHash,
+    net::ChannelPtr,
+    rpc::jsonrpc::JsonSubscriber,
+    system::sleep,
+    util::{encoding::base64, time::Timestamp},
+    validator::consensus::Proposal,
+    Error, Result,
 };
 use darkfi_serial::serialize_async;
 use rand::{prelude::SliceRandom, rngs::OsRng};
 use tinyjson::JsonValue;
-use tracing::{debug, info, warn};
+use tracing::{debug, error, info, warn};
 
 use crate::{
     proto::{
@@ -87,12 +92,25 @@ pub async fn sync_task(node: &DarkfiNodePtr, checkpoint: Option<(u32, HeaderHash
     if let Some(checkpoint) = checkpoint {
         if checkpoint.0 > last.0 {
             info!(target: "darkfid::task::sync_task", "Syncing until configured checkpoint: {} - {}", checkpoint.0, checkpoint.1);
+            // All blocks must be before the future timestamp upper bound
+            let timestamps_bound =
+                node.validator.read().await.consensus.module.future_timestamp_upper_bound()?;
+
             // Retrieve all the headers backwards until our last known one and verify them.
             // We use the next height, in order to also retrieve the checkpoint header.
-            retrieve_headers(node, &common_tip_peers, last, checkpoint.0 + 1).await?;
+            retrieve_headers(node, &common_tip_peers, last, checkpoint.0 + 1, timestamps_bound)
+                .await?;
 
             // Retrieve all the blocks for those headers and apply them to canonical
-            last = retrieve_blocks(node, &common_tip_peers, last, block_sub, true).await?;
+            last = retrieve_blocks(
+                node,
+                &common_tip_peers,
+                last,
+                block_sub,
+                true,
+                Some(timestamps_bound),
+            )
+            .await?;
             info!(target: "darkfid::task::sync_task", "Last received block: {} - {}", last.0, last.1);
 
             // Grab synced peers most common tip again
@@ -102,13 +120,25 @@ pub async fn sync_task(node: &DarkfiNodePtr, checkpoint: Option<(u32, HeaderHash
 
     // Sync headers and blocks
     loop {
+        // All blocks must be before the future timestamp upper bound
+        let timestamps_bound =
+            node.validator.read().await.consensus.module.future_timestamp_upper_bound()?;
+
         // Retrieve all the headers backwards until our last known one and verify them.
         // We use the next height, in order to also retrieve the peers tip header.
-        retrieve_headers(node, &common_tip_peers, last, common_tip_height + 1).await?;
+        retrieve_headers(node, &common_tip_peers, last, common_tip_height + 1, timestamps_bound)
+            .await?;
 
         // Retrieve all the blocks for those headers and apply them to canonical
-        let last_received =
-            retrieve_blocks(node, &common_tip_peers, last, block_sub, false).await?;
+        let last_received = retrieve_blocks(
+            node,
+            &common_tip_peers,
+            last,
+            block_sub,
+            false,
+            Some(timestamps_bound),
+        )
+        .await?;
         info!(target: "darkfid::task::sync_task", "Last received block: {} - {}", last_received.0, last_received.1);
 
         if last == last_received {
@@ -287,6 +317,7 @@ async fn retrieve_headers(
     peers: &[ChannelPtr],
     last_known: (u32, HeaderHash),
     tip_height: u32,
+    timestamps_bound: Timestamp,
 ) -> Result<()> {
     info!(target: "darkfid::task::sync::retrieve_headers", "Retrieving missing headers from peers...");
     // Communication setup
@@ -371,23 +402,30 @@ async fn retrieve_headers(
         return Ok(());
     }
 
-    // Verify headers sequence. Here we do a quick and dirty verification
-    // of just the hashes and heights sequence. We will formaly verify
-    // the blocks when we retrieve them. We verify them in batches,
-    // to not load them all in memory.
+    // Verify headers sequence. Here we do a quick and dirty
+    // verification of just the hashes and heights sequence, along with
+    // its timestamp. We will formaly verify the blocks when we
+    // retrieve them. We verify them in batches, to not load them all
+    // in memory.
     info!(target: "darkfid::task::sync::retrieve_headers", "Verifying headers sequence...");
     let mut verified_headers = 0;
     let total = validator.blockchain.headers.len_sync();
     // First we verify the first `BATCH` sequence, using the last known header
     // as the first sync header previous.
     let mut headers = validator.blockchain.headers.get_after_sync(0, BATCH)?;
-    if headers[0].previous != last_known.1 || headers[0].height != last_known.0 + 1 {
+    if headers[0].previous != last_known.1 ||
+        headers[0].height != last_known.0 + 1 ||
+        headers[0].timestamp > timestamps_bound
+    {
         validator.blockchain.headers.remove_all_sync()?;
         return Err(Error::BlockIsInvalid(headers[0].hash().as_string()))
     }
     verified_headers += 1;
     for (index, header) in headers[1..].iter().enumerate() {
-        if header.previous != headers[index].hash() || header.height != headers[index].height + 1 {
+        if header.previous != headers[index].hash() ||
+            header.height != headers[index].height + 1 ||
+            header.timestamp > timestamps_bound
+        {
             return Err(Error::BlockIsInvalid(header.hash().as_string()))
         }
         verified_headers += 1;
@@ -430,6 +468,7 @@ async fn retrieve_blocks(
     last_known: (u32, HeaderHash),
     block_sub: &JsonSubscriber,
     checkpoint_blocks: bool,
+    timestamps_bound: Option<Timestamp>,
 ) -> Result<(u32, HeaderHash)> {
     info!(target: "darkfid::task::sync::retrieve_blocks", "Retrieving missing blocks from peers...");
     let mut last_received = last_known;
@@ -518,7 +557,10 @@ async fn retrieve_blocks(
                 };
             } else {
                 for block in &response.blocks {
-                    match validator.append_proposal(&Proposal::new(block.clone())).await {
+                    match validator
+                        .append_proposal(&Proposal::new(block.clone()), timestamps_bound)
+                        .await
+                    {
                         Ok(()) | Err(Error::ProposalAlreadyExists) => continue,
                         Err(e) => {
                             debug!(target: "darkfid::task::sync::retrieve_blocks", "Error while appending proposal: {e}");
@@ -598,11 +640,27 @@ async fn sync_best_fork(node: &DarkfiNodePtr, peers: &[ChannelPtr], last_tip: &H
         return
     };
 
+    // All proposals must be before the future timestamp upper bound
+    let timestamps_bound = match node
+        .validator
+        .read()
+        .await
+        .consensus
+        .module
+        .future_timestamp_upper_bound()
+    {
+        Ok(bound) => Some(bound),
+        Err(e) => {
+            error!(target: "darkfid::task::sync::sync_best_fork", "Future timestamp upper bound retriaval failed: {e}");
+            return
+        }
+    };
+
     // Verify and store retrieved proposals
     debug!(target: "darkfid::task::sync::sync_best_fork", "Processing received proposals");
     let mut validator = node.validator.write().await;
     for proposal in &response.proposals {
-        if let Err(e) = validator.append_proposal(proposal).await {
+        if let Err(e) = validator.append_proposal(proposal, timestamps_bound).await {
             debug!(target: "darkfid::task::sync::sync_best_fork", "Error while appending proposal: {e}");
             return
         };

+ 44 - 2
bin/darkfid/src/task/unknown_proposal.rs

@@ -190,10 +190,26 @@ async fn handle_unknown_proposal(node: &DarkfiNodePtr, channel: u32, proposal: &
         return handle_reorg(node, &(&channel, &comms_timeout), proposal).await
     }
 
+    // All proposals must be before the future timestamp upper bound
+    let timestamps_bound = match node
+        .validator
+        .read()
+        .await
+        .consensus
+        .module
+        .future_timestamp_upper_bound()
+    {
+        Ok(bound) => Some(bound),
+        Err(e) => {
+            error!(target: "darkfid::task::handle_unknown_proposal", "Future timestamp upper bound retriaval failed: {e}");
+            return false
+        }
+    };
+
     // Process response proposals
     for proposal in &response.proposals {
         // Append proposal
-        match node.validator.write().await.append_proposal(proposal).await {
+        match node.validator.write().await.append_proposal(proposal, timestamps_bound).await {
             Ok(()) => { /* Do nothing */ }
             // Skip already existing proposals
             Err(ProposalAlreadyExists) => continue,
@@ -297,12 +313,22 @@ async fn handle_reorg(
     };
     drop(validator);
 
+    // All proposals must be before the future timestamp upper bound
+    let timestamps_bound = match module.future_timestamp_upper_bound() {
+        Ok(bound) => bound,
+        Err(e) => {
+            error!(target: "darkfid::task::handle_reorg", "Future timestamp upper bound retriaval failed: {e}");
+            return false
+        }
+    };
+
     // Retrieve the headers of the hashes sequence and its ranking
     let (targets_rank, hashes_rank) = match retrieve_peer_headers_sequence_ranking(
         (&last_common_height, &last_common_hash, &module, &last_difficulty),
         channel,
         proposal,
         &peer_header_hashes,
+        timestamps_bound,
     )
     .await
     {
@@ -377,6 +403,7 @@ async fn handle_reorg(
         &validator,
         (&last_common_height, &module, &last_difficulty),
         &peer_proposals,
+        Some(timestamps_bound),
     )
     .await
     {
@@ -529,6 +556,8 @@ async fn retrieve_peer_headers_sequence_ranking(
     proposal: &Proposal,
     // Peer header hashes sequence
     header_hashes: &[HeaderHash],
+    // Timestamps upper bound
+    timestamps_bound: Timestamp,
 ) -> Result<(BigUint, BigUint)> {
     // Communication setup
     let response_sub = channel.0.subscribe_msg::<ForkHeadersResponse>().await?;
@@ -584,6 +613,11 @@ async fn retrieve_peer_headers_sequence_ranking(
                 return Err(Custom(String::from("Invalid header sequence detected")))
             }
 
+            // Verify header timestamp is before the future upper bound
+            if peer_header.timestamp > timestamps_bound {
+                return Err(Custom(String::from("Header timestamp is after the future upper bound")))
+            }
+
             // Verify header hash and calculate its rank
             let (next_difficulty, target_distance_sq, hash_distance_sq) =
                 match header_rank(&mut module, peer_header) {
@@ -620,6 +654,12 @@ async fn retrieve_peer_headers_sequence_ranking(
         return Err(Custom(String::from("Invalid header sequence detected")))
     }
 
+    // Verify trigger proposal header timestamp is before the future
+    // upper bound.
+    if proposal.block.header.timestamp > timestamps_bound {
+        return Err(Custom(String::from("Header timestamp is after the future upper bound")))
+    }
+
     // Verify trigger proposal header hash and calculate its rank
     let (_, target_distance_sq, hash_distance_sq) =
         match header_rank(&mut module, &proposal.block.header) {
@@ -708,6 +748,8 @@ async fn generate_peer_fork(
     last_common_info: (&u32, &PoWModule, &BlockDifficulty),
     // Peer proposals sequence
     proposals: &[Proposal],
+    // Timestamps upper bound
+    timestamps_bound: Option<Timestamp>,
 ) -> Result<Fork> {
     // Create a fork from last common height
     let mut fork =
@@ -750,7 +792,7 @@ async fn generate_peer_fork(
         info!(target: "darkfid::task::handle_reorg", "Processing proposal: {} - {}", proposal.hash, proposal.block.header.height);
 
         // Verify proposal
-        verify_fork_proposal(&mut fork, proposal, validator.verify_fees).await?;
+        verify_fork_proposal(&mut fork, proposal, timestamps_bound, validator.verify_fees).await?;
 
         // Append proposal
         fork.append_proposal(proposal).await?;

+ 2 - 2
bin/darkfid/src/tests/harness.rs

@@ -174,7 +174,7 @@ impl Harness {
         // and then we broadcast it to rest nodes
         for block in blocks {
             let proposal = Proposal::new(block.clone());
-            self.alice.validator.write().await.append_proposal(&proposal).await?;
+            self.alice.validator.write().await.append_proposal(&proposal, None).await?;
             let message = ProposalMessage(proposal);
             self.alice.p2p_handler.p2p.broadcast(&message).await;
         }
@@ -265,7 +265,7 @@ impl Harness {
             &mut fork.module,
             &block,
             &previous,
-            true,
+            None,
             self.alice.validator.read().await.verify_fees,
         )
         .await?;

+ 1 - 1
bin/darkfid/src/tests/mod.rs

@@ -86,7 +86,7 @@ async fn sync_blocks_real(ex: Arc<Executor<'static>>) -> Result<()> {
         &mut fork.module,
         &block3,
         &block2,
-        true,
+        None,
         alice.verify_fees,
     )
     .await?;

+ 4 - 2
src/validator/consensus.rs

@@ -32,6 +32,7 @@ use crate::{
     },
     runtime::vm_runtime::GAS_LIMIT,
     tx::{Transaction, MAX_TX_CALLS},
+    util::time::Timestamp,
     validator::{
         pow::{PoWModule, RANDOMX_KEY_CHANGE_DELAY, RANDOMX_KEY_CHANGING_HEIGHT},
         utils::{best_fork_index, block_rank, find_extended_fork_index, worst_fork_index},
@@ -132,7 +133,7 @@ impl Consensus {
     pub async fn append_proposal(
         &mut self,
         proposal: &Proposal,
-        is_new: bool,
+        timestamp_bound: Option<Timestamp>,
         verify_fees: bool,
     ) -> Result<()> {
         debug!(target: "validator::consensus::append_proposal", "Appending proposal {}", proposal.hash);
@@ -157,7 +158,8 @@ impl Consensus {
         }
 
         // Verify proposal and grab corresponding fork
-        let (mut fork, index) = verify_proposal(self, proposal, is_new, verify_fees).await?;
+        let (mut fork, index) =
+            verify_proposal(self, proposal, timestamp_bound, verify_fees).await?;
 
         // Append proposal to the fork
         fork.append_proposal(proposal).await?;

+ 15 - 4
src/validator/mod.rs

@@ -31,6 +31,7 @@ use crate::{
     },
     error::TxVerifyFailed,
     tx::Transaction,
+    util::time::Timestamp,
     zk::VerifyingKey,
     Error, Result,
 };
@@ -239,8 +240,12 @@ impl Validator {
 
     /// The node tries to append provided proposal to its consensus
     /// state.
-    pub async fn append_proposal(&mut self, proposal: &Proposal) -> Result<()> {
-        self.consensus.append_proposal(proposal, self.synced, self.verify_fees).await
+    pub async fn append_proposal(
+        &mut self,
+        proposal: &Proposal,
+        timestamp_bound: Option<Timestamp>,
+    ) -> Result<()> {
+        self.consensus.append_proposal(proposal, timestamp_bound, self.verify_fees).await
     }
 
     /// The node checks if best fork can be confirmed.
@@ -448,6 +453,9 @@ impl Validator {
         let mut diffs = vec![];
         let mut inverse_diffs = vec![];
 
+        // All blocks must be before the future timestamp upper bound
+        let timestamp_bound = Some(module.future_timestamp_upper_bound()?);
+
         // Validate and insert each block
         for block in blocks {
             // Verify block
@@ -457,7 +465,7 @@ impl Validator {
                 &mut module,
                 block,
                 previous,
-                true,
+                timestamp_bound,
                 self.verify_fees,
             )
             .await
@@ -675,6 +683,9 @@ impl Validator {
         // Keep track of all block database state diffs
         let mut diffs = vec![];
 
+        // All blocks must be before the future timestamp upper bound
+        let timestamp_bound = Some(module.future_timestamp_upper_bound()?);
+
         // Validate and insert each block
         info!(target: "validator::validate_blockchain", "Validating rest blocks...");
         blocks_count -= 1;
@@ -690,7 +701,7 @@ impl Validator {
                 &mut module,
                 &block,
                 &previous,
-                false,
+                timestamp_bound,
                 self.verify_fees,
             )
             .await

+ 24 - 13
src/validator/pow.rs

@@ -255,27 +255,38 @@ impl PoWModule {
         Ok(difficulty == &self.next_difficulty()?)
     }
 
-    /// Verify provided block timestamp is not far in the future and
-    /// check its valid acorrding to current timestamps median.
-    pub fn verify_current_timestamp(&self, timestamp: Timestamp) -> Result<bool> {
-        if timestamp > Timestamp::current_time().checked_add(BLOCK_FUTURE_TIME_LIMIT)? {
-            return Ok(false)
-        }
-
-        Ok(self.verify_timestamp_by_median(timestamp))
+    /// Auxiliary funtion to retrieve currently set future timestamp
+    /// bound.
+    pub fn future_timestamp_upper_bound(&self) -> Result<Timestamp> {
+        Timestamp::current_time().checked_add(BLOCK_FUTURE_TIME_LIMIT)
     }
 
     /// Verify provided block timestamp is valid and matches certain
     /// criteria.
-    pub fn verify_timestamp_by_median(&self, timestamp: Timestamp) -> bool {
+    pub fn verify_timestamp_by_median(
+        &self,
+        timestamp: Timestamp,
+        upper_bound: Option<Timestamp>,
+    ) -> Result<bool> {
         // Check timestamp is after genesis one
         if timestamp <= self.genesis {
-            return false
+            return Ok(false)
+        }
+
+        // If an upper bound is not provided, use default future bound
+        let upper_bound = match upper_bound {
+            Some(bound) => bound,
+            None => self.future_timestamp_upper_bound()?,
+        };
+
+        // Check timestamp is not after the upper bound
+        if timestamp > upper_bound {
+            return Ok(false)
         }
 
         // If not enough blocks, no proper median yet, return true
         if self.timestamps.len() < BLOCKCHAIN_TIMESTAMP_CHECK_WINDOW {
-            return true
+            return Ok(true)
         }
 
         // Make sure the timestamp is higher or equal to the median
@@ -287,13 +298,13 @@ impl PoWModule {
             .map(|x| x.inner())
             .collect();
 
-        timestamp >= median(timestamps).into()
+        Ok(timestamp >= median(timestamps).into())
     }
 
     /// Verify provided block timestamp and hash.
     pub fn verify_current_block(&mut self, header: &Header) -> Result<()> {
         // First we verify the block's timestamp
-        if !self.verify_current_timestamp(header.timestamp)? {
+        if !self.verify_timestamp_by_median(header.timestamp, None)? {
             return Err(Error::PoWInvalidTimestamp)
         }
 

+ 14 - 13
src/validator/verification.rs

@@ -43,6 +43,7 @@ use crate::{
     error::TxVerifyFailed,
     runtime::vm_runtime::{Runtime, TxLocalState},
     tx::{Transaction, MAX_TX_CALLS, MIN_TX_CALLS},
+    util::time::Timestamp,
     validator::{
         consensus::{Consensus, Fork, Proposal, BLOCK_GAS_LIMIT},
         fees::{circuit_gas_use, GasData, PALLAS_SCHNORR_SIGNATURE_FEE},
@@ -163,7 +164,7 @@ pub fn validate_block(
     block: &BlockInfo,
     previous: &BlockInfo,
     module: &mut PoWModule,
-    is_new: bool,
+    timestamp_bound: Option<Timestamp>,
 ) -> Result<()> {
     // Check block version (1)
     if block.header.version != block_version(block.header.height) {
@@ -181,12 +182,7 @@ pub fn validate_block(
     }
 
     // Check timestamp validity (4)
-    let valid = if is_new {
-        module.verify_current_timestamp(block.header.timestamp)?
-    } else {
-        module.verify_timestamp_by_median(block.header.timestamp)
-    };
-    if !valid {
+    if !module.verify_timestamp_by_median(block.header.timestamp, timestamp_bound)? {
         return Err(Error::BlockIsInvalid(block.hash().as_string()))
     }
 
@@ -214,10 +210,14 @@ pub fn validate_blockchain(
 
     // We use block order store here so we have all blocks in order
     let blocks = blockchain.blocks.get_all_order()?;
+
+    // All blocks must be before the future timestamp upper bound
+    let timestamp_bound = Some(module.future_timestamp_upper_bound()?);
+
     for (index, block) in blocks[1..].iter().enumerate() {
         let full_blocks = blockchain.get_blocks_by_hash(&[blocks[index].1, block.1])?;
         let full_block = &full_blocks[1];
-        validate_block(full_block, &full_blocks[0], &mut module, false)?;
+        validate_block(full_block, &full_blocks[0], &mut module, timestamp_bound)?;
         // Update PoW module
         module.append(&full_block.header, &module.next_difficulty()?)?;
     }
@@ -235,7 +235,7 @@ pub async fn verify_block(
     module: &mut PoWModule,
     block: &BlockInfo,
     previous: &BlockInfo,
-    is_new: bool,
+    timestamp_bound: Option<Timestamp>,
     verify_fees: bool,
 ) -> Result<()> {
     let block_hash = block.hash();
@@ -247,7 +247,7 @@ pub async fn verify_block(
     }
 
     // Validate block, using its previous
-    validate_block(block, previous, module, is_new)?;
+    validate_block(block, previous, module, timestamp_bound)?;
 
     // Verify transactions vector contains at least one(producers)
     // transaction.
@@ -1176,7 +1176,7 @@ async fn apply_transactions(
 pub async fn verify_proposal(
     consensus: &Consensus,
     proposal: &Proposal,
-    is_new: bool,
+    timestamp_bound: Option<Timestamp>,
     verify_fees: bool,
 ) -> Result<(Fork, Option<usize>)> {
     // Check if proposal hash matches actual one (1)
@@ -1202,7 +1202,7 @@ pub async fn verify_proposal(
         &mut fork.module,
         &proposal.block,
         &previous,
-        is_new,
+        timestamp_bound,
         verify_fees,
     )
     .await
@@ -1226,6 +1226,7 @@ pub async fn verify_proposal(
 pub async fn verify_fork_proposal(
     fork: &mut Fork,
     proposal: &Proposal,
+    timestamp_bound: Option<Timestamp>,
     verify_fees: bool,
 ) -> Result<()> {
     // Check if proposal hash matches actual one (1)
@@ -1248,7 +1249,7 @@ pub async fn verify_fork_proposal(
         &mut fork.module,
         &proposal.block,
         &previous,
-        false,
+        timestamp_bound,
         verify_fees,
     )
     .await