Skip to content

Commit 1337f7f

Browse files
committed
miniscript: implement PartialOrd/Ord nonrecursively
This implements an ordering based on the display ordering, though a more efficiently than just serializing to strings and comparing those. Empirically this adds a couple kb to the total code size, but not as much as the nonrecursive fmt shrunk it. We expect somewhat larger code to implement such a nontrivial algorithm nonrecursively (and in fact the old algorithm was "wrong" in that it did not implement any coherent ordering). This is a breaking change because it changes the order of Miniscripts.
1 parent bd57608 commit 1337f7f

File tree

2 files changed

+50
-2
lines changed

2 files changed

+50
-2
lines changed

src/miniscript/decode.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ enum NonTerm {
114114
///
115115
/// The average user should always use the [`Descriptor`] APIs. Advanced users who want deal
116116
/// with Miniscript ASTs should use the [`Miniscript`] APIs.
117-
#[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
117+
#[derive(Clone, PartialEq, Eq, Hash)]
118118
pub enum Terminal<Pk: MiniscriptKey, Ctx: ScriptContext> {
119119
/// `1`
120120
True,

src/miniscript/display.rs

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
//! Miniscript Node Display
44
5-
use core::fmt;
5+
use core::{cmp, fmt};
66
use std::sync::Arc;
77

88
use bitcoin::hashes::hash160;
@@ -312,3 +312,51 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> fmt::Display for Terminal<Pk, Ctx> {
312312
self.conditional_fmt(f, DisplayTypes::None)
313313
}
314314
}
315+
316+
impl<Pk: MiniscriptKey, Ctx: ScriptContext> PartialOrd for Terminal<Pk, Ctx> {
317+
fn partial_cmp(&self, other: &Self) -> Option<cmp::Ordering> { Some(self.cmp(other)) }
318+
}
319+
320+
impl<Pk: MiniscriptKey, Ctx: ScriptContext> Ord for Terminal<Pk, Ctx> {
321+
fn cmp(&self, other: &Self) -> cmp::Ordering {
322+
// First try matching directly on the fragment name to avoid the
323+
// complexity of building an iterator.
324+
match self.fragment_name().cmp(other.fragment_name()) {
325+
cmp::Ordering::Less => cmp::Ordering::Less,
326+
cmp::Ordering::Greater => cmp::Ordering::Greater,
327+
cmp::Ordering::Equal => {
328+
// But if they are equal then we need to iterate
329+
for (me, you) in DisplayNode::Node(Type::FALSE, self)
330+
.pre_order_iter()
331+
.zip(DisplayNode::Node(Type::FALSE, other).pre_order_iter())
332+
{
333+
let me_you_cmp = match (me, you) {
334+
(DisplayNode::Node(_, me), DisplayNode::Node(_, you)) => {
335+
me.fragment_name().cmp(you.fragment_name())
336+
}
337+
(DisplayNode::ThresholdK(me), DisplayNode::ThresholdK(you)) => me.cmp(&you),
338+
(DisplayNode::Key(me), DisplayNode::Key(you)) => me.cmp(you),
339+
(DisplayNode::RawKeyHash(me), DisplayNode::RawKeyHash(you)) => me.cmp(you),
340+
(DisplayNode::After(me), DisplayNode::After(you)) => me.cmp(you),
341+
(DisplayNode::Older(me), DisplayNode::Older(you)) => me.cmp(you),
342+
(DisplayNode::Sha256(me), DisplayNode::Sha256(you)) => me.cmp(you),
343+
(DisplayNode::Hash256(me), DisplayNode::Hash256(you)) => me.cmp(you),
344+
(DisplayNode::Ripemd160(me), DisplayNode::Ripemd160(you)) => me.cmp(you),
345+
(DisplayNode::Hash160(me), DisplayNode::Hash160(you)) => me.cmp(you),
346+
_ => unreachable!(
347+
"if the type of a node differs, its parent must have differed"
348+
),
349+
};
350+
351+
match me_you_cmp {
352+
cmp::Ordering::Less => return cmp::Ordering::Less,
353+
cmp::Ordering::Greater => return cmp::Ordering::Greater,
354+
cmp::Ordering::Equal => {}
355+
}
356+
}
357+
358+
cmp::Ordering::Equal
359+
}
360+
}
361+
}
362+
}

0 commit comments

Comments
 (0)