diff --git a/payjoin-cli/src/app/v2/mod.rs b/payjoin-cli/src/app/v2/mod.rs index 73247b7e8..a127b1fe4 100644 --- a/payjoin-cli/src/app/v2/mod.rs +++ b/payjoin-cli/src/app/v2/mod.rs @@ -113,6 +113,8 @@ impl StatusText for ReceiveSession { ReceiverSessionOutcome::FallbackBroadcasted => "Fallback broadcasted", ReceiverSessionOutcome::PayjoinProposalSent => "Payjoin proposal sent, skipping monitoring as the sender is spending non-SegWit inputs", + ReceiverSessionOutcome::Unrecognized(_) => + "Settled by an unrecognized transaction", }, } } @@ -765,6 +767,12 @@ impl App { } return Ok(()); } + ReceiveSession::Closed(ReceiverSessionOutcome::Unrecognized(_)) => { + persister.print(format_args!( + "Session was already closed by an unrecognized transaction. Cannot cancel." + )); + return Ok(()); + } }; if no_broadcast { diff --git a/payjoin/src/core/receive/v2/mod.rs b/payjoin/src/core/receive/v2/mod.rs index 542f5e23c..30e2aeabe 100644 --- a/payjoin/src/core/receive/v2/mod.rs +++ b/payjoin/src/core/receive/v2/mod.rs @@ -1566,17 +1566,10 @@ impl Receiver { self, ); } - // TODO: should we check for witness and scriptsig on the tx? - let mut sender_witnesses = vec![]; - - for i in self.state.psbt_context.sender_input_indexes() { - let input = - tx.input.get(i).expect("sender_input_indexes should return valid indices"); - sender_witnesses.push((input.script_sig.clone(), input.witness.clone())); - } - // Payjoin transaction with SegWit inputs was detected. Log the signatures and complete the session. + // Payjoin transaction with SegWit inputs was detected. Complete the session, + // recording the txid of the transaction that settled it. return MaybeFatalOrSuccessTransition::success(SessionEvent::Closed( - SessionOutcome::Success(sender_witnesses), + SessionOutcome::Success(tx_id), )); } Ok(None) => {} @@ -1636,7 +1629,7 @@ pub(crate) fn pj_uri<'a>( pub mod test { use std::str::FromStr; - use bitcoin::{Amount, FeeRate, ScriptBuf, Witness}; + use bitcoin::{Amount, FeeRate}; use once_cell::sync::Lazy; use payjoin_test_utils::{ BoxError, EXAMPLE_URL, ORIGINAL_PSBT, PARSED_ORIGINAL_PSBT, PARSED_PAYJOIN_PROPOSAL, @@ -1774,10 +1767,7 @@ pub mod test { assert_eq!(persister.inner.lock().expect("Shouldn't be poisoned").events.len(), 1); assert_eq!( persister.inner.lock().expect("Shouldn't be poisoned").events.last(), - Some(&SessionEvent::Closed(SessionOutcome::Success(vec![( - ScriptBuf::default(), - Witness::default() - )]))) + Some(&SessionEvent::Closed(SessionOutcome::Success(payjoin_tx.compute_txid()))) ); // Fallback was broadcasted, should progress to success diff --git a/payjoin/src/core/receive/v2/session.rs b/payjoin/src/core/receive/v2/session.rs index b1a61222c..24e929931 100644 --- a/payjoin/src/core/receive/v2/session.rs +++ b/payjoin/src/core/receive/v2/session.rs @@ -166,7 +166,7 @@ impl SessionHistory { Some(SessionEvent::Closed(outcome)) => match outcome { SessionOutcome::Success(_) | SessionOutcome::PayjoinProposalSent => SessionStatus::Completed, - SessionOutcome::Aborted => SessionStatus::Failed, + SessionOutcome::Aborted | SessionOutcome::Unrecognized(_) => SessionStatus::Failed, SessionOutcome::FallbackBroadcasted => SessionStatus::FallbackBroadcasted, }, Some(SessionEvent::Cancelled | SessionEvent::ProtocolFailed) => @@ -192,6 +192,9 @@ pub enum SessionStatus { /// Represents a piece of information that the receiver has obtained from the session /// Each event can be used to transition the receiver state machine to a new state +/// +/// This enum is deliberately exhaustive: a caller cannot resume a session in an unknown state. A new +/// unhandled variant _should_ error at compile-time. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[allow(clippy::large_enum_variant)] pub enum SessionEvent { @@ -213,10 +216,14 @@ pub enum SessionEvent { } /// Represents all possible outcomes for a closed Payjoin session +/// +/// This enum is deliberately exhaustive: a terminal outcome is a closed set, and a wildcard arm +/// has no meaningful semantics for a caller deciding what a session's conclusion was. A new +/// unhandled variant _should_ error at compile-time. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] pub enum SessionOutcome { - /// Payjoin completed successfully - Success(Vec<(bitcoin::ScriptBuf, bitcoin::Witness)>), + /// Payjoin completed successfully: the transaction with this txid settled the session + Success(bitcoin::Txid), /// Payjoin was not successful Aborted, /// Fallback transaction was broadcasted @@ -225,6 +232,13 @@ pub enum SessionOutcome { /// the sender is using non-SegWit inputs which will change the transaction ID /// of the proposal PayjoinProposalSent, + /// The contested outpoints were settled by an unrecognized transaction (neither the + /// Payjoin nor the fallback), identified by its txid. + /// + /// NOTE: Nothing in this release produces this variant. It is reserved now because this enum is + /// deliberately exhaustive, so a new variant after 1.0 would be a semver-breaking API + /// change; naming it before the freeze lets the settlement classifier land additively. + Unrecognized(bitcoin::Txid), } #[cfg(test)] @@ -565,6 +579,11 @@ mod tests { SessionEvent::GotReplyableError(mock_err()), SessionEvent::Cancelled, SessionEvent::ProtocolFailed, + SessionEvent::Closed(SessionOutcome::Success(bitcoin::Txid::all_zeros())), + SessionEvent::Closed(SessionOutcome::Aborted), + SessionEvent::Closed(SessionOutcome::FallbackBroadcasted), + SessionEvent::Closed(SessionOutcome::PayjoinProposalSent), + SessionEvent::Closed(SessionOutcome::Unrecognized(bitcoin::Txid::all_zeros())), ]; for event in test_cases { @@ -672,7 +691,7 @@ mod tests { let success = SessionHistory::new(vec![ SessionEvent::Created(session_context.clone()), - SessionEvent::Closed(SessionOutcome::Success(vec![])), + SessionEvent::Closed(SessionOutcome::Success(bitcoin::Txid::all_zeros())), ]); assert_eq!(success.status(), SessionStatus::Completed); @@ -688,6 +707,12 @@ mod tests { ]); assert_eq!(fallback.status(), SessionStatus::FallbackBroadcasted); + let other = SessionHistory::new(vec![ + SessionEvent::Created(session_context.clone()), + SessionEvent::Closed(SessionOutcome::Unrecognized(bitcoin::Txid::all_zeros())), + ]); + assert_eq!(other.status(), SessionStatus::Failed); + // Sessions that never reached a terminal state still report Expired. let still_open = SessionHistory::new(vec![SessionEvent::Created(session_context)]); assert_eq!(still_open.status(), SessionStatus::Expired); @@ -703,7 +728,7 @@ mod tests { .save_event(SessionEvent::Created(session_context.clone())) .expect("in memory persister save should not fail"); persister - .save_event(SessionEvent::Closed(SessionOutcome::Success(vec![]))) + .save_event(SessionEvent::Closed(SessionOutcome::Success(bitcoin::Txid::all_zeros()))) .expect("in memory persister save should not fail"); let (state, _) = replay_event_log(&persister).expect("closed session should replay successfully"); @@ -715,7 +740,7 @@ mod tests { .await .expect("in memory async persister save should not fail"); persister - .save_event(SessionEvent::Closed(SessionOutcome::Success(vec![]))) + .save_event(SessionEvent::Closed(SessionOutcome::Success(bitcoin::Txid::all_zeros()))) .await .expect("in memory async persister save should not fail"); let (state, _) = replay_event_log_async(&persister) @@ -1086,7 +1111,7 @@ mod tests { }); events.push(SessionEvent::AppliedFeeRange(provisional_proposal.state.psbt_context.clone())); events.push(SessionEvent::FinalizedProposal(payjoin_proposal.psbt().clone())); - events.push(SessionEvent::Closed(SessionOutcome::Success(vec![]))); + events.push(SessionEvent::Closed(SessionOutcome::Success(bitcoin::Txid::all_zeros()))); let test = SessionHistoryTest { events, @@ -1094,7 +1119,9 @@ mod tests { fallback_tx: Some(expected_fallback), expected_status: SessionStatus::Completed, }, - expected_receiver_state: ReceiveSession::Closed(SessionOutcome::Success(vec![])), + expected_receiver_state: ReceiveSession::Closed(SessionOutcome::Success( + bitcoin::Txid::all_zeros(), + )), }; run_session_history_test(&test); run_session_history_test_async(&test).await; diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs index 6bf8dc15b..18f5b1447 100644 --- a/payjoin/tests/integration.rs +++ b/payjoin/tests/integration.rs @@ -625,23 +625,16 @@ mod integration { .save(&recv_persister) .expect("receiver should successfully monitor for the payment"); - // Receiver session should have completed with a Success, along with information on the - // sender signatures on the Payjoin that was broadcasted. + // Receiver session should have completed with a Success. let (_session, session_history) = replay_receiver_event_log(&recv_persister)?; - let sender_outpoint = session_history.fallback_tx().unwrap().input[0].previous_output; - let sender_signatures = { - let sender_txin = broadcasted_transaction - .input - .iter() - .find(|txin| txin.previous_output == sender_outpoint) - .expect("sender input must be present in payjoin_tx") - .clone(); - vec![(sender_txin.clone().script_sig, sender_txin.clone().witness)] - }; assert_eq!( recv_persister.load().unwrap().last(), - Some(payjoin::receive::v2::SessionEvent::Closed(payjoin::receive::v2::SessionOutcome::Success(sender_signatures))), - "The last event of the persister should be a SessionOutcome::Success with the correct sender signature", + Some(payjoin::receive::v2::SessionEvent::Closed( + payjoin::receive::v2::SessionOutcome::Success( + broadcasted_transaction.compute_txid() + ) + )), + "The last event of the persister should be a SessionOutcome::Success with the settlement txid", ); assert_eq!(session_history.status(), SessionStatus::Completed); Ok(()) @@ -707,23 +700,16 @@ mod integration { .save(&recv_persister) .expect("receiver should successfully monitor for the payment"); - // Receiver session should have completed with a Success, along with information on the - // sender signatures on the Payjoin that was broadcasted. + // Receiver session should have completed with a Success. let (_session, session_history) = replay_receiver_event_log(&recv_persister)?; - let sender_outpoint = session_history.fallback_tx().unwrap().input[0].previous_output; - let sender_signatures = { - let sender_txin = broadcasted_transaction - .input - .iter() - .find(|txin| txin.previous_output == sender_outpoint) - .expect("sender input must be present in payjoin_tx") - .clone(); - vec![(sender_txin.clone().script_sig, sender_txin.clone().witness)] - }; assert_eq!( recv_persister.load().unwrap().last(), - Some(payjoin::receive::v2::SessionEvent::Closed(payjoin::receive::v2::SessionOutcome::Success(sender_signatures))), - "The last event of the persister should be a SessionOutcome::Success with the correct sender signature", + Some(payjoin::receive::v2::SessionEvent::Closed( + payjoin::receive::v2::SessionOutcome::Success( + broadcasted_transaction.compute_txid() + ) + )), + "The last event of the persister should be a SessionOutcome::Success with the settlement txid", ); assert_eq!(session_history.status(), SessionStatus::Completed); Ok(())