Remove Completed at history column#1683
Conversation
d5e0f58 to
48013e2
Compare
Coverage Report for CI Build 28291062969Coverage increased (+0.5%) to 86.016%Details
Uncovered Changes
Coverage Regressions1 previously-covered line in 1 file lost coverage.
Coverage Stats
💛 - Coveralls |
caarloshenriq
left a comment
There was a problem hiding this comment.
tACK 48013e2
Reviewed and tested the changes locally, works as expected.
spacebear21
left a comment
There was a problem hiding this comment.
cACK however there seems to be an unrelated behavior change to the resume command that snuck in here, I think because of the move the SessionStatus as a source of truth.
| ReceiverSessionOutcome::Success(_) => "Session success, Payjoin proposal was broadcasted", | ||
| ReceiverSessionOutcome::FallbackBroadcasted => "Fallback broadcasted", | ||
| ReceiverSessionOutcome::Aborted => if has_fallback_tx { | ||
| "Session aborted, Fallback Tx available".to_string() |
There was a problem hiding this comment.
Is this a call to action? How do I obtain the fallback tx to broadcast?
Also, nit:
| "Session aborted, Fallback Tx available".to_string() | |
| "Session aborted, fallback transaction available".to_string() |
There was a problem hiding this comment.
running cancel on this session again will print the raw_tx to be broadcast again. from #1673
|
|
||
| if recv_session_ids.is_empty() && send_session_ids.is_empty() { | ||
| println!("No sessions to resume."); | ||
| return Ok(()); | ||
| } | ||
|
|
||
| let mut tasks: Vec<(String, tokio::task::JoinHandle<Result<()>>)> = Vec::new(); | ||
|
|
||
| // Process receiver sessions | ||
| for session_id in recv_session_ids { | ||
| let self_clone = self.clone(); | ||
| let recv_persister = ReceiverPersister::from_id(self.db.clone(), session_id.clone()); | ||
| match replay_receiver_event_log(&recv_persister) { | ||
| Ok((receiver_state, _)) => { | ||
| tasks.push(( | ||
| session_id.to_string(), | ||
| tokio::spawn(async move { | ||
| self_clone | ||
| .process_receiver_session(receiver_state, &recv_persister) | ||
| .await | ||
| }), | ||
| )); | ||
| Ok((receiver_state, history)) => { | ||
| if history.status() == ReceiverSessionStatus::Active { |
There was a problem hiding this comment.
I'm not sure I understand the rationale for this behavior change in resume. It seems unrelated to the history display changes and may have snuck in here unintentionally?
This was the result of resume on master:
❯ cargo run -- resume
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
Running `/Users/spacebear/Projects/rust-payjoin/target/debug/payjoin-cli resume`
Polling receive request...
Session 26 error: No valid relays available
❯ cargo run -- history
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.12s
Running `/Users/spacebear/Projects/rust-payjoin/target/debug/payjoin-cli history`
Session ID Sender/Receiver Completed At Status
26 Receiver Not Completed Waiting for original proposal
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))
25 Receiver 1782328670 Session expired at Time(Time(1778362082))
27 Receiver 1782334039 Session success, Payjoin proposal was broadcasted
28 Receiver 1782334213 Session success, Payjoin proposal was broadcasted
29 Receiver 1782334476 Session success, Payjoin proposal was broadcasted
On this branch:
❯ cargo run -- resume
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
Running `/Users/spacebear/Projects/rust-payjoin/target/debug/payjoin-cli resume`
Session 1 receiver expired.
2026-06-25T20:46:57.479810Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 1
Session 2 receiver expired.
2026-06-25T20:46:57.480654Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 2
Session 3 receiver expired.
2026-06-25T20:46:57.480962Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 3
Session 4 receiver expired.
2026-06-25T20:46:57.481245Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 4
Session 5 receiver expired.
2026-06-25T20:46:57.481532Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 5
Session 6 receiver expired.
2026-06-25T20:46:57.481799Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 6
Session 7 receiver expired.
2026-06-25T20:46:57.482092Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 7
Session 8 receiver expired.
2026-06-25T20:46:57.482354Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 8
Session 9 receiver expired.
2026-06-25T20:46:57.483102Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 9
Session 10 receiver expired.
2026-06-25T20:46:57.483344Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 10
Session 11 receiver expired.
2026-06-25T20:46:57.483586Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 11
Session 13 receiver expired.
2026-06-25T20:46:57.484437Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 13
Session 14 receiver expired.
2026-06-25T20:46:57.484670Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 14
Session 16 receiver expired.
2026-06-25T20:46:57.485433Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 16
Session 19 receiver expired.
2026-06-25T20:46:57.486784Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 19
Session 20 receiver expired.
2026-06-25T20:46:57.487024Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 20
Session 22 receiver expired.
2026-06-25T20:46:57.487895Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 22
Session 24 receiver expired.
2026-06-25T20:46:57.488668Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 24
Session 25 receiver expired.
2026-06-25T20:46:57.489350Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 25
Session 26 receiver expired.
2026-06-25T20:46:57.489604Z ERROR payjoin_cli::app::v2: Closed failed receiver session: 26
No sessions to resume.
❯ cargo run -- history
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.11s
Running `/Users/spacebear/Projects/rust-payjoin/target/debug/payjoin-cli history`
Session ID Sender/Receiver Status
1 Receiver Session aborted, Fallback Tx available
2 Receiver Session aborted, Fallback Tx available
3 Receiver Session aborted
4 Receiver Session aborted
5 Receiver Session aborted
6 Receiver Session aborted
7 Receiver Session aborted
8 Receiver Session aborted
9 Receiver Session aborted, Fallback Tx available
10 Receiver Session aborted
11 Receiver Session aborted
12 Receiver Session success, Payjoin proposal was broadcasted
13 Receiver Session aborted
14 Receiver Session aborted
15 Receiver Session success, Payjoin proposal was broadcasted
16 Receiver Session aborted
17 Receiver Session success, Payjoin proposal was broadcasted
18 Receiver Session success, Payjoin proposal was broadcasted
19 Receiver Session aborted
20 Receiver Session aborted
21 Receiver Session success, Payjoin proposal was broadcasted
22 Receiver Session aborted
23 Receiver Session success, Payjoin proposal was broadcasted
24 Receiver Session aborted
25 Receiver Session aborted, Fallback Tx available
26 Receiver Session aborted
27 Receiver Session success, Payjoin proposal was broadcasted
28 Receiver Session success, Payjoin proposal was broadcasted
29 Receiver Session success, Payjoin proposal was broadcasted
30 Receiver Session success, Payjoin proposal was broadcasted
There was a problem hiding this comment.
Taking a closer look at this the expired session status only lives before that session is resumed. 3df5eea forces the expired status to be inside the same match block with other outcomes that are checked with status(). However because we automatically error on an expired session and immediately close it this then changes the outcome to closed which now always fires before expired in the status() match block.
The screenshot below shows the history before and after a resume.

As an aside I think that now close_failed_session() is working properly though the tracing::error!() is jarring on an expired session, should this be info or debug. I went with debug for now
There was a problem hiding this comment.
I also wonder if close_failed_session shouldn't be using ReceiveSession::cancel() instead of SessionPersister::close() directly, forcing the PendingFallback path if a fallback transaction exists.
Removes the completed_at DB column and the "Completed At" history display column. Session status is now derived from the event log via SessionHistory::status(); resume only acts on SessionStatus::Active. SenderPersister and ReceiverPersister close() append a Closed(Aborted) event so expired/failed sessions read as terminal on subsequent replays rather than re-erroring.
9107045 to
416e949
Compare
|
This PR is beginning to grow out of scope I am going to break out the cli UX seperately from the pure remove |
416e949 to
6e55225
Compare

This PR removes the completed at column and db entry to instead rely more heavily on SessionStatus::Active as the source of truth for what can be resumed.
This additionally adds 2 minor print statements to more easily track your session_id when trying to keep track or cancel it later on. As well as a
Fallback Tx availablesuffix in the history for better understanding of what happened in an aborted session.Here are the primary screenshots demonstrating what this does.
Receiver

Sender

Coded with help from GLM-5.2
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.