Use npub as id, fix sending to nostr only contact (#544)

* Use npub as id, fix sending to nostr only contact

* Review fixes

* Review fixes

Author: tompro

CHANGELOG.md

@@ -1,38 +1,39 @@
 # 0.3.16
 
 * Set the BTC network in the identity and check, if the persisted network is the same as the one configured in the application, failing if it doesn't.
+* Nostr npub as primary key in Nostr contacts (breaking DB change)
 
 # 0.3.15
 
 * Upload and download files to and from Nostr using Blossom
-    * Add `nostr_hash` to `File` (breaking DB change)
+  * Add `nostr_hash` to `File` (breaking DB change)
 * Fix MintRequestResponse return type
 * Bill Caching for multiple identities (breaking DB change)
 
 # 0.3.14
 
 * Minting
-    * Added notifications and offers
-    * Added timestamps for status
-    * Call mint endpoint for cancelling
-    * Use mint nostr relays from network and fall back to identity ones
-    * Add endpoints to accept, or reject an offer from a mint
-    * Add logic to check keyset info, mint and create proofs
-    * Add logic to recover proofs if something goes wrong
-    * Add logic to check if proofs were spent
+  * Added notifications and offers
+  * Added timestamps for status
+  * Call mint endpoint for cancelling
+  * Use mint nostr relays from network and fall back to identity ones
+  * Add endpoints to accept, or reject an offer from a mint
+  * Add logic to check keyset info, mint and create proofs
+  * Add logic to recover proofs if something goes wrong
+  * Add logic to check if proofs were spent
 
 # 0.3.13
 
 * Minting
-    * Add default mint configuration options
-        * `default_mint_url`
-        * `default_mint_node_id`
-    * Implement `request_to_mint`
-    * Add minting status flag to bill
-    * Add endpoint to fetch minting status for a bill
-    * Add logic for checking mint request status on the mint
-    * Add cronjob to check mint requests
-    * Add endpoint to cancel mint requests
+  * Add default mint configuration options
+    * `default_mint_url`
+    * `default_mint_node_id`
+  * Implement `request_to_mint`
+  * Add minting status flag to bill
+  * Add endpoint to fetch minting status for a bill
+  * Add logic for checking mint request status on the mint
+  * Add cronjob to check mint requests
+  * Add endpoint to cancel mint requests
 * Change bitcoin addresses and descriptor to p2wpkh
 * Suppress logging from crates we don't control
 
@@ -48,36 +49,36 @@
 * Remove the `bcr-ebill-web` crate
 * Use a list of Nostr relays everywhere, instead of a single one, including in the config
 * Add Blank Endorse Bill data model implementation
-    * Rename `IdentityPublicData` to `BillIdentParticipant`
-        * same for `LightIdentityPublicData`
-    * Introduce the concept of `BillParticipant`, with the variants `Ident` and `Anon`
-        * `Anon` includes a `BillAnonParticipant`
-        * `Ident` includes a `BillIdentParticipant`
-    * Use `BillParticipant` in parts of the bill where a participant can be anonymous
+  * Rename `IdentityPublicData` to `BillIdentParticipant`
+    * same for `LightIdentityPublicData`
+  * Introduce the concept of `BillParticipant`, with the variants `Ident` and `Anon`
+    * `Anon` includes a `BillAnonParticipant`
+    * `Ident` includes a `BillIdentParticipant`
+  * Use `BillParticipant` in parts of the bill where a participant can be anonymous
 * Add the possibility to add anonymous contacts
-    * They only have Node Id, Name and E-Mail as fields
-    * E-Mail is optional
-    * This changes the data model for contacts, `email` and `postal_address` are now optional
-        * Additional validation rules ensure the fields can only be set for non-anon contacts
-    * Adds an endpoint `Api.contact().deanonymize()` to de-anonymize a contact by adding all necessary fields for a personal, or company contact
-        * It takes the same payload as creating a contact and fails for non-anon contacts
+  * They only have Node Id, Name and E-Mail as fields
+  * E-Mail is optional
+  * This changes the data model for contacts, `email` and `postal_address` are now optional
+    * Additional validation rules ensure the fields can only be set for non-anon contacts
+  * Adds an endpoint `Api.contact().deanonymize()` to de-anonymize a contact by adding all necessary fields for a personal, or company contact
+    * It takes the same payload as creating a contact and fails for non-anon contacts
 * Add the possibility to add an anonymous identity
-    * They only have Node Id, (nick)name and E-Mail as fields
-    * E-Mail is optional
-    * Adds an endpoint `Api.identity().deanonymize()` to de-anonymize an identity by adding all necessary fields for a personal identity
-        * It takes the same payload as creating an identity and fails for a non-anon identity
-    * Anon identity can't issue bills, or create a company
+  * They only have Node Id, (nick)name and E-Mail as fields
+  * E-Mail is optional
+  * Adds an endpoint `Api.identity().deanonymize()` to de-anonymize an identity by adding all necessary fields for a personal identity
+    * It takes the same payload as creating an identity and fails for a non-anon identity
+  * Anon identity can't issue bills, or create a company
 * Add the possibility to issue and endorse blank
-    * New `Api.bill()` methods for blank endorsements
-      * `issue_blank`
-      * `offer_to_sell_blank`
-      * `endorse_bill_blank`
-      * `mint_bill_blank`
-    * Can issue (non-self-drafted) blank bills (payee is anon)
-    * Can endorse/mint/offer to sell to anon endorsee/mint/buyer
-    * If caller of a bill action is anonymous in the bill, any action they take stay anonymous (e.g. endorse)
+  * New `Api.bill()` methods for blank endorsements
+    * `issue_blank`
+    * `offer_to_sell_blank`
+    * `endorse_bill_blank`
+    * `mint_bill_blank`
+  * Can issue (non-self-drafted) blank bills (payee is anon)
+  * Can endorse/mint/offer to sell to anon endorsee/mint/buyer
+  * If caller of a bill action is anonymous in the bill, any action they take stay anonymous (e.g. endorse)
 * Add endpoint to check payment for singular bill
-    * `Api.bill().check_payment_for_bill(id)`
+  * `Api.bill().check_payment_for_bill(id)`
 * Fix TS type for identity detail
 * Return identity on `create` and `deanonymize` identity for consistency
 
@@ -88,14 +89,14 @@
 * Reduce size of the WASM binary
 * Fix a small issue, where bills were recalculated instead of taken from cache, once their payment/sell/recourse/accept requests expired
 * Change behaviour of request to pay
-    * it's now possible to req to pay before the maturity date
-    * The actual payment expiry still only happens 2 workdays after the end of the maturity date,
+  * it's now possible to req to pay before the maturity date
+  * The actual payment expiry still only happens 2 workdays after the end of the maturity date,
     or end of the req to pay end of day if that was after the maturity date
-    * the `request_to_pay_timed_out` flag is set after payment expired, not after the req to pay expired
-    * The waiting state for payment is only active during the req to pay (while it's blocked)
-      * Afterwards, the bill is not blocked anymore, can still be rejected to pay and paid
-      * But recourse is only possible after the payment expired (after maturity date)
-    * An expired req to pay, which expired before the maturity date does not show up in `past_payments`
+  * the `request_to_pay_timed_out` flag is set after payment expired, not after the req to pay expired
+  * The waiting state for payment is only active during the req to pay (while it's blocked)
+    * Afterwards, the bill is not blocked anymore, can still be rejected to pay and paid
+    * But recourse is only possible after the payment expired (after maturity date)
+  * An expired req to pay, which expired before the maturity date does not show up in `past_payments`
 
 # 0.3.9
 
@@ -130,7 +131,7 @@
 
 * Add in-depth tests for bill validation
 * Add recourse reason to `Recourse` block data
-    * (breaks existing persisted bills, if they had a recourse block)
+  * (breaks existing persisted bills, if they had a recourse block)
 * Added `has_requested_funds` flag to `BillStatusWeb`, indicating the caller has requested funds (req to pay, req to recourse, offer to sell) at some point
 * Added `past_payments` endpoint to `Api.bill()`, which returns data about past payments and payment requests where the caller was the beneficiary
 
@@ -142,7 +143,7 @@
 * Add bill action validation for incoming blocks
 * Add signer verification for incoming blocks
 * Add recourse reason to `RequestRecourse` block data
-    * (breaks existing persisted bills, if they had a request recourse block)
+  * (breaks existing persisted bills, if they had a request recourse block)
 * Move bill validation logic to `bcr-ebill-core`
 
 # 0.3.2
@@ -172,7 +173,7 @@ can be passed - a list of file upload ids, to upload multiple files for one bill
 * Restructured `BitcreditBillWeb` to a more structured approach, separating `status`,
 `data` and `participants` and adding the concept of `current_waiting_state`, to
 have all data available, if the bill is in a waiting state.
-    * Added the concept of `redeemed_funds_available` on `status`, to indicate if
+  * Added the concept of `redeemed_funds_available` on `status`, to indicate if
     the caller has funds available (e.g. from a sale, or a paid bill)
 
 # 0.3.0

crates/bcr-ebill-api/src/service/contact_service.rs

@@ -7,12 +7,11 @@ use bcr_ebill_core::{
         BillParticipant,
         validation::{validate_create_contact, validate_update_contact},
     },
-    nostr_contact::{NostrContact, TrustLevel},
+    nostr_contact::{NostrContact, NostrPublicKey, TrustLevel},
 };
 use bcr_ebill_persistence::nostr::NostrContactStoreApi;
 #[cfg(test)]
 use mockall::automock;
-pub use nostr::key::PublicKey;
 
 use crate::{
     Config,
@@ -100,10 +99,13 @@ pub trait ContactServiceApi: ServiceTraitBounds {
     ) -> Result<Contact>;
 
     /// Returns whether a given npub (as hex) is in our contact list.
-    async fn is_known_npub(&self, npub: &PublicKey) -> Result<bool>;
+    async fn is_known_npub(&self, npub: &NostrPublicKey) -> Result<bool>;
 
     /// Returns the Npubs we want to subscribe to on Nostr.
-    async fn get_nostr_npubs(&self) -> Result<Vec<PublicKey>>;
+    async fn get_nostr_npubs(&self) -> Result<Vec<NostrPublicKey>>;
+
+    /// Returns a Nostr contact by node id if we have a trusted one.
+    async fn get_nostr_contact_by_node_id(&self, node_id: &str) -> Result<Option<NostrContact>>;
 
     /// opens and decrypts the attached file from the given contact
     async fn open_and_decrypt_file(
@@ -193,7 +195,7 @@ impl ContactService {
             .await?
         {
             Some(nostr_contact) => nostr_contact.merge_contact(contact),
-            None => NostrContact::from_contact(contact),
+            None => NostrContact::from_contact(contact)?,
         };
         self.nostr_contact_store.upsert(&nostr_contact).await?;
         Ok(())
@@ -570,7 +572,7 @@ impl ContactServiceApi for ContactService {
         Ok(contact)
     }
 
-    async fn is_known_npub(&self, npub: &PublicKey) -> Result<bool> {
+    async fn is_known_npub(&self, npub: &NostrPublicKey) -> Result<bool> {
         Ok(!self.config.nostr_config.only_known_contacts
             || self
                 .nostr_contact_store
@@ -581,13 +583,21 @@ impl ContactServiceApi for ContactService {
     }
 
     /// Returns the Npubs we want to subscribe to on Nostr.
-    async fn get_nostr_npubs(&self) -> Result<Vec<PublicKey>> {
+    async fn get_nostr_npubs(&self) -> Result<Vec<NostrPublicKey>> {
         Ok(self
             .nostr_contact_store
             .get_npubs(vec![TrustLevel::Trusted, TrustLevel::Participant])
             .await?)
     }
 
+    /// Returns a Nostr contact by node id if we have a trusted or participant one.
+    async fn get_nostr_contact_by_node_id(&self, node_id: &str) -> Result<Option<NostrContact>> {
+        match self.nostr_contact_store.by_node_id(node_id).await {
+            Ok(Some(c)) if c.trust_level != TrustLevel::None => Ok(Some(c)),
+            _ => Ok(None),
+        }
+    }
+
     async fn open_and_decrypt_file(
         &self,
         contact: Contact,
@@ -645,7 +655,7 @@ pub mod tests {
             empty_optional_address, init_test_cfg,
         },
     };
-    use bcr_ebill_core::nostr_contact::HandshakeStatus;
+    use bcr_ebill_core::nostr_contact::{HandshakeStatus, NostrPublicKey};
     use std::collections::HashMap;
     use util::BcrKeys;
 
@@ -1040,10 +1050,10 @@ pub mod tests {
     async fn is_known_npub_calls_store() {
         let (store, file_upload_store, file_upload_client, identity_store, mut nostr_contact) =
             get_storages();
-        let pub_key = PublicKey::from_hex(TEST_NODE_ID_SECP_AS_NPUB_HEX).unwrap();
+        let pub_key = NostrPublicKey::from_hex(TEST_NODE_ID_SECP_AS_NPUB_HEX).unwrap();
         nostr_contact.expect_by_npub().returning(|_| {
             Ok(Some(NostrContact {
-                node_id: TEST_NODE_ID_SECP.to_string(),
+                npub: NostrPublicKey::parse(TEST_NODE_ID_SECP_AS_NPUB_HEX).unwrap(),
                 name: None,
                 relays: vec![],
                 trust_level: TrustLevel::Participant,

crates/bcr-ebill-api/src/service/notification_service/default_service.rs

@@ -4,7 +4,7 @@ use std::sync::Arc;
 use async_trait::async_trait;
 use bcr_ebill_core::blockchain::BlockchainType;
 use bcr_ebill_core::blockchain::bill::block::NodeId;
-use bcr_ebill_core::contact::{BillParticipant, ContactType};
+use bcr_ebill_core::contact::{BillAnonParticipant, BillParticipant, ContactType};
 use bcr_ebill_persistence::nostr::{
     NostrChainEvent, NostrChainEventStoreApi, NostrQueuedMessage, NostrQueuedMessageStoreApi,
 };
@@ -87,6 +87,17 @@ impl DefaultNotificationService {
                     self.contact_service.get_identity_by_node_id(node_id).await
                 {
                     Some(identity)
+                } else if let Ok(Some(nostr)) = self
+                    .contact_service
+                    .get_nostr_contact_by_node_id(node_id)
+                    .await
+                {
+                    // we have no contact but a nostr contact of a participant
+                    Some(BillParticipant::Anon(BillAnonParticipant {
+                        node_id: node_id.to_string(),
+                        email: None,
+                        nostr_relays: nostr.relays,
+                    }))
                 } else {
                     None
                 }

crates/bcr-ebill-api/src/service/notification_service/nostr.rs

@@ -372,11 +372,7 @@ impl NotificationJsonTransportApi for NostrClient {
         &self,
         node_id: &str,
     ) -> Result<Option<bcr_ebill_transport::transport::NostrContactData>> {
-        if let Ok(npub) = crypto::get_nostr_npub_as_hex_from_node_id(node_id) {
-            let public_key = PublicKey::from_str(&npub).map_err(|e| {
-                error!("Failed to parse Nostr npub when sending a notification: {e}");
-                Error::Crypto("Failed to parse Nostr npub".to_string())
-            })?;
+        if let Ok(public_key) = crypto::get_npub_from_node_id(node_id) {
             match self.fetch_metadata(public_key).await? {
                 Some(meta) => {
                     let relays = self

crates/bcr-ebill-api/src/tests/mod.rs

@@ -16,7 +16,7 @@ pub mod tests {
         contact::{BillIdentParticipant, BillParticipant, Contact, ContactType},
         identity::{ActiveIdentityState, Identity, IdentityType, IdentityWithAll},
         mint::{MintOffer, MintRequest, MintRequestStatus},
-        nostr_contact::{HandshakeStatus, NostrContact, TrustLevel},
+        nostr_contact::{HandshakeStatus, NostrContact, NostrPublicKey, TrustLevel},
         notification::{ActionType, Notification, NotificationType},
         util::crypto::BcrKeys,
     };
@@ -119,12 +119,12 @@ pub mod tests {
         #[async_trait]
         impl NostrContactStoreApi for NostrContactStore {
             async fn by_node_id(&self, node_id: &str) -> Result<Option<NostrContact>>;
-            async fn by_npub(&self, npub: &nostr::key::PublicKey) -> Result<Option<NostrContact>>;
+            async fn by_npub(&self, npub: &NostrPublicKey) -> Result<Option<NostrContact>>;
             async fn upsert(&self, data: &NostrContact) -> Result<()>;
             async fn delete(&self, node_id: &str) -> Result<()>;
             async fn set_handshake_status(&self, node_id: &str, status: HandshakeStatus) -> Result<()>;
             async fn set_trust_level(&self, node_id: &str, trust_level: TrustLevel) -> Result<()>;
-            async fn get_npubs(&self, levels: Vec<TrustLevel>) -> Result<Vec<nostr::key::PublicKey>>;
+            async fn get_npubs(&self, levels: Vec<TrustLevel>) -> Result<Vec<NostrPublicKey>>;
         }
     }
 

crates/bcr-ebill-core/src/nostr_contact.rs

@@ -2,13 +2,16 @@ use std::collections::BTreeSet;
 
 use serde::{Deserialize, Serialize};
 
-use crate::contact::Contact;
+use crate::{contact::Contact, util::crypto::get_npub_from_node_id};
+
+/// Make key type clear
+pub type NostrPublicKey = nostr::key::PublicKey;
 
 /// Data we need to communicate with a Nostr contact.
 #[derive(Debug, Clone, Serialize, Deserialize)]
 pub struct NostrContact {
     /// Our node id. This is the node id and acts as the primary key.
-    pub node_id: String,
+    pub npub: NostrPublicKey,
     /// The Nostr name of the contact as retreived via Nostr metadata.
     pub name: Option<String>,
     /// The relays we found for this contact either from a message or the result of a relay list
@@ -23,14 +26,15 @@ pub struct NostrContact {
 impl NostrContact {
     /// Creates a new Nostr contact from a contact. This is used when we have a contact and want to
     /// create the Nostr contact from it. Handshake is set to complete and we trust the contact.
-    pub fn from_contact(contact: &Contact) -> Self {
-        Self {
-            node_id: contact.node_id.clone(),
+    pub fn from_contact(contact: &Contact) -> crate::util::crypto::Result<Self> {
+        let npub = get_npub_from_node_id(contact.node_id.as_str())?;
+        Ok(Self {
+            npub,
             name: Some(contact.name.clone()),
             relays: contact.nostr_relays.clone(),
             trust_level: TrustLevel::Trusted,
             handshake_status: HandshakeStatus::Added,
-        }
+        })
     }
 
     /// Merges contact data into a nostr contact. This assumes at that point the handskake is
@@ -39,7 +43,7 @@ impl NostrContact {
         let mut relays: BTreeSet<String> = BTreeSet::from_iter(self.relays.clone());
         relays.extend(contact.nostr_relays.clone());
         Self {
-            node_id: self.node_id.clone(),
+            npub: self.npub,
             name: Some(contact.name.clone()),
             relays: relays.into_iter().collect(),
             trust_level: TrustLevel::Trusted,