Identity chains

[tompro]

User description

📝 Description

Publish public block messages for identity and company chains. Also publishes company chain invites in case a signatory has been added to a company.

Relates to #511 #513

---

✅ Checklist

Please ensure the following tasks are completed before requesting a review:

• My code adheres to the coding guidelines of this project.
• I have run cargo fmt.
• I have run cargo clippy.
• I have added or updated tests (if applicable).
• All CI/CD steps were successful.
• I have updated the documentation (if applicable).
• I have checked that there are no console errors or warnings.
• I have verified that the application builds without errors.
• I've described the changes made to the API. (modification, addition, deletion).

---

🚀 Changes Made

• New Features:
  • Publish public chain events for personal identity
  • Publish public chain events and invites for company

---

💡 How to Test

Please provide clear instructions on how reviewers can test your changes:

1. Create identity or company
2. Observe logs about publishing of either identity or company messages

---

🤝 Related Issues

List any related issues, pull requests, or discussions:

• Public messages on Nostr for Private Identity #511
• Public messages on Nostr for Company Identity #513

---

📋 Review Guidelines

Please focus on the following while reviewing:

• Does the code follow the repository's contribution guidelines?
• Are there any potential bugs or performance issues?
• Are there any typos or grammatical errors in the code or comments?

---

PR Type

Enhancement

---

Description

• Publish public chain events for identity and company blockchains

• Add dynamic Nostr transport creation for companies

• Send company invite events to new signatories

• Integrate notification service into identity and company services

---

Changes diagram

flowchart LR
  A["Identity/Company Service"] --> B["Notification Service"]
  B --> C["Nostr Transport"]
  C --> D["Public Chain Events"]
  C --> E["Private Invite Events"]
  D --> F["Identity Chain Messages"]
  D --> G["Company Chain Messages"]
  E --> H["Company Signatory Invites"]

Loading

---

Changes walkthrough 📝

	Relevant files
Enhancement	10 files company_service.rs Integrate notification service for company chain events    +212/-5  identity_service.rs Add identity chain event publishing                                            +64/-12  default_service.rs Implement identity and company chain event handlers            +296/-84 bill_events.rs Update event creation method names for consistency              +3/-3      blockchain_event.rs Add identity and company blockchain event structures          +19/-3    company_events.rs Create company chain event generator                                          +78/-0    identity_events.rs Create identity chain event generator                                        +40/-0    mod.rs Add new event types and creation methods                                  +24/-3    notification_service.rs Add notification service API for chain events                        +15/-2    context.rs Inject notification service into identity and company services +2/-0
Miscellaneous	3 files nostr.rs Reduce logging verbosity for metadata events                          +3/-3      bill_chain_event_handler.rs Update import path for blockchain event                                    +1/-1      bill_invite_handler.rs Update import path and event creation calls                            +5/-5
Tests	1 files mod.rs Add mock methods for new notification service APIs              +6/-1

---

Need help?

Type /help how to ... in the comments thread for any questions about Qodo Merge usage.

Check out the documentation for more information.

[Copilot]

Pull Request Overview

This PR adds support for publishing public block events and invites for identity and company chains over Nostr, alongside extending the notification service API.

• Injects notification_service into Context, IdentityService, and CompanyService.
• Defines new event types/modules (identity_events, company_events, blockchain_event).
• Extends NotificationServiceApi with add_company_transport, send_identity_chain_events, and send_company_chain_events and implements them in DefaultNotificationService.

