Fix replayEventLog report closed session as expired.

[zealsham]

This addresses #1651. During replaying of the session event log a completed session is being reported as expired if the expiration time has elapsed when the event is replayed. The fix skips expiration check when the replay state is Closed

Pull Request Checklist

Please confirm the following before requesting review:

• I have disclosed my use of
  AI
  in the body of this PR.
• I have read CONTRIBUTING.md and rebased my branch to produce hygienic commits.

[coveralls]

Coverage Report for CI Build 28111040213

Coverage increased (+0.2%) to 85.332%

Details

• Coverage increased (+0.2%) from the base build.
• Patch coverage: 140 of 140 lines across 2 files are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	14978
Covered Lines:	12781
Line Coverage:	85.33%
Coverage Strength:	364.97 hits per line

---

💛 - Coveralls

[benalleng]

TACK d42b5f5 The history properly displays only sessions that ended with an expiry status

After running resume all the sessions still display their proper Status as the final state.

[benalleng]

Wait this may be incomplete... I think we also need to address status() itself. While this is not a direct problem in payjoin-cli any user of the payjoin crate will run into this where completed sessions will show Expired when running status()

[benalleng]

The same problem occurs here we need a simple ordering of the match to fix this though

pub fn status(&self) -> SessionStatus {
        match self.events.last() {
            Some(SessionEvent::Closed(outcome)) => match outcome {
                SessionOutcome::Success(_) | SessionOutcome::PayjoinProposalSent =>
                    SessionStatus::Completed,
                SessionOutcome::Aborted => SessionStatus::Failed,
                SessionOutcome::FallbackBroadcasted => SessionStatus::FallbackBroadcasted,
            },
            Some(SessionEvent::Cancelled | SessionEvent::ProtocolFailed) =>
                SessionStatus::PendingFallback,
            _ if self.session_context().expiration.elapsed() => SessionStatus::Expired,
            _ => SessionStatus::Active,
        }
    }

[zealsham]

Thanks for the review as well as catching this

[benalleng]

Same here we need to move the expiration SessionStatus to follow only the other non-final outcomes and drop the if statement

_ if self.pj_param().expiration().elapsed() => SessionStatus::Expired,

[bc1cindy]

concept ACK

[bc1cindy]

nit: the guard-arm form @benalleng suggested (_ if expiration.elapsed() => Expired) avoids the _ => {} + trailing if here. wdyt?

[benalleng]

TACK a37f197 I just pushed a commit to use the match block instead of the guard block

[bc1cindy]

ACK

[spacebear21]

I think we may want to take a step back and consider whether replay_event_log should error at all on expired sessions? This solution seems somewhat fragile and depends on the inner implementation details of SessionStatus.

[spacebear21]

@benalleng made a good point for keeping this shape. However I tried it with my local payjoin-cli receiver and still see some odd behavior with the history command.

❯ cargo run -- history
Session ID   Sender/Receiver           Completed At    Status
25           Receiver                  Not Completed   Session expired at Time(Time(1778362082))
1            Receiver                  1778275755      Session expired at Time(Time(1761155560))
2            Receiver                  1778275755      Session expired at Time(Time(1761234352))
3            Receiver                  1778275755      Session expired at Time(Time(1761334774))
4            Receiver                  1778275755      Session expired at Time(Time(1761335340))
5            Receiver                  1778275755      Session expired at Time(Time(1761335391))
6            Receiver                  1778275755      Session expired at Time(Time(1761336435))
7            Receiver                  1778275755      Session expired at Time(Time(1761336442))
8            Receiver                  1778275755      Session expired at Time(Time(1761336448))
9            Receiver                  1778275755      Session expired at Time(Time(1761336794))
10           Receiver                  1778275755      Session expired at Time(Time(1761336825))
11           Receiver                  1778275755      Session expired at Time(Time(1761337402))
12           Receiver                  1761318523      Session success, Payjoin proposal was broadcasted
13           Receiver                  1778275755      Session expired at Time(Time(1772828046))
14           Receiver                  1778275755      Session expired at Time(Time(1772993300))
15           Receiver                  1772907232      Session success, Payjoin proposal was broadcasted
16           Receiver                  1778275755      Session expired at Time(Time(1772993673))
17           Receiver                  1772907521      Session success, Payjoin proposal was broadcasted
18           Receiver                  1773716774      Session success, Payjoin proposal was broadcasted
19           Receiver                  1778275755      Session expired at Time(Time(1773803253))
20           Receiver                  1778275755      Session expired at Time(Time(1773803383))
21           Receiver                  1774571740      Session success, Payjoin proposal was broadcasted
22           Receiver                  1778275755      Session expired at Time(Time(1774658178))
23           Receiver                  1776873519      Session success, Payjoin proposal was broadcasted
24           Receiver                  1778275755      Session expired at Time(Time(1776960197))

Some of the sessions still have a "Completed at" timestamp but also a "Session expired" message with a different timestamp. I think there is still a bug somewhere (maybe cli?).

FWIW, this is still an improvement over master, in which all historical sessions are displaying as expired.

[benalleng]

Some of the sessions still have a "Completed at" timestamp

So all the sessions that share 1778275755 do so because that is the moment you ran payjoin-cli resume but their expirys all occured before then at their respecitve timeout times

Perhaps sessions that don't reach a Completed or FallbackBroadcasted should always show Not Completed?

[spacebear21]

tACK 3df5eea

I confirmed the above by resuming and validating that session 25 is now expired with a completed_at timestamp. I'm not convinced that this is the right behavior from payjoin-cli (at the very least "Completed At" is misleading for expired sessions), but this is unrelated to the fix in this PR.