Simply return replay error on expiration

Adhering to the principle that only primary information becomes an
event, knowing the session's expiration (in SessionContext) is
sufficient and no new information implies no outcome event.

The state machines already adhere by this and only returns an error
without resulting in a state transition. This commit applies the same
logic to `replay_event_log` for sender and receiver.

Even assuming a non-monotonic system clock, the implication of this
change is that a session that was previously considered expired may turn
out to not actually be expired, and is allowed to resume. This is
desirable behavior if e.g. the clock being incorrectly set to a future
time was what led to the session being expired in the first place.

Author: spacebear21

payjoin/src/core/error.rs

@@ -51,6 +51,7 @@ impl<SessionState: Debug, SessionEvent: Debug> std::fmt::Display
                 Some(session) => write!(f, "Invalid event ({event:?}) for session ({session:?})",),
                 None => write!(f, "Invalid first event ({event:?}) for session",),
             },
+            Expired(time) => write!(f, "Session expired at {time:?}"),
             PersistenceFailure(e) => write!(f, "Persistence failure: {e}"),
         }
     }
@@ -75,6 +76,8 @@ pub(crate) enum InternalReplayError<SessionState, SessionEvent> {
     NoEvents,
     /// Invalid initial event
     InvalidEvent(Box<SessionEvent>, Option<Box<SessionState>>),
+    /// Session is expired
+    Expired(crate::time::Time),
     /// Application storage error
     PersistenceFailure(ImplementationError),
 }

payjoin/src/core/receive/v2/session.rs

@@ -4,7 +4,7 @@ use super::{ReceiveSession, SessionContext};
 use crate::error::{InternalReplayError, ReplayError};
 use crate::output_substitution::OutputSubstitution;
 use crate::persist::SessionPersister;
-use crate::receive::v2::{extract_err_req, InternalSessionError, SessionError};
+use crate::receive::v2::{extract_err_req, SessionError};
 use crate::receive::{common, InputPair, JsonReply, OriginalPayload, PsbtContext};
 use crate::{ImplementationError, IntoUrl, PjUri, Request};
 
@@ -44,27 +44,15 @@ where
     let history = SessionHistory::new(session_events.clone());
     let ctx = history.session_context();
     if ctx.expiration.elapsed() {
-        // Session has expired: close the session and persist a fatal error
-        let err = SessionError(InternalSessionError::Expired(ctx.expiration));
-        persister
-            .save_event(SessionEvent::SessionInvalid(err.to_string(), None).into())
-            .map_err(|e| InternalReplayError::PersistenceFailure(ImplementationError::new(e)))?;
-        persister
-            .close()
-            .map_err(|e| InternalReplayError::PersistenceFailure(ImplementationError::new(e)))?;
-
-        session_events.push(SessionEvent::SessionInvalid(err.to_string(), None));
-        let history = SessionHistory::new(session_events);
-
-        return Ok((ReceiveSession::TerminalFailure, history));
+        return Err(InternalReplayError::Expired(ctx.expiration).into());
     }
 
     Ok((receiver, history))
 }
 
 /// A collection of events that have occurred during a receiver's session.
 /// It is obtained by calling [replay_event_log].
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct SessionHistory {
     events: Vec<SessionEvent>,
 }
