Merge rust-bitcoin/rust-bitcoin#3699: Explicitly define Ord for NodeInfo

428e9787d1 Explicitly define Ord for NodeInfo (Shing Him Ng)

Pull request description:

  Fixes #3654 by explicitly defining Ord so that we avoid potentially catastrophically changing addresses out from under users

ACKs for top commit:
  sanket1729:
    utACK 428e9787d1
  apoelstra:
    ACK 428e9787d181a462d06a18b7a45701790cbc0929; successfully ran local tests

Tree-SHA512: e900e07b2c53f643e3239bf7d26567b56275899b408a94e45f1cddd81217141c304c87159ce413a7a4660f1c09a0db2bbc1146948a2d9e7600abf14cd73ac691

bitcoin/src/taproot/mod.rs

@@ -7,7 +7,7 @@
 pub mod merkle_branch;
 pub mod serialized_signature;
 
-use core::cmp::Reverse;
+use core::cmp::{Ordering, Reverse};
 use core::fmt;
 use core::iter::FusedIterator;
 
@@ -789,7 +789,7 @@ impl DoubleEndedIterator for LeafNodes<'_> {
 ///
 /// You can use [`TaprootSpendInfo::from_node_info`] to a get a [`TaprootSpendInfo`] from the Merkle
 /// root [`NodeInfo`].
-#[derive(Debug, Clone, PartialOrd, Ord)]
+#[derive(Debug, Clone)]
 pub struct NodeInfo {
     /// Merkle hash for this node.
     pub(crate) hash: TapNodeHash,
@@ -799,6 +799,24 @@ pub struct NodeInfo {
     pub(crate) has_hidden_nodes: bool,
 }
 
+/// Explicitly implement Ord so future changes to NodeInfo (e.g. adding a new field) won't result in
+/// potentially changing addresses out from under users
+impl Ord for NodeInfo {
+    fn cmp(&self, other: &Self) -> Ordering {
+        match self.hash.cmp(&other.hash) {
+            Ordering::Equal => match self.leaves.cmp(&other.leaves) {
+                Ordering::Equal => self.has_hidden_nodes.cmp(&other.has_hidden_nodes),
+                other => other,
+            },
+            other => other,
+        }
+    }
+}
+
+impl PartialOrd for NodeInfo {
+    fn partial_cmp(&self, other: &NodeInfo) -> Option<Ordering> { Some(self.cmp(other)) }
+}
+
 impl PartialEq for NodeInfo {
     fn eq(&self, other: &Self) -> bool { self.hash.eq(&other.hash) }
 }