Force v1 receiver to also use a mutable reference to the state closures

By also using a mutable reference to the closure on our v1 receiver
instead of taking ownership at this level we can keep the code
consistent across versions.

Author: node-smithxby72w

payjoin-cli/src/app/v1.rs

@@ -321,17 +321,18 @@ impl App {
         let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast();
 
         // Receive Check 2: receiver can't sign for proposal inputs
-        let proposal = proposal.check_inputs_not_owned(|input| {
+        let proposal = proposal.check_inputs_not_owned(&mut |input| {
             wallet.is_mine(input).map_err(|e| ImplementationError::from(e.into_boxed_dyn_error()))
         })?;
         log::trace!("check2");
 
         // 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| Ok(self.db.insert_input_seen_before(*input)?))?;
+        let payjoin = proposal.check_no_inputs_seen_before(&mut |input| {
+            Ok(self.db.insert_input_seen_before(*input)?)
+        })?;
         log::trace!("check3");
 
-        let payjoin = payjoin.identify_receiver_outputs(|output_script| {
+        let payjoin = payjoin.identify_receiver_outputs(&mut |output_script| {
             wallet
                 .is_mine(output_script)
                 .map_err(|e| ImplementationError::from(e.into_boxed_dyn_error()))

payjoin/src/core/receive/multiparty/mod.rs

@@ -112,7 +112,7 @@ pub struct MaybeInputsOwned {
 impl MaybeInputsOwned {
     pub fn check_inputs_not_owned(
         self,
-        is_owned: impl Fn(&bitcoin::Script) -> Result<bool, ImplementationError>,
+        is_owned: &mut impl FnMut(&bitcoin::Script) -> Result<bool, ImplementationError>,
     ) -> Result<MaybeInputsSeen, Error> {
         let inner = self.v1.check_inputs_not_owned(is_owned)?;
         Ok(MaybeInputsSeen { v1: inner, contexts: self.contexts })
@@ -127,7 +127,7 @@ pub struct MaybeInputsSeen {
 impl MaybeInputsSeen {
     pub fn check_no_inputs_seen_before(
         self,
-        is_seen: impl Fn(&bitcoin::OutPoint) -> Result<bool, ImplementationError>,
+        is_seen: &mut impl FnMut(&bitcoin::OutPoint) -> Result<bool, ImplementationError>,
     ) -> Result<OutputsUnknown, Error> {
         let inner = self.v1.check_no_inputs_seen_before(is_seen)?;
         Ok(OutputsUnknown { v1: inner, contexts: self.contexts })
@@ -142,7 +142,7 @@ pub struct OutputsUnknown {
 impl OutputsUnknown {
     pub fn identify_receiver_outputs(
         self,
-        is_receiver_output: impl Fn(&bitcoin::Script) -> Result<bool, ImplementationError>,
+        is_receiver_output: &mut impl FnMut(&bitcoin::Script) -> Result<bool, ImplementationError>,
     ) -> Result<WantsOutputs, Error> {
         let inner = self.v1.identify_receiver_outputs(is_receiver_output)?;
         Ok(WantsOutputs { v1: inner, contexts: self.contexts })

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

@@ -160,9 +160,8 @@ impl MaybeInputsOwned {
     /// An attacker can try to spend the receiver's own inputs. This check prevents that.
     pub fn check_inputs_not_owned(
         self,
-        is_owned: impl FnMut(&Script) -> Result<bool, ImplementationError>,
+        is_owned: &mut impl FnMut(&Script) -> Result<bool, ImplementationError>,
     ) -> Result<MaybeInputsSeen, ReplyableError> {
-        let mut is_owned = is_owned;
         let mut err: Result<(), ReplyableError> = Ok(());
         if let Some(e) = self
             .psbt
@@ -207,9 +206,8 @@ impl MaybeInputsSeen {
     ///    original proposal PSBT of the current, new payjoin.
     pub fn check_no_inputs_seen_before(
         self,
-        is_known: impl FnMut(&OutPoint) -> Result<bool, ImplementationError>,
+        is_known: &mut impl FnMut(&OutPoint) -> Result<bool, ImplementationError>,
     ) -> Result<OutputsUnknown, ReplyableError> {
-        let mut is_known = is_known;
         self.psbt.input_pairs().try_for_each(|input| {
             match is_known(&input.txin.previous_output) {
                 Ok(false) => Ok::<(), ReplyableError>(()),
@@ -250,9 +248,8 @@ impl OutputsUnknown {
     /// outputs.
     pub fn identify_receiver_outputs(
         self,
-        is_receiver_output: impl FnMut(&Script) -> Result<bool, ImplementationError>,
+        is_receiver_output: &mut impl FnMut(&Script) -> Result<bool, ImplementationError>,
     ) -> Result<WantsOutputs, ReplyableError> {
-        let mut is_receiver_output = is_receiver_output;
         let owned_vouts: Vec<usize> = self
             .psbt
             .unsigned_tx
@@ -912,11 +909,11 @@ pub(crate) mod test {
     fn wants_outputs_from_test_vector(proposal: UncheckedProposal) -> WantsOutputs {
         proposal
             .assume_interactive_receiver()
-            .check_inputs_not_owned(|_| Ok(false))
+            .check_inputs_not_owned(&mut |_| Ok(false))
             .expect("No inputs should be owned")
-            .check_no_inputs_seen_before(|_| Ok(false))
+            .check_no_inputs_seen_before(&mut |_| Ok(false))
             .expect("No inputs should be seen before")
-            .identify_receiver_outputs(|script| {
+            .identify_receiver_outputs(&mut |script| {
                 let network = Network::Bitcoin;
                 Ok(Address::from_script(script, network).unwrap()
                     == Address::from_str("3CZZi7aWFugaCdUCS15dgrUUViupmB8bVM")
@@ -945,20 +942,20 @@ pub(crate) mod test {
             Ok(ret)
         }
 
-        let maybe_inputs_seen =
-            maybe_inputs_owned.check_inputs_not_owned(|_| mock_callback(&mut call_count, false));
+        let maybe_inputs_seen = maybe_inputs_owned
+            .check_inputs_not_owned(&mut |_| mock_callback(&mut call_count, false));
         assert_eq!(call_count, 1);
 
         let outputs_unknown = maybe_inputs_seen
             .map_err(|_| "Check inputs owned closure failed".to_string())
             .expect("Next receiver state should be accessible")
-            .check_no_inputs_seen_before(|_| mock_callback(&mut call_count, false));
+            .check_no_inputs_seen_before(&mut |_| mock_callback(&mut call_count, false));
         assert_eq!(call_count, 2);
 
         let _wants_outputs = outputs_unknown
             .map_err(|_| "Check no inputs seen closure failed".to_string())
             .expect("Next receiver state should be accessible")
-            .identify_receiver_outputs(|_| mock_callback(&mut call_count, true));
+            .identify_receiver_outputs(&mut |_| mock_callback(&mut call_count, true));
         // there are 2 receiver outputs so we should expect this callback to run twice incrementing
         // call count twice
         assert_eq!(call_count, 4);
@@ -1014,11 +1011,11 @@ pub(crate) mod test {
         let proposal = unchecked_proposal_from_test_vector();
         let wants_inputs = proposal
             .assume_interactive_receiver()
-            .check_inputs_not_owned(|_| Ok(false))
+            .check_inputs_not_owned(&mut |_| Ok(false))
             .expect("No inputs should be owned")
-            .check_no_inputs_seen_before(|_| Ok(false))
+            .check_no_inputs_seen_before(&mut |_| Ok(false))
             .expect("No inputs should be seen before")
-            .identify_receiver_outputs(|script| {
+            .identify_receiver_outputs(&mut |script| {
                 let network = Network::Bitcoin;
                 let target_address = Address::from_str("3CZZi7aWFugaCdUCS15dgrUUViupmB8bVM")
                     .map_err(ImplementationError::new)?

payjoin/src/core/receive/v2/session.rs

@@ -178,15 +178,15 @@ mod tests {
         let maybe_inputs_owned = unchecked_proposal.clone().assume_interactive_receiver();
         let maybe_inputs_seen = maybe_inputs_owned
             .clone()
-            .check_inputs_not_owned(|_| Ok(false))
+            .check_inputs_not_owned(&mut |_| Ok(false))
             .expect("No inputs should be owned");
         let outputs_unknown = maybe_inputs_seen
             .clone()
-            .check_no_inputs_seen_before(|_| Ok(false))
+            .check_no_inputs_seen_before(&mut |_| Ok(false))
             .expect("No inputs should be seen before");
         let wants_outputs = outputs_unknown
             .clone()
-            .identify_receiver_outputs(|_| Ok(true))
+            .identify_receiver_outputs(&mut |_| Ok(true))
             .expect("Outputs should be identified");
         let wants_inputs = wants_outputs.clone().commit_outputs();
         let wants_fee_range = wants_inputs.clone().commit_inputs();
@@ -375,15 +375,15 @@ mod tests {
         let maybe_inputs_owned = unchecked_proposal.clone().assume_interactive_receiver();
         let maybe_inputs_seen = maybe_inputs_owned
             .clone()
-            .check_inputs_not_owned(|_| Ok(false))
+            .check_inputs_not_owned(&mut |_| Ok(false))
             .expect("No inputs should be owned");
         let outputs_unknown = maybe_inputs_seen
             .clone()
-            .check_no_inputs_seen_before(|_| Ok(false))
+            .check_no_inputs_seen_before(&mut |_| Ok(false))
             .expect("No inputs should be seen before");
         let wants_outputs = outputs_unknown
             .clone()
-            .identify_receiver_outputs(|_| Ok(true))
+            .identify_receiver_outputs(&mut |_| Ok(true))
             .expect("Outputs should be identified");
         let wants_inputs = wants_outputs.clone().commit_outputs();
         let wants_fee_range = wants_inputs.clone().commit_inputs();
@@ -425,15 +425,15 @@ mod tests {
         let maybe_inputs_owned = unchecked_proposal.clone().assume_interactive_receiver();
         let maybe_inputs_seen = maybe_inputs_owned
             .clone()
-            .check_inputs_not_owned(|_| Ok(false))
+            .check_inputs_not_owned(&mut |_| Ok(false))
             .expect("No inputs should be owned");
         let outputs_unknown = maybe_inputs_seen
             .clone()
-            .check_no_inputs_seen_before(|_| Ok(false))
+            .check_no_inputs_seen_before(&mut |_| Ok(false))
             .expect("No inputs should be seen before");
         let wants_outputs = outputs_unknown
             .clone()
-            .identify_receiver_outputs(|_| Ok(true))
+            .identify_receiver_outputs(&mut |_| Ok(true))
             .expect("Outputs should be identified");
         let wants_inputs = wants_outputs.clone().commit_outputs();
         let wants_fee_range = wants_inputs.clone().commit_inputs();

payjoin/tests/integration.rs

@@ -1084,15 +1084,15 @@ mod integration {
                 // In a ns1r payment, the merged psbt is not broadcastable
                 let proposal =
                     multiparty_proposal.check_broadcast_suitability(None, |_| Ok(true))?;
-                let proposal = proposal.check_inputs_not_owned(|input| {
+                let proposal = proposal.check_inputs_not_owned(&mut |input| {
                     let address =
                         bitcoin::Address::from_script(input, bitcoin::Network::Regtest).unwrap();
                     Ok(receiver.get_address_info(&address).unwrap().is_mine.unwrap())
                 })?;
 
                 let payjoin = proposal
-                    .check_no_inputs_seen_before(|_| Ok(false))?
-                    .identify_receiver_outputs(|output_script| {
+                    .check_no_inputs_seen_before(&mut |_| Ok(false))?
+                    .identify_receiver_outputs(&mut |output_script| {
                         let address =
                             bitcoin::Address::from_script(output_script, bitcoin::Network::Regtest)
                                 .unwrap();
@@ -1401,7 +1401,7 @@ mod integration {
         let _to_broadcast_in_failure_case = proposal.extract_tx_to_schedule_broadcast();
 
         // Receive Check 2: receiver can't sign for proposal inputs
-        let proposal = proposal.check_inputs_not_owned(|input| {
+        let proposal = proposal.check_inputs_not_owned(&mut |input| {
             let address = bitcoin::Address::from_script(input, bitcoin::Network::Regtest)
                 .map_err(ImplementationError::new)?;
             receiver
@@ -1412,8 +1412,8 @@ mod integration {
 
         // 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(|_| Ok(false))?
-            .identify_receiver_outputs(|output_script| {
+            .check_no_inputs_seen_before(&mut |_| Ok(false))?
+            .identify_receiver_outputs(&mut |output_script| {
                 let address =
                     bitcoin::Address::from_script(output_script, bitcoin::Network::Regtest)
                         .map_err(ImplementationError::new)?;