Skip to content

Commit 43cad38

Browse files
apoelstraapoelstra
committed
satisfy: clean up a bunch of satisfaction code
There is a bunch of essentially-repeated "combine two satisfactions" code throughout satisfy.rs. It's hard to tell at a glance what order the concatenantions are in, even though this is essential, or whether the combination is actually done correctly. (In particular, we routinely use the opposite order for Witness::combine and for ||'ing the has_sig, and we routinely assume that timelock conditions are None for dissatisfactions, which is true, but it's still a lot to keep in your head.)
1 parent 9c9a2f5 commit 43cad38

File tree

1 file changed

+67
-101
lines changed

1 file changed

+67
-101
lines changed

Diff for: ‎src/miniscript/satisfy.rs

+67-101
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,34 @@ pub struct Satisfaction<T> {
892892
}
893893

894894
impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
895+
/// The empty satisfaction.
896+
///
897+
/// This has the property that, when concatenated on either side with another satisfaction
898+
/// X, the result will be X.
899+
fn empty() -> Self {
900+
Satisfaction {
901+
has_sig: false,
902+
relative_timelock: None,
903+
absolute_timelock: None,
904+
stack: Witness::Stack(vec![]),
905+
}
906+
}
907+
908+
/// Forms a satisfaction which is the concatenation of two satisfactions, with `other`'s
909+
/// stack before `self`'s.
910+
///
911+
/// This order allows callers to write `left.concatenate_rev(right)` which feels more
912+
/// natural than the opposite order, and more importantly, allows this method to be
913+
/// used when folding over an iterator of multiple satisfactions.
914+
fn concatenate_rev(self, other: Self) -> Self {
915+
Satisfaction {
916+
has_sig: self.has_sig || other.has_sig,
917+
relative_timelock: cmp::max(self.relative_timelock, other.relative_timelock),
918+
absolute_timelock: cmp::max(self.absolute_timelock, other.absolute_timelock),
919+
stack: Witness::combine(other.stack, self.stack),
920+
}
921+
}
922+
895923
pub(crate) fn build_template<P, Ctx>(
896924
term: &Terminal<Pk, Ctx>,
897925
provider: &P,
@@ -1044,20 +1072,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
10441072
}
10451073
} else {
10461074
// Otherwise flatten everything out
1047-
Satisfaction {
1048-
has_sig: ret_stack.iter().any(|sat| sat.has_sig),
1049-
relative_timelock: ret_stack
1050-
.iter()
1051-
.filter_map(|sat| sat.relative_timelock)
1052-
.max(),
1053-
absolute_timelock: ret_stack
1054-
.iter()
1055-
.filter_map(|sat| sat.absolute_timelock)
1056-
.max(),
1057-
stack: ret_stack
1058-
.into_iter()
1059-
.fold(Witness::empty(), |acc, next| Witness::combine(next.stack, acc)),
1060-
}
1075+
ret_stack
1076+
.into_iter()
1077+
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
10611078
}
10621079
}
10631080

@@ -1128,20 +1145,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
11281145

11291146
// combine the witness
11301147
// no non-malleability checks needed
1131-
Satisfaction {
1132-
has_sig: ret_stack.iter().any(|sat| sat.has_sig),
1133-
relative_timelock: ret_stack
1134-
.iter()
1135-
.filter_map(|sat| sat.relative_timelock)
1136-
.max(),
1137-
absolute_timelock: ret_stack
1138-
.iter()
1139-
.filter_map(|sat| sat.absolute_timelock)
1140-
.max(),
1141-
stack: ret_stack
1142-
.into_iter()
1143-
.fold(Witness::empty(), |acc, next| Witness::combine(next.stack, acc)),
1144-
}
1148+
ret_stack
1149+
.into_iter()
1150+
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
11451151
}
11461152