Reviewed Changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
crates/bcr-ebill-wasm/src/context.rs	Added notification_service clone to service constructors.
crates/bcr-ebill-transport/src/notification_service.rs	Extended NotificationServiceApi trait with company/identity chain methods.
crates/bcr-ebill-transport/src/event/blockchain_event.rs	Consolidated all block event types into a single module.
crates/bcr-ebill-transport/src/event/identity_events.rs	Added IdentityChainEvent definition.
crates/bcr-ebill-transport/src/event/company_events.rs	Added CompanyChainEvent definition.
crates/bcr-ebill-transport/src/event/mod.rs	Exposed new event modules/types.
crates/bcr-ebill-transport/src/handler/*	Updated handlers to use renamed event constructors.
crates/bcr-ebill-api/src/service/notification_service/default_service.rs	Implemented new chain event methods and refactored transport map.
crates/bcr-ebill-api/src/service/{identity,company}_service.rs	Populating and sending chain events after blocks.
crates/bcr-ebill-api/src/tests/mod.rs	Updated mocks for new methods on NotificationServiceApi.

Comments suppressed due to low confidence (1)

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

• In this impl NotificationServiceApi for NotificationService in tests, the async function signatures end with semicolons and lack bodies. This will not compile; you need to provide function bodies or remove the incorrect impl block.

            async fn add_company_transport(&self, company: &Company, keys: &BcrKeys) -> bcr_ebill_transport::Result<()>;

[codecov]

Codecov Report

Attention: Patch coverage is 59.74684% with 159 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...rc/service/notification_service/default_service.rs	53.55%	111 Missing ⚠️
...es/bcr-ebill-transport/src/event/company_events.rs	39.02%	25 Missing ⚠️
...s/bcr-ebill-transport/src/event/identity_events.rs	44.44%	10 Missing ⚠️
crates/bcr-ebill-transport/src/event/mod.rs	30.76%	9 Missing ⚠️
...bill-api/src/service/notification_service/nostr.rs	0.00%	2 Missing ⚠️
crates/bcr-ebill-wasm/src/context.rs	0.00%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

[qodo-code-review]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅ 511 - PR Code Verified Compliant requirements: Publish public messages on Nostr for Private Identity Enable identity chain events to be published publicly Requires further human verification: Verify that identity messages are properly published to Nostr relays Test that identity chain events contain correct data 513 - PR Code Verified Compliant requirements: Publish public messages on Nostr for Company Identity Enable company chain events to be published publicly Send company invite events when signatories are added Requires further human verification: Verify that company messages are properly published to Nostr relays Test that company invite events are sent to correct recipients Validate that company transport clients are properly created

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Thread Safety The notification_transport field is wrapped in Mutex but accessed frequently. This could create contention and performance bottlenecks, especially when multiple operations try to send notifications simultaneously. notification_transport: Mutex<HashMap<NodeId, Arc<dyn NotificationJsonTransportApi>>>, notification_store: Arc<dyn NotificationStoreApi>, Error Handling The populate_block method and notification service calls lack proper error handling context. If notification sending fails, it's unclear whether the blockchain operations should be rolled back or continue. async fn populate_block( &self, company: &Company, chain: &CompanyBlockchain, keys: &CompanyKeys, new_signatory: Option<NodeId>, ) -> Result<()> { self.notification_service .send_company_chain_events(CompanyChainEvent::new( company, chain, keys, new_signatory, true, )) .await?; Ok(()) } Resource Management Dynamic creation of NostrClient instances in add_company_client method may lead to resource leaks if clients are not properly cleaned up when companies are removed or updated. async fn add_company_client(&self, company: &Company, keys: &BcrKeys) -> Result<()> { let config = get_config(); let node_id = NodeId::new(keys.pub_key(), get_config().bitcoin_network()); let mut transports = self.notification_transport.lock().await; if transports.contains_key(&node_id) { debug!("transport for node {node_id} already present"); return Ok(()); } let nostr_config = NostrConfig::new( keys.clone(), config.nostr_config.relays.clone(), company.name.clone(), false, node_id.clone(), ); if let Ok(client) = NostrClient::new(&nostr_config).await { debug!("added nostr client for {}", &nostr_config.get_npub()); transports.insert(node_id, Arc::new(client)); } Ok(()) }

[qodo-code-review]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
General	Avoid duplicate config retrieval calls The method calls get_config() twice - once to store in config variable and again to get the bitcoin network. This is inefficient and could lead to inconsistency if config changes between calls. Use the already stored config variable for the second call. crates/bcr-ebill-api/src/service/notification_service/default_service.rs [128-131] async fn add_company_client(&self, company: &Company, keys: &BcrKeys) -> Result<()> { let config = get_config(); - let node_id = NodeId::new(keys.pub_key(), get_config().bitcoin_network()); + let node_id = NodeId::new(keys.pub_key(), config.bitcoin_network()); ... } [To ensure code accuracy, apply this suggestion manually] Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a redundant call to get_config() and proposes a good practice of reusing the existing config variable, improving code quality and consistency.	Low
	Use next() instead of nth(0) Using nth(0) is less idiomatic than first() for getting the first element of an iterator. The first() method is more explicit about the intent and is the preferred way to get the first element. crates/bcr-ebill-api/src/service/notification_service/default_service.rs [828-838] async fn resolve_contact(&self, node_id: &NodeId) -> Result<Option<NostrContactData>> { validate_node_id_network(node_id)?; let transports = self.notification_transport.lock().await; // take any transport - doesn't matter - if let Some((_node, transport)) = transports.iter().nth(0) { + if let Some((_node, transport)) = transports.iter().next() { let res = transport.resolve_contact(node_id).await?; Ok(res) } else { Ok(None) } } Apply / Chat Suggestion importance[1-10]: 4 __ Why: The suggestion correctly points out that using next() is more idiomatic and explicit than nth(0) for retrieving the first item from an iterator, which improves code readability.	Low
More

[zupzup]

Very nice 💪