Skip to content
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

Drop the Property trait entirely #652

Merged
merged 12 commits into from
Mar 6, 2024
Merged

Drop the Property trait entirely #652

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
9280609
blanket_traits: add some more bounds
apoelstra Mar 3, 2024
3c1c896
blanket_traits: remove crate:: in a bunch of places
apoelstra Mar 5, 2024
9620380
clippy: fix a couple nits
apoelstra Mar 6, 2024
dd0978d
types: don't implement Property for Correctness
apoelstra Mar 6, 2024
87294ff
types: don't implement Property for Malleability
apoelstra Mar 6, 2024
b6c3ca8
types: don't implement Property for ExtData.
apoelstra Mar 6, 2024
602fd29
types: remove all Result returns for ExtData
apoelstra Mar 6, 2024
eb854aa
compiler: stop using Property trait in CompilerExtData
apoelstra Mar 6, 2024
b25023d
compiler: remove a bunch of unused error paths
apoelstra Mar 6, 2024
fdea7f6
types: drop `Property` trait entirely
apoelstra Mar 6, 2024
d4dc226
types: make a ton of constructors const
apoelstra Mar 6, 2024
e971e77
types: remove error path from sub-fragment parameter to threshold()
apoelstra Mar 6, 2024
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
108 changes: 69 additions & 39 deletions src/miniscript/types/correctness.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@

//! Correctness/Soundness type properties

use super::{ErrorKind, Property};
use crate::ScriptContext;
use super::ErrorKind;

/// Basic type representing where the fragment can go
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)]
Expand Down Expand Up @@ -47,12 +46,24 @@ pub enum Input {
}

impl Input {
// FIXME rustc should eventually support derived == on enums in constfns
const fn constfn_eq(self, other: Self) -> bool {
matches!(
(self, other),
(Input::Zero, Input::Zero)
| (Input::One, Input::One)
| (Input::Any, Input::Any)
| (Input::OneNonZero, Input::OneNonZero)
| (Input::AnyNonZero, Input::AnyNonZero)
)
}

/// Check whether given `Input` is a subtype of `other`. That is,
/// if some Input is `OneNonZero` then it must be `One`, hence `OneNonZero` is
/// a subtype if `One`. Returns `true` for `a.is_subtype(a)`.
fn is_subtype(&self, other: Self) -> bool {
const fn is_subtype(&self, other: Self) -> bool {
match (*self, other) {
(x, y) if x == y => true,
(x, y) if x.constfn_eq(y) => true,
(Input::OneNonZero, Input::One)
| (Input::OneNonZero, Input::AnyNonZero)
| (_, Input::Any) => true,
Expand Down Expand Up @@ -95,16 +106,15 @@ impl Correctness {
/// This checks whether the argument `other` has attributes which are present
/// in the given `Type`. This returns `true` on same arguments
/// `a.is_subtype(a)` is `true`.
pub fn is_subtype(&self, other: Self) -> bool {
self.base == other.base
pub const fn is_subtype(&self, other: Self) -> bool {
self.base as u8 == other.base as u8
&& self.input.is_subtype(other.input)
&& self.dissatisfiable >= other.dissatisfiable
&& self.unit >= other.unit
}
}

impl Property for Correctness {
fn sanity_checks(&self) {
/// Confirm invariants of the correctness checker.
pub fn sanity_checks(&self) {
Copy link
Member

Choose a reason for hiding this comment

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

Aside: I feel we should not make this pub. rust-miniscript should guarantee that these hold. We should check these internally for our soundness checks.

I saw in few downstream crates where they called ms.sanity_check() frequently. This gives me C++ vibes where users call isValid before calling functions on struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good! I'll do this in a followup.

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually I'll just file an issue. There are a lot of sanity_check methods in the code, many of which are not called from within the library, and at least one has a doc example suggesting to use it to check "whether all spend paths are accessible in the Bitcoin network".

So we need to re-assess all these functions and see if they can be pulled into the type system somehow so you simply can't create the objects without running the (non-pub) methods.

match self.base {
Base::B => {}
Base::K => {
Expand All @@ -115,21 +125,19 @@ impl Property for Correctness {
debug_assert!(!self.dissatisfiable);
}
Base::W => {
debug_assert!(self.input != Input::OneNonZero);
debug_assert!(self.input != Input::AnyNonZero);
debug_assert!(!self.input.constfn_eq(Input::OneNonZero));
debug_assert!(!self.input.constfn_eq(Input::AnyNonZero));
}
}
}

fn from_true() -> Self { Self::TRUE }

fn from_false() -> Self { Self::FALSE }

fn from_pk_k<Ctx: ScriptContext>() -> Self {
/// Constructor for the correctness properties of the `pk_k` fragment.
pub const fn pk_k() -> Self {
Correctness { base: Base::K, input: Input::OneNonZero, dissatisfiable: true, unit: true }
}

fn from_pk_h<Ctx: ScriptContext>() -> Self {
/// Constructor for the correctness properties of the `pk_h` fragment.
pub const fn pk_h() -> Self {
Correctness {
base: Base::K,
input: Input::AnyNonZero,
Expand All @@ -138,23 +146,28 @@ impl Property for Correctness {
}
}

fn from_multi(_: usize, _: usize) -> Self {
/// Constructor for the correctness properties of the `multi` fragment.
pub const fn multi() -> Self {
Correctness { base: Base::B, input: Input::AnyNonZero, dissatisfiable: true, unit: true }
}

fn from_multi_a(_: usize, _: usize) -> Self {
/// Constructor for the correctness properties of the `multi_a` fragment.
pub const fn multi_a() -> Self {
Correctness { base: Base::B, input: Input::Any, dissatisfiable: true, unit: true }
}

fn from_hash() -> Self {
/// Constructor for the correctness properties of any of the hash fragments.
pub const fn hash() -> Self {
Correctness { base: Base::B, input: Input::OneNonZero, dissatisfiable: true, unit: true }
}

fn from_time(_: u32) -> Self {
/// Constructor for the correctness properties of either of the time fragments.
pub const fn time() -> Self {
Correctness { base: Base::B, input: Input::Zero, dissatisfiable: false, unit: false }
}

fn cast_alt(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `a:` fragment.
pub const fn cast_alt(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::B => Base::W,
Expand All @@ -166,7 +179,8 @@ impl Property for Correctness {
})
}

fn cast_swap(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `s:` fragment.
pub const fn cast_swap(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::B => Base::W,
Expand All @@ -181,7 +195,8 @@ impl Property for Correctness {
})
}

fn cast_check(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `c:` fragment.
pub const fn cast_check(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::K => Base::B,
Expand All @@ -193,7 +208,8 @@ impl Property for Correctness {
})
}

fn cast_dupif(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `d:` fragment.
pub const fn cast_dupif(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::V => Base::B,
Expand All @@ -208,7 +224,8 @@ impl Property for Correctness {
})
}

fn cast_verify(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `v:` fragment.
pub const fn cast_verify(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::B => Base::V,
Expand All @@ -220,8 +237,9 @@ impl Property for Correctness {
})
}

fn cast_nonzero(self) -> Result<Self, ErrorKind> {
if self.input != Input::OneNonZero && self.input != Input::AnyNonZero {
/// Constructor for the correctness properties of the `j:` fragment.
pub const fn cast_nonzero(self) -> Result<Self, ErrorKind> {
if !self.input.constfn_eq(Input::OneNonZero) && !self.input.constfn_eq(Input::AnyNonZero) {
return Err(ErrorKind::NonZeroZero);
}
Ok(Correctness {
Expand All @@ -235,7 +253,8 @@ impl Property for Correctness {
})
}

fn cast_zeronotequal(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `n:` fragment.
pub const fn cast_zeronotequal(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::B => Base::B,
Expand All @@ -247,7 +266,8 @@ impl Property for Correctness {
})
}

fn cast_true(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `t:` fragment.
pub const fn cast_true(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::V => Base::B,
Expand All @@ -259,7 +279,8 @@ impl Property for Correctness {
})
}

fn cast_or_i_false(self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `l:` and `u:` fragments.
pub const fn cast_or_i_false(self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match self.base {
Base::B => Base::B,
Expand All @@ -276,7 +297,8 @@ impl Property for Correctness {
})
}

fn and_b(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `and_b` fragment
pub const fn and_b(left: Self, right: Self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match (left.base, right.base) {
(Base::B, Base::W) => Base::B,
Expand All @@ -298,7 +320,8 @@ impl Property for Correctness {
})
}

fn and_v(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `and_v` fragment
pub const fn and_v(left: Self, right: Self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match (left.base, right.base) {
(Base::V, Base::B) => Base::B,
Expand All @@ -322,7 +345,8 @@ impl Property for Correctness {
})
}

fn or_b(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `or_b` fragment
pub const fn or_b(left: Self, right: Self) -> Result<Self, ErrorKind> {
if !left.dissatisfiable {
return Err(ErrorKind::LeftNotDissatisfiable);
}
Expand All @@ -347,7 +371,8 @@ impl Property for Correctness {
})
}

fn or_d(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `or_d` fragment
pub const fn or_d(left: Self, right: Self) -> Result<Self, ErrorKind> {
if !left.dissatisfiable {
return Err(ErrorKind::LeftNotDissatisfiable);
}
Expand All @@ -369,7 +394,8 @@ impl Property for Correctness {
})
}

fn or_c(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `or_c` fragment
pub const fn or_c(left: Self, right: Self) -> Result<Self, ErrorKind> {
if !left.dissatisfiable {
return Err(ErrorKind::LeftNotDissatisfiable);
}
Expand All @@ -391,7 +417,8 @@ impl Property for Correctness {
})
}

fn or_i(left: Self, right: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `or_i` fragment
pub const fn or_i(left: Self, right: Self) -> Result<Self, ErrorKind> {
Ok(Correctness {
base: match (left.base, right.base) {
(Base::B, Base::B) => Base::B,
Expand All @@ -408,7 +435,8 @@ impl Property for Correctness {
})
}

fn and_or(a: Self, b: Self, c: Self) -> Result<Self, ErrorKind> {
/// Constructor for the correctness properties of the `andor` fragment
pub const fn and_or(a: Self, b: Self, c: Self) -> Result<Self, ErrorKind> {
if !a.dissatisfiable {
return Err(ErrorKind::LeftNotDissatisfiable);
}
Expand Down Expand Up @@ -437,7 +465,9 @@ impl Property for Correctness {
})
}

fn threshold<S>(_k: usize, n: usize, mut sub_ck: S) -> Result<Self, ErrorKind>
/// Constructor for the correctness properties of the `thresh` fragment
// Cannot be constfn because it takes a closure.
pub fn threshold<S>(_k: usize, n: usize, mut sub_ck: S) -> Result<Self, ErrorKind>
where
S: FnMut(usize) -> Result<Self, ErrorKind>,
{
Expand Down
Loading