Skip to content

Conversation

@arminsabouri
Copy link
Collaborator

The sender session is marked as successfully closed once the payjoin proposal is received. Currently, this state is recorded in a separate event (ReceivedProposalPsbt) from the Closed event which is inconsistent with the reciever implementation.

Pull Request Checklist

Please confirm the following before requesting review:

@coveralls
Copy link
Collaborator

Pull Request Test Coverage Report for Build 18727546149

Details

  • 4 of 8 (50.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.003%) to 83.788%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin-cli/src/app/v2/mod.rs 0 2 0.0%
payjoin/src/core/send/v2/mod.rs 1 3 33.33%
Files with Coverage Reduction New Missed Lines %
payjoin-cli/src/app/v2/mod.rs 1 53.36%
Totals Coverage Status
Change from base Build 18694776764: -0.003%
Covered Lines: 8977
Relevant Lines: 10714

💛 - Coveralls

@arminsabouri arminsabouri self-assigned this Oct 22, 2025
@arminsabouri arminsabouri added this to the payjoin-1.0 milestone Oct 22, 2025
Copy link
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not totally convinced that receiving the proposal and closing the session are the same event, couldn't there be a scenario where the proposal is received but the receiver broadcasted the fallback tx or double-spent their input, so the proposal tx is no longer valid?

@arminsabouri
Copy link
Collaborator Author

I am not totally convinced that receiving the proposal and closing the session are the same event, couldn't there be a scenario where the proposal is received but the receiver broadcasted the fallback tx or double-spent their input, so the proposal tx is no longer valid?

The sessions should close once there is no more expected payjoin activity -- for sender it would be receiving the proposal. Check out the rationale here #807 (comment)
Do you think the API should capture conflicting txs (i.e a monitoring typestate) and is that just for auditability/session history reasons?

Copy link
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code ACK - makes sense after re-reading the rational in #807. Maybe we should include that rationale in code comments for the sender and receiver closed events?

The sender session is marked as successfully closed once the
payjoin proposal is received. Currently, this state is recorded in a
separate event (`ReceivedProposalPsbt`) from the `Closed` event which is
inconsistent with the reciever implementation.
@arminsabouri arminsabouri force-pushed the replace-received-proposal-psbt branch from 1365f77 to 3bdda03 Compare October 27, 2025 12:03
@arminsabouri arminsabouri merged commit a7e4283 into payjoin:master Oct 27, 2025
14 checks passed
@arminsabouri arminsabouri deleted the replace-received-proposal-psbt branch October 27, 2025 12:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants