Skip to content

[backport] cleanup: refactor a bunch of concatenation-related code in satisfy #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Apr 1, 2025
Merged

[backport] cleanup: refactor a bunch of concatenation-related code in satisfy #803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8efce62
ci: fix new clippy lint related to doccomments
apoelstra Oct 10, 2024
7be767c
satisfy: clean up a bunch of satisfaction code
apoelstra Mar 26, 2025
9d81dcf
bump version to 12.3.1
apoelstra Mar 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo-minimal.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,7 @@ dependencies = [

[[package]]
name = "miniscript"
version = "12.3.0"
version = "12.3.1"
dependencies = [
"bech32",
"bitcoin",
Expand Down
2 changes: 1 addition & 1 deletion Cargo-recent.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ dependencies = [

[[package]]
name = "miniscript"
version = "12.3.0"
version = "12.3.1"
dependencies = [
"bech32",
"bitcoin",
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "miniscript"
version = "12.3.0"
version = "12.3.1"
authors = ["Andrew Poelstra <[email protected]>, Sanket Kanjalkar <[email protected]>"]
license = "CC0-1.0"
homepage = "https://github.com/rust-bitcoin/rust-miniscript/"
Expand Down
8 changes: 6 additions & 2 deletions src/miniscript/iter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,22 +199,24 @@ impl<'a, Pk: MiniscriptKey, Ctx: ScriptContext> Iterator for PkIter<'a, Pk, Ctx>
}
}

// Module is public since it export testcase generation which may be used in
// dependent libraries for their own tasts based on Miniscript AST
/// Module is public since it export testcase generation which may be used in
/// dependent libraries for their own tasts based on Miniscript AST
#[cfg(test)]
pub mod test {
use bitcoin::hashes::{hash160, ripemd160, sha256, sha256d, Hash};

use super::Miniscript;
use crate::miniscript::context::Segwitv0;

/// Test case.
pub type TestData = (
Miniscript<bitcoin::PublicKey, Segwitv0>,
Vec<bitcoin::PublicKey>,
Vec<hash160::Hash>,
bool, // Indicates that the top-level contains public key or hashes
);

/// Generate a deterministic list of public keys of the given length.
pub fn gen_secp_pubkeys(n: usize) -> Vec<secp256k1::PublicKey> {
let mut ret = Vec::with_capacity(n);
let secp = secp256k1::Secp256k1::new();
Expand All @@ -233,13 +235,15 @@ pub mod test {
ret
}

/// Generate a deterministic list of Bitcoin public keys of the given length.
pub fn gen_bitcoin_pubkeys(n: usize, compressed: bool) -> Vec<bitcoin::PublicKey> {
gen_secp_pubkeys(n)
.into_iter()
.map(|inner| bitcoin::PublicKey { inner, compressed })
.collect()
}

/// Generate a deterministic list of test cases of the given length.
pub fn gen_testcases() -> Vec<TestData> {
let k = gen_bitcoin_pubkeys(10, true);
let _h: Vec<hash160::Hash> = k
Expand Down
168 changes: 67 additions & 101 deletions src/miniscript/satisfy.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -889,6 +889,34 @@ pub struct Satisfaction<T> {
}

impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
/// The empty satisfaction.
///
/// This has the property that, when concatenated on either side with another satisfaction
/// X, the result will be X.
fn empty() -> Self {
Satisfaction {
has_sig: false,
relative_timelock: None,
absolute_timelock: None,
stack: Witness::Stack(vec![]),
}
}

/// Forms a satisfaction which is the concatenation of two satisfactions, with `other`'s
/// stack before `self`'s.
///
/// This order allows callers to write `left.concatenate_rev(right)` which feels more
/// natural than the opposite order, and more importantly, allows this method to be
/// used when folding over an iterator of multiple satisfactions.
fn concatenate_rev(self, other: Self) -> Self {
Satisfaction {
has_sig: self.has_sig || other.has_sig,
relative_timelock: cmp::max(self.relative_timelock, other.relative_timelock),
absolute_timelock: cmp::max(self.absolute_timelock, other.absolute_timelock),
stack: Witness::combine(other.stack, self.stack),
}
}

pub(crate) fn build_template<P, Ctx>(
term: &Terminal<Pk, Ctx>,
provider: &P,
Expand Down Expand Up @@ -1041,20 +1069,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
}
} else {
// Otherwise flatten everything out
Satisfaction {
has_sig: ret_stack.iter().any(|sat| sat.has_sig),
relative_timelock: ret_stack
.iter()
.filter_map(|sat| sat.relative_timelock)
.max(),
absolute_timelock: ret_stack
.iter()
.filter_map(|sat| sat.absolute_timelock)
.max(),
stack: ret_stack
.into_iter()
.fold(Witness::empty(), |acc, next| Witness::combine(next.stack, acc)),
}
ret_stack
.into_iter()
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
}
}

Expand Down Expand Up @@ -1125,20 +1142,9 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {

// combine the witness
// no non-malleability checks needed
Satisfaction {
has_sig: ret_stack.iter().any(|sat| sat.has_sig),
relative_timelock: ret_stack
.iter()
.filter_map(|sat| sat.relative_timelock)
.max(),
absolute_timelock: ret_stack
.iter()
.filter_map(|sat| sat.absolute_timelock)
.max(),
stack: ret_stack
.into_iter()
.fold(Witness::empty(), |acc, next| Witness::combine(next.stack, acc)),
}
ret_stack
.into_iter()
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
}

fn minimum(sat1: Self, sat2: Self) -> Self {
Expand Down Expand Up @@ -1360,12 +1366,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
Self::satisfy_helper(&l.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);
let r_sat =
Self::satisfy_helper(&r.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);
Satisfaction {
stack: Witness::combine(r_sat.stack, l_sat.stack),
has_sig: l_sat.has_sig || r_sat.has_sig,
relative_timelock: cmp::max(l_sat.relative_timelock, r_sat.relative_timelock),
absolute_timelock: cmp::max(l_sat.absolute_timelock, r_sat.absolute_timelock),
}
l_sat.concatenate_rev(r_sat)
}
Terminal::AndOr(ref a, ref b, ref c) => {
let a_sat =
Expand All @@ -1383,27 +1384,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
let c_sat =
Self::satisfy_helper(&c.node, stfr, root_has_sig, leaf_hash, min_fn, thresh_fn);

min_fn(
Satisfaction {
stack: Witness::combine(b_sat.stack, a_sat.stack),
has_sig: a_sat.has_sig || b_sat.has_sig,
relative_timelock: cmp::max(
a_sat.relative_timelock,
b_sat.relative_timelock,
),
absolute_timelock: cmp::max(
a_sat.absolute_timelock,
b_sat.absolute_timelock,
),
},
Satisfaction {
stack: Witness::combine(c_sat.stack, a_nsat.stack),
has_sig: a_nsat.has_sig || c_sat.has_sig,
// timelocks can't be dissatisfied, so here we ignore a_nsat and only consider c_sat
relative_timelock: c_sat.relative_timelock,
absolute_timelock: c_sat.absolute_timelock,
},
)
min_fn(a_sat.concatenate_rev(b_sat), a_nsat.concatenate_rev(c_sat))
}
Terminal::OrB(ref l, ref r) => {
let l_sat =
Expand Down Expand Up @@ -1431,18 +1412,8 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
assert!(!r_nsat.has_sig);

min_fn(
Satisfaction {
stack: Witness::combine(r_sat.stack, l_nsat.stack),
has_sig: r_sat.has_sig,
relative_timelock: r_sat.relative_timelock,
absolute_timelock: r_sat.absolute_timelock,
},
Satisfaction {
stack: Witness::combine(r_nsat.stack, l_sat.stack),
has_sig: l_sat.has_sig,
relative_timelock: l_sat.relative_timelock,
absolute_timelock: l_sat.absolute_timelock,
},
Satisfaction::concatenate_rev(l_nsat, r_sat),
Satisfaction::concatenate_rev(l_sat, r_nsat),
)
}
Terminal::OrD(ref l, ref r) | Terminal::OrC(ref l, ref r) => {
Expand All @@ -1461,15 +1432,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {

assert!(!l_nsat.has_sig);

min_fn(
l_sat,
Satisfaction {
stack: Witness::combine(r_sat.stack, l_nsat.stack),
has_sig: r_sat.has_sig,
relative_timelock: r_sat.relative_timelock,
absolute_timelock: r_sat.absolute_timelock,
},
)
min_fn(l_sat, Satisfaction::concatenate_rev(l_nsat, r_sat))
}
Terminal::OrI(ref l, ref r) => {
let l_sat =
Expand All @@ -1492,7 +1455,24 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
)
}
Terminal::Thresh(ref thresh) => {
thresh_fn(thresh, stfr, root_has_sig, leaf_hash, min_fn)
if thresh.k() == thresh.n() {
// this is just an and
thresh
.iter()
.map(|s| {
Self::satisfy_helper(
&s.node,
stfr,
root_has_sig,
leaf_hash,
min_fn,
thresh_fn,
)
})
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev)
} else {
thresh_fn(thresh, stfr, root_has_sig, leaf_hash, min_fn)
}
}
Terminal::Multi(ref thresh) => {
// Collect all available signatures
Expand Down Expand Up @@ -1682,12 +1662,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
min_fn,
thresh_fn,
);
Satisfaction {
stack: Witness::combine(odissat.stack, vsat.stack),
has_sig: vsat.has_sig || odissat.has_sig,
relative_timelock: None,
absolute_timelock: None,
}
vsat.concatenate_rev(odissat)
}
Terminal::AndB(ref l, ref r)
| Terminal::OrB(ref l, ref r)
Expand All @@ -1709,12 +1684,7 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
min_fn,
thresh_fn,
);
Satisfaction {
stack: Witness::combine(rnsat.stack, lnsat.stack),
has_sig: rnsat.has_sig || lnsat.has_sig,
relative_timelock: None,
absolute_timelock: None,
}
lnsat.concatenate_rev(rnsat)
}
Terminal::OrI(ref l, ref r) => {
let lnsat = Self::dissatisfy_helper(
Expand Down Expand Up @@ -1750,23 +1720,19 @@ impl<Pk: MiniscriptKey + ToPublicKey> Satisfaction<Placeholder<Pk>> {
// Dissatisfactions don't need to non-malleable. Use minimum_mall always
Satisfaction::minimum_mall(dissat_1, dissat_2)
}
Terminal::Thresh(ref thresh) => Satisfaction {
stack: thresh.iter().fold(Witness::empty(), |acc, sub| {
let nsat = Self::dissatisfy_helper(
&sub.node,
Terminal::Thresh(ref thresh) => thresh
.iter()
.map(|s| {
Self::dissatisfy_helper(
&s.node,
stfr,
root_has_sig,
leaf_hash,
min_fn,
thresh_fn,
);
assert!(!nsat.has_sig);
Witness::combine(nsat.stack, acc)
}),
has_sig: false,
relative_timelock: None,
absolute_timelock: None,
},
)
})
.fold(Satisfaction::empty(), Satisfaction::concatenate_rev),
Terminal::Multi(ref thresh) => Satisfaction {
stack: Witness::Stack(vec![Placeholder::PushZero; thresh.k() + 1]),
has_sig: false,
Expand Down
Loading