Decapsulate v1 provisional and payjoin typestate from v2 typestate#938
Merged
spacebear21 merged 2 commits intopayjoin:masterfrom Aug 13, 2025
Merged
Decapsulate v1 provisional and payjoin typestate from v2 typestate#938spacebear21 merged 2 commits intopayjoin:masterfrom
spacebear21 merged 2 commits intopayjoin:masterfrom
Conversation
Collaborator
Pull Request Test Coverage Report for Build 16918472197Details
💛 - Coveralls |
spacebear21
reviewed
Aug 12, 2025
Collaborator
spacebear21
left a comment
spacebear21
left a comment
There was a problem hiding this comment.
Concept ACK, this is a nice decoupling.
| pub struct ProvisionalProposal { | ||
| v1: v1::ProvisionalProposal, | ||
| psbt_context: PsbtContext, | ||
| context: SessionContext, |
Collaborator
There was a problem hiding this comment.
As a follow up I'm thinking this other context should be renamed to session_context across all the receiver states.
arminsabouri
force-pushed
the
psbt-ctx-recv
branch
from
94b9d6e to
83df259
Compare
arminsabouri
force-pushed
the
psbt-ctx-recv
branch
from
83df259 to
e156acf
Compare
We currently represent a failure to finalize a Payjoin proposal using the `ReplayableError(ImplementationError)` variant. While this is appropriate for a v1 Payjoin receiver, it misrepresents the expected behavior for a v2 receiver. In v2, encountering an implementation error is considered a transient failure. i.e the receiver should retry the session rather than respond to the sender and terminate it. This misclassification stems from the current v2 -> v1 type hierarchy, which forces the v2 receiver to adopt `ReplayableError`, when a more suitable variant exists. In reality, the v2 receiver only requires a shared abstraction (`PsbtContext`) to finalize the PSBT and construct a payjoin proposal. It does not need to embed or depend on the v1 receiver’s typestates. This commit introduces `PsbtContext` as a common abstraction for proposal finalization and decouples the `ProvisionalProposal` and `PayjoinProposal` typestates from the v1 typestate.
In the async. payjoin context, we need a mechanism to represent integration errors that should not be relayed to the counterparty. Currently, these errors are incorrectly represented as `ReplayableError::ImplementationError`. This commit corrects the error handling by updating `finalize_proposal` to return `SessionError::IntegrationError`, which provides a more accurate and semantically correct representation for integration failures. Moreover, `SessionError`s cannot be accidentally converted and relayed to the sender as a json reply as they do not impl the conversion.
arminsabouri
force-pushed
the
psbt-ctx-recv
branch
from
e156acf to
08ae332
Compare
spacebear21
approved these changes
Aug 13, 2025
Collaborator
spacebear21
left a comment
spacebear21
left a comment
There was a problem hiding this comment.
ACK 08ae332
The failing CI check is due to server issues with Coveralls, bypassing this for merge.
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
3 participants
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.
We currently represent a failure to finalize a Payjoin proposal using
the
ReplayableError(ImplementationError)variant. While this isappropriate for a v1 Payjoin receiver, it misrepresents the expected
behavior for a v2 receiver. In v2, encountering an implementation error
is considered a transient failure. i.e the receiver should retry the
session rather than respond to the sender and terminate it.
This misclassification stems from the current v2 -> v1 type hierarchy,
which forces the v2 receiver to adopt
ReplayableError, when amore suitable variant exists. In reality, the v2 receiver only requires
a shared abstraction (
PsbtContext) to finalize the PSBT and construct apayjoin proposal. It does not need to embed or depend on the v1
receiver’s typestates.
This PR introduces
PsbtContextas a common abstraction forproposal finalization and decouples the
ProvisionalProposalandPayjoinProposaltypestates from the v1 typestate. Additionally we mapthe finalize proposal error to a new SessionError integration error variant.