Use hashable_ord on short_ids (#932)

core/src/core/id.rs

@@ -14,6 +14,7 @@
 
 //! short ids for compact blocks
 
+use std::cmp::Ordering;
 use std::cmp::min;
 
 use byteorder::{ByteOrder, LittleEndian};
@@ -70,9 +71,13 @@ impl<H: Hashed> ShortIdentifiable for H {
 }
 
 /// Short id for identifying inputs/outputs/kernels
-#[derive(PartialEq, Clone, PartialOrd, Ord, Eq, Serialize, Deserialize)]
+#[derive(Clone, Serialize, Deserialize)]
 pub struct ShortId([u8; 6]);
 
+/// We want to sort short_ids in a canonical and consistent manner so we can
+/// verify sort order in the same way we do for full inputs|outputs|kernels themselves.
+hashable_ord!(ShortId);
+
 impl ::std::fmt::Debug for ShortId {
 	fn fmt(&self, f: &mut ::std::fmt::Formatter) -> ::std::fmt::Result {
 		try!(write!(f, "{}(", stringify!(ShortId)));
@@ -131,6 +136,29 @@ mod test {
 	use super::*;
 	use ser::{Writeable, Writer};
 
+	#[test]
+	fn short_id_ord() {
+		let id_1 = ShortId::from_bytes(&[1, 1, 1, 1]);
+		let id_2 = ShortId::from_bytes(&[2, 2, 2, 2]);
+		let id_3 = ShortId::from_bytes(&[3, 3, 3, 3]);
+
+		let mut ids = vec![id_1.clone(), id_2.clone(), id_3.clone()];
+		println!("{:?}", ids);
+
+		let mut hashes = ids.iter().map(|x| x.hash()).collect::<Vec<_>>();
+		println!("{:?}", hashes);
+
+		// NOTE: after sorting hash(3) comes before hash(2)
+		hashes.sort();
+		println!("{:?}", hashes);
+		assert_eq!(hashes, [id_1.hash(), id_3.hash(), id_2.hash()]);
+
+		// NOTE: this also applies to sorting the ids (we sort based on hashes)
+		ids.sort();
+		println!("{:?}", ids);
+		assert_eq!(ids, [id_1, id_3, id_2]);
+	}
+
 	#[test]
 	fn test_short_id() {
 		// minimal struct for testing

core/src/core/mod.rs

@@ -36,7 +36,7 @@ use util::secp::pedersen::*;
 pub use self::block::*;
 pub use self::transaction::*;
 pub use self::id::ShortId;
-use self::hash::Hashed;
+use core::hash::Hashed;
 use ser::{Error, Readable, Reader, Writeable, Writer};
 use global;
 

core/src/core/transaction.rs

@@ -45,29 +45,6 @@ bitflags! {
 	}
 }
 
-// don't seem to be able to define an Ord implementation for Hash due to
-// Ord being defined on all pointers, resorting to a macro instead
-macro_rules! hashable_ord {
-	($hashable: ident) => {
-		impl Ord for $hashable {
-			fn cmp(&self, other: &$hashable) -> Ordering {
-				self.hash().cmp(&other.hash())
-			}
-		}
-		impl PartialOrd for $hashable {
-			fn partial_cmp(&self, other: &$hashable) -> Option<Ordering> {
-				Some(self.hash().cmp(&other.hash()))
-			}
-		}
-		impl PartialEq for $hashable {
-			fn eq(&self, other: &$hashable) -> bool {
-				self.hash() == other.hash()
-			}
-		}
-		impl Eq for $hashable {}
-	}
-}
-
 /// Errors thrown by Block validation
 #[derive(Clone, Debug, PartialEq)]
 pub enum Error {

core/src/macros.rs

@@ -92,3 +92,26 @@ macro_rules! ser_multiwrite {
     $( try!($wrtr.$write_call($val)) );*
   }
 }
+
+// don't seem to be able to define an Ord implementation for Hash due to
+// Ord being defined on all pointers, resorting to a macro instead
+macro_rules! hashable_ord {
+	($hashable: ident) => {
+		impl Ord for $hashable {
+			fn cmp(&self, other: &$hashable) -> Ordering {
+				self.hash().cmp(&other.hash())
+			}
+		}
+		impl PartialOrd for $hashable {
+			fn partial_cmp(&self, other: &$hashable) -> Option<Ordering> {
+				Some(self.cmp(other))
+			}
+		}
+		impl PartialEq for $hashable {
+			fn eq(&self, other: &$hashable) -> bool {
+				self.hash() == other.hash()
+			}
+		}
+		impl Eq for $hashable {}
+	}
+}