Validation bugfix (#2089)

Adds missing validation steps

  * validate timestamp (against system time and previous block)
  * wait for l1_finalized
  * wait for l1_head
  * validate blocks are not decrementing
  * small update to pee-existing tests to avoid hitting these cases
  * adds TODOs for followups


---------

Co-authored-by: tbro <tbro@users.noreply.github.com>
Co-authored-by: Jeb Bearer <jeb@espressosys.com>
Co-authored-by: Mathis <sveitser@gmail.com>

.github/workflows/test.yml

@@ -63,7 +63,7 @@ jobs:
       - name: Test
         # Build test binary with `testing` feature, which requires `hotshot_example` config
         run: cargo nextest run --locked --release --workspace --all-features --verbose --no-fail-fast
-        timeout-minutes: 5
+        timeout-minutes: 20
 
       - name: Install process-compose
         run: nix profile install nixpkgs#process-compose

types/src/v0/impls/header.rs

@@ -1374,6 +1374,7 @@ mod test_headers {
 
     #[async_std::test]
     async fn test_validate_proposal_error_cases() {
+        // TODO add assertion for timestamp validation
         let genesis = GenesisForTest::default().await;
         let vid_common = vid_scheme(1).disperse([]).unwrap().common;
 
@@ -1390,6 +1391,8 @@ mod test_headers {
         validated_state.block_merkle_tree = block_merkle_tree.clone();
         *parent_header.block_merkle_tree_root_mut() = block_merkle_tree_root;
         let mut proposal = parent_header.clone();
+        *proposal.timestamp_mut() = OffsetDateTime::now_utc().unix_timestamp() as u64;
+        *proposal.l1_head_mut() = 5;
 
         let ver = StaticVersion::<0, 1>::version();
 

types/src/v0/impls/l1.rs

@@ -79,6 +79,34 @@ impl L1Client {
         L1Snapshot { head, finalized }
     }
 
+    pub async fn wait_for_block(&self, number: u64) -> L1BlockInfo {
+        let interval = self.provider.get_interval();
+        loop {
+            let block = match self.provider.get_block(number).await {
+                Ok(Some(block)) => block,
+                Ok(None) => {
+                    tracing::info!(number, "no such block");
+                    sleep(interval).await;
+                    continue;
+                }
+                Err(err) => {
+                    tracing::error!(%err, number, "failed to get L1 block");
+                    sleep(interval).await;
+                    continue;
+                }
+            };
+            let Some(hash) = block.hash else {
+                tracing::error!(number, ?block, "L1 block has no hash");
+                sleep(interval).await;
+                continue;
+            };
+            break L1BlockInfo {
+                number,
+                hash,
+                timestamp: block.timestamp,
+            };
+        }
+    }
     /// Get information about the given block.
     ///
     /// If the desired block number is not finalized yet, this function will block until it becomes
@@ -107,31 +135,7 @@ impl L1Client {
 
         // The finalized block may have skipped over the block of interest. In this case, our block
         // is still finalized, since it is before the finalized block. We just need to fetch it.
-        loop {
-            let block = match self.provider.get_block(number).await {
-                Ok(Some(block)) => block,
-                Ok(None) => {
-                    tracing::error!(number, "no such block");
-                    sleep(interval).await;
-                    continue;
-                }
-                Err(err) => {
-                    tracing::error!(%err, number, "failed to get L1 block");
-                    sleep(interval).await;
-                    continue;
-                }
-            };
-            let Some(hash) = block.hash else {
-                tracing::error!(number, ?block, "L1 block has no hash");
-                sleep(interval).await;
-                continue;
-            };
-            break L1BlockInfo {
-                number,
-                hash,
-                timestamp: block.timestamp,
-            };
-        }
+        self.wait_for_block(number).await
     }
 
     /// Proxy to `Provider.get_block_number`.

types/src/v0/impls/state.rs

@@ -23,6 +23,7 @@ use num_traits::CheckedSub;
 use serde::{Deserialize, Serialize};
 use std::ops::Add;
 use thiserror::Error;
+use time::OffsetDateTime;
 use vbs::version::Version;
 
 use super::{
@@ -94,6 +95,20 @@ pub enum ProposalValidationError {
     InvalidNsTable { err: NsTableValidationError },
     #[error("Some fee amount or their sum total out of range")]
     SomeFeeAmountOutOfRange,
+    #[error("Invalid timestamp: proposal={proposal_timestamp}, parent={parent_timestamp}")]
+    InvalidTimestampNonIncrementing {
+        proposal_timestamp: u64,
+        parent_timestamp: u64,
+    },
+    #[error("Invalid timestamp: local:={local_timestamp}, proposal={proposal_timestamp}")]
+    InvalidTimestampDrift {
+        proposal_timestamp: u64,
+        local_timestamp: u64,
+    },
+    #[error("l1_finalized has `None` value")]
+    L1FinalizedNotFound,
+    #[error("Invalid timestamp: parent:={parent}, proposal={proposal}")]
+    NonIncrementingL1Head { parent: u64, proposal: u64 },
 }
 
 impl StateDelta for Delta {}
@@ -306,6 +321,26 @@ pub fn validate_proposal(
         });
     }
 
+    // Validate timestamp increasing.
+    // TODO add test https://github.com/EspressoSystems/espresso-sequencer/issues/2100
+    if proposal.timestamp() < parent_header.timestamp() {
+        return Err(ProposalValidationError::InvalidTimestampNonIncrementing {
+            proposal_timestamp: proposal.timestamp(),
+            parent_timestamp: parent_header.timestamp(),
+        });
+    }
+
+    // Validate timestamp hasn't drifted too much from system time.
+    let system_time: u64 = OffsetDateTime::now_utc().unix_timestamp() as u64;
+    // TODO 2 seconds of tolerance should be enough for reasonably
+    // configured nodes, but we should make this configurable.
+    if proposal.timestamp().abs_diff(system_time) > 2 {
+        return Err(ProposalValidationError::InvalidTimestampDrift {
+            proposal_timestamp: proposal.timestamp(),
+            local_timestamp: system_time,
+        });
+    }
+
     let ValidatedState {
         block_merkle_tree,
         fee_merkle_tree,
@@ -663,6 +698,40 @@ impl HotShotState<SeqTypes> for ValidatedState {
             .resolve()
             .expect("Chain Config not found in validated state");
 
+        // Validate l1_finalized.
+        // TODO add test https://github.com/EspressoSystems/espresso-sequencer/issues/2100
+        let proposed_finalized = proposed_header.l1_finalized();
+        let parent_finalized = parent_leaf.block_header().l1_finalized();
+        if proposed_finalized < parent_finalized {
+            tracing::error!(
+                "L1 finalized not incrementing. parent: {:?}, proposal: {:?}",
+                parent_finalized,
+                proposed_finalized,
+            );
+        }
+        if let Some(proposed_finalized) = proposed_finalized {
+            let finalized = instance
+                .l1_client
+                .wait_for_finalized_block(proposed_finalized.number())
+                .await;
+
+            if finalized != proposed_finalized {
+                tracing::error!("Invalid proposal: l1_finalized mismatch");
+                return Err(BlockError::InvalidBlockHeader);
+            }
+        }
+        // Validate `l1_head`.
+        // TODO add test https://github.com/EspressoSystems/espresso-sequencer/issues/2100
+        if proposed_header.l1_head() < parent_leaf.block_header().l1_head() {
+            tracing::error!("Invalid proposal: l1_head decreasing");
+            return Err(BlockError::InvalidBlockHeader);
+        }
+
+        let _ = instance
+            .l1_client
+            .wait_for_block(proposed_header.l1_head())
+            .await;
+
         // validate the proposal
         if let Err(err) = validate_proposal(
             &validated_state,
@@ -671,7 +740,7 @@ impl HotShotState<SeqTypes> for ValidatedState {
             proposed_header,
             &vid_common,
         ) {
-            tracing::error!("invalid proposal: {err:#}");
+            tracing::error!("Invalid proposal: {err:#}");
             return Err(BlockError::InvalidBlockHeader);
         }