Modify try_preserving_privacy function signature

spacebear21 · spacebear21 · commit b2b63c6fb83c · 2024-10-24T15:10:46.000-04:00
`try_preserving_privacy` now takes a (PsbtInput, TxIn) iterator instead
of a bespoke (Amount, Outpoint) iterator which is lossy and unwieldy. It
also returns a single (PsbtInput, TxIn) instead of just an OutPoint.

This is the same type that `contribute_inputs` takes as a parameter.
diff --git a/payjoin-cli/src/app/mod.rs b/payjoin-cli/src/app/mod.rs
@@ -124,7 +124,7 @@ fn read_local_cert() -> Result<Vec<u8>> {
 }
 
 pub fn input_pair_from_list_unspent(
-    utxo: &bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,
+    utxo: bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,
 ) -> (PsbtInput, TxIn) {
     let psbtin = PsbtInput {
         // NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies
diff --git a/payjoin-cli/src/app/v1.rs b/payjoin-cli/src/app/v1.rs
@@ -375,28 +375,18 @@ fn try_contributing_inputs(
     payjoin: payjoin::receive::WantsInputs,
     bitcoind: &bitcoincore_rpc::Client,
 ) -> Result<payjoin::receive::ProvisionalProposal> {
-    use bitcoin::OutPoint;
-
-    let available_inputs = bitcoind
+    let candidate_inputs = bitcoind
         .list_unspent(None, None, None, None, None)
-        .context("Failed to list unspent from bitcoind")?;
-    let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
-        .iter()
-        .map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
-        .collect();
-
-    let selected_outpoint = payjoin
+        .context("Failed to list unspent from bitcoind")?
+        .into_iter()
+        .map(input_pair_from_list_unspent);
+    let selected_input = payjoin
         .try_preserving_privacy(candidate_inputs)
         .map_err(|e| anyhow!("Failed to make privacy preserving selection: {}", e))?;
-    let selected_utxo = available_inputs
-        .iter()
-        .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 input_pair = input_pair_from_list_unspent(selected_utxo);
+    log::debug!("selected input: {:#?}", selected_input);
 
     Ok(payjoin
-        .contribute_inputs(vec![input_pair])
+        .contribute_inputs(vec![selected_input])
         .expect("This shouldn't happen. Failed to contribute inputs.")
         .commit_inputs())
 }
diff --git a/payjoin-cli/src/app/v2.rs b/payjoin-cli/src/app/v2.rs
@@ -365,28 +365,18 @@ fn try_contributing_inputs(
     payjoin: payjoin::receive::v2::WantsInputs,
     bitcoind: &bitcoincore_rpc::Client,
 ) -> Result<payjoin::receive::v2::ProvisionalProposal> {
-    use bitcoin::OutPoint;
-
-    let available_inputs = bitcoind
+    let candidate_inputs = bitcoind
         .list_unspent(None, None, None, None, None)
-        .context("Failed to list unspent from bitcoind")?;
-    let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
-        .iter()
-        .map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
-        .collect();
-
-    let selected_outpoint = payjoin
+        .context("Failed to list unspent from bitcoind")?
+        .into_iter()
+        .map(input_pair_from_list_unspent);
+    let selected_input = payjoin
         .try_preserving_privacy(candidate_inputs)
         .map_err(|e| anyhow!("Failed to make privacy preserving selection: {}", e))?;
-    let selected_utxo = available_inputs
-        .iter()
-        .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 input_pair = input_pair_from_list_unspent(selected_utxo);
+    log::debug!("selected input: {:#?}", selected_input);
 
     Ok(payjoin
-        .contribute_inputs(vec![input_pair])
+        .contribute_inputs(vec![selected_input])
         .expect("This shouldn't happen. Failed to contribute inputs.")
         .commit_inputs())
 }
diff --git a/payjoin/src/receive/error.rs b/payjoin/src/receive/error.rs
@@ -260,6 +260,8 @@ pub(crate) enum InternalSelectionError {
     TooManyOutputs,
     /// No selection candidates improve privacy
     NotFound,
+    /// Missing previous txout information
+    PrevTxOut(crate::psbt::PrevTxOutError),
 }
 
 impl fmt::Display for SelectionError {
@@ -272,6 +274,8 @@ impl fmt::Display for SelectionError {
             ),
             InternalSelectionError::NotFound =>
                 write!(f, "No selection candidates improve privacy"),
+            InternalSelectionError::PrevTxOut(e) =>
+                write!(f, "Missing previous txout information: {}", e),
         }
     }
 }
