Simply return replay error on expiration (payjoin#1107)

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]