Receiver fallback typestate#1558
Open
spacebear21 wants to merge 7 commits into
Open
Conversation
2 tasks
f4454b7 to
f175e01
Compare
f175e01 to
56641ee
Compare
Collaborator
Coverage Report for CI Build 26202708607Coverage increased (+0.04%) to 85.179%Details
Uncovered Changes
Coverage Regressions21 previously-covered lines in 3 files lost coverage.
Coverage Stats
💛 - Coveralls |
Contributor
|
I read the writeups and concept ACK. I wonder if |
Introduce a sealed::FallbackTx trait with a non-Option fallback_tx() method, implemented inside the sealed module for the in-protocol receiver states whose contract includes a confirmed broadcastable fallback. Expose access through HasFallbackTx, a public marker trait that has no methods of its own and is implemented for any type satisfying sealed::FallbackTx via a blanket impl. External crates can bound on HasFallbackTx but cannot implement it, and the method itself is only callable from inside the receive::v2 module where the sealed trait is in scope. - UncheckedOriginalPayload is deliberately excluded; it holds the sender's Original PSBT but has not yet run check_broadcast_suitability, so the PSBT is not yet verified as broadcastable. - HasReplyableError is also excluded; it will gain an optional fallback field in a later commit and continues to model the absent-fallback case at runtime. To avoid naming conflicts in intermediate commits, the existing `fallback_tx() -> Option<Transaction>` implementation is renamed to `maybe_fallback_tx`. It is removed entirely in a later commit.
PendingFallback represents a receiver session that was cancelled or hit a fatal protocol error, and has a fallback transaction available to broadcast. While the session sits in PendingFallback the implementer holds an obligation to broadcast, discard, or otherwise handle the fallback transaction (e.g. save it to wallet DB for later broadcasting). This state is preserved across restarts and session replays until the implemeter calls `close()`, indicating that the handoff of the fallback transaction is complete and no longer a payjoin concern.
HasReplyableError represents a receiver session that hit a replyable error before reaching PendingFallback. The struct must model the runtime fact that some sources can hand it a verified broadcastable fallback and others cannot. Encoding the field as Option<Transaction> keeps that distinction at the type level without weakening the HasFallback trait contract.
Introduce MaybeTerminalTransition for the no-error fork (used by cancel) and MaybeTerminalSuccessTransition for the error-bearing fork (used by process_error_response). Both expose advance and terminate constructors that map to Save and SaveAndClose actions respectively. The success variant returns Option<NextState>; the error variants preserve the caller's distinction between transient, fatal-advance, and fatal-terminate.
Collaborator
Author
In the latest iteration of this PR the typestate is called |
56641ee to
e6828cc
Compare
The receiver side of v2 had a single blanket cancel implementation that always terminated the session and handed the wallet an Option<Transaction>. Fatal protocol errors emitted Closed(Failure) directly. Both shapes lost the wallet's obligation to broadcast the original transaction across a restart whenever a fallback existed. Replace the blanket cancel with typestate-aware impls: - impl<S: HasFallback> Receiver<S>::cancel advances to PendingFallback - Receiver<Initialized>::cancel and Receiver<UncheckedOriginalPayload> ::cancel terminate with Closed(Cancel); neither holds a verified fallback - Receiver<HasReplyableError>::cancel forks on the optional fallback: Some advances to PendingFallback, None terminates with Closed(Cancel)
Drop dead code.
e6828cc to
21722b1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Supersedes #1542.
Receiver counterpart to #1557 . It's a much bigger PR because a) the receiver state machine is more complicated than the sender's, and b) it also implements the "protocol failure" fallback path (i.e. not a manually-initiated cancel, but an irrecoverable error that would also warrant broadcasting the fallback tx).
stateDiagram-v2 [*] --> Initialized Initialized --> UncheckedOriginalPayload: RetrievedOriginalPayload UncheckedOriginalPayload --> MaybeInputsOwned: CheckedBroadcastSuitability state "9 HasFallbackTx in-protocol states" as InProtocol { MaybeInputsOwned --> MaybeInputsSeen: CheckedInputsNotOwned MaybeInputsSeen --> OutputsUnknown: CheckedNoInputsSeenBefore OutputsUnknown --> WantsOutputs: IdentifiedReceiverOutputs WantsOutputs --> WantsInputs: CommittedOutputs WantsInputs --> WantsFeeRange: CommittedInputs WantsFeeRange --> ProvisionalProposal: AppliedFeeRange ProvisionalProposal --> PayjoinProposal: FinalizedProposal PayjoinProposal --> Monitor: PostedPayjoinProposal } UncheckedOriginalPayload --> HasReplyableError: GotReplyableError, fallback_tx is None (broadcast unverified) InProtocol --> HasReplyableError: GotReplyableError, fallback_tx is Some InProtocol --> PendingFallback: Cancelled (cancel from any HasFallbackTx state) PayjoinProposal --> PendingFallback: ProtocolFailed (fatal process_response) HasReplyableError --> PendingFallback: Cancelled or ProtocolFailed, when fallback_tx is Some Initialized --> Closed: Closed(Cancel) on cancel, or Closed(Failure) on fatal process_response UncheckedOriginalPayload --> Closed: Closed(Cancel) on cancel HasReplyableError --> Closed: Closed(Cancel) on cancel, or Closed(Failure) on process_error_response, when fallback_tx is None Monitor --> Closed: check_payment observes Success, FallbackBroadcasted, or PayjoinProposalSent PendingFallback --> Closed: close() emits Closed(Cancel) after Cancelled entry, or Closed(Failure) after ProtocolFailed entry Closed --> [*] note right of PendingFallback Holds the wallet's obligation to broadcast or explicitly discard the original tx. Stays active until close() so it survives resume cycles. end notePlanned with Claude Opus 4.7, implemented by Codex 5.5
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.