From 9c86f03902249e431cc403c4a7c10bfd8b312cb8 Mon Sep 17 00:00:00 2001 From: Kieran Date: Fri, 24 Nov 2023 12:52:25 +0000 Subject: [PATCH] refactor: drop hexrange Signed-off-by: Greg Heartsfield --- src/hexrange.rs | 159 ------------------------------- src/lib.rs | 1 - src/repo/postgres.rs | 216 ++++++++++++++----------------------------- src/repo/sqlite.rs | 42 +-------- 4 files changed, 75 insertions(+), 343 deletions(-) delete mode 100644 src/hexrange.rs diff --git a/src/hexrange.rs b/src/hexrange.rs deleted file mode 100644 index a9e3e83..0000000 --- a/src/hexrange.rs +++ /dev/null @@ -1,159 +0,0 @@ -//! Utilities for searching hexadecimal -use crate::utils::is_hex; -use hex; - -/// Types of hexadecimal queries. -#[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone)] -pub enum HexSearch { - // when no range is needed, exact 32-byte - Exact(Vec), - // lower (inclusive) and upper range (exclusive) - Range(Vec, Vec), - // lower bound only, upper bound is MAX inclusive - LowerOnly(Vec), -} - -/// Check if a string contains only f chars -fn is_all_fs(s: &str) -> bool { - s.chars().all(|x| x == 'f' || x == 'F') -} - -/// Find the next hex sequence greater than the argument. -#[must_use] -pub fn hex_range(s: &str) -> Option { - let mut hash_base = s.to_owned(); - if !is_hex(&hash_base) || hash_base.len() > 64 { - return None; - } - if hash_base.len() == 64 { - return Some(HexSearch::Exact(hex::decode(&hash_base).ok()?)); - } - // if s is odd, add a zero - let mut odd = hash_base.len() % 2 != 0; - if odd { - // extend the string to make it even - hash_base.push('0'); - } - let base = hex::decode(hash_base).ok()?; - // check for all ff's - if is_all_fs(s) { - // there is no higher bound, we only want to search for blobs greater than this. - return Some(HexSearch::LowerOnly(base)); - } - - // return a range - let mut upper = base.clone(); - let mut byte_len = upper.len(); - - // for odd strings, we made them longer, but we want to increment the upper char (+16). - // we know we can do this without overflowing because we explicitly set the bottom half to 0's. - while byte_len > 0 { - byte_len -= 1; - // check if byte can be incremented, or if we need to carry. - let b = upper[byte_len]; - if b == u8::MAX { - // reset and carry - upper[byte_len] = 0; - } else if odd { - // check if first char in this byte is NOT 'f' - if b < 240 { - // bump up the first character in this byte - upper[byte_len] = b + 16; - // increment done, stop iterating through the vec - break; - } - // if it is 'f', reset the byte to 0 and do a carry - // reset and carry - upper[byte_len] = 0; - // done with odd logic, so don't repeat this - odd = false; - } else { - // bump up the first character in this byte - upper[byte_len] = b + 1; - // increment done, stop iterating - break; - } - } - Some(HexSearch::Range(base, upper)) -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::error::Result; - - #[test] - fn hex_range_exact() -> Result<()> { - let hex = "abcdef00abcdef00abcdef00abcdef00abcdef00abcdef00abcdef00abcdef00"; - let r = hex_range(hex); - assert_eq!( - r, - Some(HexSearch::Exact(hex::decode(hex).expect("invalid hex"))) - ); - Ok(()) - } - #[test] - fn hex_full_range() -> Result<()> { - let hex = "aaaa"; - let hex_upper = "aaab"; - let r = hex_range(hex); - assert_eq!( - r, - Some(HexSearch::Range( - hex::decode(hex).expect("invalid hex"), - hex::decode(hex_upper).expect("invalid hex") - )) - ); - Ok(()) - } - - #[test] - fn hex_full_range_odd() -> Result<()> { - let r = hex_range("abc"); - assert_eq!( - r, - Some(HexSearch::Range( - hex::decode("abc0").expect("invalid hex"), - hex::decode("abd0").expect("invalid hex") - )) - ); - Ok(()) - } - - #[test] - fn hex_full_range_odd_end_f() -> Result<()> { - let r = hex_range("abf"); - assert_eq!( - r, - Some(HexSearch::Range( - hex::decode("abf0").expect("invalid hex"), - hex::decode("ac00").expect("invalid hex") - )) - ); - Ok(()) - } - - #[test] - fn hex_no_upper() -> Result<()> { - let r = hex_range("ffff"); - assert_eq!( - r, - Some(HexSearch::LowerOnly( - hex::decode("ffff").expect("invalid hex") - )) - ); - Ok(()) - } - - #[test] - fn hex_no_upper_odd() -> Result<()> { - let r = hex_range("fff"); - assert_eq!( - r, - Some(HexSearch::LowerOnly( - hex::decode("fff0").expect("invalid hex") - )) - ); - Ok(()) - } -} diff --git a/src/lib.rs b/src/lib.rs index a6f6268..0d0a5f7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,7 +6,6 @@ pub mod db; pub mod delegation; pub mod error; pub mod event; -pub mod hexrange; pub mod info; pub mod nauthz; pub mod nip05; diff --git a/src/repo/postgres.rs b/src/repo/postgres.rs index 2e2f48f..dd120ab 100644 --- a/src/repo/postgres.rs +++ b/src/repo/postgres.rs @@ -14,7 +14,6 @@ use sqlx::{Error, Execute, FromRow, Postgres, QueryBuilder, Row}; use std::time::{Duration, Instant}; use crate::error; -use crate::hexrange::{hex_range, HexSearch}; use crate::repo::postgres_migration::run_migrations; use crate::server::NostrMetrics; use crate::utils::{self, is_hex, is_lower_hex}; @@ -60,8 +59,7 @@ async fn cleanup_expired(conn: PostgresPool, frequency: Duration) -> Result<()> } } } - } - ; + }; } }); Ok(()) @@ -151,19 +149,19 @@ impl NostrRepo for PostgresRepo { VALUES($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (id) DO NOTHING"#, ) - .bind(&id_blob) - .bind(&pubkey_blob) - .bind(Utc.timestamp_opt(e.created_at as i64, 0).unwrap()) - .bind( - e.expiration() - .and_then(|x| Utc.timestamp_opt(x as i64, 0).latest()), - ) - .bind(e.kind as i64) - .bind(event_str.into_bytes()) - .bind(delegator_blob) - .execute(&mut tx) - .await? - .rows_affected(); + .bind(&id_blob) + .bind(&pubkey_blob) + .bind(Utc.timestamp_opt(e.created_at as i64, 0).unwrap()) + .bind( + e.expiration() + .and_then(|x| Utc.timestamp_opt(x as i64, 0).latest()), + ) + .bind(e.kind as i64) + .bind(event_str.into_bytes()) + .bind(delegator_blob) + .execute(&mut tx) + .await? + .rows_affected(); if ins_count == 0 { // if the event was a duplicate, no need to insert event or @@ -283,10 +281,10 @@ ON CONFLICT (id) DO NOTHING"#, LEFT JOIN tag t ON e.id = t.event_id \ WHERE e.pub_key = $1 AND t.\"name\" = 'e' AND e.kind = 5 AND t.value = $2 LIMIT 1", ) - .bind(&pubkey_blob) - .bind(&id_blob) - .fetch_optional(&mut tx) - .await?; + .bind(&pubkey_blob) + .bind(&id_blob) + .fetch_optional(&mut tx) + .await?; // check if a the query returned a result, meaning we should // hid the current event @@ -577,10 +575,10 @@ ON CONFLICT (id) DO NOTHING"#, sqlx::query( "UPDATE account SET is_admitted = TRUE, balance = balance - $1 WHERE pubkey = $2", ) - .bind(admission_cost as i64) - .bind(pub_key) - .execute(&self.conn_write) - .await?; + .bind(admission_cost as i64) + .bind(pub_key) + .execute(&self.conn_write) + .await?; Ok(()) } @@ -726,6 +724,7 @@ fn query_from_filter(f: &ReqFilter) -> Option> { } let mut query = QueryBuilder::new("SELECT e.\"content\", e.created_at FROM \"event\" e WHERE "); + // This tracks whether we need to push a prefix AND before adding another clause let mut push_and = false; // Query for "authors", allowing prefix matches @@ -736,63 +735,19 @@ fn query_from_filter(f: &ReqFilter) -> Option> { if auth_vec.is_empty() { return None; } - query.push("("); + query.push("(e.pub_key in ("); - // shortcut authors into "IN" query - let any_is_range = auth_vec.iter().any(|pk| pk.len() != 64); - if !any_is_range { - query.push("e.pub_key in ("); - let mut pk_sep = query.separated(", "); - for pk in auth_vec.iter() { - pk_sep.push_bind(hex::decode(pk).ok()); - } - query.push(") OR e.delegated_by in ("); - let mut pk_delegated_sep = query.separated(", "); - for pk in auth_vec.iter() { - pk_delegated_sep.push_bind(hex::decode(pk).ok()); - } - query.push(")"); - push_and = true; - } else { - let mut range_authors = query.separated(" OR "); - for auth in auth_vec { - match hex_range(auth) { - Some(HexSearch::Exact(ex)) => { - range_authors - .push("(e.pub_key = ") - .push_bind_unseparated(ex.clone()) - .push_unseparated(" OR e.delegated_by = ") - .push_bind_unseparated(ex) - .push_unseparated(")"); - } - Some(HexSearch::Range(lower, upper)) => { - range_authors - .push("((e.pub_key > ") - .push_bind_unseparated(lower.clone()) - .push_unseparated(" AND e.pub_key < ") - .push_bind_unseparated(upper.clone()) - .push_unseparated(") OR (e.delegated_by > ") - .push_bind_unseparated(lower) - .push_unseparated(" AND e.delegated_by < ") - .push_bind_unseparated(upper) - .push_unseparated("))"); - } - Some(HexSearch::LowerOnly(lower)) => { - range_authors - .push("(e.pub_key > ") - .push_bind_unseparated(lower.clone()) - .push_unseparated(" OR e.delegated_by > ") - .push_bind_unseparated(lower) - .push_unseparated(")"); - } - None => { - info!("Could not parse hex range from author {:?}", auth); - } - } - push_and = true; - } + let mut pk_sep = query.separated(", "); + for pk in auth_vec.iter() { + pk_sep.push_bind(hex::decode(pk).ok()); } - query.push(")"); + query.push(") OR e.delegated_by in ("); + let mut pk_delegated_sep = query.separated(", "); + for pk in auth_vec.iter() { + pk_delegated_sep.push_bind(hex::decode(pk).ok()); + } + push_and = true; + query.push("))"); } // Query for Kind @@ -813,7 +768,7 @@ fn query_from_filter(f: &ReqFilter) -> Option> { query.push(")"); } - // Query for event, allowing prefix matches + // Query for event, if let Some(id_vec) = &f.ids { // filter out non-hex values let id_vec: Vec<&String> = id_vec.iter().filter(|a| is_hex(a)).collect(); @@ -827,48 +782,12 @@ fn query_from_filter(f: &ReqFilter) -> Option> { } push_and = true; - // shortcut ids into "IN" query - let any_is_range = id_vec.iter().any(|pk| pk.len() != 64); - if !any_is_range { - query.push("id in ("); - let mut sep = query.separated(", "); - for id in id_vec.iter() { - sep.push_bind(hex::decode(id).ok()); - } - query.push(")"); - } else { - // take each author and convert to a hex search - let mut id_query = query.separated(" OR "); - for id in id_vec { - match hex_range(id) { - Some(HexSearch::Exact(ex)) => { - id_query - .push("(id = ") - .push_bind_unseparated(ex) - .push_unseparated(")"); - } - Some(HexSearch::Range(lower, upper)) => { - id_query - .push("(id > ") - .push_bind_unseparated(lower) - .push_unseparated(" AND id < ") - .push_bind_unseparated(upper) - .push_unseparated(")"); - } - Some(HexSearch::LowerOnly(lower)) => { - id_query - .push("(id > ") - .push_bind_unseparated(lower) - .push_unseparated(")"); - } - None => { - info!("Could not parse hex range from id {:?}", id); - } - } - } + query.push("id in ("); + let mut sep = query.separated(", "); + for id in id_vec.iter() { + sep.push_bind(hex::decode(id).ok()); } - - query.push(")"); + query.push("))"); } // Query for tags @@ -888,7 +807,8 @@ fn query_from_filter(f: &ReqFilter) -> Option> { if push_or { query.push(" OR "); } - query.push("(t.\"name\" = ") + query + .push("(t.\"name\" = ") .push_bind(key.to_string()) .push(" AND ("); @@ -898,8 +818,7 @@ fn query_from_filter(f: &ReqFilter) -> Option> { query.push("value in ("); // plain value match first let mut tag_query = query.separated(", "); - for v in val.iter() - .filter(|v| !is_lower_hex(v)) { + for v in val.iter().filter(|v| !is_lower_hex(v)) { tag_query.push_bind(v.as_bytes()); } } @@ -910,8 +829,7 @@ fn query_from_filter(f: &ReqFilter) -> Option> { query.push("value_hex in ("); // plain value match first let mut tag_query = query.separated(", "); - for v in val.iter() - .filter(|v| v.len() % 2 == 0 && is_lower_hex(v)) { + for v in val.iter().filter(|v| v.len() % 2 == 0 && is_lower_hex(v)) { tag_query.push_bind(hex::decode(v).ok()); } } @@ -988,11 +906,10 @@ impl FromRow<'_, PgRow> for VerificationRecord { } } - #[cfg(test)] mod tests { - use std::collections::{HashMap, HashSet}; use super::*; + use std::collections::{HashMap, HashSet}; #[test] fn test_query_gen_tag_value_hex() { @@ -1001,11 +918,16 @@ mod tests { kinds: Some(vec![1000]), since: None, until: None, - authors: Some(vec!["84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned()]), + authors: Some(vec![ + "84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned(), + ]), limit: None, - tags: Some(HashMap::from([ - ('p', HashSet::from(["63fe6318dc58583cfe16810f86dd09e18bfd76aabc24a0081ce2856f330504ed".to_owned()])) - ])), + tags: Some(HashMap::from([( + 'p', + HashSet::from([ + "63fe6318dc58583cfe16810f86dd09e18bfd76aabc24a0081ce2856f330504ed".to_owned(), + ]), + )])), force_no_match: false, }; @@ -1020,11 +942,11 @@ mod tests { kinds: Some(vec![1000]), since: None, until: None, - authors: Some(vec!["84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned()]), + authors: Some(vec![ + "84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned(), + ]), limit: None, - tags: Some(HashMap::from([ - ('d', HashSet::from(["test".to_owned()])) - ])), + tags: Some(HashMap::from([('d', HashSet::from(["test".to_owned()]))])), force_no_match: false, }; @@ -1039,11 +961,17 @@ mod tests { kinds: Some(vec![1000]), since: None, until: None, - authors: Some(vec!["84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned()]), + authors: Some(vec![ + "84de35e2584d2b144aae823c9ed0b0f3deda09648530b93d1a2a146d1dea9864".to_owned(), + ]), limit: None, - tags: Some(HashMap::from([ - ('d', HashSet::from(["test".to_owned(), "63fe6318dc58583cfe16810f86dd09e18bfd76aabc24a0081ce2856f330504ed".to_owned()])) - ])), + tags: Some(HashMap::from([( + 'd', + HashSet::from([ + "test".to_owned(), + "63fe6318dc58583cfe16810f86dd09e18bfd76aabc24a0081ce2856f330504ed".to_owned(), + ]), + )])), force_no_match: false, }; @@ -1062,7 +990,7 @@ mod tests { limit: None, tags: Some(HashMap::from([ ('d', HashSet::from(["follow".to_owned()])), - ('t', HashSet::from(["siamstr".to_owned()])) + ('t', HashSet::from(["siamstr".to_owned()])), ])), force_no_match: false, }; @@ -1079,11 +1007,9 @@ mod tests { until: None, authors: None, limit: None, - tags: Some(HashMap::from([ - ('a', HashSet::new()) - ])), + tags: Some(HashMap::from([('a', HashSet::new())])), force_no_match: false, }; assert!(query_from_filter(&filter).is_none()); } -} \ No newline at end of file +} diff --git a/src/repo/sqlite.rs b/src/repo/sqlite.rs index c297847..e12ae8e 100644 --- a/src/repo/sqlite.rs +++ b/src/repo/sqlite.rs @@ -4,8 +4,6 @@ use crate::config::Settings; use crate::db::QueryResult; use crate::error::{Error::SqlError, Result}; use crate::event::{single_char_tagname, Event}; -use crate::hexrange::hex_range; -use crate::hexrange::HexSearch; use crate::nip05::{Nip05Name, VerificationRecord}; use crate::payment::{InvoiceInfo, InvoiceStatus}; use crate::repo::sqlite_migration::{upgrade_db, STARTUP_SQL}; @@ -994,24 +992,8 @@ fn query_from_filter(f: &ReqFilter) -> (String, Vec>, Option = vec![]; for auth in authvec { - match hex_range(auth) { - Some(HexSearch::Exact(ex)) => { - auth_searches.push("author=?".to_owned()); - params.push(Box::new(ex)); - } - Some(HexSearch::Range(lower, upper)) => { - auth_searches.push("(author>? AND author { - auth_searches.push("author>?".to_owned()); - params.push(Box::new(lower)); - } - None => { - trace!("Could not parse hex range from author {:?}", auth); - } - } + auth_searches.push("author=?".to_owned()); + params.push(Box::new(auth.clone())); } if !authvec.is_empty() { let auth_clause = format!("({})", auth_searches.join(" OR ")); @@ -1032,24 +1014,8 @@ fn query_from_filter(f: &ReqFilter) -> (String, Vec>, Option = vec![]; for id in idvec { - match hex_range(id) { - Some(HexSearch::Exact(ex)) => { - id_searches.push("event_hash=?".to_owned()); - params.push(Box::new(ex)); - } - Some(HexSearch::Range(lower, upper)) => { - id_searches.push("(event_hash>? AND event_hash { - id_searches.push("event_hash>?".to_owned()); - params.push(Box::new(lower)); - } - None => { - info!("Could not parse hex range from id {:?}", id); - } - } + id_searches.push("event_hash=?".to_owned()); + params.push(Box::new(id.clone())); } if idvec.is_empty() { // if the ids list was empty, we should never return