From 6d1244434b8cf4bdec00a8d0324d1cb3a7bac239 Mon Sep 17 00:00:00 2001
From: William Casarin <jb55@jb55.com>
Date: Fri, 11 Nov 2022 07:20:36 -0800
Subject: [PATCH] feat(NIP-20): improve invalid event error messages

Instead of returning a NOTICE for invalid events, return a `OK false`
command result with a reason as to why the event is invalid.
---
 src/error.rs  | 12 +++++++++---
 src/event.rs  | 34 ++++++++++++++++++++--------------
 src/server.rs |  5 +++--
 3 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/src/error.rs b/src/error.rs
index bca9fc6..525c459 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -17,10 +17,16 @@ pub enum Error {
     ConnWriteError,
     #[error("EVENT parse failed")]
     EventParseFailed,
-    #[error("ClOSE message parse failed")]
+    #[error("CLOSE message parse failed")]
     CloseParseFailed,
-    #[error("Event validation failed")]
-    EventInvalid,
+    #[error("Event invalid signature")]
+    EventInvalidSignature,
+    #[error("Event invalid id")]
+    EventInvalidId,
+    #[error("Event malformed pubkey")]
+    EventMalformedPubkey,
+    #[error("Event could not canonicalize")]
+    EventCouldNotCanonicalize,
     #[error("Event too large")]
     EventMaxLengthError(usize),
     #[error("Subscription identifier max length exceeded")]
diff --git a/src/event.rs b/src/event.rs
index 60d5cb1..9f7137e 100644
--- a/src/event.rs
+++ b/src/event.rs
@@ -27,6 +27,12 @@ pub struct EventCmd {
     event: Event,
 }
 
+impl EventCmd {
+    pub fn event_id(&self) -> &str {
+        return &self.event.id;
+    }
+}
+
 /// Parsed nostr event.
 #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)]
 pub struct Event {
@@ -83,13 +89,13 @@ impl From<EventCmd> for Result<Event> {
         // ensure command is correct
         if ec.cmd != "EVENT" {
             Err(CommandUnknownError)
-        } else if ec.event.is_valid() {
-            let mut e = ec.event;
-            e.build_index();
-            e.update_delegation();
-            Ok(e)
         } else {
-            Err(EventInvalid)
+            ec.event.validate().map(|_| {
+                let mut e = ec.event;
+                e.build_index();
+                e.update_delegation();
+                e
+            })
         }
     }
 }
@@ -220,7 +226,7 @@ impl Event {
     }
 
     /// Check if this event has a valid signature.
-    fn is_valid(&self) -> bool {
+    fn validate(&self) -> Result<()> {
         // TODO: return a Result with a reason for invalid events
         // validation is performed by:
         // * parsing JSON string into event fields
@@ -229,8 +235,8 @@ impl Event {
         // * serialize with no spaces/newlines
         let c_opt = self.to_canonical();
         if c_opt.is_none() {
-            debug!("event could not be canonicalized");
-            return false;
+            debug!("could not canonicalize");
+            return Err(EventCouldNotCanonicalize);
         }
         let c = c_opt.unwrap();
         // * compute the sha256sum.
@@ -239,21 +245,21 @@ impl Event {
         // * ensure the id matches the computed sha256sum.
         if self.id != hex_digest {
             debug!("event id does not match digest");
-            return false;
+            return Err(EventInvalidId);
         }
         // * validate the message digest (sig) using the pubkey & computed sha256 message hash.
         let sig = schnorr::Signature::from_str(&self.sig).unwrap();
         if let Ok(msg) = secp256k1::Message::from_slice(digest.as_ref()) {
             if let Ok(pubkey) = XOnlyPublicKey::from_str(&self.pubkey) {
-                let verify = SECP.verify_schnorr(&sig, &msg, &pubkey);
-                matches!(verify, Ok(()))
+                SECP.verify_schnorr(&sig, &msg, &pubkey)
+                    .map_err(|_| EventInvalidSignature)
             } else {
                 debug!("client sent malformed pubkey");
-                false
+                Err(EventMalformedPubkey)
             }
         } else {
             info!("error converting digest to secp256k1 message");
-            false
+            Err(EventInvalidSignature)
         }
     }
 
diff --git a/src/server.rs b/src/server.rs
index 29337a2..abd1973 100644
--- a/src/server.rs
+++ b/src/server.rs
@@ -572,6 +572,7 @@ async fn nostr_server(
                     Ok(NostrMessage::EventMsg(ec)) => {
                         // An EventCmd needs to be validated to be converted into an Event
                         // handle each type of message
+                        let evid = ec.event_id().to_owned();
                         let parsed : Result<Event> = Result::<Event>::from(ec);
                         match parsed {
                             Ok(e) => {
@@ -592,9 +593,9 @@ async fn nostr_server(
                     }
                 }
                             },
-                            Err(_) => {
+                            Err(e) => {
                                 info!("client: {} sent an invalid event", cid);
-                                ws_stream.send(make_notice_message(Notice::message("event was invalid".into()))).await.ok();
+                                ws_stream.send(make_notice_message(Notice::invalid(evid, &format!("{}", e)))).await.ok();
                             }
                         }
                     },