diff --git a/payjoin/src/receive/mod.rs b/payjoin/src/receive/mod.rs
@@ -459,8 +459,8 @@ impl WantsInputs {
     /// A simple consolidation is otherwise chosen if available.
     pub fn try_preserving_privacy(
         &self,
-        candidate_inputs: impl IntoIterator<Item = (Amount, OutPoint)>,
-    ) -> Result<OutPoint, SelectionError> {
+        candidate_inputs: impl IntoIterator<Item = (PsbtInput, TxIn)>,
+    ) -> Result<(PsbtInput, TxIn), SelectionError> {
         let mut candidate_inputs = candidate_inputs.into_iter().peekable();
         if candidate_inputs.peek().is_none() {
             return Err(InternalSelectionError::Empty.into());
@@ -486,8 +486,8 @@ impl WantsInputs {
     /// https://eprint.iacr.org/2022/589.pdf
     fn avoid_uih(
         &self,
-        candidate_inputs: impl IntoIterator<Item = (Amount, OutPoint)>,
-    ) -> Result<OutPoint, SelectionError> {
+        candidate_inputs: impl IntoIterator<Item = (PsbtInput, TxIn)>,
+    ) -> Result<(PsbtInput, TxIn), SelectionError> {
         let min_original_out_sats = self
             .payjoin_psbt
             .unsigned_tx
@@ -506,15 +506,17 @@ impl WantsInputs {
 
         let prior_payment_sats = self.payjoin_psbt.unsigned_tx.output[self.change_vout].value;
 
-        for candidate in candidate_inputs {
-            let candidate_sats = candidate.0;
+        for (psbtin, txin) in candidate_inputs {
+            let input_pair = InputPair { txin: &txin, psbtin: &psbtin };
+            let candidate_sats =
+                input_pair.previous_txout().map_err(InternalSelectionError::PrevTxOut)?.value;
             let candidate_min_out = min(min_original_out_sats, prior_payment_sats + candidate_sats);
             let candidate_min_in = min(min_original_in_sats, candidate_sats);
 
             if candidate_min_in > candidate_min_out {
                 // The candidate avoids UIH2 but conforms to UIH1: Optimal change heuristic.
                 // It implies the smallest output is the sender's change address.
-                return Ok(candidate.1);
+                return Ok((psbtin, txin));
             }
         }
 
@@ -524,12 +526,12 @@ impl WantsInputs {
 
     fn select_first_candidate(
         &self,
-        candidate_inputs: impl IntoIterator<Item = (Amount, OutPoint)>,
-    ) -> Result<OutPoint, SelectionError> {
+        candidate_inputs: impl IntoIterator<Item = (PsbtInput, TxIn)>,
+    ) -> Result<(PsbtInput, TxIn), SelectionError> {
         candidate_inputs
             .into_iter()
             .next()
-            .map_or(Err(InternalSelectionError::NotFound.into()), |(_, outpoint)| Ok(outpoint))
+            .map_or(Err(InternalSelectionError::NotFound.into()), |input_pair| Ok(input_pair))
     }
 
     /// Add the provided list of inputs to the transaction.
diff --git a/payjoin/src/receive/v2/mod.rs b/payjoin/src/receive/v2/mod.rs
@@ -387,8 +387,8 @@ impl WantsInputs {
     /// https://eprint.iacr.org/2022/589.pdf
     pub fn try_preserving_privacy(
         &self,
-        candidate_inputs: impl IntoIterator<Item = (Amount, OutPoint)>,
-    ) -> Result<OutPoint, SelectionError> {
+        candidate_inputs: impl IntoIterator<Item = (PsbtInput, TxIn)>,
+    ) -> Result<(PsbtInput, TxIn), SelectionError> {
         self.inner.try_preserving_privacy(candidate_inputs)
     }
 
diff --git a/payjoin/tests/integration.rs b/payjoin/tests/integration.rs
@@ -560,7 +560,7 @@ mod integration {
                 // POST payjoin
                 let proposal =
                     session.process_res(response.bytes().await?.to_vec().as_slice(), ctx)?.unwrap();
-                let inputs = receiver_utxos.iter().map(input_pair_from_list_unspent).collect();
+                let inputs = receiver_utxos.into_iter().map(input_pair_from_list_unspent).collect();
                 let mut payjoin_proposal =
                     handle_directory_proposal(&receiver, proposal, Some(inputs));
                 assert!(!payjoin_proposal.is_output_substitution_disabled());
@@ -874,24 +874,18 @@ mod integration {
             let inputs = match custom_inputs {
                 Some(inputs) => inputs,
                 None => {
-                    let available_inputs =
-                        receiver.list_unspent(None, None, None, None, None).unwrap();
-                    let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
-                        .iter()
-                        .map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
-                        .collect();
-
-                    let selected_outpoint = payjoin
+                    let candidate_inputs = receiver
+                        .list_unspent(None, None, None, None, None)
+                        .unwrap()
+                        .into_iter()
+                        .map(input_pair_from_list_unspent);
+                    let selected_input = payjoin
                         .try_preserving_privacy(candidate_inputs)
-                        .expect("Failed to make privacy preserving selection");
-                    let selected_utxo = available_inputs
-                        .iter()
-                        .find(|i| {
-                            i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout
+                        .map_err(|e| {
+                            format!("Failed to make privacy preserving selection: {:?}", e)
                         })
                         .unwrap();
-                    let input_pair = input_pair_from_list_unspent(selected_utxo);
-                    vec![input_pair]
+                    vec![selected_input]
                 }
             };
             let payjoin = payjoin.contribute_inputs(inputs).unwrap().commit_inputs();
@@ -1043,7 +1037,7 @@ mod integration {
                     .script_pubkey(),
             }];
             let drain_script = outputs[0].script_pubkey.clone();
-            let inputs = receiver_utxos.iter().map(input_pair_from_list_unspent).collect();
+            let inputs = receiver_utxos.into_iter().map(input_pair_from_list_unspent).collect();
             let response = handle_v1_pj_request(
                 req,
                 headers,
@@ -1309,21 +1303,14 @@ mod integration {
         let inputs = match custom_inputs {
             Some(inputs) => inputs,
             None => {
-                let available_inputs = receiver.list_unspent(None, None, None, None, None)?;
-                let candidate_inputs: HashMap<Amount, OutPoint> = available_inputs
-                    .iter()
-                    .map(|i| (i.amount, OutPoint { txid: i.txid, vout: i.vout }))
-                    .collect();
-
-                let selected_outpoint = payjoin
+                let candidate_inputs = receiver
+                    .list_unspent(None, None, None, None, None)?
+                    .into_iter()
+                    .map(input_pair_from_list_unspent);
+                let selected_input = payjoin
                     .try_preserving_privacy(candidate_inputs)
                     .map_err(|e| format!("Failed to make privacy preserving selection: {:?}", e))?;
-                let selected_utxo = available_inputs
-                    .iter()
-                    .find(|i| i.txid == selected_outpoint.txid && i.vout == selected_outpoint.vout)
-                    .unwrap();
-                let input_pair = input_pair_from_list_unspent(selected_utxo);
-                vec![input_pair]
+                vec![selected_input]
             }
         };
         let payjoin = payjoin
@@ -1388,7 +1375,7 @@ mod integration {
     }
 
     fn input_pair_from_list_unspent(
-        utxo: &bitcoind::bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,
+        utxo: bitcoind::bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,
     ) -> (PsbtInput, TxIn) {
         let psbtin = PsbtInput {
             // NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies

Original file line number	Diff line number	Diff line change
`@@ -124,7 +124,7 @@ fn read_local_cert() -> Result<Vec<u8>> {`
`124`	`124`	`}`
`125`	`125`
`126`	`126`	`pub fn input_pair_from_list_unspent(`
`127`		`- utxo: &bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,`
	`127`	`+ utxo: bitcoincore_rpc::bitcoincore_rpc_json::ListUnspentResultEntry,`
`128`	`128`	`) -> (PsbtInput, TxIn) {`
`129`	`129`	`let psbtin = PsbtInput {`
`130`	`130`	`// NOTE: non_witness_utxo is not necessary because bitcoin-cli always supplies`
Original file line number	Diff line number	Diff line change
`@@ -260,6 +260,8 @@ pub(crate) enum InternalSelectionError {`
`260`	`260`	`TooManyOutputs,`
`261`	`261`	`/// No selection candidates improve privacy`
`262`	`262`	`NotFound,`
	`263`	`+ /// Missing previous txout information`
	`264`	`+ PrevTxOut(crate::psbt::PrevTxOutError),`
`263`	`265`	`}`
`264`	`266`
`265`	`267`	`impl fmt::Display for SelectionError {`
`@@ -272,6 +274,8 @@ impl fmt::Display for SelectionError {`
`272`	`274`	`),`
`273`	`275`	`InternalSelectionError::NotFound =>`
`274`	`276`	`write!(f, "No selection candidates improve privacy"),`
	`277`	`+ InternalSelectionError::PrevTxOut(e) =>`
	`278`	`+ write!(f, "Missing previous txout information: {}", e),`
`275`	`279`	`}`
`276`	`280`	`}`
`277`	`281`	`}`