Correct the pjos parameter parity (payjoin#546)

alter and corrected the logic for `pjos` parameter

fixes payjoin#545

Author: DanGould

payjoin-cli/src/app/v1.rs

@@ -139,8 +139,11 @@ impl App {
         let pj_part = payjoin::Url::parse(pj_part)
             .map_err(|e| anyhow!("Failed to parse pj_endpoint: {}", e))?;
 
-        let mut pj_uri =
-            payjoin::receive::v1::build_v1_pj_uri(&pj_receiver_address, &pj_part, false)?;
+        let mut pj_uri = payjoin::receive::v1::build_v1_pj_uri(
+            &pj_receiver_address,
+            &pj_part,
+            payjoin::OutputSubstitution::Enabled,
+        )?;
         pj_uri.amount = Some(amount);
 
         Ok(pj_uri.to_string())

payjoin/src/lib.rs

@@ -54,8 +54,11 @@ mod request;
 #[cfg(feature = "_core")]
 pub use request::*;
 #[cfg(feature = "_core")]
+pub(crate) mod output_substitution;
+#[cfg(feature = "v1")]
+pub use output_substitution::OutputSubstitution;
+#[cfg(feature = "_core")]
 mod uri;
-
 #[cfg(feature = "_core")]
 pub use into_url::{Error as IntoUrlError, IntoUrl};
 #[cfg(feature = "_core")]

payjoin/src/output_substitution.rs

@@ -0,0 +1,20 @@
+/// Whether the receiver is allowed to substitute original outputs or not.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+#[cfg_attr(feature = "v2", derive(serde::Serialize, serde::Deserialize))]
+pub enum OutputSubstitution {
+    Enabled,
+    Disabled,
+}
+
+impl OutputSubstitution {
+    /// Combine two output substitution flags.
+    ///
+    /// If both are enabled, the result is enabled.
+    /// If one is disabled, the result is disabled.
+    pub(crate) fn combine(self, other: Self) -> Self {
+        match (self, other) {
+            (Self::Enabled, Self::Enabled) => Self::Enabled,
+            _ => Self::Disabled,
+        }
+    }
+}

payjoin/src/receive/optional_parameters.rs

@@ -4,12 +4,14 @@ use std::fmt;
 use bitcoin::FeeRate;
 use log::warn;
 
+use crate::output_substitution::OutputSubstitution;
+
 #[derive(Debug, Clone)]
 pub(crate) struct Params {
     // version
     pub v: usize,
     // disableoutputsubstitution
-    pub disable_output_substitution: bool,
+    pub output_substitution: OutputSubstitution,
     // maxadditionalfeecontribution, additionalfeeoutputindex
     pub additional_fee_contribution: Option<(bitcoin::Amount, usize)>,
     // minfeerate
@@ -23,7 +25,7 @@ impl Default for Params {
     fn default() -> Self {
         Params {
             v: 1,
-            disable_output_substitution: false,
+            output_substitution: OutputSubstitution::Enabled,
             additional_fee_contribution: None,
             min_fee_rate: FeeRate::BROADCAST_MIN,
             #[cfg(feature = "_multiparty")]
@@ -88,7 +90,11 @@ impl Params {
                         Err(_) => return Err(Error::FeeRate),
                     },
                 ("disableoutputsubstitution", v) =>
-                    params.disable_output_substitution = v == "true",
+                    params.output_substitution = if v == "true" {
+                        OutputSubstitution::Disabled
+                    } else {
+                        OutputSubstitution::Enabled
+                    },
                 #[cfg(feature = "_multiparty")]
                 ("optimisticmerge", v) => params.optimistic_merge = v == "true",
                 _ => (),

payjoin/src/receive/v1/exclusive/mod.rs

@@ -16,10 +16,9 @@ pub trait Headers {
 pub fn build_v1_pj_uri<'a>(
     address: &bitcoin::Address,
     endpoint: impl IntoUrl,
-    disable_output_substitution: bool,
+    output_substitution: OutputSubstitution,
 ) -> Result<crate::uri::PjUri<'a>, crate::into_url::Error> {
-    let extras =
-        crate::uri::PayjoinExtras { endpoint: endpoint.into_url()?, disable_output_substitution };
+    let extras = crate::uri::PayjoinExtras { endpoint: endpoint.into_url()?, output_substitution };
     Ok(bitcoin_uri::Uri::with_extras(address.clone(), extras))
 }
 

payjoin/src/receive/v1/mod.rs

@@ -39,6 +39,7 @@ use super::optional_parameters::Params;
 use super::{
     ImplementationError, InputPair, OutputSubstitutionError, ReplyableError, SelectionError,
 };
+use crate::output_substitution::OutputSubstitution;
 use crate::psbt::PsbtExt;
 use crate::receive::InternalPayloadError;
 
@@ -264,9 +265,8 @@ pub struct WantsOutputs {
 }
 
 impl WantsOutputs {
-    pub fn is_output_substitution_disabled(&self) -> bool {
-        self.params.disable_output_substitution
-    }
+    /// Whether the receiver is allowed to substitute original outputs or not.
+    pub fn output_substitution(&self) -> OutputSubstitution { self.params.output_substitution }
 
     /// Substitute the receiver output script with the provided script.
     pub fn substitute_receiver_script(
@@ -306,7 +306,7 @@ impl WantsOutputs {
                     // Select an output with the same address if one was provided
                     Some(pos) => {
                         let txo = replacement_outputs.swap_remove(pos);
-                        if self.params.disable_output_substitution
+                        if self.output_substitution() == OutputSubstitution::Disabled
                             && txo.value < original_output.value
                         {
                             return Err(
@@ -317,7 +317,7 @@ impl WantsOutputs {
                     }
                     // Otherwise randomly select one of the replacement outputs
                     None => {
-                        if self.params.disable_output_substitution {
+                        if self.output_substitution() == OutputSubstitution::Disabled {
                             return Err(
                                 InternalOutputSubstitutionError::ScriptPubKeyChangedWhenDisabled
                                     .into(),
@@ -708,7 +708,7 @@ impl ProvisionalProposal {
             self.payjoin_psbt.inputs[i].tap_key_sig = None;
         }
 
-        PayjoinProposal { payjoin_psbt: self.payjoin_psbt, params: self.params }
+        PayjoinProposal { payjoin_psbt: self.payjoin_psbt }
     }
 
     /// Return the indexes of the sender inputs
@@ -763,18 +763,13 @@ impl ProvisionalProposal {
 #[derive(Debug, Clone)]
 pub struct PayjoinProposal {
     payjoin_psbt: Psbt,
-    params: Params,
 }
 
 impl PayjoinProposal {
     pub fn utxos_to_be_locked(&self) -> impl '_ + Iterator<Item = &bitcoin::OutPoint> {
         self.payjoin_psbt.unsigned_tx.input.iter().map(|input| &input.previous_output)
     }
 
-    pub fn is_output_substitution_disabled(&self) -> bool {
-        self.params.disable_output_substitution
-    }
-
     pub fn psbt(&self) -> &Psbt { &self.payjoin_psbt }
 }
 
@@ -949,8 +944,7 @@ pub(crate) mod test {
     #[test]
     fn test_pjos_disabled() {
         let mut proposal = proposal_from_test_vector().unwrap();
-        // Specify outputsubstitution is disabled
-        proposal.params.disable_output_substitution = true;
+        proposal.params.output_substitution = OutputSubstitution::Disabled;
         let wants_outputs = wants_outputs_from_test_vector(proposal).unwrap();
 
         let output_value =

payjoin/src/receive/v2/mod.rs

@@ -18,6 +18,7 @@ use super::{
 };
 use crate::hpke::{decrypt_message_a, encrypt_message_b, HpkeKeyPair, HpkePublicKey};
 use crate::ohttp::{ohttp_decapsulate, ohttp_encapsulate, OhttpEncapsulationError, OhttpKeys};
+use crate::output_substitution::OutputSubstitution;
 use crate::receive::{parse_payload, InputPair};
 use crate::uri::ShortId;
 use crate::{IntoUrl, IntoUrlError, Request};
@@ -192,7 +193,7 @@ impl Receiver {
         //
         // see: https://github.com/bitcoin/bips/blob/master/bip-0078.mediawiki#unsecured-payjoin-server
         if params.v == 1 {
-            params.disable_output_substitution = true;
+            params.output_substitution = OutputSubstitution::Disabled;
 
             // Additionally V1 sessions never have an optimistic merge opportunity
             #[cfg(feature = "_multiparty")]
@@ -212,7 +213,8 @@ impl Receiver {
         pj.set_receiver_pubkey(self.context.s.public_key().clone());
         pj.set_ohttp(self.context.ohttp_keys.clone());
         pj.set_exp(self.context.expiry);
-        let extras = PayjoinExtras { endpoint: pj, disable_output_substitution: false };
+        let extras =
+            PayjoinExtras { endpoint: pj, output_substitution: OutputSubstitution::Enabled };
         bitcoin_uri::Uri::with_extras(self.context.address.clone(), extras)
     }
 
@@ -385,9 +387,8 @@ pub struct WantsOutputs {
 }
 
 impl WantsOutputs {
-    pub fn is_output_substitution_disabled(&self) -> bool {
-        self.v1.is_output_substitution_disabled()
-    }
+    /// Whether the receiver is allowed to substitute original outputs or not.
+    pub fn output_substitution(&self) -> OutputSubstitution { self.v1.output_substitution() }
 
     /// Substitute the receiver output script with the provided script.
     pub fn substitute_receiver_script(
@@ -503,10 +504,6 @@ impl PayjoinProposal {
         self.v1.utxos_to_be_locked()
     }
 
-    pub fn is_output_substitution_disabled(&self) -> bool {
-        self.v1.is_output_substitution_disabled()
-    }
-
     pub fn psbt(&self) -> &Psbt { self.v1.psbt() }
 
     pub fn extract_v2_req(
@@ -652,6 +649,6 @@ mod test {
     fn test_v2_pj_uri() {
         let uri = Receiver { context: SHARED_CONTEXT.clone() }.pj_uri();
         assert_ne!(uri.extras.endpoint, EXAMPLE_URL.clone());
-        assert!(!uri.extras.disable_output_substitution);
+        assert_eq!(uri.extras.output_substitution, OutputSubstitution::Enabled);
     }
 }

payjoin/src/send/mod.rs

@@ -17,6 +17,7 @@ pub use error::{BuildSenderError, ResponseError, ValidationError, WellKnownError
 pub(crate) use error::{InternalBuildSenderError, InternalProposalError, InternalValidationError};
 use url::Url;
 
+use crate::output_substitution::OutputSubstitution;
 use crate::psbt::PsbtExt;
 
 // See usize casts
@@ -51,7 +52,7 @@ pub(crate) struct AdditionalFeeContribution {
 #[derive(Debug, Clone)]
 pub struct PsbtContext {
     original_psbt: Psbt,
-    disable_output_substitution: bool,
+    output_substitution: OutputSubstitution,
     fee_contribution: Option<AdditionalFeeContribution>,
     min_fee_rate: FeeRate,
     payee: ScriptBuf,
@@ -253,7 +254,7 @@ impl PsbtContext {
                     if original_output.script_pubkey == self.payee =>
                 {
                     ensure!(
-                        !self.disable_output_substitution
+                        self.output_substitution == OutputSubstitution::Enabled
                             || (proposed_txout.script_pubkey == original_output.script_pubkey
                                 && proposed_txout.value >= original_output.value),
                         DisallowedOutputSubstitution
@@ -414,14 +415,14 @@ fn determine_fee_contribution(
 
 fn serialize_url(
     endpoint: Url,
-    disable_output_substitution: bool,
+    output_substitution: OutputSubstitution,
     fee_contribution: Option<AdditionalFeeContribution>,
     min_fee_rate: FeeRate,
     version: &str,
 ) -> Url {
     let mut url = endpoint;
     url.query_pairs_mut().append_pair("v", version);
-    if disable_output_substitution {
+    if output_substitution == OutputSubstitution::Disabled {
         url.query_pairs_mut().append_pair("disableoutputsubstitution", "true");
     }
     if let Some(AdditionalFeeContribution { max_amount, vout }) = fee_contribution {
@@ -449,14 +450,15 @@ mod test {
     use super::{
         check_single_payee, clear_unneeded_fields, determine_fee_contribution, serialize_url,
     };
+    use crate::output_substitution::OutputSubstitution;
     use crate::psbt::PsbtExt;
     use crate::send::{AdditionalFeeContribution, InternalBuildSenderError, InternalProposalError};
 
     pub(crate) fn create_psbt_context() -> Result<super::PsbtContext, BoxError> {
         let payee = PARSED_ORIGINAL_PSBT.unsigned_tx.output[1].script_pubkey.clone();
         Ok(super::PsbtContext {
             original_psbt: PARSED_ORIGINAL_PSBT.clone(),
-            disable_output_substitution: false,
+            output_substitution: OutputSubstitution::Enabled,
             fee_contribution: Some(AdditionalFeeContribution {
                 max_amount: bitcoin::Amount::from_sat(182),
                 vout: 0,
@@ -641,10 +643,22 @@ mod test {
 
     #[test]
     fn test_disable_output_substitution_query_param() -> Result<(), BoxError> {
-        let url = serialize_url(Url::parse("http://localhost")?, true, None, FeeRate::ZERO, "2");
+        let url = serialize_url(
+            Url::parse("http://localhost")?,
+            OutputSubstitution::Disabled,
+            None,
+            FeeRate::ZERO,
+            "2",
+        );
         assert_eq!(url, Url::parse("http://localhost?v=2&disableoutputsubstitution=true")?);
 
-        let url = serialize_url(Url::parse("http://localhost")?, false, None, FeeRate::ZERO, "2");
+        let url = serialize_url(
+            Url::parse("http://localhost")?,
+            OutputSubstitution::Enabled,
+            None,
+            FeeRate::ZERO,
+            "2",
+        );
         assert_eq!(url, Url::parse("http://localhost?v=2")?);
         Ok(())
     }