feat(transport): Phase 2 — protocol-v2 (NIP-44 direct) selection#177
Conversation
Phase 2 of docs/TRANSPORT_V2_SPEC.md — the CLI can now speak the node's wire transport (protocol v1 gift wrap, kind 1059, or protocol v2 NIP-44 direct, kind 14), enabling trading against a `transport = "nip44"` node and exercising the daemon's anti-spam gates (MostroP2P/mostro#780). - Config: `--transport <gift-wrap|nip44>` (-t) sets a TRANSPORT env var, resolved via `messaging::parse_transport_env()` (default gift-wrap → wire-identical to today). Read from the env like POW/SECRET already are, so send_dm's signature and its ~14 callers are untouched. - Send: send_dm / send_plain_text_dm route through a new `publish_wrapped` → `wrap_message_with(transport, …)`, replacing the hard-wired `wrap_message`. The NIP-17 peer-chat path (to_user) is untouched. - Receive: `wait_for_dm` subscribes on `transport.event_kind()` and matches it in the notification loop; on v2 it additionally pins `author = mostro_pubkey` so a Mostro reply is never confused with NIP-17 peer chat (same kind 14). - Unwrap: `parse_dm_events` gains `mostro_protocol: bool` — true → decode via `unwrap_incoming` (dispatches on kind 1059/14); false → NIP-17 peer-chat path. All Mostro-reply / Mostro→user-DM call sites pass true; the one peer-chat listing passes false. Tests: Transport::from_str→event_kind mapping; a wrap_message_with(Nip44Direct) → unwrap_incoming roundtrip (kind 14, author = trade key, message round-trips). Full suite green; clippy --all-targets --all-features and fmt clean. Known gap (Phase 3): the get-dm historical-listing filter still hard-codes gift wrap, so listing past Mostro DMs on a v2 node returns nothing. The interactive request/response path (what exercises the anti-spam gate) is fully v2. Based on the mostro-core 0.13.0 bump (Phase 1, #176). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 27 minutes and 58 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughAdds transport v2 (NIP-44) support to mostro-cli via a ChangesTransport v2 (NIP-44) Implementation
Sequence Diagram(s)sequenceDiagram
actor User
participant CLI as mostro-cli (--transport nip44)
participant parse_transport_env
participant publish_wrapped
participant wrap_message_with as wrap_message_with (mostro-core)
participant RelayPool as Nostr Relay
participant parse_dm_events
User->>CLI: run command with --transport nip44
CLI->>CLI: set TRANSPORT=nip44 env var
CLI->>parse_transport_env: read TRANSPORT
parse_transport_env-->>CLI: Transport::Nip44Direct
CLI->>publish_wrapped: send DM (transport=Nip44Direct)
publish_wrapped->>wrap_message_with: wrap_message_with(Nip44Direct, …)
wrap_message_with-->>publish_wrapped: kind-14 Event
publish_wrapped->>RelayPool: publish event
RelayPool-->>CLI: incoming event (kind=14, author=mostro_pubkey)
CLI->>parse_dm_events: parse(events, keys, since, mostro_protocol=true)
parse_dm_events->>parse_dm_events: unwrap_incoming(event)
parse_dm_events-->>CLI: Vec<(Message, timestamp, sender)>
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07a3b1807e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .fetch_events(subscription, FETCH_EVENTS_TIMEOUT) | ||
| .await?; | ||
| let messages = parse_dm_events(events, &next_trade_key, Some(&2)).await; | ||
| let messages = parse_dm_events(events, &next_trade_key, Some(&2), true).await; |
There was a problem hiding this comment.
When --transport nip44 is used and a release of a range order generates the follow-up NewOrder, this path still feeds parse_dm_events from create_filter(ListKind::DirectMessagesUser, ...), whose DirectMessages branch is hard-coded to Kind::GiftWrap in src/util/events.rs. The v2 child-order event is kind 14 and authored by Mostro, so it is never fetched here and print_commands_results is not reached to display/record the replacement order. Please make this follow-up subscription transport-aware as well, including the Mostro author pin for v2.
Useful? React with 👍 / 👎.
…mpat guard Review follow-ups on docs/TRANSPORT_V2_SPEC.md (no code change): - §2 kind-14 note: reference the public `wrap_message_with(transport, …)` dispatcher the CLI actually calls, instead of the lower-level `wrap_message_nip44`, matching §3/§4. - Phase 2 acceptance: state explicitly that the daemon arms its anti-spam gates on the event kind (14), not on `Message.version` — so Phase 1 (gift-wrap, version = 2) never hits the gate and stays backward-compatible. - Phase 3: document that the `protocol_versions` info-event probe is the backward-compat guard for the version-skew risk Phase 1 flagged (a pre-0.13 daemon publishes no tag → treat as v1 + warn), and that until it lands the explicit `--transport` flag is the only negotiation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…dex review) Addresses the Codex P2 on PR #177: the range-order child-order follow-up fetched after a `release` (send_msg.rs) used `create_filter(DirectMessagesUser, …)`, whose DirectMessages branch hard-coded `Kind::GiftWrap`. On a v2 node the child `NewOrder` is a kind-14 event authored by Mostro, so it was never fetched and `print_commands_results` never ran. Fix: make `create_filter` transport-aware for the `DirectMessages*` kinds — `transport.event_kind()` plus an `author = mostro_pubkey` pin on v2 (new `mostro_pubkey` param threaded through its call sites + the integration test). This also makes the `get-dm` historical listing v2-aware, so the whole interactive path is fully v2 (spec note updated; the item is no longer deferred to Phase 3). cargo build/test/clippy --all-targets --all-features -D warnings/fmt all clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Phase 3 of docs/TRANSPORT_V2_SPEC.md — completes the CLI's transport-v2 support. - Auto-detection: `events::fetch_protocol_version_with` reads the node's `protocol_versions` tag (kind-38385 info event) with a short INFO_PROBE_TIMEOUT. `init_context` → `resolve_transport` runs it once at startup when `--transport`/`TRANSPORT` is unset: "2" → TRANSPORT=nip44; "1"/absent/unreachable → leave unset (messaging defaults to gift-wrap). An explicit `--transport` is authoritative and skips the probe. A pre-v2 daemon publishes no tag, so the CLI stays on v1 instead of mis-pairing (backward-compat guard for the version-skew risk Phase 1 flagged). - get-dm listing is transport-aware: `create_filter` for the DirectMessages* kinds now uses `transport.event_kind()` and pins `author = mostro_pubkey` on v2 (new `mostro_pubkey` param threaded through call sites), closing the Phase 2 gap. - Verbose: `resolve_transport` logs the active transport and how it was chosen. - Docs: docs/commands.md gains a Global options section documenting `--transport` + auto-detection; spec marked complete. Tests: deterministic `protocol_versions` tag read/parse. Full suite green; clippy --all-targets --all-features -D warnings and fmt clean. Based on Phase 2 (#177). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/integration_tests.rs (1)
60-78: ⚡ Quick winConsider testing DirectMessages kinds to exercise transport-aware filtering.*
The test correctly passes
ctx.mostro_pubkeyas the new fourth parameter, but since it testsListKind::Orders, themostro_pubkeyparameter isn't actually used in the filter logic (that parameter only affectsDirectMessagesAdminandDirectMessagesUserkinds, where it pinsauthor = mostro_pubkeyon NIP-44 transport). A test for one of those kinds would provide stronger validation of the Phase 2 transport-aware filtering behavior.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration_tests.rs` around lines 60 - 78, The test_filter_creation_integration function tests create_filter with ListKind::Orders, but the fourth parameter (ctx.mostro_pubkey) is not actually used in the filter logic for that kind. To properly validate the transport-aware filtering behavior for Phase 2, create an additional test function that calls create_filter with ListKind::DirectMessagesAdmin or ListKind::DirectMessagesUser instead, which will exercise the code path where the mostro_pubkey parameter is actually used to pin the author field. Verify in this new test that the filter correctly sets author = mostro_pubkey for these kinds.src/util/events.rs (1)
256-258: 💤 Low valueStale comment describes v1 only, but code path is now transport-aware.
The comment says "Mostro→user DMs (gift wrap, v1)" but the filter above uses
transport.event_kind()which handles both v1 (gift-wrap) and v2 (NIP-44 direct). Consider updating the comment.📝 Suggested comment update
- // Mostro→user DMs (gift wrap, v1): Mostro-protocol messages. + // Mostro→user DMs: Mostro-protocol messages (transport-aware).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/events.rs` around lines 256 - 258, The comment above the parse_dm_events call for direct_messages_for_trade_key only mentions v1 (gift wrap) but the code is transport-aware and handles both v1 (gift-wrap) and v2 (NIP-44 direct) protocols. Update the comment to accurately reflect that it now supports both transport versions rather than just describing v1.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/util/events.rs`:
- Around line 256-258: The comment above the parse_dm_events call for
direct_messages_for_trade_key only mentions v1 (gift wrap) but the code is
transport-aware and handles both v1 (gift-wrap) and v2 (NIP-44 direct)
protocols. Update the comment to accurately reflect that it now supports both
transport versions rather than just describing v1.
In `@tests/integration_tests.rs`:
- Around line 60-78: The test_filter_creation_integration function tests
create_filter with ListKind::Orders, but the fourth parameter
(ctx.mostro_pubkey) is not actually used in the filter logic for that kind. To
properly validate the transport-aware filtering behavior for Phase 2, create an
additional test function that calls create_filter with
ListKind::DirectMessagesAdmin or ListKind::DirectMessagesUser instead, which
will exercise the code path where the mostro_pubkey parameter is actually used
to pin the author field. Verify in this new test that the filter correctly sets
author = mostro_pubkey for these kinds.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 72f22aac-5789-4065-a752-e600c13f9878
📒 Files selected for processing (12)
docs/TRANSPORT_V2_SPEC.mdsrc/cli.rssrc/cli/last_trade_index.rssrc/cli/orders_info.rssrc/cli/restore.rssrc/cli/send_msg.rssrc/cli/take_dispute.rssrc/parser/dms.rssrc/util/events.rssrc/util/messaging.rstests/integration_tests.rstests/parser_dms.rs
…r author pin Two review nitpicks on PR #177: - src/util/events.rs: the comment above the DirectMessagesUser parse only said "gift wrap, v1", but the fetch is transport-aware now (the filter selects kind 1059 or 14 per transport and unwrap_incoming decodes either). Reworded to reflect both v1 and v2. - tests/integration_tests.rs: add `direct_messages_filter_is_transport_aware`, which exercises the code path where `mostro_pubkey` is actually used — asserts that on v2 (TRANSPORT=nip44) create_filter selects kind 14 and pins author = mostro_pubkey, and that the default gift-wrap stays kind 1059 with no author pin. Full suite green; clippy --all-targets --all-features -D warnings + fmt clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 participant
Phase 2 of
docs/TRANSPORT_V2_SPEC.md— protocol-v2 transport selectionTeaches
mostro-clito speak the node's wire transport — protocol v1 (gift wrap, kind 1059) or protocol v2 (NIP-44 direct, signed kind 14). This is what lets the CLI trade against atransport = "nip44"node and exercise the daemon's Phase 2 anti-spam gates (MostroP2P/mostro#780).How transport is chosen
--transport <gift-wrap|nip44>(-t) sets aTRANSPORTenv var, resolved bymessaging::parse_transport_env()into mostro-core'sTransport(defaultgift-wrap, wire-identical to today). It's read from the environment exactly likePOW/SECRETalready are — sosend_dm's signature and its ~14 call sites are untouched. Mirrors the daemon's[mostro] transportknob.Send
send_dm/send_plain_text_dmnow route the Mostro-protocol path through a newpublish_wrapped→wrap_message_with(transport, …)(replacing the hard-wiredwrap_message). The NIP-17 peer-chat path (to_user) is untouched.Receive (the careful part)
Kind 14 is shared between v2 Mostro messages and the CLI's existing NIP-17 peer chat, so the two are disambiguated:
wait_for_dmsubscribes ontransport.event_kind()and matches it in the notification loop; on v2 it additionally pinsauthor = mostro_pubkey, so a Mostro reply is never confused with peer chat on the same kind.parse_dm_eventsgains amostro_protocol: bool:true→ decode viaunwrap_incoming(dispatches on kind 1059/14 — the mostro 3-tuple + identity proof);false→ the existing NIP-17 peer-chat decode. Every Mostro-reply / Mostro→user-DM call site passestrue; the single peer-chat listing passesfalse.Tests
Transport::from_str→event_kindmapping (gift-wrap→1059,nip44→14, default, bogus errors).wrap_message_with(Nip44Direct) → unwrap_incomingroundtrip: produces a kind-14 event authored by the trade key, and decodes back to the same message.cargo clippy --all-targets --all-features+cargo fmt --checkclean.Known gap (deferred to Phase 3)
The
get-dmhistorical listing filter (create_filterfor theDirectMessages*kinds) still hard-codes gift wrap, so listing past Mostro DMs on a v2 node returns nothing. The interactive request/response path — the one that exercises the anti-spam gate — is fully v2. Phase 3 closes this plus capability auto-detection from theprotocol_versionsinfo tag.🧪 Manual testing — step by step
Needs a Mostro daemon on mostro-core 0.13 (e.g. MostroP2P/mostro#780) reachable on a relay.
A. Regression — gift-wrap unchanged (default)
export RELAYS=ws://localhost:7000 MOSTRO_PUBKEY=<node pubkey>with the daemon at its defaulttransport = "gift-wrap".--transport(e.g.mostro-cli listorders, then anewordr/takesell… round-trip). Behaviour is identical to before. ✔️B. v2 happy path — drive a nip44 node
settings.tomlsettransport = "nip44"and restart it.--transport nip44(orexport TRANSPORT=nip44):mostro-cli --transport nip44 newordr -k sell -c USD -f 100 -m "bank transfer" -a 0wait_for_dmreceives the kind-14 reply authored by Mostro. Complete a fulltakesell/takebuy→addinvoice→fiatsent→releaseround-trip. ✔️nak req -k 14 -a <your trade pubkey>): your outbound events are kind 14, not 1059.C. Transport mismatch behaves sanely
--transport(so it defaults to gift-wrap). The CLI sends kind-1059; the node (listening on kind 14) never sees it → the command times out ("No response"/timeout). Re-run with--transport nip44→ it works. (Phase 3 will auto-detect this fromprotocol_versionsand warn.) ✔️D. First-contact PoW lane (exercises the daemon gate, #780)
pow = 0andpow_first_contact = <N>(e.g. 8).newordrwith--transport nip44and no--pow: the daemon drops it before decrypt → CLI times out.--pow <N>(≥ the node'spow_first_contact): it's accepted. Once you have an active order, follow-up commands from that trade key fast-path without--pow(known-keys lane). ✔️E. Invalid transport value
mostro-cli --transport bogus listorders→ fails fast withInvalid TRANSPORT 'bogus': …. ✔️🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--transportCLI argument to select message transport protocol: gift-wrap (v1) or nip44 (v2).Documentation