From b7da4b1a28728bba62fb08b87b233be49f1fe5e7 Mon Sep 17 00:00:00 2001 From: Mike Dilger Date: Tue, 9 Apr 2024 08:53:27 +1200 Subject: [PATCH] Simplify get_event_by_offset() output, avoid a panic --- chorus-bin/src/main.rs | 31 ++--- chorus-lib/src/error.rs | 5 + chorus-lib/src/store/event_store.rs | 16 ++- chorus-lib/src/store/migrations.rs | 60 ++++---- chorus-lib/src/store/mod.rs | 209 ++++++++++++++-------------- 5 files changed, 161 insertions(+), 160 deletions(-) diff --git a/chorus-bin/src/main.rs b/chorus-bin/src/main.rs index 303d96a..eb75efe 100644 --- a/chorus-bin/src/main.rs +++ b/chorus-bin/src/main.rs @@ -500,26 +500,25 @@ impl WebSocketService { return Ok(()); } - if let Some(event) = GLOBALS + let event = GLOBALS .store .get() .unwrap() - .get_event_by_offset(new_event_offset)? - { - let event_flags = nostr::event_flags(&event, &self.user); - let authorized_user = nostr::authorized_user(&self.user); + .get_event_by_offset(new_event_offset)?; - 'subs: for (subid, filters) in self.subscriptions.iter() { - for filter in filters.iter() { - if filter.as_filter()?.event_matches(&event)? - && nostr::screen_outgoing_event(&event, &event_flags, authorized_user) - { - let message = NostrReply::Event(subid, event); - self.websocket - .send(Message::text(message.as_json())) - .await?; - continue 'subs; - } + let event_flags = nostr::event_flags(&event, &self.user); + let authorized_user = nostr::authorized_user(&self.user); + + 'subs: for (subid, filters) in self.subscriptions.iter() { + for filter in filters.iter() { + if filter.as_filter()?.event_matches(&event)? + && nostr::screen_outgoing_event(&event, &event_flags, authorized_user) + { + let message = NostrReply::Event(subid, event); + self.websocket + .send(Message::text(message.as_json())) + .await?; + continue 'subs; } } } diff --git a/chorus-lib/src/error.rs b/chorus-lib/src/error.rs index c9b4cde..6880336 100644 --- a/chorus-lib/src/error.rs +++ b/chorus-lib/src/error.rs @@ -112,6 +112,9 @@ pub enum ChorusError { // Protected Event ProtectedEvent, + // Range error + RangeError, + // Restricted Restricted, @@ -191,6 +194,7 @@ impl std::fmt::Display for ChorusError { ChorusError::NoPrivateKey => write!(f, "Private Key Not Found"), ChorusError::NoSuchSubscription => write!(f, "No such subscription"), ChorusError::ProtectedEvent => write!(f, "Protected event"), + ChorusError::RangeError => write!(f, "Range error"), ChorusError::Restricted => write!(f, "Restricted"), ChorusError::Rustls(e) => write!(f, "{e}"), ChorusError::TimedOut => write!(f, "Timed out"), @@ -261,6 +265,7 @@ impl ChorusError { ChorusError::NoPrivateKey => 0.0, ChorusError::NoSuchSubscription => 0.05, ChorusError::ProtectedEvent => 0.35, + ChorusError::RangeError => 0.0, ChorusError::Restricted => 0.1, ChorusError::Rustls(_) => 0.0, ChorusError::TimedOut => 0.1, diff --git a/chorus-lib/src/store/event_store.rs b/chorus-lib/src/store/event_store.rs index 89cb903..249c4d5 100644 --- a/chorus-lib/src/store/event_store.rs +++ b/chorus-lib/src/store/event_store.rs @@ -1,4 +1,4 @@ -use crate::error::Error; +use crate::error::{ChorusError, Error}; use crate::types::Event; use mmap_append::MmapAppend; use std::fs::{File, OpenOptions}; @@ -77,10 +77,12 @@ impl EventStore { } /// Get an event by its offset in the map - pub fn get_event_by_offset(&self, offset: usize) -> Result, Error> { - // deserialize event + pub fn get_event_by_offset(&self, offset: usize) -> Result { + if offset >= self.read_event_map_end() { + return Err(ChorusError::EndOfInput.into()); + } let event = Event::delineate(&self.event_map[offset..])?; - Ok(Some(event)) + Ok(event) } // This stores an event @@ -162,19 +164,19 @@ mod tests { println!("Event map has {} used bytes", store.read_event_map_end()); - if let Some(event) = store.get_event_by_offset(offset1).unwrap() { + if let Ok(event) = store.get_event_by_offset(offset1) { assert_eq!(event, event1); } else { panic!("EVENT 1 IS WRONG"); } - if let Some(event) = store.get_event_by_offset(offset2).unwrap() { + if let Ok(event) = store.get_event_by_offset(offset2) { assert_eq!(event, event2); } else { panic!("EVENT 2 IS WRONG"); } - if let Some(event) = store.get_event_by_offset(offset3).unwrap() { + if let Ok(event) = store.get_event_by_offset(offset3) { assert_eq!(event, event3); } else { panic!("EVENT 3 IS WRONG"); diff --git a/chorus-lib/src/store/migrations.rs b/chorus-lib/src/store/migrations.rs index 8fa6781..829da86 100644 --- a/chorus-lib/src/store/migrations.rs +++ b/chorus-lib/src/store/migrations.rs @@ -52,14 +52,12 @@ impl Store { let iter = self.i_index.iter(&loop_txn)?; for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - // Index in ci - self.ci_index.put( - txn, - &Self::key_ci_index(event.created_at(), event.id()), - &offset, - )?; - } + let event = self.events.get_event_by_offset(offset)?; + self.ci_index.put( + txn, + &Self::key_ci_index(event.created_at(), event.id()), + &offset, + )?; } Ok(()) @@ -71,30 +69,30 @@ impl Store { let iter = self.i_index.iter(&loop_txn)?; for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - // Add to ac_index - self.ac_index.put( - txn, - &Self::key_ac_index(event.pubkey(), event.created_at(), event.id()), - &offset, - )?; + let event = self.events.get_event_by_offset(offset)?; - // Add to tc_index - for mut tsi in event.tags()?.iter() { - if let Some(tagname) = tsi.next() { - if tagname.len() == 1 { - if let Some(tagvalue) = tsi.next() { - self.tc_index.put( - txn, - &Self::key_tc_index( - tagname[0], - tagvalue, - event.created_at(), - event.id(), - ), - &offset, - )?; - } + // Add to ac_index + self.ac_index.put( + txn, + &Self::key_ac_index(event.pubkey(), event.created_at(), event.id()), + &offset, + )?; + + // Add to tc_index + for mut tsi in event.tags()?.iter() { + if let Some(tagname) = tsi.next() { + if tagname.len() == 1 { + if let Some(tagvalue) = tsi.next() { + self.tc_index.put( + txn, + &Self::key_tc_index( + tagname[0], + tagvalue, + event.created_at(), + event.id(), + ), + &offset, + )?; } } } diff --git a/chorus-lib/src/store/mod.rs b/chorus-lib/src/store/mod.rs index 2c86c36..424360f 100644 --- a/chorus-lib/src/store/mod.rs +++ b/chorus-lib/src/store/mod.rs @@ -305,7 +305,7 @@ impl Store { } /// Get an event by its offset. - pub fn get_event_by_offset(&self, offset: usize) -> Result, Error> { + pub fn get_event_by_offset(&self, offset: usize) -> Result { self.events.get_event_by_offset(offset) } @@ -313,7 +313,7 @@ impl Store { pub fn get_event_by_id(&self, id: Id) -> Result, Error> { let txn = self.env.read_txn()?; if let Some(offset) = self.i_index.get(&txn, id.0.as_slice())? { - self.events.get_event_by_offset(offset) + Some(self.events.get_event_by_offset(offset)).transpose() } else { Ok(None) } @@ -376,37 +376,37 @@ impl Store { 'per_event: for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - // If we have gone beyond since, we can stop early - // (We have to check because `since` might change in this loop) - if event.created_at() < since { + let event = self.events.get_event_by_offset(offset)?; + + // If we have gone beyond since, we can stop early + // (We have to check because `since` might change in this loop) + if event.created_at() < since { + break 'per_event; + } + + // check against the rest of the filter + if filter.event_matches(&event)? && screen(&event) { + // Accept the event + output.insert(event); + paircount += 1; + + // Stop this pair if limited + if paircount >= filter.limit() as usize { + // Since we found the limit just among this pair, + // potentially move since forward + if event.created_at() > since { + since = event.created_at(); + } break 'per_event; } - // check against the rest of the filter - if filter.event_matches(&event)? && screen(&event) { - // Accept the event - output.insert(event); - paircount += 1; - - // Stop this pair if limited - if paircount >= filter.limit() as usize { - // Since we found the limit just among this pair, - // potentially move since forward - if event.created_at() > since { - since = event.created_at(); - } - break 'per_event; - } - - // If kind is replaceable (and not parameterized) - // then don't take any more events for this author-kind - // pair. - // NOTE that this optimization is difficult to implement - // for other replaceable event situations - if kind.is_replaceable() { - break 'per_event; - } + // If kind is replaceable (and not parameterized) + // then don't take any more events for this author-kind + // pair. + // NOTE that this optimization is difficult to implement + // for other replaceable event situations + if kind.is_replaceable() { + break 'per_event; } } } @@ -450,28 +450,28 @@ impl Store { 'per_event: for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - // If we have gone beyond since, we can stop early - // (We have to check because `since` might change in this loop) - if event.created_at() < since { - break 'per_event; - } + let event = self.events.get_event_by_offset(offset)?; - // check against the rest of the filter - if filter.event_matches(&event)? && screen(&event) { - // Accept the event - output.insert(event); - paircount += 1; + // If we have gone beyond since, we can stop early + // (We have to check because `since` might change in this loop) + if event.created_at() < since { + break 'per_event; + } - // Stop this pair if limited - if paircount >= filter.limit() as usize { - // Since we found the limit just among this pair, - // potentially move since forward - if event.created_at() > since { - since = event.created_at(); - } - break 'per_event; + // check against the rest of the filter + if filter.event_matches(&event)? && screen(&event) { + // Accept the event + output.insert(event); + paircount += 1; + + // Stop this pair if limited + if paircount >= filter.limit() as usize { + // Since we found the limit just among this pair, + // potentially move since forward + if event.created_at() > since { + since = event.created_at(); } + break 'per_event; } } } @@ -517,28 +517,28 @@ impl Store { 'per_event: for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - // If we have gone beyond since, we can stop early - // (We have to check because `since` might change in this loop) - if event.created_at() < since { - break 'per_event; - } + let event = self.events.get_event_by_offset(offset)?; - // check against the rest of the filter - if filter.event_matches(&event)? && screen(&event) { - // Accept the event - output.insert(event); - paircount += 1; + // If we have gone beyond since, we can stop early + // (We have to check because `since` might change in this loop) + if event.created_at() < since { + break 'per_event; + } - // Stop this pair if limited - if paircount >= filter.limit() as usize { - // Since we found the limit just among this pair, - // potentially move since forward - if event.created_at() > since { - since = event.created_at(); - } - break 'per_event; + // check against the rest of the filter + if filter.event_matches(&event)? && screen(&event) { + // Accept the event + output.insert(event); + paircount += 1; + + // Stop this pair if limited + if paircount >= filter.limit() as usize { + // Since we found the limit just among this pair, + // potentially move since forward + if event.created_at() > since { + since = event.created_at(); } + break 'per_event; } } } @@ -575,24 +575,24 @@ impl Store { 'per_event: for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - if event.created_at() < since { - break 'per_event; - } + let event = self.events.get_event_by_offset(offset)?; - // check against the rest of the filter - if filter.event_matches(&event)? && screen(&event) { - // Accept the event - output.insert(event); - rangecount += 1; + if event.created_at() < since { + break 'per_event; + } - // Stop this limited - if rangecount >= filter.limit() as usize { - if event.created_at() > since { - since = event.created_at(); - } - break 'per_event; + // check against the rest of the filter + if filter.event_matches(&event)? && screen(&event) { + // Accept the event + output.insert(event); + rangecount += 1; + + // Stop this limited + if rangecount >= filter.limit() as usize { + if event.created_at() > since { + since = event.created_at(); } + break 'per_event; } } } @@ -623,24 +623,24 @@ impl Store { 'per_event: for result in iter { let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - if event.created_at() < filter.since() { - break 'per_event; - } + let event = self.events.get_event_by_offset(offset)?; - // check against the rest of the filter - if filter.event_matches(&event)? && screen(&event) { - // Accept the event - output.insert(event); - rangecount += 1; + if event.created_at() < filter.since() { + break 'per_event; + } - // Stop this limited - if rangecount >= filter.limit() as usize { - if event.created_at() > since { - since = event.created_at(); - } - break 'per_event; + // check against the rest of the filter + if filter.event_matches(&event)? && screen(&event) { + // Accept the event + output.insert(event); + rangecount += 1; + + // Stop this limited + if rangecount >= filter.limit() as usize { + if event.created_at() > since { + since = event.created_at(); } + break 'per_event; } } } @@ -674,10 +674,10 @@ impl Store { break; } let (_key, offset) = result?; - if let Some(event) = self.events.get_event_by_offset(offset)? { - if filter.event_matches(&event)? && screen(&event) { - output.insert(event); - } + let event = self.events.get_event_by_offset(offset)?; + + if filter.event_matches(&event)? && screen(&event) { + output.insert(event); } } } @@ -860,10 +860,7 @@ impl Store { self.deleted_offsets.put(txn, &offset_u64, &())?; // Get event - let event = match self.events.get_event_by_offset(offset)? { - Some(event) => event, - None => return Ok(()), - }; + let event = self.events.get_event_by_offset(offset)?; // Remove from indexes self.deindex(txn, &event)?;