Does the payjoin-cli cancel have the correct semantics?

[benalleng]

Quoted from #1558 (review)

I thought about the cancel --no-broadcast semantics in review and confirm I think this is the correct semantics. cancel matches cancel() & SessionOutcome::Cancel and defaults to a broadcast. If you want the special case where you don't broadcast that's an option with --no-broadcast.

Perhaps though, we separate cancel (which can mean stop broadcasting a transaction that pays the counterparty in RBF cancel) into abort and abort --broadcast, flipping the semantics. What's weird here is after cancel payjoin-cli has no way to broadcast a transaction by replaying until the PendingFallback state. I think we want there to be a way to broadcast the fallback on a closed session, but that's a matter for payjoin-cli-1.0, not the payjoin-1.0 milestone issue this PR tries to address.

[benalleng]

Just to additionally weigh in here I do think its important to keep in mind that the payjoin-cli commands are payjoin focused so that a bare cargo run cancel will simply not proceed the payjoin but the transaction can still proceed whereas cargo run cancel --no-broadcast is a special case beyond just cancelling the payjoin.

[spacebear21]

What's weird here is after cancel payjoin-cli has no way to broadcast a transaction by replaying until the PendingFallback state. I think we want there to be a way to broadcast the fallback on a closed session

Would it be sufficient to add a way to retrieve the raw fallback transaction so that it can be broadcasted with e.g. bitcoin-cli sendrawtransaction? Perhaps in payjoin-cli history output or similar. For example on failure closes we simply print:

println!("Payjoin failed. To broadcast the fallback transaction, run:");
println!(
    "  bitcoin-cli -rpcwallet=<wallet> sendrawtransaction {:#}",

[benalleng]

I'm a bit confused looking at the cli which session state you are hoping to add this additional information at. Would we want this print to go alongside the print here?

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

Lines 679 to 684 in dac9c9b

	SendSession::PendingFallback(_) => {
	let id = persister.session_id();
	println!(
	"Session {id} was cancelled. Run `payjoin-cli cancel {id}` to cancel and broadcast the fallback transaction."
	);
	return Ok(());

and for the receiver

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

Lines 791 to 796 in dac9c9b

	ReceiveSession::PendingFallback(_) => {
	let id = persister.session_id();
	println!(
	"Session {id} was cancelled. Run `payjoin-cli cancel {id}` to cancel and broadcast the fallback transaction."
	);
	return Ok(());

Or should this be instead of the cancel suggestion?

[bc1cindy]

concept ACK