Fix `no_std` when `bitcoinconsensus` is enabled

`default-features = false` was missing previously but blindly adding it
would lead to subtle risk of breaking when a crate not needing `std`
depends on `bitcoinconsensus` and simultaneously another crate not
needing `bitcoinconsensus` depends on `std` and another crate depends on
them both.

This change fixes it by introducing `bitcoinconsensus-std` feature flag
and provides a fallback if the flag is off. Unfortunately the fallback
has to use a bit of reasonable `unsafe` due to limitations of upcasting.

The only safe alternatives are not do it and provide worse experience
for crates that are affected by the problem above or break the API,
which couldn't be backported and would be more annoying to use.

Closes #1343

bitcoin/Cargo.toml

@@ -19,6 +19,7 @@ rand = ["secp256k1/rand-std"]
 serde = ["actual-serde", "bitcoin_hashes/serde", "secp256k1/serde"]
 secp-lowmemory = ["secp256k1/lowmemory"]
 secp-recovery = ["secp256k1/recovery"]
+bitcoinconsensus-std = ["bitcoinconsensus/std", "std"]
 
 # At least one of std, no-std must be enabled.
 #
@@ -40,7 +41,7 @@ secp256k1 = { version = "0.24.0", default-features = false, features = ["bitcoin
 core2 = { version = "0.3.0", optional = true, default-features = false }
 
 base64 = { version = "0.13.0", optional = true }
-bitcoinconsensus = { version = "0.20.2-0.5.0", optional = true }
+bitcoinconsensus = { version = "0.20.2-0.5.0", optional = true, default-features = false }
 # Do NOT use this as a feature! Use the `serde` feature instead.
 actual-serde = { package = "serde", version = "1.0.103", default-features = false, features = [ "derive", "alloc" ], optional = true }
 hashbrown = { version = "0.8", optional = true }

bitcoin/src/blockdata/script.rs

@@ -170,6 +170,46 @@ pub enum Error {
     Serialization
 }
 
+// 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 fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         match *self {
@@ -177,7 +217,7 @@ impl fmt::Display for Error {
             Error::EarlyEndOfScript => f.write_str("unexpected end of script"),
             Error::NumericOverflow => f.write_str("numeric overflow (number on stack larger than 4 bytes)"),
             #[cfg(feature = "bitcoinconsensus")]
-            Error::BitcoinConsensus(ref e) => write_err!(f, "bitcoinconsensus verification failed"; e),
+            Error::BitcoinConsensus(ref e) => write_err!(f, "bitcoinconsensus verification failed"; bitcoinconsensus_hack::wrap_error(e)),
             Error::UnknownSpentOutput(ref point) => write!(f, "unknown spent output: {}", point),
             Error::Serialization => f.write_str("can not serialize the spending transaction in Transaction::verify()"),
         }
@@ -197,7 +237,7 @@ impl std::error::Error for Error {
             | UnknownSpentOutput(_)
             | Serialization => None,
             #[cfg(feature = "bitcoinconsensus")]
-            BitcoinConsensus(ref e) => Some(e),
+            BitcoinConsensus(ref e) => Some(bitcoinconsensus_hack::wrap_error(e)),
         }
     }
 }

bitcoin/src/lib.rs

@@ -26,14 +26,17 @@
 //! * `no-std` - enables additional features required for this crate to be usable
 //!              without std. Does **not** disable `std`. Depends on `hashbrown`
 //!              and `core2`.
-//!
+//! * `bitcoinconsensus-std` - enables `std` in `bitcoinconsensus` and communicates it
+//!                            to this crate so it knows how to implement
+//!                            `std::error::Error`. At this time there's a hack to
+//!                            achieve the same without this feature but it could
+//!                            happen the implementations diverge one day.
 
 #![cfg_attr(all(not(feature = "std"), not(test)), no_std)]
 // Experimental features we need.
 #![cfg_attr(bench, feature(test))]
 #![cfg_attr(docsrs, feature(doc_cfg))]
 // Coding conventions
-#![forbid(unsafe_code)]
 #![deny(non_upper_case_globals)]
 #![deny(non_camel_case_types)]
 #![deny(non_snake_case)]