Merge pull request #26 from 'refactor/replace-get-smallest-outpoint-by-lexmin'

Author: nymius

silentpayments/src/lib.rs

@@ -63,34 +63,62 @@ pub fn tag_txin(txin: &TxIn, script_pubkey: &ScriptBuf) -> Option<SpInputs> {
     }
 }
 
-pub fn get_smallest_lexicographic_outpoint(outpoints: &[OutPoint]) -> [u8; 36] {
-    // Find the outpoint with the smallest lexicographic order
-    let smallest = outpoints
-        .iter()
-        .min_by(|a, b| {
-            // Compare txids first
-            let a_txid = a.txid.to_raw_hash();
-            let b_txid = b.txid.to_raw_hash();
-
-            // If txids are different, compare them
-            match a_txid.as_byte_array().cmp(b_txid.as_byte_array()) {
-                std::cmp::Ordering::Equal => {
-                    // If txids are equal, compare vouts directly
-                    let a_vout_bytes = a.vout.to_le_bytes();
-                    let b_vout_bytes = b.vout.to_le_bytes();
-                    a_vout_bytes.cmp(&b_vout_bytes)
+#[derive(Debug)]
+pub enum LexMinError {
+    NoMinOutpoint,
+}
+
+impl std::fmt::Display for LexMinError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            Self::NoMinOutpoint => write!(f, "No minimal outpoint, update at least once"),
+        }
+    }
+}
+
+#[derive(Default)]
+pub struct LexMin<'a> {
+    current_min: Option<&'a OutPoint>,
+}
+
+impl<'a> LexMin<'a> {
+    pub fn update(&mut self, outpoint: &'a OutPoint) -> &'a OutPoint {
+        if let Some(min) = self.current_min {
+            let new_min = std::cmp::min_by(outpoint, min, |a, b| {
+                // Compare txids first
+                let a_txid = a.txid.to_raw_hash();
+                let b_txid = b.txid.to_raw_hash();
+
+                // If txids are different, compare them
+                match a_txid.as_byte_array().cmp(b_txid.as_byte_array()) {
+                    std::cmp::Ordering::Equal => {
+                        // If txids are equal, compare vouts directly
+                        let a_vout_bytes = a.vout.to_le_bytes();
+                        let b_vout_bytes = b.vout.to_le_bytes();
+                        a_vout_bytes.cmp(&b_vout_bytes)
+                    }
+                    other => other,
                 }
-                other => other,
-            }
-        })
-        .expect("cannot create silent payment script pubkey without outpoints");
+            });
+            self.current_min = Some(new_min);
+            new_min
+        } else {
+            self.current_min = Some(outpoint);
+            outpoint
+        }
+    }
 
-    // Only allocate the result array once we have the smallest outpoint
-    let mut result = [0u8; 36];
-    result[..32].copy_from_slice(smallest.txid.to_raw_hash().as_byte_array());
-    result[32..36].copy_from_slice(&smallest.vout.to_le_bytes());
+    pub fn bytes(&self) -> Result<[u8; 36], LexMinError> {
+        if let Some(min) = self.current_min {
+            let mut result = [0u8; 36];
+            result[..32].copy_from_slice(min.txid.to_raw_hash().as_byte_array());
+            result[32..36].copy_from_slice(&min.vout.to_le_bytes());
 
-    result
+            Ok(result)
+        } else {
+            Err(LexMinError::NoMinOutpoint)
+        }
+    }
 }
 
 // Do not report coverage for this function as it is a wrapper around external lib function