11471153
fn minimum(sat1: Self, sat2: Self) -> Self {
@@ -1363,12 +1369,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
13631369
Self::satisfy_helper(&l.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);
13641370
let r_sat =
13651371
Self::satisfy_helper(&r.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);
1366-
Satisfaction {
1367-
stack: Witness::combine(r_sat.stack, l_sat.stack),
1368-
has_sig: l_sat.has_sig || r_sat.has_sig,
1369-
relative_timelock: cmp::max(l_sat.relative_timelock, r_sat.relative_timelock),
1370-
absolute_timelock: cmp::max(l_sat.absolute_timelock, r_sat.absolute_timelock),
1371-
}
1372+
l_sat.concatenate_rev(r_sat)
13721373
}
13731374
Terminal::AndOr(ref a, ref b, ref c) => {
13741375
let a_sat =
@@ -1386,27 +1387,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
13861387
let c_sat =
13871388
Self::satisfy_helper(&c.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);
13881389

1389-
min_fn(
1390-
Satisfaction {
1391-
stack: Witness::combine(b_sat.stack, a_sat.stack),
1392-
has_sig: a_sat.has_sig || b_sat.has_sig,
1393-
relative_timelock: cmp::max(
1394-
a_sat.relative_timelock,
1395-
b_sat.relative_timelock,
1396-
),
1397-
absolute_timelock: cmp::max(
1398-
a_sat.absolute_timelock,
1399-
b_sat.absolute_timelock,
1400-
),
1401-
},
1402-
Satisfaction {
1403-
stack: Witness::combine(c_sat.stack, a_nsat.stack),
1404-
has_sig: a_nsat.has_sig || c_sat.has_sig,
1405-
// timelocks can't be dissatisfied, so here we ignore a_nsat and only consider c_sat
1406-
relative_timelock: c_sat.relative_timelock,
1407-
absolute_timelock: c_sat.absolute_timelock,
1408-
},
1409-
)
1390+
min_fn(a_sat.concatenate_rev(b_sat), a_nsat.concatenate_rev(c_sat))
14101391
}
14111392
Terminal::OrB(ref l, ref r) => {
14121393
let l_sat =
@@ -1434,18 +1415,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
14341415
assert!(!r_nsat.has_sig);
14351416

14361417
min_fn(
1437-
Satisfaction {
1438-
stack: Witness::combine(r_sat.stack, l_nsat.stack),
1439-
has_sig: r_sat.has_sig,
1440-
relative_timelock: r_sat.relative_timelock,
1441-
absolute_timelock: r_sat.absolute_timelock,
1442-
},
1443-
Satisfaction {
1444-
stack: Witness::combine(r_nsat.stack, l_sat.stack),
1445-
has_sig: l_sat.has_sig,
1446-
relative_timelock: l_sat.relative_timelock,
1447-
absolute_timelock: l_sat.absolute_timelock,
1448-
},
1418+
Satisfaction::concatenate_rev(l_nsat, r_sat),
1419+
Satisfaction::concatenate_rev(l_sat, r_nsat),
14491420
)
14501421
}
14511422
Terminal::OrD(ref l, ref r) | Terminal::OrC(ref l, ref r) => {
@@ -1464,15 +1435,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
14641435

14651436
assert!(!l_nsat.has_sig);
14661437

1467-
min_fn(
1468-
l_sat,
1469-
Satisfaction {
1470-
stack: Witness::combine(r_sat.stack, l_nsat.stack),
1471-
has_sig: r_sat.has_sig,
1472-
relative_timelock: r_sat.relative_timelock,
1473-
absolute_timelock: r_sat.absolute_timelock,
1474-
},
1475-
)
1438+
min_fn(l_sat, Satisfaction::concatenate_rev(l_nsat, r_sat))
14761439
}
14771440
Terminal::OrI(ref l, ref r) => {
14781441
let l_sat =
@@ -1495,7 +1458,24 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
14951458
)
14961459
}
14971460
Terminal::Thresh(ref thresh) => {
1498-
thresh_fn(thresh, stfr, root_has_sig, leaf_hash, min_fn)
1461+
if thresh.k() == thresh.n() {
1462+
// this is just an and
1463+
thresh
1464+
.iter()
1465+
.map(|s| {
1466+
Self::satisfy_helper(
1467+
&s.node,
1468+
stfr,
1469+
root_has_sig,
1470+
leaf_hash,
1471+
min_fn,
1472+
thresh_fn,
1473+
)
1474+
})
1475+
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
1476+
} else {
1477+
thresh_fn(thresh, stfr, root_has_sig, leaf_hash, min_fn)
1478+
}
14991479
}
15001480
Terminal::Multi(ref thresh) => {
15011481
// Collect all available signatures
@@ -1685,12 +1665,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
16851665
min_fn,
16861666
thresh_fn,
16871667
);
1688-
Satisfaction {
1689-
stack: Witness::combine(odissat.stack, vsat.stack),
1690-
has_sig: vsat.has_sig || odissat.has_sig,
1691-
relative_timelock: None,
1692-
absolute_timelock: None,
1693-
}
1668+
vsat.concatenate_rev(odissat)
16941669
}
16951670
Terminal::AndB(ref l, ref r)
16961671
| Terminal::OrB(ref l, ref r)
@@ -1712,12 +1687,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
17121687
min_fn,
17131688
thresh_fn,
17141689
);
1715-
Satisfaction {
1716-
stack: Witness::combine(rnsat.stack, lnsat.stack),
1717-
has_sig: rnsat.has_sig || lnsat.has_sig,
1718-
relative_timelock: None,
1719-
absolute_timelock: None,
1720-
}
1690+
lnsat.concatenate_rev(rnsat)
17211691
}
17221692
Terminal::OrI(ref l, ref r) => {
17231693
let lnsat = Self::dissatisfy_helper(
@@ -1753,23 +1723,19 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
17531723
// Dissatisfactions don't need to non-malleable. Use minimum_mall always
17541724
Satisfaction::minimum_mall(dissat_1, dissat_2)
17551725
}
1756-
Terminal::Thresh(ref thresh) => Satisfaction {
1757-
stack: thresh.iter().fold(Witness::empty(), |acc, sub| {
1758-
let nsat = Self::dissatisfy_helper(
1759-
&sub.node,
1726+
Terminal::Thresh(ref thresh) => thresh
1727+
.iter()
1728+
.map(|s| {
1729+
Self::dissatisfy_helper(
1730+
&s.node,
17601731
stfr,
17611732
root_has_sig,
17621733
leaf_hash,
17631734
min_fn,
17641735
thresh_fn,
1765-
);
1766-
assert!(!nsat.has_sig);
1767-
Witness::combine(nsat.stack, acc)
1768-
}),
1769-
has_sig: false,
1770-
relative_timelock: None,
1771-
absolute_timelock: None,
1772-
},
1736+
)
1737+
})
1738+
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev),
17731739
Terminal::Multi(ref thresh) => Satisfaction {
17741740
stack: Witness::Stack(vec![Placeholder::PushZero; thresh.k() + 1]),
17751741
has_sig: false,

0 commit comments

Comments
 (0)