Skip to content

Process sessions is no longer halted by a failed session#24

Merged
arminsabouri merged 2 commits into
payjoin:payjoin-v4from
benalleng:pdk-payjoin-v4
Aug 18, 2025
Merged

Process sessions is no longer halted by a failed session#24
arminsabouri merged 2 commits into
payjoin:payjoin-v4from
benalleng:pdk-payjoin-v4

Conversation

@benalleng

@benalleng benalleng commented Aug 14, 2025

Copy link
Copy Markdown

The for loop in our receiver and sender session was causing any error in a session to bubble up and completely stop the loop meaning follow-up sessions could not proceed forward.

Comment thread lianad/src/payjoin/receiver.rs Outdated
let mut db_conn = db.connection();
for session_id in db_conn.get_all_active_receiver_session_ids() {
let persister = ReceiverPersister::from_id(Arc::new(db.clone()), session_id.clone());
let persister = ReceiverPersister::from_id(Arc::new(db.clone()), session_id.clone());

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We had talked about moving db up and just passing the db_conn since it locks so perhaps we could get into trouble locking it above in payjoin_receiver_check

impl DatabaseInterface for sync::Arc<sync::Mutex<dyn DatabaseInterface>> {
    fn connection(&self) -> Box<dyn DatabaseConnection> {
        self.lock().unwrap().connection()
    }
}

However, we access the entire DatabaseInterface in addition to the DatabaseConnection in this function since we need to get the persister for each session_id, perhaps also pass down the persister for each session?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah that would make more sense

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually the persister is session specific. can we pass down db_conn?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am currently passing both db_conn and persister down since the current way we construct the persister is from a DatabaseInterface via db not db_conn, I just construct the persister within the loop.

@arminsabouri arminsabouri left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

The for loop in our receiver session was causing any error in a session
to bubble up and completely stop the loop meaning follow-up sessions
could not proceed forward.

@arminsabouri arminsabouri left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

utACK

Comment thread lianad/src/payjoin/sender.rs
@benalleng benalleng changed the title Process Receiver sessions is no longer halted by a failed session Process sessions is no longer halted by a failed session Aug 14, 2025
@benalleng

Copy link
Copy Markdown
Author

Even with this change I am still finding there is a fatal panic when there is an expired session.

ERROR liana_gui:614: Panic occured

  2025-08-18T14:53:49.818719Z ERROR liana_gui:633: panic occurred at line 45 of file lianad/src/payjoin/receiver.rs: So
me("Failed to extract request: V2(SessionError(Expired(SystemTime { tv_sec: 1755275866, tv_nsec: 22088869 })))")
   0: liana_gui::setup_panic_hook::{{closure}}

@arminsabouri

Copy link
Copy Markdown

Even with this change I am still finding there is a fatal panic when there is an expired session.

ERROR liana_gui:614: Panic occured

  2025-08-18T14:53:49.818719Z ERROR liana_gui:633: panic occurred at line 45 of file lianad/src/payjoin/receiver.rs: So
me("Failed to extract request: V2(SessionError(Expired(SystemTime { tv_sec: 1755275866, tv_nsec: 22088869 })))")
   0: liana_gui::setup_panic_hook::{{closure}}

I thin this is documented #17 . Probably unrelated to the changes in this PR

Comment thread lianad/src/payjoin/sender.rs
@arminsabouri arminsabouri merged commit d7f1bff into payjoin:payjoin-v4 Aug 18, 2025
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants