From 0afd8324dfb1fb0850efc6edfc3a554c4dcddd45 Mon Sep 17 00:00:00 2001 From: Riccardo Casatta Date: Mon, 26 Feb 2024 11:40:36 +0100 Subject: [PATCH] Remove generics from Error by making fragment a String This cause a reduction of IR lines emitted by the compiler of about 2% (using cargo llvm-lines), so this should improve compile times. I've seen using the parse example to check the produced binary size, but it's a small snippet I am not sure it calls a representative portion of the API surface. From a library perspective this is equivalent since the field is used only to be converted to string. In theory externally someone could use the field in the error to do things so it comes at a cost. --- src/lib.rs | 8 +-- src/miniscript/types/extra_props.rs | 20 +++---- src/miniscript/types/mod.rs | 90 +++++++++++++++-------------- src/policy/compiler.rs | 4 +- 4 files changed, 60 insertions(+), 62 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index c4e3a15c9..a06dd62c0 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -631,12 +631,8 @@ impl error::Error for Error { } #[doc(hidden)] -impl From> for Error -where - Pk: MiniscriptKey, - Ctx: ScriptContext, -{ - fn from(e: miniscript::types::Error) -> Error { Error::TypeCheck(e.to_string()) } +impl From for Error { + fn from(e: miniscript::types::Error) -> Error { Error::TypeCheck(e.to_string()) } } #[doc(hidden)] diff --git a/src/miniscript/types/extra_props.rs b/src/miniscript/types/extra_props.rs index 8cbb4aa3c..e6d8c3108 100644 --- a/src/miniscript/types/extra_props.rs +++ b/src/miniscript/types/extra_props.rs @@ -860,7 +860,7 @@ impl Property for ExtData { fn type_check_with_child( _fragment: &Terminal, mut _child: C, - ) -> Result> + ) -> Result where C: FnMut(usize) -> Self, Pk: MiniscriptKey, @@ -871,13 +871,13 @@ impl Property for ExtData { /// Compute the type of a fragment assuming all the children of /// Miniscript have been computed already. - fn type_check(fragment: &Terminal) -> Result> + fn type_check(fragment: &Terminal) -> Result where Ctx: ScriptContext, Pk: MiniscriptKey, { let wrap_err = |result: Result| { - result.map_err(|kind| Error { fragment: fragment.clone(), error: kind }) + result.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) }; let ret = match *fragment { @@ -888,13 +888,13 @@ impl Property for ExtData { Terminal::Multi(k, ref pks) | Terminal::MultiA(k, ref pks) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > pks.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, pks.len()), }); } @@ -910,7 +910,7 @@ impl Property for ExtData { // only consumes 4 bytes from the stack. if t == absolute::LockTime::ZERO.into() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -919,7 +919,7 @@ impl Property for ExtData { Terminal::Older(t) => { if t == Sequence::ZERO || !t.is_relative_lock_time() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -975,20 +975,20 @@ impl Property for ExtData { Terminal::Thresh(k, ref subs) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > subs.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, subs.len()), }); } let res = Self::threshold(k, subs.len(), |n| Ok(subs[n].ext)); - res.map_err(|kind| Error { fragment: fragment.clone(), error: kind }) + res.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) } }; if let Ok(ref ret) = ret { diff --git a/src/miniscript/types/mod.rs b/src/miniscript/types/mod.rs index e95585c9b..7c27c0100 100644 --- a/src/miniscript/types/mod.rs +++ b/src/miniscript/types/mod.rs @@ -8,6 +8,8 @@ pub mod correctness; pub mod extra_props; pub mod malleability; +#[cfg(all(not(feature = "std"), not(test)))] +use alloc::string::{String, ToString}; use core::fmt; #[cfg(feature = "std")] use std::error; @@ -82,91 +84,91 @@ pub enum ErrorKind { /// Error type for typechecking #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] -pub struct Error { +pub struct Error { /// The fragment that failed typecheck - pub fragment: Terminal, + pub fragment_string: String, /// The reason that typechecking failed pub error: ErrorKind, } -impl fmt::Display for Error { +impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match self.error { ErrorKind::InvalidTime => write!( f, "fragment «{}» represents a timelock which value is invalid (time must be in [1; 0x80000000])", - self.fragment, + self.fragment_string, ), ErrorKind::NonZeroDupIf => write!( f, "fragment «{}» represents needs to be `z`, needs to consume zero elements from the stack", - self.fragment, + self.fragment_string, ), ErrorKind::ZeroThreshold => write!( f, "fragment «{}» has a threshold value of 0", - self.fragment, + self.fragment_string, ), ErrorKind::OverThreshold(k, n) => write!( f, "fragment «{}» is a {}-of-{} threshold, which does not make sense", - self.fragment, k, n, + self.fragment_string, k, n, ), ErrorKind::NoStrongChild => write!( f, "fragment «{}» requires at least one strong child \ (a 3rd party cannot create a witness without having \ seen one before) to prevent malleability", - self.fragment, + self.fragment_string, ), ErrorKind::LeftNotDissatisfiable => write!( f, "fragment «{}» requires its left child be dissatisfiable", - self.fragment, + self.fragment_string, ), ErrorKind::RightNotDissatisfiable => write!( f, "fragment «{}» requires its right child be dissatisfiable", - self.fragment, + self.fragment_string, ), ErrorKind::SwapNonOne => write!( f, "fragment «{}» attempts to use `SWAP` to prefix something which does not take exactly one input", - self.fragment, + self.fragment_string, ), ErrorKind::NonZeroZero => write!( f, "fragment «{}» attempts to use use the `j:` wrapper around a fragment which might be satisfied by an input of size zero", - self.fragment, + self.fragment_string, ), ErrorKind::LeftNotUnit => write!( f, "fragment «{}» requires its left child be a unit (outputs exactly 1 given a satisfying input)", - self.fragment, + self.fragment_string, ), ErrorKind::ChildBase1(base) => write!( f, "fragment «{}» cannot wrap a fragment of type {:?}", - self.fragment, base, + self.fragment_string, base, ), ErrorKind::ChildBase2(base1, base2) => write!( f, "fragment «{}» cannot accept children of types {:?} and {:?}", - self.fragment, base1, base2, + self.fragment_string, base1, base2, ), ErrorKind::ChildBase3(base1, base2, base3) => write!( f, "fragment «{}» cannot accept children of types {:?}, {:?} and {:?}", - self.fragment, base1, base2, base3, + self.fragment_string, base1, base2, base3, ), ErrorKind::ThresholdBase(idx, base) => write!( f, "fragment «{}» sub-fragment {} has type {:?} rather than {:?}", - self.fragment, + self.fragment_string, idx, base, if idx == 0 { Base::B } else { Base::W }, @@ -175,20 +177,20 @@ impl fmt::Display for Error { f, "fragment «{}» sub-fragment {} can not be dissatisfied \ and cannot be used in a threshold", - self.fragment, idx, + self.fragment_string, idx, ), ErrorKind::ThresholdNonUnit(idx) => write!( f, "fragment «{}» sub-fragment {} is not a unit (does not put \ exactly 1 on the stack given a satisfying input)", - self.fragment, idx, + self.fragment_string, idx, ), ErrorKind::ThresholdNotStrong { k, n, n_strong } => write!( f, "fragment «{}» is a {}-of-{} threshold, and needs {} of \ its children to be strong to prevent malleability; however \ only {} children were strong.", - self.fragment, + self.fragment_string, k, n, n - k, @@ -199,7 +201,7 @@ impl fmt::Display for Error { } #[cfg(feature = "std")] -impl error::Error for Error { +impl error::Error for Error { fn cause(&self) -> Option<&dyn error::Error> { None } } @@ -351,14 +353,14 @@ pub trait Property: Sized { fn type_check_common<'a, Pk, Ctx, C>( fragment: &'a Terminal, mut get_child: C, - ) -> Result> + ) -> Result where - C: FnMut(&'a Terminal, usize) -> Result>, + C: FnMut(&'a Terminal, usize) -> Result, Pk: MiniscriptKey, Ctx: ScriptContext, { let wrap_err = |result: Result| { - result.map_err(|kind| Error { fragment: fragment.clone(), error: kind }) + result.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) }; let ret = match *fragment { @@ -369,13 +371,13 @@ pub trait Property: Sized { Terminal::Multi(k, ref pks) | Terminal::MultiA(k, ref pks) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > pks.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, pks.len()), }); } @@ -391,7 +393,7 @@ pub trait Property: Sized { // only consumes 4 bytes from the stack. if t == absolute::LockTime::ZERO.into() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -400,7 +402,7 @@ pub trait Property: Sized { Terminal::Older(t) => { if t == Sequence::ZERO || !t.is_relative_lock_time() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -458,13 +460,13 @@ pub trait Property: Sized { Terminal::Thresh(k, ref subs) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > subs.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, subs.len()), }); } @@ -473,13 +475,13 @@ pub trait Property: Sized { let res = Self::threshold(k, subs.len(), |n| match get_child(&subs[n].node, n) { Ok(x) => Ok(x), Err(e) => { - last_err_frag = Some(e.fragment); + last_err_frag = Some(e.fragment_string); Err(e.error) } }); res.map_err(|kind| Error { - fragment: last_err_frag.unwrap_or_else(|| fragment.clone()), + fragment_string: last_err_frag.unwrap_or_else(|| fragment.to_string()), error: kind, }) } @@ -495,7 +497,7 @@ pub trait Property: Sized { fn type_check_with_child( fragment: &Terminal, mut child: C, - ) -> Result> + ) -> Result where C: FnMut(usize) -> Self, Pk: MiniscriptKey, @@ -506,7 +508,7 @@ pub trait Property: Sized { } /// Compute the type of a fragment. - fn type_check(fragment: &Terminal) -> Result> + fn type_check(fragment: &Terminal) -> Result where Pk: MiniscriptKey, Ctx: ScriptContext, @@ -697,7 +699,7 @@ impl Property for Type { fn type_check_with_child( _fragment: &Terminal, mut _child: C, - ) -> Result> + ) -> Result where C: FnMut(usize) -> Self, Pk: MiniscriptKey, @@ -708,13 +710,13 @@ impl Property for Type { /// Compute the type of a fragment assuming all the children of /// Miniscript have been computed already. - fn type_check(fragment: &Terminal) -> Result> + fn type_check(fragment: &Terminal) -> Result where Pk: MiniscriptKey, Ctx: ScriptContext, { let wrap_err = |result: Result| { - result.map_err(|kind| Error { fragment: fragment.clone(), error: kind }) + result.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) }; let ret = match *fragment { @@ -725,13 +727,13 @@ impl Property for Type { Terminal::Multi(k, ref pks) | Terminal::MultiA(k, ref pks) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > pks.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, pks.len()), }); } @@ -747,7 +749,7 @@ impl Property for Type { // only consumes 4 bytes from the stack. if t == absolute::LockTime::ZERO.into() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -756,7 +758,7 @@ impl Property for Type { Terminal::Older(t) => { if t == Sequence::ZERO || !t.is_relative_lock_time() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::InvalidTime, }); } @@ -812,20 +814,20 @@ impl Property for Type { Terminal::Thresh(k, ref subs) => { if k == 0 { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::ZeroThreshold, }); } if k > subs.len() { return Err(Error { - fragment: fragment.clone(), + fragment_string: fragment.to_string(), error: ErrorKind::OverThreshold(k, subs.len()), }); } let res = Self::threshold(k, subs.len(), |n| Ok(subs[n].ty)); - res.map_err(|kind| Error { fragment: fragment.clone(), error: kind }) + res.map_err(|kind| Error { fragment_string: fragment.to_string(), error: kind }) } }; if let Ok(ref ret) = ret { diff --git a/src/policy/compiler.rs b/src/policy/compiler.rs index fabf27d3c..8aa4cea22 100644 --- a/src/policy/compiler.rs +++ b/src/policy/compiler.rs @@ -450,7 +450,7 @@ impl AstElemExt { ast: Terminal, l: &AstElemExt, r: &AstElemExt, - ) -> Result, types::Error> { + ) -> Result, types::Error> { let lookup_ext = |n| match n { 0 => l.comp_ext_data, 1 => r.comp_ext_data, @@ -472,7 +472,7 @@ impl AstElemExt { a: &AstElemExt, b: &AstElemExt, c: &AstElemExt, - ) -> Result, types::Error> { + ) -> Result, types::Error> { let lookup_ext = |n| match n { 0 => a.comp_ext_data, 1 => b.comp_ext_data,