@@ -372,21 +360,16 @@ mod tests {
 
     #[test]
     fn test_replaying_session_creation_with_expired_session() -> Result<(), BoxError> {
-        let session_context = SessionContext {
-            expiration: (SystemTime::now() - Duration::from_secs(1)).try_into().unwrap(),
-            ..SHARED_CONTEXT.clone()
-        };
-        let test = SessionHistoryTest {
-            events: vec![SessionEvent::Created(session_context.clone())],
-            expected_session_history: SessionHistoryExpectedOutcome {
-                psbt_with_fee_contributions: None,
-                fallback_tx: None,
-                expected_status: SessionStatus::Expired,
-            },
-            expected_receiver_state: ReceiveSession::TerminalFailure,
-        };
-        // TODO: should check for the expired error message off the session history
-        run_session_history_test(test)
+        let expiration = (SystemTime::now() - Duration::from_secs(1)).try_into().unwrap();
+        let session_context = SessionContext { expiration, ..SHARED_CONTEXT.clone() };
+        let persister = InMemoryTestPersister::<SessionEvent>::default();
+        persister.save_event(SessionEvent::Created(session_context))?;
+
+        let err = replay_event_log(&persister).expect_err("session should be expired");
+        let expected_err: ReplayError<ReceiveSession, SessionEvent> =
+            InternalReplayError::Expired(expiration).into();
+        assert_eq!(err.to_string(), expected_err.to_string());
+        Ok(())
     }
 
     #[test]

payjoin/src/core/send/v2/session.rs

@@ -40,24 +40,12 @@ where
     let history = SessionHistory::new(session_events.clone());
     let pj_param = history.pj_param();
     if pj_param.expiration().elapsed() {
-        // Session has expired: close the session and persist a fatal error
-        let session_event = SessionEvent::SessionInvalid("Session expired".to_string());
-        persister
-            .save_event(session_event.clone().into())
-            .map_err(|e| InternalReplayError::PersistenceFailure(ImplementationError::new(e)))?;
-        persister
-            .close()
-            .map_err(|e| InternalReplayError::PersistenceFailure(ImplementationError::new(e)))?;
-
-        session_events.push(session_event);
-        let history = SessionHistory::new(session_events);
-
-        return Ok((SendSession::TerminalFailure, history));
+        return Err(InternalReplayError::Expired(pj_param.expiration()).into());
     }
     Ok((sender, history))
 }
 
-#[derive(Clone)]
+#[derive(Debug, Clone)]
 pub struct SessionHistory {
     events: Vec<SessionEvent>,
 }
@@ -225,12 +213,13 @@ mod tests {
         .unwrap();
         let reply_key = HpkeKeyPair::gen_keypair();
         let endpoint = sender.endpoint().clone();
-        let fallback_tx = sender.psbt_ctx.original_psbt.clone().extract_tx_unchecked_fee_rate();
         let id = crate::uri::ShortId::try_from(&b"12345670"[..]).expect("valid short id");
+        let expiration =
+            (std::time::SystemTime::now() - std::time::Duration::from_secs(1)).try_into().unwrap();
         let pj_param = crate::uri::v2::PjParam::new(
             endpoint,
             id,
-            (std::time::SystemTime::now() - std::time::Duration::from_secs(1)).try_into().unwrap(),
+            expiration,
             crate::OhttpKeys(
                 ohttp::KeyConfig::new(KEY_ID, KEM, Vec::from(SYMMETRIC)).expect("valid key config"),
             ),
@@ -241,13 +230,15 @@ mod tests {
             psbt_ctx: sender.psbt_ctx.clone(),
             reply_key: reply_key.0,
         };
-        let test = SessionHistoryTest {
-            events: vec![SessionEvent::CreatedReplyKey(with_reply_key)],
-            expected_session_history: SessionHistoryExpectedOutcome { fallback_tx, pj_param },
-            expected_sender_state: SendSession::TerminalFailure,
-            expected_error: Some("Session expired".to_string()),
-        };
-        run_session_history_test(test);
+        let persister = InMemoryTestPersister::<SessionEvent>::default();
+        persister
+            .save_event(SessionEvent::CreatedReplyKey(with_reply_key))
+            .expect("save_event should succeed");
+
+        let err = replay_event_log(&persister).expect_err("session should be expired");
+        let expected_err: ReplayError<SendSession, SessionEvent> =
+            InternalReplayError::Expired(expiration).into();
+        assert_eq!(err.to_string(), expected_err.to_string());
     }
 
     #[test]