@@ -110,9 +138,7 @@ pub fn compute_shared_secret(sk: &SecretKey, pk: &PublicKey) -> PublicKey {
 #[cfg(test)]
 #[cfg_attr(coverage_nightly, coverage(off))]
 mod tests {
-    use super::{
-        get_smallest_lexicographic_outpoint, tag_txin, Hash, ScriptBuf, SpInputs, TxIn, NUMS_H,
-    };
+    use super::{tag_txin, Hash, LexMin, ScriptBuf, SpInputs, TxIn, NUMS_H};
     use bitcoin::{
         hex::test_hex_unwrap as hex, taproot::TAPROOT_ANNEX_PREFIX, OutPoint, Sequence, Txid,
         Witness,
@@ -344,8 +370,9 @@ mod tests {
     }
 
     #[test]
-    fn test_get_smallest_outpoint_different_txids_and_vouts() {
-        let outpoints = vec![
+    fn test_lex_min_different_txids_and_vouts() {
+        let mut lex_min = LexMin::default();
+        let outpoints = [
             OutPoint {
                 txid: Txid::from_slice(&[3u8; 32]).unwrap(),
                 vout: 2,
@@ -360,7 +387,11 @@ mod tests {
             },
         ];
 
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [2u8; 36];
         expected_bytes[32..36].copy_from_slice(&1u32.to_le_bytes());
@@ -369,32 +400,37 @@ mod tests {
     }
 
     #[test]
-    #[should_panic(expected = "cannot create silent payment script pubkey without outpoints")]
-    fn test_get_smallest_outpoint_empty() {
-        let outpoints: Vec<OutPoint> = vec![];
-        get_smallest_lexicographic_outpoint(&outpoints);
+    fn test_lex_min_no_update() {
+        let e = LexMin::default().bytes().expect_err("should fail");
+        assert_eq!("No minimal outpoint, update at least once", e.to_string());
     }
 
     // Additional test: same txid, different vouts
     #[test]
-    fn test_get_smallest_outpoint_identical_txid_different_vouts() {
+    fn test_lex_min_identical_txid_different_vouts() {
+        let mut lex_min = LexMin::default();
         let txid = Txid::from_slice(&[0u8; 32]).unwrap();
-        let outpoints = vec![
+        let outpoints = [
             OutPoint { txid, vout: 10 },
             OutPoint { txid, vout: 2 },
             OutPoint { txid, vout: 5 },
         ];
 
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [0u8; 36];
         expected_bytes[32..36].copy_from_slice(&2u32.to_le_bytes());
         assert_eq!(result, expected_bytes);
     }
 
     #[test]
-    fn test_get_smallest_outpoint_same_vout_different_txid() {
-        let outpoints = vec![
+    fn test_lex_min_same_vout_different_txid() {
+        let mut lex_min = LexMin::default();
+        let outpoints = [
             OutPoint {
                 txid: Txid::from_slice(&[2u8; 32]).unwrap(),
                 vout: 7,
@@ -409,16 +445,21 @@ mod tests {
             },
         ];
 
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [1u8; 36];
         expected_bytes[32..36].copy_from_slice(&7u32.to_le_bytes());
         assert_eq!(result, expected_bytes);
     }
 
     #[test]
-    fn test_get_smallest_outpoint_edge_case_max_vout() {
-        let outpoints = vec![
+    fn test_lex_min_edge_case_max_vout() {
+        let mut lex_min = LexMin::default();
+        let outpoints = [
             OutPoint {
                 txid: Txid::from_slice(&[1u8; 32]).unwrap(),
                 vout: u32::MAX,
@@ -429,7 +470,11 @@ mod tests {
             },
         ];
 
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [1u8; 36];
         expected_bytes[..32].copy_from_slice(&[1u8; 32]);
@@ -438,8 +483,9 @@ mod tests {
     }
 
     #[test]
-    fn test_get_smallest_outpoint_txid_takes_precedence() {
-        let outpoints = vec![
+    fn test_lex_min_txid_takes_precedence() {
+        let mut lex_min = LexMin::default();
+        let outpoints = [
             OutPoint {
                 txid: Txid::from_slice(&[8u8; 32]).unwrap(),
                 vout: 0,
@@ -450,7 +496,11 @@ mod tests {
             },
         ];
 
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [5u8; 36];
         expected_bytes[32..36].copy_from_slice(&100u32.to_le_bytes());
@@ -459,6 +509,7 @@ mod tests {
 
     #[test]
     fn test_get_smallest_outpoint_txid_endianness_matters() {
+        let mut lex_min = LexMin::default();
         // big endian: 0x[00][00][00][01]
         // big endian: 0x[a1][b1][c1][d1]
         let mut txid_bytes_be = [0u8; 32];
@@ -469,7 +520,7 @@ mod tests {
         let mut txid_bytes_le = [0u8; 32];
         txid_bytes_le[31] = 1;
 
-        let outpoints = vec![
+        let outpoints = [
             OutPoint {
                 txid: Txid::from_slice(&txid_bytes_be).unwrap(),
                 vout: 1,
@@ -480,9 +531,13 @@ mod tests {
             },
         ];
 
+        for outpoint in outpoints.iter() {
+            lex_min.update(outpoint);
+        }
+
         // if Txid is big endian then: [a1] < [a2] => expected_bytes = txid_bytes_be
         // if Txid is little endian then: [d2] < [d1] => expected_bytes = txid_bytes_le
-        let result = get_smallest_lexicographic_outpoint(&outpoints);
+        let result = lex_min.bytes().expect("should succeed");
 
         let mut expected_bytes = [0u8; 36];
         expected_bytes[31] = 1;

silentpayments/src/receive/error.rs

@@ -6,6 +6,14 @@ pub enum SpReceiveError {
     Secp256k1Error(bitcoin::secp256k1::Error),
     /// Secp256k1 error
     SliceError(bitcoin::key::FromSliceError),
+    /// Cannot derive silent payment output without input prevout outpoints
+    NoOutpoints(crate::LexMinError),
+}
+
+impl From<crate::LexMinError> for SpReceiveError {
+    fn from(e: crate::LexMinError) -> Self {
+        Self::NoOutpoints(e)
+    }
 }
 
 impl From<bitcoin::secp256k1::Error> for SpReceiveError {
@@ -32,6 +40,7 @@ impl std::fmt::Display for SpReceiveError {
             }
             SpReceiveError::Secp256k1Error(e) => write!(f, "Silent payment receive error: {e}"),
             SpReceiveError::SliceError(e) => write!(f, "Silent payment receive error: {e}"),
+            Self::NoOutpoints(e) => write!(f, "Silent payment sending error: {e}"),
         }
     }
 }

silentpayments/src/receive/mod.rs

@@ -4,9 +4,8 @@ pub mod scan;
 pub use self::error::SpReceiveError;
 
 use crate::{
-    get_smallest_lexicographic_outpoint,
     hashes::{InputsHash, SharedSecretHash},
-    tag_txin, SpInputs,
+    tag_txin, LexMin, SpInputs,
 };
 
 use std::collections::BTreeMap;
@@ -231,33 +230,26 @@ pub fn compute_tweak_data(
 ) -> Result<PublicKey, SpReceiveError> {
     let secp = Secp256k1::verification_only();
 
-    let input_pubkeys = tx
-        .input
-        .clone()
-        .into_iter()
-        .zip(prevouts)
-        .filter_map(|(txin, prevout)| {
-            // NOTE: Public keys which couldn't be extracted will be ignored
-            extract_pubkey(txin, &prevout.script_pubkey).map(|(_input_type, key)| key)
-        })
-        .collect::<Vec<PublicKey>>();
+    let mut input_pubkeys = <Vec<PublicKey>>::new();
+    let mut lex_min = LexMin::default();
+    for (txin, prevout) in tx.input.iter().zip(prevouts) {
+        lex_min.update(&txin.previous_output);
+        // NOTE: Public keys which couldn't be extracted will be ignored
+        if let Some((_, key)) = extract_pubkey(txin.clone(), &prevout.script_pubkey) {
+            input_pubkeys.push(key)
+        }
+    }
 
     let input_pubkey_refs: Vec<&PublicKey> = input_pubkeys.iter().collect();
 
     #[allow(non_snake_case)]
     // NOTE: cannot ignore malicious crafting of transaction with input public keys that cancel
     // themselves
     let A_sum = PublicKey::combine_keys(&input_pubkey_refs)?;
-    let outpoints = tx
-        .input
-        .iter()
-        .map(|txin| txin.previous_output)
-        .collect::<Vec<OutPoint>>();
-    let smallest_outpoint = get_smallest_lexicographic_outpoint(&outpoints);
 
     let input_hash = {
         let mut eng = InputsHash::engine();
-        eng.input(&smallest_outpoint);
+        eng.input(&lex_min.bytes()?);
         eng.input(&A_sum.serialize());
         let hash = InputsHash::from_engine(eng);
         // NOTE: Why big endian bytes??? Doesn't matter. Look at: https://github.com/rust-bitcoin/rust-bitcoin/issues/1896

silentpayments/src/send/bip32.rs

@@ -2,10 +2,10 @@ use std::collections::HashMap;
 
 use crate::{
     encoding::SilentPaymentCode,
-    get_smallest_lexicographic_outpoint,
     send::{
         create_silentpayment_partial_secret, create_silentpayment_scriptpubkeys, error::SpSendError,
     },
+    LexMin,
 };
 
 use bitcoin::{
@@ -30,10 +30,12 @@ impl XprivSilentPaymentSender {
         outputs: &[SilentPaymentCode],
     ) -> Result<HashMap<SilentPaymentCode, Vec<XOnlyPublicKey>>, SpSendError> {
         let secp = Secp256k1::new();
-        let (outpoints, spks_with_derivations): (Vec<_>, Vec<_>) = inputs.iter().cloned().unzip();
 
         let mut spks_with_keys = <Vec<(ScriptBuf, SecretKey)>>::new();
-        for (spk, derivation_path) in spks_with_derivations {
+        let mut lex_min = LexMin::default();
+        for (outpoint, (spk, derivation_path)) in inputs.iter() {
+            lex_min.update(outpoint);
+
             let bip32_privkey = self.xpriv.derive_priv(&secp, &derivation_path)?;
             let mut internal_privkey = bip32_privkey.private_key;
             let (x_only_internal, parity) = internal_privkey.x_only_public_key(&secp);
@@ -49,13 +51,11 @@ impl XprivSilentPaymentSender {
             let external_privkey = internal_privkey.add_tweak(&tap_tweak.to_scalar())
                 .expect("computationally unreachable: can only fail if tap_tweak = -internal_privkey, but tap_tweak is the output of a hash function");
 
-            spks_with_keys.push((spk, external_privkey));
+            spks_with_keys.push((spk.clone(), external_privkey));
         }
 
-        let smallest_outpoint_bytes = get_smallest_lexicographic_outpoint(&outpoints);
-
         let partial_secret =
-            create_silentpayment_partial_secret(&smallest_outpoint_bytes, &spks_with_keys)?;
+            create_silentpayment_partial_secret(&lex_min.bytes()?, &spks_with_keys)?;
 
         create_silentpayment_scriptpubkeys(partial_secret, outputs)
     }