From 946cd83106ea05fd55a2560309cae97921a5dc4f Mon Sep 17 00:00:00 2001
From: "Tobin C. Harding" <me@tobin.cc>
Date: Tue, 14 Jun 2022 09:33:48 +1000
Subject: [PATCH] Improve Error display

The current display code for `Error` is a little unusual. We typically
just implement `Display` and if a `str` is needed use `format!`.

Improve the `Error` type by doing

- Remove the `as_str` function and implement `Display` directly.
- Remove the 'secp: ' prefix of all the error messages.
- Use a newly defined macro `write_err` that writes the error if `std`
  feature is not enabled so that no-std builds do not loose error info.

Note: The `write_err` macro is currently being introduced in
`rust-bitcoin` also. Elect to just duplicate it here and not share it
between the crates.
---
 src/lib.rs    | 35 +++++++++++++++--------------------
 src/macros.rs | 19 +++++++++++++++++++
 2 files changed, 34 insertions(+), 20 deletions(-)

diff --git a/src/lib.rs b/src/lib.rs
index f1cc959..327d4af 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -349,28 +349,23 @@ pub enum Error {
     InvalidParityValue(key::InvalidParityValue),
 }
 
-impl Error {
-    fn as_str(&self) -> &str {
-        match *self {
-            Error::IncorrectSignature => "secp: signature failed verification",
-            Error::InvalidMessage => "secp: message was not 32 bytes (do you need to hash?)",
-            Error::InvalidPublicKey => "secp: malformed public key",
-            Error::InvalidSignature => "secp: malformed signature",
-            Error::InvalidSecretKey => "secp: malformed or out-of-range secret key",
-            Error::InvalidSharedSecret => "secp: malformed or out-of-range shared secret",
-            Error::InvalidRecoveryId => "secp: bad recovery id",
-            Error::InvalidTweak => "secp: bad tweak",
-            Error::NotEnoughMemory => "secp: not enough memory allocated",
-            Error::InvalidPublicKeySum => "secp: the sum of public keys was invalid or the input vector lengths was less than 1",
-            Error::InvalidParityValue(_) => "couldn't create parity",
-        }
-    }
-}
-
-// Passthrough Debug to Display, since errors should be user-visible.
 impl fmt::Display for Error {
     fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
-        f.write_str(self.as_str())
+        use Error::*;
+
+        match *self {
+            IncorrectSignature => f.write_str("signature failed verification"),
+            InvalidMessage => f.write_str("message was not 32 bytes (do you need to hash?)"),
+            InvalidPublicKey => f.write_str("malformed public key"),
+            InvalidSignature => f.write_str("malformed signature"),
+            InvalidSecretKey => f.write_str("malformed or out-of-range secret key"),
+            InvalidSharedSecret => f.write_str("malformed or out-of-range shared secret"),
+            InvalidRecoveryId => f.write_str("bad recovery id"),
+            InvalidTweak => f.write_str("bad tweak"),
+            NotEnoughMemory => f.write_str("not enough memory allocated"),
+            InvalidPublicKeySum => f.write_str("the sum of public keys was invalid or the input vector lengths was less than 1"),
+            InvalidParityValue(e) => write_err!(f, "couldn't create parity"; e),
+        }
     }
 }
 
diff --git a/src/macros.rs b/src/macros.rs
index 6cd1d66..152baa9 100644
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -26,3 +26,22 @@ macro_rules! impl_pretty_debug {
         }
      }
 }
+
+/// Formats error. If `std` feature is OFF appends error source (delimited by `: `). We do this
+/// because `e.source()` is only available in std builds, without this macro the error source is
+/// lost for no-std builds.
+macro_rules! write_err {
+    ($writer:expr, $string:literal $(, $args:expr),*; $source:expr) => {
+        {
+            #[cfg(feature = "std")]
+            {
+                let _ = &$source;   // Prevents clippy warnings.
+                write!($writer, $string $(, $args)*)
+            }
+            #[cfg(not(feature = "std"))]
+            {
+                write!($writer, concat!($string, ": {}") $(, $args)*, $source)
+            }
+        }
+    }
+}