Rate user feature (#404)

* Rate-user: first draft modification to test - user rate is saved to mostro and sent as event too

* rate user improve: users now are saved correctly based on their identity key - need to improve check on message arrival for latest trade index

* some rabbit things...

* Update src/db.rs

Add comprehensive validation for rating parameters.

Consider adding validation for all rating parameters to ensure data consistency:

    Validate min_rating and max_rating range
    Ensure min_rating <= last_rating <= max_rating
    Validate total_rating against total_reviews
    Add public_key format validation

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update src/app/rate_user.rs

total_reviews before calculation

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Update src/db.rs

Add consistent validation and improve error messages.

For consistency with other functions and better error handling:

    Add public key validation (same as in add_new_user and update_user_rating)
    Add trade index validation
    Improve error message to include the public key

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

* Fix: calculation of user rating and last rating save on db

* Fix: improved checks in rate order

* Fix: math corrected with all rabbit suggestion

* Fix: wrong check on incoming messages, need to check signature against rumor pubkey - changed to float total rating in user table

* Fix total_rating calculation

* Fix: last trade index check working

* Comment on test_get_nostr_client_failure test - fails on gh actions locally working weird

* Fix test: modified not passing test

* Bug fixes found on this PR review

* Fix: first review special case - fix on user rate update for sqlx issue documented

* Fix: rate user now updates correctly max and min rate of a user in db

* Shows trade public key on rate event

---------

Co-authored-by: Francisco Calderón <[email protected]>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

Cargo.toml

@@ -38,7 +38,7 @@ uuid = { version = "1.8.0", features = [
   "serde",
 ] }
 reqwest = { version = "0.12.1", features = ["json"] }
-mostro-core = { version = "0.6.18", features = ["sqlx"] }
+mostro-core = { version = "0.6.19", features = ["sqlx"] }
 tracing = "0.1.40"
 tracing-subscriber = { version = "0.3.18", features = ["env-filter"] }
 config = "0.14.0"

migrations/20231005195154_users.sql

@@ -1,13 +1,12 @@
 CREATE TABLE IF NOT EXISTS users (
-  id char(36) primary key not null,
-  pubkey char(64) unique not null,
+  pubkey char(64) primary key not null,
   is_admin integer not null default 0,
   is_solver integer not null default 0,
   is_banned integer not null default 0,
   category integer not null default 0,
   last_trade_index integer not null default 0,
   total_reviews integer not null default 0,
-  total_rating integer not null default 0,
+  total_rating real not null default 0.0,
   last_rating integer not null default 0,
   max_rating integer not null default 0,
   min_rating integer not null default 0,

sqlx-data.json

@@ -10,6 +10,16 @@
     },
     "query": "\n            UPDATE orders\n            SET\n            master_seller_pubkey = ?1\n            WHERE id = ?2\n        "
   },
+  "071917afd7e1d00e2ae860779a1471d281f7aa45da61f7a7bdbd4625a3966a5d": {
+    "describe": {
+      "columns": [],
+      "nullable": [],
+      "parameters": {
+        "Right": 6
+      }
+    },
+    "query": "\n            UPDATE users SET last_rating = ?1, min_rating = ?2, max_rating = ?3, total_reviews = ?4, total_rating = ?5 WHERE pubkey = ?6\n        "
+  },
   "77ea98f6af16fa6e5a7d604965593700c563f88d88cb10b348bdc4200c87ad1d": {
     "describe": {
       "columns": [],
@@ -69,5 +79,15 @@
       }
     },
     "query": "\n            UPDATE orders\n            SET\n            seller_pubkey = ?1\n            WHERE id = ?2\n        "
+  },
+  "fb8c9a8f42f6fc3c70e734db9db4af88c9602143afdbfcf57562f3fecb80b3a7": {
+    "describe": {
+      "columns": [],
+      "nullable": [],
+      "parameters": {
+        "Right": 2
+      }
+    },
+    "query": "\n            UPDATE users SET last_trade_index = ?1 WHERE pubkey = ?2\n        "
   }
 }

src/app.rs

@@ -31,7 +31,9 @@ use crate::app::release::release_action;
 use crate::app::take_buy::take_buy_action;
 use crate::app::take_sell::take_sell_action;
 
+use crate::db::update_user_trade_index;
 // Core functionality imports
+use crate::db::add_new_user;
 use crate::lightning::LndConnector;
 use crate::nip59::unwrap_gift_wrap;
 use crate::util::send_cant_do_msg;
@@ -44,7 +46,6 @@ use mostro_core::message::{Action, CantDoReason, Message};
 use mostro_core::user::User;
 use nostr_sdk::prelude::*;
 use sqlx::{Pool, Sqlite};
-use sqlx_crud::Crud;
 use std::sync::Arc;
 use tokio::sync::Mutex;
 /// Helper function to log warning messages for action errors
