close
Skip to content

Remove redundant fields from WantsOutputs event#1102

Merged
arminsabouri merged 1 commit into
payjoin:masterfrom
arminsabouri:wants-outputs-event-primary-info
Sep 22, 2025
Merged

Remove redundant fields from WantsOutputs event#1102
arminsabouri merged 1 commit into
payjoin:masterfrom
arminsabouri:wants-outputs-event-primary-info

Conversation

@arminsabouri
Copy link
Copy Markdown
Contributor

@arminsabouri arminsabouri commented Sep 22, 2025

The primary information obtained from OutputsUnknown to WantsOutputs is the index of the receiver outputs. The rest of the information can be obtained from the previous state(s).

related ticket #1103

Pull Request Checklist

Please confirm the following before requesting review:

The primary information obtained from `OutputsUnknown` to `WantsOutputs` is
the index of the receiver outputs. The rest of the information can be obtained
from the previous state(s).
@coveralls
Copy link
Copy Markdown
Collaborator

Pull Request Test Coverage Report for Build 17917096584

Details

  • 7 of 7 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.001%) to 84.744%

Totals Coverage Status
Change from base Build 17883934451: 0.001%
Covered Lines: 8560
Relevant Lines: 10101

💛 - Coveralls

@arminsabouri arminsabouri requested review from nothingmuch and removed request for spacebear21 September 22, 2025 15:38
Copy link
Copy Markdown
Contributor

@nothingmuch nothingmuch left a comment

Choose a reason for hiding this comment

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

utACK

@arminsabouri arminsabouri merged commit ea179a1 into payjoin:master Sep 22, 2025
10 checks passed
@arminsabouri arminsabouri deleted the wants-outputs-event-primary-info branch September 22, 2025 15:45
params: Params,
change_vout: usize,
owned_vouts: Vec<usize>,
pub(crate) owned_vouts: Vec<usize>,
Copy link
Copy Markdown
Contributor

@DanGould DanGould Sep 22, 2025

Choose a reason for hiding this comment

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

big nit, but wouldn't the cleaner pattern be a SessionEvents constructor from owned_vouts or a pub(crate) fn owned_vouts getter? Incongruent field accessibility is a tiny smell.

( a SessionEvent constructor or From impl on Wantsoutputs might also work)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

A getter would work fine but it will need to be feature flagged. IIRC in previous Prs we have a preference for pub(crate) over feature flags in common code.

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.

4 participants