feat: migrate chat to mostro-core primitives by arkanoider · Pull Request #66 · MostroP2P/mostrix

arkanoider · 2026-05-08T13:40:49Z

Summary

Migrate admin/user/observer chat GiftWrap flows to mostro_core::chat (SharedKey, wrap_chat_message, unwrap_chat_message, chat_filter).
Remove custom chat GiftWrap builder and shared-key ECDH helpers based on nostr_sdk::util::generate_shared_key.
Add focused rustdoc for shared-key semantics and a roundtrip wrap/unwrap test.

Notes

Chat GiftWraps are sent without PoW (per migration choice), while protocol DMs still use the instance PoW path.

Summary by CodeRabbit

Documentation
- Updated documentation to clarify that PoW difficulty may be 0 until cached instance info loads, which could cause message rejection by stricter relays until refreshed.
Refactor
- Consolidated chat message handling to use a centralized library for wrapping and unwrapping encrypted messages.
- Simplified message sending logic to use a unified approach for all message types.

coderabbitai · 2026-05-08T13:41:02Z

Warning

Rate limit exceeded

@arkanoider has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 22 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7554a056-3218-4bee-8071-955d2a2bfad1

📥 Commits

Reviewing files that changed from the base of the PR and between 186613b and 81d4143.

📒 Files selected for processing (1)

docs/POW_AND_OUTBOUND_EVENTS.md

Walkthrough

This PR consolidates GiftWrap and direct message handling by migrating from local NIP-44/NIP-59 implementations to mostro_core chat helpers. It removes the MessageType enum, deletes the create_private_dm_event function, and unifies all DM sending through wrap_message with a single signed giftwrap path.

Changes

GiftWrap and Direct Message Consolidation

Layer / File(s)	Summary
Type and Contract Simplification `src/util/types.rs`	Removes `MessageType` enum and `determine_message_type` helper function that previously branched message creation logic.
Core Chat Wrapping and Shared Key Derivation `src/util/chat_utils.rs`	Imports mostro_core chat helpers (`SharedKey`, `wrap_chat_message`, `unwrap_chat_message`, `chat_filter`). Refactors shared-key derivation to use `SharedKey::derive` and persist via `to_hex`/`from_hex`. Admin chat sending now uses `wrap_chat_message` for wrapping. GiftWrap unwrapping delegates to `unwrap_chat_message`. Filter construction uses `chat_filter` and removes redundant in-loop recipient checking.
DM Utilities Refactoring `src/util/dm_utils/dm_helpers.rs`, `src/util/dm_utils/mod.rs`	Removes `create_private_dm_event` helper and its associated imports (`base64`, `nip44`). Refactors `send_dm` to always deserialize payload to `Message` and send via `wrap_message(signed: true)`, eliminating `MessageType`-based branching.
Tests and Documentation `src/util/chat_utils.rs`, `docs/POW_AND_OUTBOUND_EVENTS.md`	Adds tokio test validating wrap→unwrap round-trip preserves sender pubkey and content. Updates PoW documentation to clarify that difficulty may be 0 until instance info loads. Removes stale NIP-17 DM section from docs.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application Code
    participant MostroChat as mostro_core::chat
    participant SharedKeyDeriv as SharedKey Derivation
    participant Relay as Relay Filter
    
    rect rgba(0, 100, 200, 0.5)
    note right of App: Message Sending
    App->>SharedKeyDeriv: derive_shared_key_hex(counterparty_pubkey)
    SharedKeyDeriv->>MostroChat: SharedKey::derive(our_keys, counterparty_key)
    MostroChat-->>SharedKeyDeriv: shared_secret
    SharedKeyDeriv->>SharedKeyDeriv: to_hex() for persistence
    SharedKeyDeriv-->>App: shared_key_hex
    end
    
    rect rgba(100, 0, 200, 0.5)
    note right of MostroChat: Message Wrapping
    App->>MostroChat: wrap_chat_message(content, shared_pubkey)
    MostroChat-->>App: wrapped_giftwrap_event
    end
    
    rect rgba(0, 200, 100, 0.5)
    note right of Relay: Message Reception
    App->>Relay: chat_filter(shared_pubkey)
    Relay-->>App: filtered_giftwrap_events
    App->>MostroChat: unwrap_chat_message(event)
    MostroChat-->>App: (content, timestamp, sender_pubkey)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

