Skip to content

Commit c91653e

Browse files
committed
Force cli to cancel and broadcast fallback when available mid loop
Previously the cli only checked for expiry at program start during long polling loops there was not an expiry check leading to the possiblity of an indefinite poll past expiry where the counterparty had already given up.
1 parent 099fc58 commit c91653e

4 files changed

Lines changed: 161 additions & 39 deletions

File tree

payjoin-cli/src/app/v2/mod.rs

Lines changed: 133 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -337,6 +337,12 @@ impl AppTrait for App {
337337
Err(e) => {
338338
if e.is_expired() {
339339
println!("Session {session_id} receiver expired.");
340+
match e.fallback_tx() {
341+
Some(tx) => self.broadcast_fallback_tx(tx, &session_id, "receiver"),
342+
None => println!(
343+
"No fallback transaction available for expired receiver session {session_id}."
344+
),
345+
}
340346
} else {
341347
tracing::error!(
342348
"An error {:?} occurred while replaying receiver session",
@@ -365,6 +371,12 @@ impl AppTrait for App {
365371
Err(e) => {
366372
if e.is_expired() {
367373
println!("Session {session_id} sender expired.");
374+
match e.fallback_tx() {
375+
Some(tx) => self.broadcast_fallback_tx(tx, &session_id, "sender"),
376+
None => println!(
377+
"No fallback transaction available for expired sender session {session_id}."
378+
),
379+
}
368380
} else {
369381
tracing::error!("An error {:?} occurred while replaying Sender session", e);
370382
println!("Session {session_id} sender failed to replay - {e}");
@@ -688,10 +700,52 @@ impl App {
688700
if let Err(close_err) = SessionPersister::close(persister) {
689701
tracing::error!("Failed to close {} session {}: {:?}", role, session_id, close_err);
690702
} else {
691-
tracing::error!("Closed failed {} session: {}", role, session_id);
703+
tracing::debug!("Closed failed {} session: {}", role, session_id);
704+
}
705+
}
706+
707+
/// Broadcast a fallback transaction with graceful error handling. Logs
708+
/// success/failure, never propagates the broadcast error.
709+
fn broadcast_fallback_tx(
710+
&self,
711+
tx: &payjoin::bitcoin::Transaction,
712+
session_id: &SessionId,
713+
role: &str,
714+
) {
715+
match self.wallet().broadcast_tx(tx) {
716+
Ok(txid) =>
717+
println!("Broadcasted fallback transaction for {role} session {session_id}: {txid}"),
718+
Err(err) => {
719+
println!(
720+
"Expired {role} session {session_id} has a fallback transaction \
721+
but it failed to broadcast: {err}\n\
722+
Recover it manually with `payjoin-cli history` / `cancel`."
723+
);
724+
tracing::error!(
725+
"Fallback broadcast failed for {role} session {session_id}: {err:?}"
726+
);
727+
}
692728
}
693729
}
694730

731+
// / Best-effort broadcast of a fallback transaction recovered from an expired
732+
// / session's `ReplayError`. A sender always has a fallback (its original
733+
// / PSBT); a receiver only has one if it had already received and validated
734+
// / the sender's original proposal.
735+
// fn broadcast_fallback_on_expiry<SessionState, SessionEvent>(
736+
// &self,
737+
// e: &payjoin::error::ReplayError<SessionState, SessionEvent>,
738+
// session_id: &SessionId,
739+
// role: &str,
740+
// ) {
741+
// match e.fallback_tx() {
742+
// Some(tx) => self.broadcast_fallback_tx(tx, session_id, role),
743+
// None => println!(
744+
// "No fallback transaction available for expired {role} session {session_id}."
745+
// ),
746+
// }
747+
// }
748+
695749
async fn process_sender_session(
696750
&self,
697751
session: SendSession,
@@ -739,26 +793,53 @@ impl App {
739793
persister: &SenderPersister,
740794
) -> Result<()> {
741795
let mut session = sender.clone();
742-
// Long poll until we get a response
796+
// Long poll until we get a response. The session's expiration is enforced
797+
// here: once `create_poll_request` reports expiry, the session is driven to
798+
// a terminal `Closed(Aborted)` state and the fallback transaction is surfaced
799+
// so it can be broadcast manually. The persisted terminal outcome prevents
800+
// the session from being mistakenly resumed.
743801
loop {
744-
let (response, ctx) =
745-
self.post_via_relay(|relay| session.create_poll_request(relay)).await?;
746-
let res = session.process_response(&response.bytes().await?, ctx).save(persister);
747-
match res {
748-
Ok(OptionalTransitionOutcome::Progress(psbt)) => {
749-
println!("Proposal received. Processing...");
750-
self.process_pj_response(psbt)?;
751-
return Ok(());
802+
let relay = self.mailroom_manager.choose_relay()?;
803+
let (req, ctx) = match session.create_poll_request(relay.as_str()) {
804+
Ok(r) => r,
805+
Err(e) if e.is_expired() => {
806+
let pending = session.cancel().save(persister)?;
807+
self.broadcast_fallback_tx(
808+
pending.fallback_tx(),
809+
&persister.session_id(),
810+
"sender",
811+
);
812+
pending.close().save(persister)?;
813+
return Err(anyhow!("Sender session expired"));
752814
}
753-
Ok(OptionalTransitionOutcome::Stasis(current_state)) => {
754-
println!("No response yet.");
755-
session = current_state;
756-
continue;
815+
Err(e) => return Err(e.into()),
816+
};
817+
match self.post_request(req).await {
818+
Ok(response) => {
819+
let bytes = response.bytes().await?;
820+
let res = session.process_response(&bytes, ctx).save(persister);
821+
match res {
822+
Ok(OptionalTransitionOutcome::Progress(psbt)) => {
823+
println!("Proposal received. Processing...");
824+
self.process_pj_response(psbt)?;
825+
return Ok(());
826+
}
827+
Ok(OptionalTransitionOutcome::Stasis(current_state)) => {
828+
println!("No response yet.");
829+
session = current_state;
830+
continue;
831+
}
832+
Err(re) => {
833+
println!("{re}");
834+
tracing::debug!("{re:?}");
835+
return Err(anyhow!("Response error").context(re));
836+
}
837+
}
757838
}
758-
Err(re) => {
759-
println!("{re}");
760-
tracing::debug!("{re:?}");
761-
return Err(anyhow!("Response error").context(re));
839+
Err(e) => {
840+
tracing::debug!("Request to relay {relay} failed: {e:?}");
841+
self.mailroom_manager.add_failed_relay(relay);
842+
continue;
762843
}
763844
}
764845
}
@@ -771,22 +852,44 @@ impl App {
771852
) -> Result<Receiver<UncheckedOriginalPayload>> {
772853
let mut session = session;
773854
loop {
855+
let relay = self.mailroom_manager.choose_relay()?;
856+
let (req, context) = match session.create_poll_request(relay.as_str()) {
857+
Ok(r) => r,
858+
Err(e) if e.is_expired() => {
859+
session.cancel().save(persister)?;
860+
println!(
861+
"Receiver session expired before any proposal was received; \
862+
no fallback transaction to broadcast."
863+
);
864+
return Err(anyhow!("Receiver session expired"));
865+
}
866+
Err(e) => return Err(e.into()),
867+
};
774868
println!("Polling receive request...");
775-
let (ohttp_response, context) =
776-
self.post_via_relay(|relay| session.create_poll_request(relay)).await?;
777-
let state_transition = session
778-
.process_response(ohttp_response.bytes().await?.to_vec().as_slice(), context)
779-
.save(persister);
780-
match state_transition {
781-
Ok(OptionalTransitionOutcome::Progress(next_state)) => {
782-
println!("Got a request from the sender. Responding with a Payjoin proposal.");
783-
return Ok(next_state);
869+
match self.post_request(req).await {
870+
Ok(ohttp_response) => {
871+
let bytes = ohttp_response.bytes().await?.to_vec();
872+
let state_transition =
873+
session.process_response(bytes.as_slice(), context).save(persister);
874+
match state_transition {
875+
Ok(OptionalTransitionOutcome::Progress(next_state)) => {
876+
println!(
877+
"Got a request from the sender. Responding with a Payjoin proposal."
878+
);
879+
return Ok(next_state);
880+
}
881+
Ok(OptionalTransitionOutcome::Stasis(current_state)) => {
882+
session = current_state;
883+
continue;
884+
}
885+
Err(e) => return Err(e.into()),
886+
}
784887
}
785-
Ok(OptionalTransitionOutcome::Stasis(current_state)) => {
786-
session = current_state;
888+
Err(e) => {
889+
tracing::debug!("Request to relay {relay} failed: {e:?}");
890+
self.mailroom_manager.add_failed_relay(relay);
787891
continue;
788892
}
789-
Err(e) => return Err(e.into()),
790893
}
791894
}
792895
}

payjoin/src/core/error.rs

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ impl<SessionState: Debug, SessionEvent: Debug> std::fmt::Display
5151
Some(session) => write!(f, "Invalid event ({event:?}) for session ({session:?})",),
5252
None => write!(f, "Invalid first event ({event:?}) for session",),
5353
},
54-
Expired(time) => write!(f, "Session expired at {time:?}"),
54+
Expired(time, _) => write!(f, "Session expired at {time:?}"),
5555
PersistenceFailure(e) => write!(f, "Persistence failure: {e}"),
5656
}
5757
}
@@ -73,7 +73,20 @@ impl<SessionState: Debug, SessionEvent: Debug> From<InternalReplayError<SessionS
7373
impl<SessionState, SessionEvent> ReplayError<SessionState, SessionEvent> {
7474
/// Returns `true` if the event log could not be replayed because the
7575
/// session has expired.
76-
pub fn is_expired(&self) -> bool { matches!(self.0, InternalReplayError::Expired(_)) }
76+
pub fn is_expired(&self) -> bool { matches!(self.0, InternalReplayError::Expired(..)) }
77+
78+
/// Returns the fallback transaction that should be broadcast when the
79+
/// session has expired, if one is recoverable from the event log.
80+
///
81+
/// Returns `None` for non-expired errors, or for an expired session that
82+
/// never produced a fallback transaction (e.g. a receiver that expired
83+
/// before receiving the sender's original proposal).
84+
pub fn fallback_tx(&self) -> Option<&bitcoin::Transaction> {
85+
match &self.0 {
86+
InternalReplayError::Expired(_, tx) => tx.as_ref(),
87+
_ => None,
88+
}
89+
}
7790
}
7891

7992
#[cfg(feature = "v2")]
@@ -83,8 +96,8 @@ pub(crate) enum InternalReplayError<SessionState, SessionEvent> {
8396
NoEvents,
8497
/// Invalid initial event
8598
InvalidEvent(Box<SessionEvent>, Option<Box<SessionState>>),
86-
/// Session is expired
87-
Expired(crate::time::Time),
99+
/// Session is expired. Carries the recoverable fallback transaction, if any.
100+
Expired(crate::time::Time, Option<bitcoin::Transaction>),
88101
/// Application storage error
89102
PersistenceFailure(ImplementationError),
90103
}
@@ -96,8 +109,10 @@ mod tests {
96109

97110
#[test]
98111
fn replay_error_is_expired() {
99-
let expired: ReplayError<(), ()> = ReplayError(InternalReplayError::Expired(Time::now()));
112+
let expired: ReplayError<(), ()> =
113+
ReplayError(InternalReplayError::Expired(Time::now(), None));
100114
assert!(expired.is_expired());
115+
assert!(expired.fallback_tx().is_none());
101116

102117
let other: ReplayError<(), ()> = ReplayError(InternalReplayError::NoEvents);
103118
assert!(!other.is_expired());

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ fn construct_history(
3333
if !matches!(receiver, ReceiveSession::Closed(_)) {
3434
let ctx = history.session_context();
3535
if ctx.expiration.elapsed() {
36-
return Err(InternalReplayError::Expired(ctx.expiration).into());
36+
return Err(InternalReplayError::Expired(ctx.expiration, history.fallback_tx()).into());
3737
}
3838
}
3939
Ok(history)
@@ -395,7 +395,7 @@ mod tests {
395395
.expect("in memory persister save should not fail");
396396
let err = replay_event_log(&persister).expect_err("session should be expired");
397397
let expected_err: ReplayError<ReceiveSession, SessionEvent> =
398-
InternalReplayError::Expired(expiration).into();
398+
InternalReplayError::Expired(expiration, None).into();
399399
assert_eq!(err.to_string(), expected_err.to_string());
400400

401401
let persister = InMemoryAsyncPersister::<SessionEvent>::default();
@@ -405,7 +405,7 @@ mod tests {
405405
.expect("in memory async persister save should not fail");
406406
let err = replay_event_log_async(&persister).await.expect_err("session should be expired");
407407
let expected_err: ReplayError<ReceiveSession, SessionEvent> =
408-
InternalReplayError::Expired(expiration).into();
408+
InternalReplayError::Expired(expiration, None).into();
409409
assert_eq!(err.to_string(), expected_err.to_string());
410410
}
411411

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,11 @@ fn construct_history(
3030
if !matches!(sender, SendSession::Closed(_)) {
3131
let pj_param = history.pj_param();
3232
if pj_param.expiration().elapsed() {
33-
return Err(InternalReplayError::Expired(pj_param.expiration()).into());
33+
return Err(InternalReplayError::Expired(
34+
pj_param.expiration(),
35+
Some(history.fallback_tx()),
36+
)
37+
.into());
3438
}
3539
}
3640
Ok(history)

0 commit comments

Comments
 (0)