Allow mixed input scripts in v2

payjoin-cli/src/app/mod.rs

@@ -2,6 +2,8 @@ use std::collections::HashMap;
 use std::str::FromStr;
 
 use anyhow::{anyhow, Context, Result};
+use bitcoin::psbt::Input as PsbtInput;
+use bitcoin::TxIn;
 use bitcoincore_rpc::bitcoin::Amount;
 use bitcoincore_rpc::RpcApi;
 use payjoin::bitcoin::psbt::Psbt;
@@ -120,3 +122,24 @@ fn read_local_cert() -> Result<Vec<u8>> {
     local_cert_path.push(LOCAL_CERT_FILE);
     Ok(std::fs::read(local_cert_path)?)
 }
+
+pub fn input_pair_from_list_unspent(
+    utxo: &bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,
+) -> (PsbtInput, TxIn) {
+    let psbtin = PsbtInput {
+        // NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies
+        // witness_utxo, even for non-witness inputs
+        witness_utxo: Some(bitcoin::TxOut {
+            value: utxo.amount,
+            script_pubkey: utxo.script_pub_key.clone(),
+        }),
+        redeem_script: utxo.redeem_script.clone(),
+        witness_script: utxo.witness_script.clone(),
+        ..Default::default()
+    };
+    let txin = TxIn {
+        previous_output: bitcoin::OutPoint { txid: utxo.txid, vout: utxo.vout },
+        ..Default::default()
+    };
+    (psbtin, txin)
+}

payjoin-cli/src/app/v1.rs

@@ -21,7 +21,7 @@ use tokio::net::TcpListener;
 
 use super::config::AppConfig;
 use super::App as AppTrait;
-use crate::app::http_agent;
+use crate::app::{http_agent, input_pair_from_list_unspent};
 use crate::db::Database;
 #[cfg(feature = "danger-local-https")]
 pub const LOCAL_CERT_FILE: &str = "localhost.der";
@@ -319,15 +319,12 @@ impl App {
             }
         })?;
         log::trace!("check2");
-        // Receive Check 3: receiver can't sign for proposal inputs
-        let proposal = proposal.check_no_mixed_input_scripts()?;
-        log::trace!("check3");
 
-        // Receive Check 4: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
+        // Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
         let payjoin = proposal.check_no_inputs_seen_before(|input| {
             self.db.insert_input_seen_before(*input).map_err(|e| Error::Server(e.into()))
         })?;
-        log::trace!("check4");
+        log::trace!("check3");
 
         let payjoin = payjoin.identify_receiver_outputs(|output_script| {
             if let Ok(address) = bitcoin::Address::from_script(output_script, network) {
@@ -396,13 +393,10 @@ fn try_contributing_inputs(
         .find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
         .context("This shouldn't happen. Failed to retrieve the privacy preserving utxo from those we provided to the seclector.")?;
     log::debug!("selected utxo: {:#?}", selected_utxo);
-    let txo_to_contribute = bitcoin::TxOut {
-        value: selected_utxo.amount,
-        script_pubkey: selected_utxo.script_pub_key.clone(),
-    };
+    let input_pair = input_pair_from_list_unspent(selected_utxo);
 
     Ok(payjoin
-        .contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)])
+        .contribute_inputs(vec![input_pair])
         .expect("This shouldn't happen. Failed to contribute inputs.")
         .commit_inputs())
 }

payjoin-cli/src/app/v2.rs

@@ -15,7 +15,7 @@ use tokio::sync::watch;
 
 use super::config::AppConfig;
 use super::App as AppTrait;
-use crate::app::http_agent;
+use crate::app::{http_agent, input_pair_from_list_unspent};
 use crate::db::Database;
 
 #[derive(Clone)]
@@ -306,15 +306,12 @@ impl App {
             }
         })?;
         log::trace!("check2");
-        // Receive Check 3: receiver can't sign for proposal inputs
-        let proposal = proposal.check_no_mixed_input_scripts()?;
-        log::trace!("check3");
 
-        // Receive Check 4: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
+        // Receive Check 3: have we seen this input before? More of a check for non-interactive i.e. payment processor receivers.
         let payjoin = proposal.check_no_inputs_seen_before(|input| {
             self.db.insert_input_seen_before(*input).map_err(|e| Error::Server(e.into()))
         })?;
