feat: implement Phase 4 dev fee audit events via Nostr #559
Conversation
Add transparent audit trail for dev fee payments through Nostr events, enabling third-party verification and analytics while maintaining user privacy. Changes: - Add DEV_FEE_AUDIT_EVENT_KIND constant (38383) for audit events - Implement publish_dev_fee_audit_event() function with complete event creation - Integrate audit event publishing in scheduler after successful payments - Non-blocking implementation ensures payment reliability over audit completeness - Event includes order details, amounts, payment hash for transparency - Excludes buyer/seller pubkeys for privacy protection - Update documentation to mark Phase 4 as complete Event structure uses standard Nostr tags (y, z, order, amount, hash, t, currency, network) for queryability via any Nostr client. Events are published to configured relays after database verification.
WalkthroughAdds non-blocking Nostr audit event publishing for dev-fee payments: new Changes
Sequence Diagram(s)sequenceDiagram
actor Scheduler
participant Verifier as Payment Verifier
participant Audit as publish_dev_fee_audit_event
participant Keys as Signing Keys
participant NostrClient as Nostr Client
participant Relays as Nostr Relays
Scheduler->>Verifier: Verify dev-fee payment (order, payment_hash)
activate Verifier
Verifier->>Verifier: Update payment status
Verifier-->>Scheduler: Success
deactivate Verifier
rect rgb(230,245,230)
Note over Scheduler,Relays: Non-blocking audit publish (does not affect payment success)
Scheduler->>Audit: invoke publish_dev_fee_audit_event(order, payment_hash)
activate Audit
Audit->>Audit: Build event payload (order_id, dev_fee_sats, payment_hash, timestamp, tags...)
Audit->>Keys: sign event
activate Keys
Keys-->>Audit: signature
deactivate Keys
Audit->>NostrClient: send signed event
activate NostrClient
NostrClient->>Relays: publish event (kind 8383)
Relays-->>NostrClient: ack / response
deactivate NostrClient
Audit-->>Scheduler: Ok() / Err() (logged)
deactivate Audit
end
Scheduler->>Scheduler: Continue payment flow (regardless of audit result)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/util.rs (1)
554-592: Tag structure is comprehensive, but network tag is hardcoded.The queryable tags are well-designed for filtering and aggregation. However, the
networktag is hardcoded as "mainnet" (line 590), which could be incorrect if the instance runs on testnet, signet, or regtest.Recommendation: Consider making the network tag configurable via Settings or detecting it from the LND connection. For now, this can be deferred as an optional enhancement since most production instances will run on mainnet.
Would you like me to generate a refactor that adds a network configuration setting and uses it here?
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/DEV_FEE.mdsrc/config/constants.rssrc/scheduler.rssrc/util.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Runtime code lives in
src/directory with subdirectories:src/app/for order flows and business logic,src/lightning/for LND bindings and Lightning helpers,src/rpc/for gRPC service and types, andsrc/config/for settings and loaders
Files:
src/config/constants.rssrc/util.rssrc/scheduler.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Use 4-space indent,snake_casefor functions,PascalCasefor types, andSCREAMING_SNAKE_CASEfor constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash///documentation comments
Prefertracingspans over ad-hoc logging in Rust code
Co-locate tests in their modules undermod testsblocks and name tests descriptively
Files:
src/config/constants.rssrc/util.rssrc/scheduler.rs
🧠 Learnings (6)
📚 Learning: 2025-12-17T13:04:13.036Z
Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.
Applied to files:
src/config/constants.rssrc/util.rsdocs/DEV_FEE.md
📚 Learning: 2025-03-11T23:26:33.763Z
Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.
Applied to files:
src/util.rs
📚 Learning: 2025-06-25T22:48:44.741Z
Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.
Applied to files:
src/util.rs
📚 Learning: 2025-10-09T14:06:50.067Z
Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 530
File: src/app/order.rs:278-279
Timestamp: 2025-10-09T14:06:50.067Z
Learning: In the Mostro project, tests in files like `src/app/order.rs` may use structural checks to ensure functions like `order_action` do not panic during message construction, rather than validating specific error variants.
Applied to files:
src/util.rs
📚 Learning: 2025-06-25T22:48:44.741Z
Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.
Applied to files:
src/util.rs
📚 Learning: 2025-04-27T20:07:24.558Z
Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 475
File: src/db.rs:792-802
Timestamp: 2025-04-27T20:07:24.558Z
Learning: The decrypt_data function from mostro_core::order returns a type that needs to be wrapped with MostroInternalErr to convert it to MostroError, so calls should keep the .map_err(MostroInternalErr) conversion.
Applied to files:
src/util.rs
🧬 Code graph analysis (1)
src/scheduler.rs (1)
src/util.rs (4)
get_keys(617-627)get_nostr_relays(1119-1125)publish_dev_fee_audit_event(528-615)send_dm(464-509)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (7)
src/scheduler.rs (2)
20-23: LGTM: Import addition is clean and well-organized.The
publish_dev_fee_audit_eventfunction is correctly added to the existingutilmodule imports alongside related functions.
695-709: Excellent non-blocking audit implementation.The audit event publishing is correctly placed after database verification and follows best practices:
- Only publishes after confirmed database persistence
- Uses
verified_orderfrom database rather than in-memory state- Failures are logged as warnings without affecting payment flow
- Clear warning message guides manual review when needed
src/util.rs (3)
511-531: Excellent documentation and function signature.The function documentation is comprehensive, covering purpose, parameters, returns, and importantly, privacy considerations. The signature appropriately uses references to avoid unnecessary data copying.
532-552: Content structure is well-designed for audit purposes.The JSON content includes all essential audit information:
- Order identification and amount details
- Payment hash and timestamp for verification
- Destination address for transparency
- Hardcoded
"success"status is appropriate since function is only called after successful payment
594-614: Event creation and publishing logic is solid.The event is properly constructed, signed, and published with appropriate error handling and logging. The error propagation allows the caller (scheduler) to handle failures non-blockingly.
docs/DEV_FEE.md (2)
72-82: Phase 4 status correctly reflects implementation.The documentation accurately describes the non-blocking behavior and marks Phase 4 as complete, which aligns with the implemented code. However, note the event kind issue flagged in
src/config/constants.rsthat should be addressed.
1212-1219: Documentation correctly describes intended behavior.The rationale for using a regular (non-replaceable) event is well-justified:
- Complete history preservation
- Third-party auditing capabilities
- Total calculation support
- Immutable record keeping
However, this documentation highlights the critical issue: the code uses event kind 38383 (parameterized replaceable range) which contradicts these requirements. See the issue flagged in
src/config/constants.rs.Note: Documentation describes correct intended behavior, but implementation uses wrong event kind range (30000-39999 instead of 1000-9999).
Event kind 38383 was incorrectly in the parameterized replaceable events range (30000-39999) per NIP-01 and NIP-33. This would cause relays to replace events with the same "d" tag, defeating the audit trail purpose. Changed to kind 8383 which is in the regular events range (1000-9999), ensuring all dev fee payment events are preserved as separate, permanent records for complete audit history. Changes: - Update DEV_FEE_AUDIT_EVENT_KIND from 38383 to 8383 - Clarify comment referencing NIP-01 regular events range - Update all documentation references to use correct event kind
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/util.rs (1)
532-533: Consider moving imports to module level.Local imports inside functions are valid but unusual in Rust. Consider moving these to the module-level imports at the top of the file for consistency with project style.
🔎 Proposed refactor
Move to module-level imports (after line 15):
use crate::NOSTR_CLIENT; +use crate::config::constants::{DEV_FEE_AUDIT_EVENT_KIND, DEV_FEE_LIGHTNING_ADDRESS}; use chrono::Duration;Then remove from function body:
) -> Result<(), MostroError> { - use crate::config::constants::{DEV_FEE_AUDIT_EVENT_KIND, DEV_FEE_LIGHTNING_ADDRESS}; - use std::borrow::Cow; let ln_network = match LN_STATUS.get() {Note:
std::borrow::Cowcan remain local if only used in this function.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/util.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Runtime code lives in
src/directory with subdirectories:src/app/for order flows and business logic,src/lightning/for LND bindings and Lightning helpers,src/rpc/for gRPC service and types, andsrc/config/for settings and loaders
Files:
src/util.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Use 4-space indent,snake_casefor functions,PascalCasefor types, andSCREAMING_SNAKE_CASEfor constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash///documentation comments
Prefertracingspans over ad-hoc logging in Rust code
Co-locate tests in their modules undermod testsblocks and name tests descriptively
Files:
src/util.rs
🧠 Learnings (4)
📚 Learning: 2025-12-17T13:04:13.036Z
Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.
Applied to files:
src/util.rs
📚 Learning: 2025-03-11T23:26:33.763Z
Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.
Applied to files:
src/util.rs
📚 Learning: 2025-06-25T22:48:44.741Z
Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.
Applied to files:
src/util.rs
📚 Learning: 2025-10-09T14:06:50.067Z
Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 530
File: src/app/order.rs:278-279
Timestamp: 2025-10-09T14:06:50.067Z
Learning: In the Mostro project, tests in files like `src/app/order.rs` may use structural checks to ensure functions like `order_action` do not panic during message construction, rather than validating specific error variants.
Applied to files:
src/util.rs
🪛 GitHub Actions: CI
src/util.rs
[error] 572-572: cargo fmt --check failed due to formatting changes needed. Diff shows a reflow of Tag::custom(...) across lines; run 'cargo fmt --all' to fix code style issues in this file.
🔇 Additional comments (2)
src/util.rs (2)
528-610: Implementation correctly preserves privacy and handles errors.The function properly excludes buyer and seller pubkeys from the audit event (verified in the JSON content at lines 545-555), maintaining user privacy while providing transparency for dev fee payments. Error handling is appropriate at each failure point (keys, client, signing, publishing).
558-587: Fix formatting to pass CI checks.The CI pipeline reports that
cargo fmt --checkfailed at line 572. Runcargo fmt --allto automatically fix the formatting issues in the tag creation block.#!/bin/bash # Run cargo fmt to fix formatting cargo fmt --all⛔ Skipped due to learnings
Learnt from: CR Repo: MostroP2P/mostro PR: 0 File: AGENTS.md:0-0 Timestamp: 2025-12-23T17:33:03.588Z Learning: Link the motivating issue in pull requests, include `cargo test` output in PR descriptions, and call out schema or config changes
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
@grunch just read Json message is totally good imo, it cannot be forged with signature and I like the different kind to let users who wants to collect and control us to collect them quickly from relays. Only niptick, citing the rabbit, maybe I would write not only [
"EVENT",
"b54f3759-e362-4da9-9872-4ab6503efe0a",
{
"tags": [
[
"order-id",
"f639d735-308c-4045-b768-3b4a7039c527"
],
[
"amount",
"4"
],
[
"hash",
"7fcadb44d67acb09c7ba2fe62db67d86a7b182a214a24b00b9c8095505e114f0"
],
[
"currency",
"ARS"
],
[
"destination",
"pivotaldeborah52@walletofsatoshi.com"
],
[
"network",
"mainnet"
],
[
"y",
"mostro"
],
[
"z",
"dev-fee-payment"
]
],
"content": "",
"sig": "dc93c056e2eb93c080167b893046042e18e1a816f27f17bbbd603fb5596d4471e494881406595002480a85971802045f8ae05876e10587b3b33f0a4f29c2f0be",
"id": "f857e56db92c3885805d82902d3b10f9e1ea10cf6b61f7b502fab32b1d2771d1",
"pubkey": "dbe0b1be7aafd3cfba92d7463edbd4e33b2969f61bd554d37ac56f032e13355a",
"created_at": 1767121354,
"kind": 8383
}
`]`` |
|
No me compila, eliminé la db antigua que tenía y tampoco compila así: |
@Catrya this is the case where you have to call This is a manual thing still to do when database tables change, this time for dev-fee introduction. I was lookin a bit this morning how to automate this, but seems still a thing not completely possibile. Maybe on gh actions...will check... Please try a ./init-db.sh and then cargo build |
I think in order to avoid confusion we can remove the currency tag and just let the amount, audits can be done with the order-id |
I thought this change on sqlx-data.json would be enough, do you have any idea on why wasn't? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/util.rs (2)
511-591: LGTM! Documentation issue from previous review has been resolved.The function is well-implemented with proper error handling and privacy preservation. The documentation now correctly states "kind 8383" which matches the
DEV_FEE_AUDIT_EVENT_KINDconstant.💡 Optional improvement: Use tracing span for better observability
As per coding guidelines, consider using a tracing span instead of a single
info!log for better observability:pub async fn publish_dev_fee_audit_event( order: &Order, payment_hash: &str, ) -> Result<(), MostroError> { + let span = tracing::info_span!("publish_dev_fee_audit_event", order_id = %order.id, dev_fee = order.dev_fee); + let _enter = span.enter(); + use crate::config::constants::{DEV_FEE_AUDIT_EVENT_KIND, DEV_FEE_LIGHTNING_ADDRESS}; use std::borrow::Cow;This would provide better context in distributed tracing systems.
550-553: Consider validating dev_fee before publishing.The audit event will be published even if
order.dev_feeis 0. While this may be intentional for audit completeness, consider adding a validation check if zero-amount audit events are not meaningful.Optional defensive check
pub async fn publish_dev_fee_audit_event( order: &Order, payment_hash: &str, ) -> Result<(), MostroError> { use crate::config::constants::{DEV_FEE_AUDIT_EVENT_KIND, DEV_FEE_LIGHTNING_ADDRESS}; use std::borrow::Cow; + + if order.dev_fee <= 0 { + return Err(MostroInternalErr(ServiceError::InvalidAmount)); + } + let ln_network = match LN_STATUS.get() {
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/util.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
Runtime code lives in
src/directory with subdirectories:src/app/for order flows and business logic,src/lightning/for LND bindings and Lightning helpers,src/rpc/for gRPC service and types, andsrc/config/for settings and loaders
Files:
src/util.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Use 4-space indent,snake_casefor functions,PascalCasefor types, andSCREAMING_SNAKE_CASEfor constants in Rust code (Rust 2021 edition)
Document non-obvious public APIs with triple-slash///documentation comments
Prefertracingspans over ad-hoc logging in Rust code
Co-locate tests in their modules undermod testsblocks and name tests descriptively
Files:
src/util.rs
🧠 Learnings (5)
📚 Learning: 2025-12-17T13:04:13.036Z
Learnt from: grunch
Repo: MostroP2P/mostro PR: 554
File: docs/DEV_FEE_TECHNICAL_SPEC.md:1-434
Timestamp: 2025-12-17T13:04:13.036Z
Learning: In the Mostro development fee implementation, the Lightning address `mostro_p2psats.mobi` in src/config/constants.rs is temporary for testing purposes. The production address will be `developmentmostro.network` as documented in docs/DEV_FEE_TECHNICAL_SPEC.md. This intentional mismatch is planned to be resolved before production deployment.
Applied to files:
src/util.rs
📚 Learning: 2025-06-25T22:48:44.741Z
Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: The PaymentFailedInfo struct used in src/app/release.rs is defined in mostro-core PR #111, creating a cross-repository dependency that causes compilation errors until the dependency is resolved.
Applied to files:
src/util.rs
📚 Learning: 2025-03-11T23:26:33.763Z
Learnt from: bilthon
Repo: MostroP2P/mostro PR: 464
File: src/app/add_invoice.rs:73-82
Timestamp: 2025-03-11T23:26:33.763Z
Learning: In the Mostro codebase, the `update_order_event` function does not perform database operations - it only updates an order in memory and sends a Nostr event. The actual database update happens separately when calling `.update(pool)` afterward.
Applied to files:
src/util.rs
📚 Learning: 2025-10-09T14:06:50.067Z
Learnt from: arkanoider
Repo: MostroP2P/mostro PR: 530
File: src/app/order.rs:278-279
Timestamp: 2025-10-09T14:06:50.067Z
Learning: In the Mostro project, tests in files like `src/app/order.rs` may use structural checks to ensure functions like `order_action` do not panic during message construction, rather than validating specific error variants.
Applied to files:
src/util.rs
📚 Learning: 2025-06-25T22:48:44.741Z
Learnt from: Catrya
Repo: MostroP2P/mostro PR: 502
File: src/app/release.rs:52-55
Timestamp: 2025-06-25T22:48:44.741Z
Learning: PaymentFailedInfo struct and Payload::PaymentFailed variant are defined in mostro-core PR #111, creating a cross-repository dependency with the Mostro repository that requires careful coordination of merging and version updates.
Applied to files:
src/util.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: test
yes right! i just suggested the script to @Catrya because i thought she was more used to use it. It should the prepare command...we should find a way to automate this. |
Add transparent audit trail for dev fee payments through Nostr events, enabling third-party verification and analytics while maintaining user privacy.
Changes:
Event structure uses standard Nostr tags (y, z, order, amount, hash, t, currency, network) for queryability via any Nostr client. Events are published to configured relays after database verification.
Summary by CodeRabbit
Documentation
New Features
✏️ Tip: You can customize this high-level summary in your review settings.