payjoin
Should we save the BIP21 URI verbatim
The receiver generates a BIP 21 URI with payjoin parameters. Storing the amount in the context (and therefore initialization event) is useful for its original intent, but also in case we might want to verify that the fallback transaction doesn't under-pay (e.g. as an optional method WantsOutputs::check_expected_amout(&self) -> Result<(), SenderIsMiserlyErr>
The sender receives a BIP 21 URI which may have payjoin parameters along with other data. SenderBuilder currently accepts parsed PjParams which omits the amount, but the amount is recorded in the PsbtContext as the value of the payment output of the fallback transaction. This is not sufficient to reconstruct the original URI as amount encoding might not be canonical and any unrecognized fields will be omitted. Recording the verbatim URI as part of the sender initialization could be useful for error handling, and possibly future proofing (e.g. if we want to allow the sender to recover the URI, for example to display to users, then being able to recover the exact URI could prevent confusion).
The original intention from 4926384a655d0defdca9948babfaf218dc3bc1b1 was application dev convenience
I updated the main body as requested, and I think as a result this issue can be closed, or we can broaden its scope to the verbatim URI in the sender assuming the reasoning for recording that is strong enough.
I'm leaning towards it being a good idea in principle, not so sure in practice, since that implies the SenderBuilder::from_parts constructor should be removed, and implies that either SenderBuilder::new should take a &str not a PjUri (which is already the parsed bitcoin_uri::Uri internally and therefore lossy), or alternatively that bitcoin_uri should implement lossless deserialization (this would be the nicer option IMO, but seems like a big undertaking for something that is just future proofing). I don't like &str because the correct SenderBuilder is used depending on the parsing of the URI, it's weird/surprising to require that to be given again to the sender builder with no justifications.
As such I think we shouldn't do anything for now, discourage wallet developers from taking a Sender and reconstructing the URI from it by whatever means, and instead encourage them to save the verbatim URI for reporting purposes if appropriate, and should a real use case crop up, handle that with an API breaking change. Adding an event to serialize the verbatim URI is OK to do later in an API breaking change, it's semantically equivalent to having an Option<String> for it in the sender already but for now only ever setting it to None.
@arminsabouri needs to review this and decide if we should close based on the rationale above
Rationale above seems reasonable and I agree no change needs to happen now. Removing from the milestone but leaving this open.