From: Savil Srivastava <savil@cs.stanford.edu>
Date: Mon, 23 Jul 2018 00:03:13 +0000 (-0700)
Subject: [RFC][Tx Sort] Implement sorting of inputs
X-Git-Tag: v0.0.12~364^2~1
X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=commitdiff_plain;h=eb1eddcea0e3591e4065806a3951d791745ec6cd;p=rust-lightning

[RFC][Tx Sort] Implement sorting of inputs

Follows BIP69: https://github.com/bitcoin/bips/blob/master/bip-0069.mediawiki

1. Implements sorting of transactoin inputs.
- BIP says to use "reversed byte-order" for the `prev_hash`. I interpreted this as: little-endian.
- TODO need to add tests

2. Re: improve sorting of TxOut's script_pubkey to use lexicographic ordering, and not length.
From the test-cases i've included it seems that the current code already does lexicographic ordering (and not length based). Am i missing something?
---

diff --git a/src/util/transaction_utils.rs b/src/util/transaction_utils.rs
index 8ecd78acc..4f5018f60 100644
--- a/src/util/transaction_utils.rs
+++ b/src/util/transaction_utils.rs
@@ -1,19 +1,92 @@
-use bitcoin::blockdata::transaction::TxOut;
+use bitcoin::blockdata::transaction::{TxIn, TxOut};
 
 use std::cmp::Ordering;
 
-pub fn sort_outputs<T>(outputs: &mut Vec<(TxOut, T)>) { //TODO: Make static and put it in some utils somewhere (+inputs sorting)
+pub fn sort_outputs<T>(outputs: &mut Vec<(TxOut, T)>) {
 	outputs.sort_unstable_by(|a, b| {
 		if a.0.value < b.0.value {
 			Ordering::Less
 		} else if b.0.value < a.0.value {
 			Ordering::Greater
-		} else if a.0.script_pubkey[..] < b.0.script_pubkey[..] { //TODO: ordering of scripts shouldn't be len-based
+		} else if a.0.script_pubkey[..] < b.0.script_pubkey[..] {
 			Ordering::Less
-		} else if b.0.script_pubkey[..] < a.0.script_pubkey[..] { //TODO: ordering of scripts shouldn't be len-based
+		} else if b.0.script_pubkey[..] < a.0.script_pubkey[..] {
 			Ordering::Greater
 		} else {
 			Ordering::Equal
 		}
 	});
 }
+
+// TODO savil. Add tests.
+pub fn sort_inputs<T>(inputs: &mut Vec<(TxIn, T)>) {
+    inputs.sort_unstable_by(|a, b| {
+        if a.0.prev_hash.into_le() < b.0.prev_hash.into_le() {
+            Ordering::Less
+        } else if b.0.prev_hash.into_le() < a.0.prev_hash.into_le() {
+            Ordering::Greater
+        } else if a.0.prev_index < b.0.prev_index {
+            Ordering::Less
+        } else if b.0.prev_index < a.0.prev_index {
+            Ordering::Greater
+        } else {
+            Ordering::Equal
+        }
+    });
+}
+
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use bitcoin::blockdata::opcodes;
+    use bitcoin::blockdata::script::Builder;
+    use bitcoin::blockdata::transaction::TxOut;
+
+    #[test]
+    fn sort_output_by_value() {
+        let txout1 = TxOut {
+            value:  100,
+            script_pubkey: Builder::new().push_int(0).into_script()
+        };
+        let txout1_ = txout1.clone();
+
+        let txout2 = TxOut {
+            value: 99,
+            script_pubkey: Builder::new().push_int(0).into_script()
+        };
+        let txout2_ = txout2.clone();
+
+        let mut outputs = vec![(txout1, "ignore"), (txout2, "ignore")];
+        sort_outputs(&mut outputs);
+
+        assert_eq!(
+            &outputs,
+            &vec![(txout2_, "ignore"), (txout1_, "ignore")]
+        );
+    }
+
+    #[test]
+    fn sort_output_by_script_pubkey() {
+        let txout1 = TxOut {
+            value:  100,
+            script_pubkey: Builder::new().push_int(3).into_script(),
+        };
+        let txout1_ = txout1.clone();
+
+        let txout2 = TxOut {
+            value: 100,
+            script_pubkey: Builder::new().push_int(1).push_int(2).into_script()
+        };
+        let txout2_ = txout2.clone();
+
+        let mut outputs = vec![(txout1, "ignore"), (txout2, "ignore")];
+        sort_outputs(&mut outputs);
+
+        assert_eq!(
+            &outputs,
+            &vec![(txout2_, "ignore"), (txout1_, "ignore")]
+        );
+    }
+}