Event-kinds-update #154
Event-kinds-update #154
Conversation
Update mostro-core to 0.7.0 which introduces separate Nostr event kinds for orders (38383), disputes (38386), ratings (38384), and info (38385). All event kind constants are now imported from mostro-core::prelude and used consistently throughout the codebase. No hardcoded event kind values. BREAKING CHANGE: Requires mostro-core 0.7.0+
WalkthroughBumps multiple Rust crate versions, adds extensive Mostro CLI documentation (keys, orders, messaging, disputes, DB, coding standards), switches event-filter construction to numeric Nostr kinds, and changes key-derivation account kind from replaceable-event to order-event. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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: 3
🤖 Fix all issues with AI agents
In `@docs/DATABASE_SCHEMA.md`:
- Around line 15-29: The orders table documentation is missing columns that
exist in the actual schema; update the `orders` table docs to match the schema
defined in src/db.rs by adding entries for min_amount, max_amount,
payment_method, premium, counterparty_pubkey, buyer_invoice, request_id, and
expires_at (include types and short descriptions consistent with the DB
definitions), and verify other existing columns (e.g., trade_keys, fiat_amount)
match their types in src/db.rs to keep the docs and the `orders` table schema
synchronized.
In `@docs/KEY_IMPLEMENTATION.md`:
- Around line 66-74: Clarify the apparent contradiction by updating the DO/DON'T
section to state that identity keys (identity key / identity signing) are only
to be used for explicit authentication or registration workflows (e.g., proving
account ownership), while all public trade-related events must be authored and
signed with trade-specific keys obtained via get_next_trade_keys(); explicitly
mention that pubkey in public events must refer to the trade key, not the
identity key, and keep guidance about never reusing trade keys and updating
last_trade_index after creating a trade event.
In `@docs/TRADE_LIFECYCLE.md`:
- Around line 7-39: The fenced diagram block in TRADE_LIFECYCLE.md is missing a
language tag which triggers MD040; update the opening triple-backtick fence that
surrounds the ASCII trade lifecycle diagram to include a language tag (e.g.,
text) so it becomes ```text, ensuring the diagram renders consistently and
linter warnings are resolved.
🧹 Nitpick comments (1)
docs/KEY_MANAGEMENT.md (1)
1-94: Excellent key management documentation with clear privacy rationale.This document provides comprehensive coverage of:
- HD key hierarchy using BIP-32/44 standards
- Clear distinction between identity keys (index 0) and trade keys (index 1+)
- Privacy benefits through key separation
- Practical usage patterns
The use of
38383'(NOSTR_ORDER_EVENT_KIND) in the BIP-44 path (line 26) correctly aligns with the event kinds update in this PR.Optional: Add language specifiers to fenced code blocks for markdown linting compliance.
The visual diagrams at lines 11, 61, and 72 could be marked with a language specifier (e.g.,
text) to satisfy markdown linters, though this is purely cosmetic since they're intentional visual representations.📝 Optional markdown linting fix
For line 11, change:
-``` +```text Master Seed (BIP-39 mnemonic - 12 words)For line 61, change:
-``` +```text ❌ Without trade keys (bad for privacy):For line 72, change:
-``` +```text ✅ With trade keys (good for privacy):Based on learnings: The documentation correctly identifies index 0 as the identity key and 1+ as trade keys, which aligns with the implementation where index 0 should be skipped when iterating through trade keys.
| ### DO ✅ | ||
| - **Use unique keys**: Always use `get_next_trade_keys()` for new orders. | ||
| - **Sign with identity**: Prove authenticity while maintaining sender privacy. | ||
| - **Update indices**: Ensure `last_trade_index` is updated after successful creation. | ||
|
|
||
| ### DON'T ❌ | ||
| - **Reuse keys**: Never use the same trade key for two different orders. | ||
| - **Author with identity**: Never set the `pubkey` of a public event to your identity key. | ||
| - **Lose mnemonic**: Keys cannot be recovered without the seed phrase. |
There was a problem hiding this comment.
Clarify identity vs trade-key signing guidance.
“Sign with identity” and “Never set the pubkey of a public event to your identity key” read as contradictory in Nostr. Please clarify when identity signing is appropriate (e.g., authentication/registration only) and that public events should be authored/signed with trade keys.
✏️ Suggested rewording
-### DO ✅
-- **Use unique keys**: Always use `get_next_trade_keys()` for new orders.
-- **Sign with identity**: Prove authenticity while maintaining sender privacy.
-- **Update indices**: Ensure `last_trade_index` is updated after successful creation.
+### DO ✅
+- **Use unique keys**: Always use `get_next_trade_keys()` for new orders.
+- **Sign public events with trade keys**: Preserve privacy by authoring events with per-trade keys.
+- **Use identity key only for authentication/registration** (if required by protocol).
+- **Update indices**: Ensure `last_trade_index` is updated after successful creation.🤖 Prompt for AI Agents
In `@docs/KEY_IMPLEMENTATION.md` around lines 66 - 74, Clarify the apparent
contradiction by updating the DO/DON'T section to state that identity keys
(identity key / identity signing) are only to be used for explicit
authentication or registration workflows (e.g., proving account ownership),
while all public trade-related events must be authored and signed with
trade-specific keys obtained via get_next_trade_keys(); explicitly mention that
pubkey in public events must refer to the trade key, not the identity key, and
keep guidance about never reusing trade keys and updating last_trade_index after
creating a trade event.
There was a problem hiding this comment.
that's correct! Identity key is used to seal the nip59 giftwrap, you can read about here. So identity is used in every message but never revealed.
Read about here --> https://mostro.network/protocol/key_management.html
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
mostro.network protocol key management NIP-59 giftwrap
💡 Result:
Summary — how Mostro uses NIP-59 (Gift Wrap) for key management
-
Mostro clients generate new (throwaway) Nostr private keys per order/operation and use those ephemeral keys to publish NIP‑59 “gift wrap” events so buyers/sellers and Mostrod cannot be linked to a single long‑lived key. This is explicit in Mostro’s protocol/FAQ and privacy docs. [1][2]
-
Mechanism: Mostro wraps a rumor (unsigned inner event) inside a Seal (signed by the true author), then places that encrypted Seal inside a Gift Wrap (outer event) signed with the ephemeral key. Encryption uses NIP‑44 conventions (conversation/shared‑key derived via ECDH). This three‑layer pattern (rumor → seal → gift wrap) is the NIP‑59 design Mostro implements. Clients should rotate throwaway keys, randomize timestamps, and publish only to recipient relays where possible to reduce metadata leakage. [3][4]
-
Practical notes: Mostro’s implementation automates key generation/rotation for each order; private keys are not shared with the Mostro instance; the gift‑wrap content is NIP‑44 encrypted and the seal reveals the true author only inside the encrypted seal. For developer examples and SDK usage (wrapping/unwrapping), see NDK/Rust examples referenced in the NIP‑59 docs and Mostro docs. [3][4][1]
Sources
[1] Mostro — “How it works” / order flow (key rotation per order).
[2] Mostro — “Privacy in Mostro” (explains ephemeral keys / not sharing private keys).
[3] NIP‑59 specification (Gift Wrap; rumor → seal → gift wrap; uses NIP‑44).
[4] Kind 1059 / implementation notes and SDK examples (gift‑wrap usage).
@arkanoider, thank you for the clarification! That makes perfect sense now. The identity key signs the encrypted seal within the NIP-59 giftwrap, so while it authenticates every message, it's never exposed publicly. The public events are authored with trade keys, maintaining privacy.
Your current documentation is technically correct, but it might be helpful to add a brief note explicitly mentioning the NIP-59 giftwrap mechanism to make this clearer for readers unfamiliar with the protocol. Something like:
Sign with identity: Prove authenticity via NIP-59 seal (encrypted, never publicly revealed) while maintaining sender privacy.
This would help prevent confusion like mine! But the current version is accurate given the link you provided for additional context.
✏️ Learnings added
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 154
File: docs/KEY_IMPLEMENTATION.md:70-80
Timestamp: 2026-01-17T13:10:54.496Z
Learning: In mostro-cli, the identity key is used to sign the NIP-59 giftwrap seal for authentication. The seal is encrypted within the giftwrap, so the identity key signature is never publicly revealed while still proving message authenticity. Public events are authored with trade keys for privacy.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli, index 0 represents the master key and should be skipped when iterating through trade keys. The loop should start from index 1 when fetching user trade keys in get_dm.rs.
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli.rs:301-306
Timestamp: 2025-09-12T20:02:14.291Z
Learning: The mostro-cli is designed as a test client that allows users to test both regular user operations and admin operations from the terminal. Therefore, MOSTRO_PUBKEY should always be required regardless of whether NSEC_PRIVKEY is present, as both user and admin operations need to know which Mostro instance to interact with.
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 146
File: src/cli/dm_to_user.rs:25-32
Timestamp: 2025-10-18T11:48:11.944Z
Learning: In mostro-cli's DM functionality (dm_to_user.rs and similar), message content consists of ephemeral public keys that change every time and contains no sensitive personal data. Logging message content to stdout is acceptable and not a privacy concern.
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/add_invoice.rs:57-79
Timestamp: 2025-09-09T19:18:57.161Z
Learning: arkanoider prefers to bubble up errors with anyhow::Result instead of using tokio::spawn with eprintln! error handling in Nostr DM sending scenarios, as the spawn is often overkill for simple send operations.
Summary
This PR upgrades
mostro-clito support mostro-core 0.7.0 with distinct Nostr event kinds and adds comprehensive modular documentation for AI-assisted development.Changes
🔧 Core Upgrade: mostro-core 0.7.0
mostro-core = "0.7.0"src/util/events.rsto use correct event kindsNOSTR_ORDER_EVENT_KINDfor BIP-44 paths📚 Documentation Structure
Added modular, AI-friendly documentation (all files < 400 lines):
Navigation & Standards:
docs/README.md- Central documentation hubdocs/CODING_STANDARDS.md- Rust best practices and conventionsKey Management:
docs/KEY_MANAGEMENT.md- Identity vs trade keys overviewdocs/KEY_IMPLEMENTATION.md- BIP-44 derivation technical detailsTrading Operations:
docs/CREATING_ORDERS.md- Creating buy/sell orders guidedocs/ORDER_FLOW_TECHNICAL.md- Internal message flow and structuresdocs/TAKING_ORDERS.md- Taking orders (takesell/takebuy) guidedocs/TRADE_LIFECYCLE.md- Step-by-step trade examplesSystem Operations:
docs/DISPUTE_MANAGEMENT.md- User and admin dispute flowsdocs/MESSAGE_PROTOCOL.md- NIP-59/17 encryption detailsdocs/DATABASE_SCHEMA.md- SQLite schema and modelsBenefits
Testing
Migration Notes
This is a breaking change due to the mostro-core 0.7.0 upgrade. Users must ensure their Mostro coordinator is also running version 0.7.0 or later for compatibility.
Summary by CodeRabbit
Documentation
Chores
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.