From f3114e617edf202b0c7d48116b21e196601db5a1 Mon Sep 17 00:00:00 2001 From: Mathis Date: Wed, 9 Oct 2024 21:48:17 +0200 Subject: [PATCH] Check timestamp first when validating header (#2137) * Check timestamp first when validating header * log timestamp drift validation error (cherry picked from commit ebe8a26e590df4b578d5e8ce0abde686668bcd11) --- types/src/v0/impls/state.rs | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/types/src/v0/impls/state.rs b/types/src/v0/impls/state.rs index 0a0c130393..1783728345 100644 --- a/types/src/v0/impls/state.rs +++ b/types/src/v0/impls/state.rs @@ -330,17 +330,6 @@ pub fn validate_proposal( }); } - // Validate timestamp hasn't drifted too much from system time. - let system_time: u64 = OffsetDateTime::now_utc().unix_timestamp() as u64; - // TODO 12 seconds of tolerance should be enough for reasonably - // configured nodes, but we should make this configurable. - if proposal.timestamp().abs_diff(system_time) > 12 { - return Err(ProposalValidationError::InvalidTimestampDrift { - proposal_timestamp: proposal.timestamp(), - local_timestamp: system_time, - }); - } - let ValidatedState { block_merkle_tree, fee_merkle_tree, @@ -680,6 +669,22 @@ impl HotShotState for ValidatedState { vid_common: VidCommon, version: Version, ) -> Result<(Self, Self::Delta), Self::Error> { + // Validate timestamp hasn't drifted too much from system time. + // Do this check first so we don't add unnecessary drift. + let system_time: u64 = OffsetDateTime::now_utc().unix_timestamp() as u64; + // TODO 12 seconds of tolerance should be enough for reasonably + // configured nodes, but we should make this configurable. + let diff = proposed_header.timestamp().abs_diff(system_time); + if diff > 12 { + tracing::warn!( + "Timestamp drift too high proposed={} system={} diff={}", + proposed_header.timestamp(), + system_time, + diff + ); + return Err(BlockError::InvalidBlockHeader); + } + //validate builder fee if let Err(err) = validate_builder_fee(proposed_header) { tracing::error!("invalid builder fee: {err:#}");