From 29a4f9b114f65de72833987a2a36d44a8dac6671 Mon Sep 17 00:00:00 2001 From: "Tobin C. Harding" Date: Mon, 7 Aug 2023 08:39:59 +1000 Subject: [PATCH] Wrap the bitcoinconsensus error Currently the `bitcoinconsensus` error is part of the public API. This hinders maintainability because changes to the verison of `bitcoinconsensus` force a re-release in `rust-bitcoin`. This is an unnecessary maintenance burden, we can wrap the error instead. --- bitcoin/src/consensus/validation.rs | 91 +++++++++++++---------------- 1 file changed, 40 insertions(+), 51 deletions(-) diff --git a/bitcoin/src/consensus/validation.rs b/bitcoin/src/consensus/validation.rs index 523bcacf..7667cc9d 100644 --- a/bitcoin/src/consensus/validation.rs +++ b/bitcoin/src/consensus/validation.rs @@ -31,7 +31,7 @@ pub fn verify_script( index: usize, amount: Amount, spending_tx: &[u8], -) -> Result<(), bitcoinconsensus::Error> { +) -> Result<(), BitcoinconsensusError> { verify_script_with_flags(script, index, amount, spending_tx, bitcoinconsensus::VERIFY_ALL) } @@ -50,14 +50,14 @@ pub fn verify_script_with_flags>( amount: Amount, spending_tx: &[u8], flags: F, -) -> Result<(), bitcoinconsensus::Error> { - bitcoinconsensus::verify_with_flags( +) -> Result<(), BitcoinconsensusError> { + Ok(bitcoinconsensus::verify_with_flags( script.as_bytes(), amount.to_sat(), spending_tx, index, flags.into(), - ) + )?) } /// Verifies that this transaction is able to spend its inputs. @@ -121,7 +121,7 @@ impl Script { index: usize, amount: crate::Amount, spending_tx: &[u8], - ) -> Result<(), bitcoinconsensus::Error> { + ) -> Result<(), BitcoinconsensusError> { verify_script(self, index, amount, spending_tx) } @@ -140,7 +140,7 @@ impl Script { amount: crate::Amount, spending_tx: &[u8], flags: F, - ) -> Result<(), bitcoinconsensus::Error> { + ) -> Result<(), BitcoinconsensusError> { verify_script_with_flags(self, index, amount, spending_tx, flags) } } @@ -172,11 +172,40 @@ impl Transaction { } } +/// Wrapped error from `bitcoinconsensus`. +// We do this for two reasons: +// 1. We don't want the error to be part of the public API because we do not want to expose the +// unusual versioning used in `bitcoinconsensus` to users of `rust-bitcoin`. +// 2. We want to implement `std::error::Error` if the "std" feature is enabled in `rust-bitcoin` but +// not in `bitcoinconsensus`. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct BitcoinconsensusError(bitcoinconsensus::Error); + +impl fmt::Display for BitcoinconsensusError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write_err!(f, "bitcoinconsensus error"; &self.0) + } +} + +#[cfg(all(feature = "std", feature = "bitcoinconsensus-std"))] +impl std::error::Error for BitcoinconsensusError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) } +} + +#[cfg(all(feature = "std", not(feature = "bitcoinconsensus-std")))] +impl std::error::Error for BitcoinconsensusError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } +} + +impl From for BitcoinconsensusError { + fn from(e: bitcoinconsensus::Error) -> Self { Self(e) } +} + /// An error during transaction validation. #[derive(Debug, Clone, PartialEq, Eq)] pub enum TxVerifyError { /// Error validating the script with bitcoinconsensus library. - ScriptVerification(bitcoinconsensus::Error), + ScriptVerification(BitcoinconsensusError), /// Can not find the spent output. UnknownSpentOutput(OutPoint), } @@ -186,9 +215,7 @@ impl fmt::Display for TxVerifyError { use TxVerifyError::*; match *self { - ScriptVerification(ref e) => { - write_err!(f, "bitcoinconsensus verification failed"; bitcoinconsensus_hack::wrap_error(e)) - } + ScriptVerification(ref e) => write_err!(f, "bitcoinconsensus verification failed"; e), UnknownSpentOutput(ref p) => write!(f, "unknown spent output: {}", p), } } @@ -200,50 +227,12 @@ impl std::error::Error for TxVerifyError { use TxVerifyError::*; match *self { - ScriptVerification(ref e) => Some(bitcoinconsensus_hack::wrap_error(e)), + ScriptVerification(ref e) => Some(e), UnknownSpentOutput(_) => None, } } } -impl From for TxVerifyError { - fn from(e: bitcoinconsensus::Error) -> Self { TxVerifyError::ScriptVerification(e) } -} - -// If bitcoinonsensus-std is off but bitcoinconsensus is present we patch the error type to -// implement `std::error::Error`. -#[cfg(all(feature = "std", feature = "bitcoinconsensus", not(feature = "bitcoinconsensus-std")))] -mod bitcoinconsensus_hack { - use core::fmt; - - #[repr(transparent)] - pub(crate) struct Error(bitcoinconsensus::Error); - - impl fmt::Debug for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Debug::fmt(&self.0, f) } - } - - impl fmt::Display for Error { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { fmt::Display::fmt(&self.0, f) } - } - - // bitcoinconsensus::Error has no sources at this time - impl std::error::Error for Error {} - - pub(crate) fn wrap_error(error: &bitcoinconsensus::Error) -> &Error { - // Unfortunately, we cannot have the reference inside `Error` struct because of the 'static - // bound on `source` return type, so we have to use unsafe to overcome the limitation. - // SAFETY: the type is repr(transparent) and the lifetimes match - unsafe { &*(error as *const _ as *const Error) } - } -} - -#[cfg(not(all( - feature = "std", - feature = "bitcoinconsensus", - not(feature = "bitcoinconsensus-std") -)))] -mod bitcoinconsensus_hack { - #[allow(unused_imports)] // conditionally used - pub(crate) use core::convert::identity as wrap_error; +impl From for TxVerifyError { + fn from(e: BitcoinconsensusError) -> Self { TxVerifyError::ScriptVerification(e) } }