@@ -76,11 +77,17 @@ async fn check_trade_index(pool: &Pool<Sqlite>, event: &UnwrappedGift, msg: &Mes
                     if index > user.last_trade_index
                         && msg
                             .get_inner_message_kind()
-                            .verify_signature(event.sender, sig)
+                            .verify_signature(event.rumor.pubkey, sig)
                     {
                         user.last_trade_index = index;
-                        if user.update(pool).await.is_ok() {
-                            tracing::info!("Update user trade index");
+                        if let Err(e) = update_user_trade_index(
+                            pool,
+                            event.sender.to_string(),
+                            user.last_trade_index,
+                        )
+                        .await
+                        {
+                            tracing::error!("Error updating user trade index: {}", e);
                         }
                     } else {
                         tracing::info!("Invalid signature or trade index");
@@ -96,13 +103,22 @@ async fn check_trade_index(pool: &Pool<Sqlite>, event: &UnwrappedGift, msg: &Mes
             }
             Err(_) => {
                 if let (true, last_trade_index) = message_kind.has_trade_index() {
-                    let new_user = User {
+                    let new_user: User = User {
                         pubkey: event.sender.to_string(),
                         last_trade_index,
                         ..Default::default()
                     };
-                    if new_user.create(pool).await.is_ok() {
-                        tracing::info!("Added new user for rate");
+                    if let Err(e) =
+                        add_new_user(pool, new_user.pubkey, new_user.last_trade_index).await
+                    {
+                        tracing::error!("Error creating new user: {}", e);
+                        send_cant_do_msg(
+                            None,
+                            msg.get_inner_message_kind().id,
+                            Some(CantDoReason::InvalidTextMessage),
+                            &event.rumor.pubkey,
+                        )
+                        .await;
                     }
                 }
             }

src/app/admin_add_solver.rs

@@ -1,3 +1,4 @@
+use crate::db::add_new_user;
 use crate::util::{send_cant_do_msg, send_dm};
 
 use anyhow::Result;
@@ -6,7 +7,6 @@ use mostro_core::user::User;
 use nostr::nips::nip59::UnwrappedGift;
 use nostr_sdk::prelude::*;
 use sqlx::{Pool, Sqlite};
-use sqlx_crud::Crud;
 use tracing::{error, info};
 
 pub async fn admin_add_solver_action(
@@ -42,7 +42,7 @@ pub async fn admin_add_solver_action(
     let public_key = PublicKey::from_bech32(npubkey)?.to_hex();
     let user = User::new(public_key, 0, 1, 0, 0, trade_index);
     // Use CRUD to create user
-    match user.create(pool).await {
+    match add_new_user(pool, user.pubkey, user.last_trade_index).await {
         Ok(r) => info!("Solver added: {:#?}", r),
         Err(ee) => error!("Error creating solver: {:#?}", ee),
     }

src/app/rate_user.rs

@@ -1,8 +1,9 @@
 use crate::util::{send_cant_do_msg, send_new_order_msg, update_user_rating_event};
 use crate::NOSTR_CLIENT;
 
+use crate::db::{is_user_present, update_user_rating};
 use anyhow::{Error, Result};
-use mostro_core::message::{Action, Message, Payload};
+use mostro_core::message::{Action, CantDoReason, Message, Payload};
 use mostro_core::order::{Order, Status};
 use mostro_core::rating::Rating;
 use mostro_core::NOSTR_REPLACEABLE_EVENT_KIND;
@@ -15,8 +16,8 @@ use std::time::Duration;
 use tokio::sync::Mutex;
 use tracing::error;
 
-const MAX_RATING: u8 = 5;
-const MIN_RATING: u8 = 1;
+pub const MAX_RATING: u8 = 5;
+pub const MIN_RATING: u8 = 1;
 
 pub async fn get_user_reputation(user: &str, my_keys: &Keys) -> Result<Option<Rating>> {
     // Request NIP33 of the counterparts
@@ -85,22 +86,39 @@ pub async fn update_user_reputation_action(
     }
     // Get counterpart pubkey
     let mut counterpart: String = String::new();
+    let mut counterpart_trade_pubkey: String = String::new();
     let mut buyer_rating: bool = false;
     let mut seller_rating: bool = false;
 
     // Find the counterpart public key
     if message_sender == buyer {
-        counterpart = seller;
+        counterpart = order
+            .master_seller_pubkey
+            .ok_or_else(|| Error::msg("Missing master seller pubkey"))?;
         buyer_rating = true;
+        counterpart_trade_pubkey = order
+            .buyer_pubkey
+            .ok_or_else(|| Error::msg("Missing buyer pubkey"))?;
     } else if message_sender == seller {
-        counterpart = buyer;
+        counterpart = order
+            .master_buyer_pubkey
+            .ok_or_else(|| Error::msg("Missing master buyer pubkey"))?;
         seller_rating = true;
+        counterpart_trade_pubkey = order
+            .seller_pubkey
+            .ok_or_else(|| Error::msg("Missing seller pubkey"))?;
     };
 
     // Add a check in case of no counterpart found
     if counterpart.is_empty() {
         // We create a Message
-        send_cant_do_msg(request_id, Some(order.id), None, &event.rumor.pubkey).await;
+        send_cant_do_msg(
+            request_id,
+            Some(order.id),
+            Some(CantDoReason::InvalidPeer),
+            &event.rumor.pubkey,
+        )
+        .await;
         return Ok(());
     };
 
@@ -118,53 +136,77 @@ pub async fn update_user_reputation_action(
     };
 
     // Check if content of Peer is the same of counterpart
-    let rating;
-
-    if let Some(Payload::RatingUser(v)) = msg.get_inner_message_kind().payload.to_owned() {
-        if !(MIN_RATING..=MAX_RATING).contains(&v) {
-            return Err(Error::msg(format!(
-                "Rating must be between {} and {}",
-                MIN_RATING, MAX_RATING
-            )));
-        }
-        rating = v;
+    let rating =
+        if let Some(Payload::RatingUser(v)) = msg.get_inner_message_kind().payload.to_owned() {
+            if !(MIN_RATING..=MAX_RATING).contains(&v) {
+                return Err(Error::msg(format!(
+                    "Rating must be between {} and {}",
+                    MIN_RATING, MAX_RATING
+                )));
+            }
+            v
+        } else {
+            return Err(Error::msg("No rating present"));
+        };
+
+    // Get counter to vote from db
+    let mut user_to_vote = is_user_present(pool, counterpart.clone()).await?;
+
+    // Update user reputation
+    // Going on with calculation
+    // increment first
+    user_to_vote.total_reviews += 1;
+    let old_rating = user_to_vote.total_rating as f64;
+    // recompute new rating
+    if user_to_vote.total_reviews <= 1 {
+        user_to_vote.total_rating = rating.into();
+        user_to_vote.max_rating = rating.into();
+        user_to_vote.min_rating = rating.into();
     } else {
-        return Err(Error::msg("No rating present"));
+        user_to_vote.total_rating = old_rating
+            + ((user_to_vote.last_rating as f64) - old_rating)
+                / (user_to_vote.total_reviews as f64);
+        if user_to_vote.max_rating < rating.into() {
+            user_to_vote.max_rating = rating.into();
+        }
+        if user_to_vote.min_rating > rating.into() {
+            user_to_vote.min_rating = rating.into();
+        }
     }
-
-    // Ask counterpart reputation
-    let rep = get_user_reputation(&counterpart, my_keys).await?;
-    // Here we have to update values of the review of the counterpart
-    let mut reputation;
-
-    if let Some(r) = rep {
-        // Update user reputation
-        // Going on with calculation
-        reputation = r;
-        let old_rating = reputation.total_rating;
-        let last_rating = reputation.last_rating;
-        let new_rating =
-            old_rating + (last_rating as f64 - old_rating) / (reputation.total_reviews as f64);
-
-        reputation.last_rating = rating;
-        reputation.total_reviews += 1;
-        // Format with two decimals
-        let new_rating = format!("{:.2}", new_rating).parse::<f64>()?;
-
-        // Assing new total rating to review
-        reputation.total_rating = new_rating;
-    } else {
-        reputation = Rating::new(1, rating as f64, rating, MIN_RATING, MAX_RATING);
+    // Store last rating
+    user_to_vote.last_rating = rating.into();
+    // Create new rating event
+    let reputation_event = Rating::new(
+        user_to_vote.total_reviews as u64,
+        user_to_vote.total_rating as f64,
+        user_to_vote.last_rating as u8,
+        user_to_vote.min_rating as u8,
+        user_to_vote.max_rating as u8,
+    )
+    .to_tags()?;
+
+    // Save new rating to db
+    if let Err(e) = update_user_rating(
+        pool,
+        user_to_vote.pubkey,
+        user_to_vote.last_rating,
+        user_to_vote.min_rating,
+        user_to_vote.max_rating,
+        user_to_vote.total_reviews,
+        user_to_vote.total_rating,
+    )
+    .await
+    {
+        return Err(Error::msg(format!("Error updating user rating : {}", e)));
     }
-    let reputation = reputation.to_tags()?;
 
     if buyer_rating || seller_rating {
         // Update db with rate flags
         update_user_rating_event(
-            &counterpart,
+            &counterpart_trade_pubkey,
             update_buyer_rate,
             update_seller_rate,
-            reputation,
+            reputation_event,
             order.id,
             my_keys,
             pool,

src/app/take_buy.rs

@@ -101,7 +101,9 @@ pub async fn take_buy_action(
         order.fee = fee;
     }
 
-    // Update trade index for seller
+    // Add seller identity pubkey to order
+    order.master_seller_pubkey = Some(event.sender.to_string());
+    // Add seller trade index to order
     order.trade_index_seller = msg.get_inner_message_kind().trade_index;
     // Timestamp order take time
     order.taken_at = Timestamp::now().as_u64() as i64;

src/app/take_sell.rs

@@ -127,6 +127,9 @@ pub async fn take_sell_action(
 
     // Add buyer pubkey to order
     order.buyer_pubkey = Some(buyer_trade_pubkey.to_string());
+    // Add buyer identity pubkey to order
+    order.master_buyer_pubkey = Some(event.sender.to_string());
+    // Add buyer trade index to order
     order.trade_index_buyer = msg.get_inner_message_kind().trade_index;
     // Timestamp take order time
     order.taken_at = Timestamp::now().as_u64() as i64;