Fix PR review issues from PR #812

[Mshehu5]

This PR addresses the review comments from @nothingmuch and @arminsabouri on PR #812 (Deduplicate payjoin URI creation logic)

Changes Made

• Import Cleanup: Add direct import of bitcoin_uri::Uri instead of using relative imports
• Output Substitution Fix: Fix output substitution handling to properly default to enabled for v2 and disabled for v1

Implementation Details

The pj_uri function in SessionHistory now follows a clearer flow:

1. Find the session context: First locates the session context from the Created event in the history
2. Determine version and set output substitution: Checks if there's an UncheckedProposal event to determine the protocol version, then sets output substitution to disabled for v1 and enabled for v2
3. Generate the URI: Creates the Payjoin URI with the appropriate context and output substitution setting

[coveralls]

Pull Request Test Coverage Report for Build 15976925469

Details

• 9 of 13 (69.23%) changed or added relevant lines in 2 files are covered.
• 1 unchanged line in 1 file lost coverage.
• Overall coverage decreased (-0.03%) to 85.453%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin/src/core/receive/v2/session.rs	8	12	66.67%

Files with Coverage Reduction	New Missed Lines	%
payjoin/src/core/receive/v2/session.rs	1	92.59%

Totals
Change from base Build 15949315495:	-0.03%
Covered Lines:	7255
Relevant Lines:	8490

---

💛 - Coveralls

[arminsabouri]

CodeACK. It would be great to have a unit test for the change in session.rs.

@Mshehu5 do you mind splitting out these changes in to two seperate commits? In general commits should just do one thing. So here I would expect one commit for import and another for the output substitution fix. This reasoning is explained in our contribution guide.

Thank you for the fast follow up on this 🔥

[arminsabouri]

Do you mind adding a test below to prevent future regressions?

[nothingmuch]

the behavior of pj_uri before this change did not match the spec

the fix is simple (but this does need a regression unit test)

sorry for the confusion

[nothingmuch]

unfortunately there is a bug (#843) that appears to have caused some confusion with this behavior

pj_uri should not depend at all on the events, it should return what the receiver generated at the session initialization, and there output substitution is always disabled, because a malicious directory can reply to bip 78 senders with malicious proposal PSBTs that steal the intended receiver's payment.

since v2 messages are end to end encrypted with integrity protection, a v2 receiver receiving from a v2 sender can substitute outputs even though it specified pjos=0 in the uri, so this code fragment checking the event data is correct for determining the correct behavior for the receiver, it's just not the right value to use when constructing pj_uri.

Feel free to fix this bug here, but then please edit the pull request title and description accordingly

Suggested change

	let mut output_substitution = OutputSubstitution::Enabled;
	for event in &self.events {
	if let SessionEvent::UncheckedProposal((unchecked_proposal, _)) = event {
	if unchecked_proposal.params.v == Version::One {
	output_substitution = OutputSubstitution::Disabled;
	}
	break;
	}
	}
	Some(crate::receive::v2::pj_uri(session_context, output_substitution))
	Some(crate::receive::v2::pj_uri(session_context, OutputSubstitution::Disabled))

[nothingmuch]

also I guess this method should be documented to say that it returns the pj_url that was sent to the sender, and to note that although pjos=0 is set, this does not disable output substitution if the sender also supports bip 77

[nothingmuch]

See #847