-        log::trace!("check4");
+        log::trace!("check3");
 
         let payjoin = payjoin
             .identify_receiver_outputs(|output_script| {
@@ -375,13 +372,10 @@ fn try_contributing_inputs(
         .find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
         .context("This shouldn't happen. Failed to retrieve the privacy preserving utxo from those we provided to the seclector.")?;
     log::debug!("selected utxo: {:#?}", selected_utxo);
-    let txo_to_contribute = bitcoin::TxOut {
-        value: selected_utxo.amount,
-        script_pubkey: selected_utxo.script_pub_key.clone(),
-    };
+    let input_pair = input_pair_from_list_unspent(selected_utxo);
 
     Ok(payjoin
-        .contribute_witness_inputs(vec![(selected_outpoint, txo_to_contribute)])
+        .contribute_inputs(vec![input_pair])
         .expect("This shouldn't happen. Failed to contribute inputs.")
         .commit_inputs())
 }

payjoin/src/psbt.rs

@@ -195,17 +195,24 @@ impl<'a> InputPair<'a> {
         // Get the input weight prediction corresponding to spending an output of this address type
         let iwp = match self.address_type()? {
             P2pkh => Ok(InputWeightPrediction::P2PKH_COMPRESSED_MAX),
-            P2sh =>
-                match self.psbtin.final_script_sig.as_ref().and_then(|s| redeem_script(s.as_ref()))
-                {
+            P2sh => {
+                // redeemScript can be extracted from scriptSig for signed P2SH inputs
+                let redeem_script = if let Some(ref script_sig) = self.psbtin.final_script_sig {
+                    redeem_script(script_sig)
+                // try the PSBT redeem_script field for unsigned inputs.
+                } else {
+                    self.psbtin.redeem_script.as_ref().map(|script| script.as_ref())
+                };
+                match redeem_script {
                     // Nested segwit p2wpkh.
                     Some(script) if script.is_witness_program() && script.is_p2wpkh() =>
                         Ok(NESTED_P2WPKH_MAX),
                     // Other script or witness program.
                     Some(_) => Err(InputWeightError::NotSupported),
                     // No redeem script provided. Cannot determine the script type.
-                    None => Err(InputWeightError::NotFinalized),
-                },
+                    None => Err(InputWeightError::NoRedeemScript),
+                }
+            }
             P2wpkh => Ok(InputWeightPrediction::P2WPKH_MAX),
             P2wsh => Err(InputWeightError::NotSupported),
             P2tr => Ok(InputWeightPrediction::P2TR_KEY_DEFAULT_SIGHASH),
@@ -323,15 +330,15 @@ impl From<FromScriptError> for AddressTypeError {
 #[derive(Debug)]
 pub(crate) enum InputWeightError {
     AddressType(AddressTypeError),
-    NotFinalized,
+    NoRedeemScript,
     NotSupported,
 }
 
 impl fmt::Display for InputWeightError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match self {
             Self::AddressType(_) => write!(f, "invalid address type"),
-            Self::NotFinalized => write!(f, "input not finalized"),
+            Self::NoRedeemScript => write!(f, "p2sh input missing a redeem script"),
             Self::NotSupported => write!(f, "weight prediction not supported"),
         }
     }
@@ -341,7 +348,7 @@ impl std::error::Error for InputWeightError {
     fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
         match self {
             Self::AddressType(error) => Some(error),
-            Self::NotFinalized => None,
+            Self::NoRedeemScript => None,
             Self::NotSupported => None,
         }
     }

payjoin/src/receive/error.rs

@@ -72,10 +72,6 @@ pub(crate) enum InternalRequestError {
     OriginalPsbtNotBroadcastable,
     /// The sender is trying to spend the receiver input
     InputOwned(bitcoin::ScriptBuf),
-    /// The original psbt has mixed input address types that could harm privacy
-    MixedInputScripts(bitcoin::AddressType, bitcoin::AddressType),
-    /// The address type could not be determined
-    AddressType(crate::psbt::AddressTypeError),
     /// The expected input weight cannot be determined
     InputWeight(crate::psbt::InputWeightError),
     /// Original PSBT input has been seen before. Only automatic receivers, aka "interactive" in the spec
@@ -152,13 +148,6 @@ impl fmt::Display for RequestError {
             ),
             InternalRequestError::InputOwned(_) =>
                 write_error(f, "original-psbt-rejected", "The receiver rejected the original PSBT."),
-            InternalRequestError::MixedInputScripts(type_a, type_b) => write_error(
-                f,
-                "original-psbt-rejected",
-                &format!("Mixed input scripts: {}; {}.", type_a, type_b),
-            ),
-            InternalRequestError::AddressType(e) =>
-                write_error(f, "original-psbt-rejected", &format!("AddressType Error: {}", e)),
             InternalRequestError::InputWeight(e) =>
                 write_error(f, "original-psbt-rejected", &format!("InputWeight Error: {}", e)),
             InternalRequestError::InputSeen(_) =>
@@ -200,7 +189,6 @@ impl std::error::Error for RequestError {
             InternalRequestError::SenderParams(e) => Some(e),
             InternalRequestError::InconsistentPsbt(e) => Some(e),
             InternalRequestError::PrevTxOut(e) => Some(e),
-            InternalRequestError::AddressType(e) => Some(e),
             InternalRequestError::InputWeight(e) => Some(e),
             #[cfg(feature = "v2")]
             InternalRequestError::ParsePsbt(e) => Some(e),
@@ -301,13 +289,32 @@ pub struct InputContributionError(InternalInputContributionError);
 
 #[derive(Debug)]
 pub(crate) enum InternalInputContributionError {
+    /// Missing previous txout information
+    PrevTxOut(crate::psbt::PrevTxOutError),
+    /// The address type could not be determined
+    AddressType(crate::psbt::AddressTypeError),
+    /// The original PSBT has no inputs
+    NoSenderInputs,
+    /// The proposed receiver inputs would introduce mixed input script types
+    MixedInputScripts(bitcoin::AddressType, bitcoin::AddressType),
     /// Total input value is not enough to cover additional output value
     ValueTooLow,
 }
 
 impl fmt::Display for InputContributionError {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match &self.0 {
+            InternalInputContributionError::PrevTxOut(e) =>
+                write!(f, "Missing previous txout information: {}", e),
+            InternalInputContributionError::AddressType(e) =>
+                write!(f, "The address type could not be determined: {}", e),
+            InternalInputContributionError::NoSenderInputs =>
+                write!(f, "The original PSBT has no inputs"),
+            InternalInputContributionError::MixedInputScripts(type_a, type_b) => write!(
+                f,
+                "The proposed receiver inputs would introduce mixed input script types: {}; {}.",
+                type_a, type_b
+            ),
             InternalInputContributionError::ValueTooLow =>
                 write!(f, "Total input value is not enough to cover additional output value"),
         }