feat(nostr_manager event_processor): add timestamp validation with future skew protection and giftwrap timestamp handling

Author: jgmontoya

src/integration_tests/test_cases/subscription_processing/verify_last_synced_timestamp.rs

@@ -1,8 +1,10 @@
-use crate::integration_tests::core::*;
-use crate::WhitenoiseError;
+use std::time::Duration;
+
 use async_trait::async_trait;
 use nostr_sdk::prelude::*;
-use std::time::Duration;
+
+use crate::integration_tests::core::*;
+use crate::WhitenoiseError;
 
 pub struct VerifyLastSyncedTimestampTestCase {
     mode: Mode,
@@ -31,8 +33,8 @@ impl VerifyLastSyncedTimestampTestCase {
         context: &ScenarioContext,
         pubkey: PublicKey,
     ) -> Result<Option<chrono::DateTime<chrono::Utc>>, WhitenoiseError> {
-        let fresh = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
-        Ok(fresh.last_synced_at)
+        let account = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
+        Ok(account.last_synced_at)
     }
 
     async fn assert_advanced(
@@ -43,13 +45,13 @@ impl VerifyLastSyncedTimestampTestCase {
         description: &str,
     ) -> Result<(), WhitenoiseError> {
         retry(
-            50,
-            Duration::from_millis(50),
+            100,
+            Duration::from_millis(75),
             || async {
-                let refreshed = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
-                match (before, refreshed.last_synced_at) {
+                let account = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
+                match (before, account.last_synced_at) {
                     (None, Some(_)) => Ok(()),
-                    (Some(b), Some(a)) if a > b => Ok(()),
+                    (Some(before_time), Some(after_time)) if after_time > before_time => Ok(()),
                     _ => Err(WhitenoiseError::Other(anyhow::anyhow!(
                         "last_synced_at not advanced yet"
                     ))),
@@ -71,12 +73,8 @@ impl VerifyLastSyncedTimestampTestCase {
             50,
             Duration::from_millis(50),
             || async {
-                let after = context
-                    .whitenoise
-                    .find_account_by_pubkey(&pubkey)
-                    .await?
-                    .last_synced_at;
-                if after == before {
+                let account = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
+                if account.last_synced_at == before {
                     Ok(())
                 } else {
                     Err(WhitenoiseError::Other(anyhow::anyhow!(
@@ -89,38 +87,49 @@ impl VerifyLastSyncedTimestampTestCase {
         .await
     }
 
-    async fn publish_account_follow_event(
+    async fn publish_account_follow_event_with_timestamp(
         &self,
         context: &ScenarioContext,
         pubkey: PublicKey,
+        event_timestamp: Timestamp,
     ) -> Result<(), WhitenoiseError> {
-        let account_owned = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
-        let nsec = context
-            .whitenoise
-            .export_account_nsec(&account_owned)
-            .await?;
+        let account = context.whitenoise.find_account_by_pubkey(&pubkey).await?;
+        let nsec = context.whitenoise.export_account_nsec(&account).await?;
         let keys = Keys::parse(&nsec)?;
-        let client = create_test_client(&context.dev_relays, keys).await?;
+        let client = create_test_client(&context.dev_relays, keys.clone()).await?;
         let contact = Keys::generate().public_key();
-        publish_follow_list(&client, &[contact]).await?;
+
+        let tags = vec![Tag::custom(TagKind::p(), [contact.to_hex()])];
+        let event = EventBuilder::new(Kind::ContactList, "")
+            .tags(tags)
+            .custom_created_at(event_timestamp)
+            .sign_with_keys(&keys)
+            .map_err(|e| WhitenoiseError::Other(e.into()))?;
+
+        client.send_event(&event).await?;
         tokio::time::sleep(tokio::time::Duration::from_secs(1)).await;
         client.disconnect().await;
         Ok(())
     }
 
-    async fn publish_global_metadata_event(
+    async fn publish_global_metadata_event_with_timestamp(
         &self,
         context: &ScenarioContext,
+        event_timestamp: Timestamp,
     ) -> Result<(), WhitenoiseError> {
-        let ext = Keys::generate();
-        let client = create_test_client(&context.dev_relays, ext).await?;
+        let keys = Keys::generate();
+        let client = create_test_client(&context.dev_relays, keys.clone()).await?;
         let metadata = Metadata {
-            name: Some("No-op for account sync".to_string()),
+            name: Some("Test metadata for sync verification".to_string()),
             ..Default::default()
         };
-        client
-            .send_event_builder(EventBuilder::metadata(&metadata))
-            .await?;
+
+        let event = EventBuilder::metadata(&metadata)
+            .custom_created_at(event_timestamp)
+            .sign_with_keys(&keys)
+            .map_err(|e| WhitenoiseError::Other(e.into()))?;
+
+        client.send_event(&event).await?;
         tokio::time::sleep(tokio::time::Duration::from_millis(600)).await;
         client.disconnect().await;
         Ok(())
@@ -132,9 +141,37 @@ impl TestCase for VerifyLastSyncedTimestampTestCase {
     async fn run(&self, context: &mut ScenarioContext) -> Result<(), WhitenoiseError> {
         let pubkey = { context.get_account("subscription_test_account")?.pubkey };
         let before = self.baseline(context, pubkey).await?;
+
+        // Create deterministic base timestamp for this test run
+        let base_timestamp = Timestamp::now();
+
         match self.mode {
             Mode::AccountFollowEvent => {
-                self.publish_account_follow_event(context, pubkey).await?;
+                // Wait to ensure that when the event timestamp gets capped to now(),
+                // it will still be greater than the baseline last_synced_at
+                if let Some(before_time) = before {
+                    let before_timestamp_secs = before_time.timestamp() as u64;
+                    let current_timestamp_secs = Timestamp::now().as_u64();
+
+                    if current_timestamp_secs <= before_timestamp_secs {
+                        // Wait enough time to ensure now() > baseline when event gets processed
+                        let wait_time = (before_timestamp_secs - current_timestamp_secs + 2) * 1000; // +2 seconds buffer
+                        tracing::debug!(
+                            target: "verify_last_synced_timestamp",
+                            "Waiting {}ms to ensure capped timestamp > baseline ({} vs {})",
+                            wait_time,
+                            current_timestamp_secs,
+                            before_timestamp_secs
+                        );
+                        tokio::time::sleep(std::time::Duration::from_millis(wait_time)).await;
+                    }
+                }
+
+                // Use base timestamp + 10 seconds for guaranteed advancement
+                // Even if this gets capped to now(), it should be > baseline due to our wait
+                let event_timestamp = Timestamp::from_secs(base_timestamp.as_u64() + 10);
+                self.publish_account_follow_event_with_timestamp(context, pubkey, event_timestamp)
+                    .await?;
                 self.assert_advanced(
                     context,
                     pubkey,
@@ -144,7 +181,10 @@ impl TestCase for VerifyLastSyncedTimestampTestCase {
                 .await?;
             }
             Mode::GlobalMetadataEvent => {
-                self.publish_global_metadata_event(context).await?;
+                // Use base timestamp + 5 seconds (should not affect account sync)
+                let event_timestamp = Timestamp::from_secs(base_timestamp.as_u64() + 5);
+                self.publish_global_metadata_event_with_timestamp(context, event_timestamp)
+                    .await?;
                 self.assert_unchanged(
                     context,
                     pubkey,

src/nostr_manager/query.rs

@@ -1,17 +1,12 @@
 //! This module contains functions for querying Nostr events from relays.
 
-use std::time::Duration;
-
 use nostr_sdk::prelude::*;
 
 use crate::{
-    nostr_manager::{NostrManager, Result},
+    nostr_manager::{utils::is_event_timestamp_valid, NostrManager, Result},
     RelayType,
 };
 
-/// Maximum allowed skew for event timestamps in the future (1 hour)
-const MAX_FUTURE_SKEW: Duration = Duration::from_secs(60 * 60);
-
 impl NostrManager {
     pub(crate) async fn fetch_metadata_from(
         &self,
@@ -54,11 +49,9 @@ impl NostrManager {
     }
 
     fn latest_from_events(events: Events) -> Result<Option<Event>> {
-        // Filter out events with timestamps too far in the future
-        let cutoff = Timestamp::now() + MAX_FUTURE_SKEW;
         let latest = events
             .into_iter()
-            .filter(|e| e.created_at <= cutoff)
+            .filter(is_event_timestamp_valid)
             .max_by_key(|e| (e.created_at, e.id));
         Ok(latest)
     }

src/nostr_manager/subscriptions.rs

@@ -9,7 +9,9 @@ use sha2::{Digest, Sha256};
 
 const MAX_USERS_PER_GLOBAL_SUBSCRIPTION: usize = 1000;
 
-use crate::nostr_manager::{NostrManager, NostrManagerError, Result};
+use crate::nostr_manager::{
+    utils::adjust_since_for_giftwrap, NostrManager, NostrManagerError, Result,
+};
 
 impl NostrManager {
     /// Create a short hash from a pubkey for use in subscription IDs
@@ -372,7 +374,9 @@ impl NostrManager {
     ) -> Result<()> {
         tracing::debug!(
             target: "whitenoise::nostr_manager::setup_user_follow_list_subscription",
-            "Setting up user follow list subscription"
+            "Setting up subscription for pubkey {} on {} relays",
+            pubkey.to_hex(),
+            user_relays.len()
         );
         let pubkey_hash = self.create_pubkey_hash(&pubkey);
         let subscription_id = SubscriptionId::new(format!("{}_user_follow_list", pubkey_hash));
@@ -383,12 +387,14 @@ impl NostrManager {
         }
 
         self.client
-            .subscribe_with_id_to(user_relays, subscription_id, user_follow_list_filter, None)
+            .subscribe_with_id_to(user_relays, subscription_id.clone(), user_follow_list_filter, None)
             .await?;
 
         tracing::debug!(
             target: "whitenoise::nostr_manager::setup_user_follow_list_subscription",
-            "User follow list subscription set up"
+            "FollowList subscription '{}' set up successfully for {}",
+            subscription_id,
+            pubkey.to_hex()
         );
         Ok(())
     }
@@ -408,7 +414,12 @@ impl NostrManager {
 
         let mut giftwrap_filter = Filter::new().kind(Kind::GiftWrap).pubkey(pubkey);
         if let Some(since) = since {
-            giftwrap_filter = giftwrap_filter.since(since);
+            // Account for NIP-59 backdated timestamps - giftwrap events may be timestamped
+            // in the past for privacy, so we look back further than last_synced_at
+            let adjusted_since = adjust_since_for_giftwrap(Some(since));
+            if let Some(adjusted) = adjusted_since {
+                giftwrap_filter = giftwrap_filter.since(adjusted);
+            }
         }
 
         self.client
@@ -507,4 +518,30 @@ mod tests {
         assert_eq!(hash1, hash2);
         assert_eq!(hash1.len(), 12); // Should be 12 characters as specified
     }
+
+    #[tokio::test]
+    async fn test_giftwrap_subscription_lookback_buffer() {
+        use crate::nostr_manager::utils::GIFTWRAP_LOOKBACK_BUFFER;
+        use std::time::Duration;
+
+        // Test that adjust_since_for_giftwrap extends the lookback period correctly
+        let original_timestamp = Timestamp::now();
+        let adjusted = adjust_since_for_giftwrap(Some(original_timestamp));
+
+        assert!(adjusted.is_some());
+        let adjusted_ts = adjusted.unwrap();
+
+        // Should be exactly GIFTWRAP_LOOKBACK_BUFFER earlier
+        assert_eq!(adjusted_ts, original_timestamp - GIFTWRAP_LOOKBACK_BUFFER);
+
+        // Verify the buffer is significant (7 days)
+        assert_eq!(
+            GIFTWRAP_LOOKBACK_BUFFER,
+            Duration::from_secs(7 * 24 * 60 * 60)
+        );
+
+        // Test with None - should return None
+        let none_result = adjust_since_for_giftwrap(None);
+        assert!(none_result.is_none());
+    }
 }