refactor(lib): replace get_smallest_lexicographic_outpoint by LexMin

The new LexMin is a zero allocation cost struct that allows computing
the minimal lexicographic outpoint in the same loop where other logic is
contained, avoiding the intermediate step of extracting all the
outpoints from their TxIn into a separated collection to sort them.

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)
     }