MostroP2P/mostrix#64: Both PRs modify the same DM/GiftWrap flow—removing local NIP-59/NIP-44 builders and updating send_dm/dm_helpers to use mostro-core wrap/unwrap APIs.
MostroP2P/mostrix#65: Both PRs replace in-repo nip59/nip44 GiftWrap/DM wrap–unwrap logic with mostro_core's wrap/unwrap/shared-key APIs.
MostroP2P/mostrix#51: Both PRs modify DM/GiftWrap and chat utilities—this PR switches wrapping/unwrapping to mostro_core chat helpers and removes custom NIP‑44/NIP‑59 code.

Suggested reviewers

mostronatorcoder
grunch

Poem

🐰 With whiskers twitched and hoppy cheer,
Our GiftWraps now use shared-key spheres,
No NIP-59 tangled—clean paths reign,
One wrap_message to make it plain,
Mostro's core now guides our way,
A cleaner chat for every day! 🎁

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'feat: migrate chat to mostro-core primitives' directly and concisely summarizes the main objective: migrating chat functionality to use mostro_core primitives, which is the primary focus across all changed files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/mostro-core-chat

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

Generate code and open pull requests
Plan features and break down work
Investigate incidents and troubleshoot customer tickets together
Automate recurring tasks and respond to alerts with triggers
Summarize progress and report instantly

Built for teams:

Shared memory across your entire org—no repeating context
Per-thread sandboxes to safely plan and execute work
Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

mostronatorcoder

I found one blocker before this can be approved.

Blocker

This PR intentionally migrates chat GiftWrap construction to mostro_core::chat::wrap_chat_message, and the PR description explicitly says chat GiftWraps are now sent without PoW.

That part is fine if it is the intended migration choice, but the repository documentation was not updated consistently.

docs/POW_AND_OUTBOUND_EVENTS.md still says:

"Admin dispute chat gift wraps use the same instance-derived PoW via send_admin_chat_message_via_shared_key"

That is no longer true after this refactor.

Before this PR, send_admin_chat_message_via_shared_key() called the local custom wrapper path that applied instance-derived PoW to the outer GiftWrap. After this PR it calls wrap_chat_message(...) directly, mostro_instance is renamed to _mostro_instance, and no PoW is applied in that path anymore.

So right now the code and docs disagree on an externally relevant behavior change.

I am treating this as a blocker, not a doc nit, because PoW policy directly affects relay acceptance and operational expectations. Anyone reading the current doc will walk away with the wrong guarantee about outbound admin chat behavior.

Please update the documentation so it clearly reflects the new semantics for chat GiftWraps in this PR.

mostronatorcoder

Re-reviewed the latest commit.

The blocker I raised is resolved now.

The documentation now clearly distinguishes the two behaviors:

protocol DMs toward Mostro still use instance-derived PoW on the published outer GiftWrap
shared-key chat messages are relay-scoped chat wraps and intentionally do not use PoW

That was the missing piece for this migration. The refactor itself still looks coherent, and the code/doc semantics are aligned now.

feat: Step 4 of new mostro-core primitived: introduced chat primitives

186613b

mostronatorcoder Bot requested changes May 8, 2026

View reviewed changes

chore: mostronator rant

81d4143

mostronatorcoder Bot approved these changes May 8, 2026

View reviewed changes

arkanoider merged commit 3504f48 into main May 8, 2026
11 checks passed

coderabbitai Bot mentioned this pull request May 31, 2026

feat(my-trades): receive attachments #80

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: migrate chat to mostro-core primitives#66

feat: migrate chat to mostro-core primitives#66
arkanoider merged 2 commits into
mainfrom
feat/mostro-core-chat

arkanoider commented May 8, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented May 8, 2026 •

edited

Loading

Rate limit exceeded

Uh oh!

mostronatorcoder Bot left a comment

Uh oh!

mostronatorcoder Bot left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

arkanoider commented May 8, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Notes

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Rate limit exceeded

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

mostronatorcoder Bot left a comment

Choose a reason for hiding this comment

Blocker

Uh oh!

mostronatorcoder Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

arkanoider commented May 8, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 8, 2026 •

edited

Loading