Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
23 changes: 19 additions & 4 deletions src/miniscript/analyzable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,30 @@
//! Tools for determining whether the guarantees offered by the library
//! actually hold.

use crate::miniscript::types::Malleability;
use crate::prelude::*;
use crate::{Miniscript, MiniscriptKey, ScriptContext, Terminal};

impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
/// Whether all spend paths of miniscript require a signature
pub fn requires_sig(&self) -> bool { self.ty.mall.signed }
/// Whether the miniscript is non-malleable and all of its spend paths
/// require a signature.
///
/// Returns `false` for a malleable miniscript. The "s" property only
/// describes the satisfactions of an expression that is non-malleable to
/// begin with, so nothing is known for a malleable one, and `false` is the
/// assumption that keeps a caller from relying on a signature being
/// required.
#[deprecated(
since = "TBD",
note = "check the `signed` property of the malleability type instead"
)]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In 4817382:

This deprecation is in the wrong place and its message makes no sense. We should deprecate the requires_sig method, telling users to use non_malleable_and_requires_sig instead. And we should not deprecate the new method.

Did Claude do this (introducing a new method and immediately deprecating it)?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I might have misunderstood your earlier comment.

The requires_sig method did exist before this PR. And your comment stated: "In addition to updating the docs we should also deprecate this and rename it to non_malleable_and_requires_sig.".
Which is what I did (with Claude, yes).

But it sounds like you might have meant something like "introduce a new, fixed method called non_malleable_and_requires_sig"?

And yes, the deprecation comment definitely isn't my best work, happy to use a better wording if you have a suggestion.

pub fn non_malleable_and_requires_sig(&self) -> bool {
matches!(self.ty.mall, Malleability::NonMalleable { signed: true, .. })
}

/// Whether the miniscript is malleable
pub fn is_non_malleable(&self) -> bool { self.ty.mall.non_malleable }
/// Whether the miniscript is guaranteed to have a non-malleable
/// satisfaction, if it has a satisfaction at all.
pub fn is_non_malleable(&self) -> bool { self.ty.mall.is_non_malleable() }

/// Whether the miniscript can exceed the resource limits(Opcodes, Stack limit etc)
// It maybe possible to return a detail error type containing why the miniscript
Expand Down
14 changes: 7 additions & 7 deletions src/miniscript/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,7 +365,7 @@ mod private {
}

// Sigless branches can be fixed by adding a conjunction with a signature.
if !params.allow_sigless_branch && !self.requires_sig() {
if !params.allow_sigless_branch && !self.non_malleable_and_requires_sig() {
return Err(ValidationError::SiglessBranch);
}

Expand Down Expand Up @@ -636,7 +636,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
let satisfaction = satisfy::Satisfaction::satisfy(
self,
&satisfier,
self.ty.mall.signed,
self.non_malleable_and_requires_sig(),
self.leaf_hash_internal(),
);
self._satisfy(satisfaction)
Expand All @@ -654,7 +654,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
let satisfaction = satisfy::Satisfaction::satisfy_mall(
self,
&satisfier,
self.ty.mall.signed,
self.non_malleable_and_requires_sig(),
self.leaf_hash_internal(),
);
self._satisfy(satisfaction)
Expand Down Expand Up @@ -683,7 +683,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
satisfy::Satisfaction::build_template(
self,
provider,
self.ty.mall.signed,
self.non_malleable_and_requires_sig(),
self.leaf_hash_internal(),
)
}
Expand All @@ -699,7 +699,7 @@ impl<Pk: MiniscriptKey, Ctx: ScriptContext> Miniscript<Pk, Ctx> {
satisfy::Satisfaction::build_template_mall(
self,
provider,
self.ty.mall.signed,
self.non_malleable_and_requires_sig(),
self.leaf_hash_internal(),
)
}
Expand Down Expand Up @@ -1345,8 +1345,8 @@ mod tests {
match (ms, valid) {
(Ok(ms), true) => {
assert_eq!(format!("{:x}", ms.encode()), expected_hex);
assert_eq!(ms.ty.mall.non_malleable, non_mal);
assert_eq!(ms.ty.mall.signed, need_sig);
assert_eq!(ms.is_non_malleable(), non_mal);
assert_eq!(ms.non_malleable_and_requires_sig(), non_mal && need_sig);
assert_eq!(ms.ext.static_ops + ms.ext.sat_data.unwrap().max_exec_op_count, ops);
}
(Err(_), false) => {}
Expand Down
Loading