Return current state in transient errors

[spacebear21]

Closes #1422. Closes #1709.

Problem

Transient state transitions consume the typestate and return only an error. An implementer who hits a transient failure (a directory 5xx, a wallet RPC hiccup) has no way to resume: they must either defensively clone the state before every transition call or replay the whole session event log. On top of that, PersistedError collapsed the transient/fatal distinction that the internal ApiError variants already encode, so poll loops could not even tell whether retrying was appropriate.

Approach

Transient rejections now return ownership of the current typestate inside the error, the way mpsc::SendError hands back the unsent value. This preserves typestate linearity: at every point exactly one live handle exists, either Ok(next_state), the current state inside the transient Err (via the new PersistedError::transient_state()), or none on a fatal error that closed the session. Transient rejections persist nothing, so the carried state is always identical to what replaying the event log would reconstruct, and a retried run leaves an event log indistinguishable from one that never failed. Both properties are covered by new tests.

• RejectTransient, Rejection, ApiError, and PersistedError gain a CurrentState type parameter defaulting to (). MaybeFatalTransition, MaybeTransientTransition, and MaybeTerminalSuccessTransition gain a trailing CurrentState parameter; the WithNoResults and FatalOrSuccess transitions reuse the CurrentState they already carry for the stasis arm. Every transient constructor now carries the state, uniformly.
• process_error_response now consumes self like every other transition. It was the one borrowing transition, which left a live handle able to re-post and append a duplicate event after a successful save; consuming self extends the exactly-one-live-handle invariant to the error-reply path.
• PersistedError gains is_transient() / is_fatal() predicates (Be specific about error handling for methods that don't return state transitions #1422). Storage errors are neither: the transition outcome is unknown and recovery is replay.
• Over FFI, uniffi error enums cannot carry methods, so the classification is encoded in the variant structure: ReceiverPersistedError and SenderPersistedError become Transient/Fatal/Storage, surfacing as distinct exception types in each binding language. The sender's per-kind variants move into a new SenderError enum mirroring ReceiverError. This breaks binding consumers matching the old variant names, which seems acceptable pre-1.0.
• The FFI builder error is renamed from BuildSenderError to SenderBuilderError, mirroring ReceiverBuilderError. Besides the symmetry, this avoids a class-name collision in the generated Dart, where uniffi-dart flattens the SenderError::Build variant and the BuildSenderError object to the same top-level BuildSenderException class.
• payjoin-cli demonstrates the API end to end: both poll loops and all three one-shot POST flows (Original PSBT, proposal, error reply) retry transient errors from the state carried in the error, paced by a fixed 5 second delay and bounded by session expiry. The receiver's proposal-checking chain intentionally keeps propagating transient errors, since their recovery path is resuming the session, and apply_fee_range's transient error is deterministic for a given proposal, so an in-place retry loop would spin until session expiry.

Trade-off worth reviewer attention

Three receiver transitions (identify_receiver_outputs, apply_fee_range, finalize_proposal) moved inner data into by-value calls before the transient return, so keeping self intact costs one PSBT-sized clone per call on the happy path. The alternative was changing inner by-value signatures shared with the v1 receiver, which seemed worse.

Since this change is breaking (new type parameters), it must land before 1.0; the #[non_exhaustive] escape hatch would not have covered it later.

Disclosure: co-authored by Claude Code

🤖 Generated with Claude Code

[coveralls]

Coverage Report for CI Build 29110198013

Coverage increased (+0.02%) to 86.026%

Details

• Coverage increased (+0.02%) from the base build.
• Patch coverage: 73 uncovered changes across 4 files (332 of 405 lines covered, 81.98%).
• 7 coverage regressions across 2 files.

Uncovered Changes

File	Changed	Covered	%
payjoin-cli/src/app/v2/mod.rs	72	27	37.5%
payjoin/src/core/receive/v2/mod.rs	106	84	79.25%
payjoin/src/core/persist.rs	223	219	98.21%
payjoin/src/core/send/v2/mod.rs	4	2	50.0%

Coverage Regressions

7 previously-covered lines in 2 files lost coverage.

File	Lines Losing Coverage	Coverage
payjoin/src/core/receive/v2/mod.rs	4	92.01%
payjoin-cli/src/app/v2/mod.rs	3	50.66%

---

Coverage Stats

Relevant Lines:	15887
Covered Lines:	13667
Line Coverage:	86.03%
Coverage Strength:	343.55 hits per line

---

💛 - Coveralls

[benalleng]

I am ok with any of the shapes this takes and this is looking good. I'm curious what we want to change in the cli with this approach. There is a dart exception naming problem

[benalleng]

CACK I would still like to see the how the usage of e.is_transient() and e.transient_state() takes its shape in the cli

[DanGould]

CLI impl with e.is_transient() & e.transient_state https://github.com/DanGould/rust-payjoin/commits/dangould/1724-cli-transient-impl/

• sender.clone() removed for mut sender in the function signatures
• continue removed because now all of the error arms are covered

[spacebear21]

I cherry-picked Dan's commit onto this PR and made two minor modifications: added a 5 second retry delay and updated send_payjoin_proposal to make the proposal parameter mut instead of rebinding in the function body, matching the other two function styles.

[xstoicunicornx]

Haven't completed thorough review but offhand it looks like Dan's commit is still missing the transient error handling in handle_error (the code snippet that I pasted in #1709)

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

Lines 1040 to 1053 in f60825c

	if let Err(e) = session.process_error_response(&err_bytes, err_ctx).save(persister) {
	if let Some(api_err) = e.api_error_ref() {
	tracing::warn!("Failed to confirm error response delivery: {api_err}");
	}
	match e.error_state() {
	Some(_) => {
	let id = persister.session_id();
	println!(
	"Session {id} failed. Run `payjoin-cli cancel {id}` to cancel and broadcast the fallback transaction."
	);
	}
	None => return Err(anyhow!("Failed to process error response")),
	}
	}

[spacebear21]

Updated to add retry for handle_error and also post_original_proposal.

Note that there are remaining transient errors in the receiver checklist chain that are intentionally left out here as they come from wallet/db callbacks, and in-place retry loops wouldn't achieve anything there. We could add case-by-case handling there in follow-ups (e.g prompting for a new set of inputs, different fee preferences, etc.)

[xstoicunicornx]

I think were still missing current event handling for one of the transient errors.

[benalleng]

ACK a7c000e transient errors now carry state where relevant in the cli.

Given the expiration nature of payjoins I think a 5 second retry is appropriate rather than a give-up after n retires path.

addressed in latest push

[xstoicunicornx]

ACK a7c000e

Overall looks good. State machine feels much more consistent overall.

A few suggestions, 2 of which touch API for payjoin crate. PRs for the API updates incoming.

[xstoicunicornx]

May want rename to fatal_state for consistency/clarity?

[xstoicunicornx]

May want to also consume self here for consistency?

[xstoicunicornx]

Rather than introducing a loop couldn't we just call process_receiver_session with the returned transient current state?

[xstoicunicornx]

Renaming this to align with the ReceiverBuilderError from ffi crate makes it not follow the naming convention from PDK base crate. Why not rename ReceiverBuilderError from ffi crate to be BuildReceiverError instead to bring them both into alignment?