docs: finish protocol v2 migration of the spec (action pages, anti-spam gates)#50
docs: finish protocol v2 migration of the spec (action pages, anti-spam gates)#50grunch wants to merge 1 commit into
Conversation
…am gates) Complete the documentation gaps left after the Phase 3 transport docs: - Migrate all action pages to the v2 wire format: JSON examples now show version: 2 and the 3-element content tuple (identity proof placeholder in reputation-mode requests, null/null in Mostro replies and full-privacy mode), with transport-agnostic wording linking to Overview -> Transports instead of gift-wrap/rumor terminology. - Replace the five kind-1059 envelope examples with the signed kind-14 NIP-44 envelope (p + NIP-40 expiration tags), keeping a pointer to the v1 envelope in Keys management. - Document the v2 anti-spam gates in the migration guide: base PoW, 60-second replay guard, first-contact PoW with the active trade-key set fast path, the 10-second freshness window, silent drops (no cant-do replies), and the pow_first_contact discovery caveat. - Clarify that the event kind is authoritative over the inner version field, and note the mostro-core 0.14.1 gift-wrap deprecation. - Fix leftovers in already-migrated pages: overview.md new-order example version, other_events.md info-event sample protocol_version tag and base-pow description. - Add scope notes to chat.md and dispute_chat.md: the P2P/dispute chat keeps its simplified NIP-59 scheme and is not affected by the v0.19.0 removal of the v1 transport. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018kz9Xo4SSaXz5Hxs57gfYJ
WalkthroughThis PR updates the Mostro protocol documentation to reflect version 2 of the wire format across nearly all message-flow documents: ChangesProtocol v2 documentation examples
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 6cc96eac75
ℹ️ 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".
| "<index N signature of the sha256 hash of the serialized first element of content>", | ||
| ["<identity pubkey>", "<identity signature>"] |
There was a problem hiding this comment.
This example is explicitly a Mostro → seller message, but the v2 tuple now shows a trade-key signature and identity proof. The v2 overview/key-management rules say Mostro-authored replies are unsigned and both tuple slots must be null; clients following this will expect identity material that Mostro never sends and can fail to handle the pay-invoice step.
Useful? React with 👍 / 👎.
| "<index N signature of the sha256 hash of the serialized first element of content>", | ||
| ["<identity pubkey>", "<identity signature>"] |
There was a problem hiding this comment.
This is a Mostro-authored waiting-seller-to-pay response, so the added v2 identity proof is not valid: Mostro replies use null, null for the trade signature and identity-proof tuple elements. Leaving the proof placeholders here documents an impossible response shape for clients that implement the v2 decoder/verification rules.
Useful? React with 👍 / 👎.
| null, | ||
| null |
There was a problem hiding this comment.
The message includes trade_index: 1, which marks the request as reputation mode, but the v2 tuple is documented here with both proof elements as null (the full-privacy shape). A seller following this range-order example would omit the trade-key signature and identity proof, so the take is not linked to reputation and may be treated differently from the adjacent non-range take-buy example.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/last_trade_index.md (1)
40-40: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the
restore.versiondescription.This still says the current version is
1, which conflicts with the updated v2 examples above. It should read2.🤖 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/last_trade_index.md` at line 40, The documentation for restore.version is outdated and still states the current protocol version as 1. Update the restore.version description in last_trade_index.md to reflect the new version 2, matching the updated v2 examples above and keeping the wording consistent with the surrounding restore field descriptions.
🤖 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.
Inline comments:
In `@src/cancel.md`:
- Line 22: The prose in cancel documentation is inconsistent with the emitted
action name; update the wording in the cancel message example so it matches the
actual action used by Mostro. Align the text in src/cancel.md with the action
emitted by the cancellation flow, using the same naming as the relevant
cancel/canceled example to avoid ambiguity for implementers.
In `@src/other_events.md`:
- Line 122: The instance-status example is internally inconsistent because the
sample daemon version in the example predates protocol v2 while the protocol tag
is set to "2". Update the example in the relevant instance-status section of the
documentation so the version fields match, either by changing the daemon version
to a v2-era value or by setting protocol_version back to "1" and keeping the
example consistent.
In `@src/take_sell_range_order.md`:
- Line 25: Update the range-order explanation in the take_sell_range_order.md
content so it no longer says the order amount is `0`; instead, describe that
Mostro uses the selected fiat amount from the range order to calculate the
required sats. Keep the rest of the flow intact, but make the wording in this
section consistent with the range-order example and its invoice payload rather
than the zero-amount sell flow.
In `@src/take_sell.md`:
- Line 114: The second “Mostro response” heading in the take_sell markdown
duplicates the earlier anchor, so rename this heading to a distinct title in the
same section to avoid repeated rendered anchors. Update the Markdown heading
near the repeated “Mostro response” text so it remains clear and unique without
changing the surrounding content structure.
---
Outside diff comments:
In `@src/last_trade_index.md`:
- Line 40: The documentation for restore.version is outdated and still states
the current protocol version as 1. Update the restore.version description in
last_trade_index.md to reflect the new version 2, matching the updated v2
examples above and keeping the wording consistent with the surrounding restore
field descriptions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1433d3bc-ea82-4d1d-a05f-f2c5aecf325c
📒 Files selected for processing (30)
src/add_bond_invoice.mdsrc/admin_add_solver.mdsrc/admin_cancel_order.mdsrc/admin_settle_order.mdsrc/bond_slashed.mdsrc/cancel.mdsrc/chat.mdsrc/dispute.mdsrc/dispute_chat.mdsrc/fiatsent.mdsrc/last_trade_index.mdsrc/new_buy_order.mdsrc/new_buy_order_ln_address.mdsrc/new_sell_order.mdsrc/new_sell_range_order.mdsrc/orders.mdsrc/other_events.mdsrc/overview.mdsrc/pay_bond_invoice.mdsrc/payment_failed.mdsrc/release.mdsrc/restore_session.mdsrc/seller_pay_hold_invoice.mdsrc/take_buy.mdsrc/take_buy_range_order.mdsrc/take_sell.mdsrc/take_sell_ln_address.mdsrc/take_sell_range_order.mdsrc/transport_migration.mdsrc/user_rating.md
| ## Mostro response | ||
|
|
||
| Mostro will send a message with action `cancel` confirming the order was canceled, here an example of rumor's content of the message: | ||
| Mostro will send a message with action `cancel` confirming the order was canceled, here an example of decrypted content of the message: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the prose to the emitted action.
The text says cancel, but the example emits canceled. Keep those aligned so implementers don't key off the wrong action name.
Suggested fix
- Mostro will send a message with action `cancel` confirming the order was canceled, here an example of decrypted content of the message:
+ Mostro will send a message with action `canceled` confirming the order was canceled, here an example of decrypted content of the message:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Mostro will send a message with action `cancel` confirming the order was canceled, here an example of decrypted content of the message: | |
| Mostro will send a message with action `canceled` confirming the order was canceled, here an example of decrypted content of the message: |
🤖 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/cancel.md` at line 22, The prose in cancel documentation is inconsistent
with the emitted action name; update the wording in the cancel message example
so it matches the actual action used by Mostro. Align the text in src/cancel.md
with the action emitted by the cancellation flow, using the same naming as the
relevant cancel/canceled example to avoid ambiguity for implementers.
| [ | ||
| "protocol_version", | ||
| "1" | ||
| "2" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant markdown file structure first
wc -l src/other_events.md
ast-grep outline src/other_events.md || true
# Show the nearby lines around the cited snippet
sed -n '90,150p' src/other_events.md
# Find other protocol/version mentions in the repo for context
rg -n '"protocol_version"|mostro_version|v2|transport timeline|instance-status' src src/other_events.md . -g '!**/node_modules/**' -g '!**/dist/**' -g '!**/build/**' || trueRepository: MostroP2P/protocol
Length of output: 13458
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Read the instance-status example around the cited lines
sed -n '60,135p' src/other_events.md
# Read the instance-status field descriptions
sed -n '200,235p' src/other_events.md
# Read the migration timeline and the v1/v2 mapping
sed -n '1,130p' src/transport_migration.mdRepository: MostroP2P/protocol
Length of output: 10645
Keep the instance-status example internally consistent. mostro_version: "0.12.8" predates protocol v2, so pairing it with protocol_version: "2" is misleading. Either update the sample to a v2-era daemon version or set the protocol tag back to "1".
🤖 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/other_events.md` at line 122, The instance-status example is internally
inconsistent because the sample daemon version in the example predates protocol
v2 while the protocol tag is set to "2". Update the example in the relevant
instance-status section of the documentation so the version fields match, either
by changing the daemon version to a v2-era value or by setting protocol_version
back to "1" and keeping the example consistent.
| ## Mostro response | ||
|
|
||
| In order to continue the buyer needs to send a lightning network invoice to Mostro, in this case the amount of the order is `0`, so Mostro will need to calculate the amount of sats for this order, then Mostro will send back a message asking for a LN invoice indicating the correct amount of sats that the invoice should have, here the rumor's content of the message: | ||
| In order to continue the buyer needs to send a lightning network invoice to Mostro, in this case the amount of the order is `0`, so Mostro will need to calculate the amount of sats for this order, then Mostro will send back a message asking for a LN invoice indicating the correct amount of sats that the invoice should have, here the decrypted content of the message: |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Correct the range-order response description.
This paragraph still says the order amount is 0, but this flow already carries the selected fiat amount and the example payload computes sats from that value. That reads like the zero-amount sell flow was copied here unchanged.
Suggested fix
- In order to continue the buyer needs to send a lightning network invoice to Mostro, in this case the amount of the order is `0`, so Mostro will need to calculate the amount of sats for this order, then Mostro will send back a message asking for a LN invoice indicating the correct amount of sats that the invoice should have, here the decrypted content of the message:
+ In order to continue the buyer needs to send a lightning network invoice to Mostro for the selected fiat amount, so Mostro can calculate the sats amount from that selection and then send back a message asking for a LN invoice with that exact amount:📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| In order to continue the buyer needs to send a lightning network invoice to Mostro, in this case the amount of the order is `0`, so Mostro will need to calculate the amount of sats for this order, then Mostro will send back a message asking for a LN invoice indicating the correct amount of sats that the invoice should have, here the decrypted content of the message: | |
| In order to continue the buyer needs to send a lightning network invoice to Mostro for the selected fiat amount, so Mostro can calculate the sats amount from that selection and then send back a message asking for a LN invoice with that exact amount: |
🤖 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/take_sell_range_order.md` at line 25, Update the range-order explanation
in the take_sell_range_order.md content so it no longer says the order amount is
`0`; instead, describe that Mostro uses the selected fiat amount from the range
order to calculate the required sats. Keep the rest of the flow intact, but make
the wording in this section consistent with the range-order example and its
invoice payload rather than the zero-amount sell flow.
|
|
||
| If the invoice includes an amount, the last element of the `payment_request` array should be set to `null`. | ||
|
|
||
| ## Mostro response |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Rename the second Mostro response heading.
This repeats the earlier heading and creates a duplicate anchor in rendered Markdown.
Suggested fix
-## Mostro response
+## Mostro response (buyer LN invoice)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Mostro response | |
| ## Mostro response (buyer LN invoice) |
🧰 Tools
🪛 LanguageTool
[grammar] ~114-~114: Ensure spelling is correct
Context: ...estarray should be set tonull`. ## Mostro response Mostro sends a message to the...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.22.1)
[warning] 114-114: Multiple headings with the same content
(MD024, no-duplicate-heading)
🤖 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/take_sell.md` at line 114, The second “Mostro response” heading in the
take_sell markdown duplicates the earlier anchor, so rename this heading to a
distinct title in the same section to avoid repeated rendered anchors. Update
the Markdown heading near the repeated “Mostro response” text so it remains
clear and unique without changing the surrounding content structure.
Source: Linters/SAST tools
2 participants
Completes the documentation gaps left after the Phase 3 transport docs (TRANSPORT_V2_SPEC.md §6 in mostrod tracks this as the pending protocol-repo work). The gaps were identified by auditing the v2 implementation in
mostro(PRs #776, #780, #781, #782, #785, #800) andmostro-core(PRs #152, #158) against the current spec.Action pages migrated to the v2 wire format (~25 pages)
Every action/flow page still described only the deprecated v1 transport. Now:
version: 2and the 3-element content tuple: identity-proof placeholder["<identity pubkey>", "<identity signature>"]in reputation-mode requests,null, nullin Mostro replies and full-privacy mode.1059envelope examples (new_sell_order,new_buy_order,new_buy_order_ln_address,take_buy,take_sell_ln_address) replaced with the signed kind-14NIP-44 envelope (p+ NIP-40expirationtags), keeping a pointer to the v1 envelope in Keys management.Anti-spam gates documented (
transport_migration.md)The Phase 2 gates shipped in mostrod v0.18.0 (#780) had no spec coverage. New section documents, in evaluation order: base PoW (the advertised
powtag), the 60-second replay guard, first-contact PoW (pow_first_contactwith fallback to basepow) with the active trade-key set fast path (non-terminal orders — disputes included — plus active-dispute solvers, refreshed every ~60 s), and the 10-second freshness window. Also states that all gate rejections are silent (nocant-doreply), and flags the discovery caveat: the info event does not advertise the first-contact difficulty.Other fixes
versionfield semantics: the event kind is authoritative for unwrap; the innerversionvalue is informative on the wire (matches #785 behavior).overview.md: mainnew-orderexample bumped toversion: 2.other_events.md: info-event sample now shows["protocol_version", "2"];powdescription notes it is the base difficulty on v2.transport_migration.md: mostro-core reference updated (0.14.1 marks the gift-wrap variant deprecated at the library level).chat.md/dispute_chat.md: scope notes clarifying the P2P/dispute chat keeps its simplified NIP-59 (kind1059) scheme and is not affected by the v0.19.0 removal of the v1 daemon transport.Intentionally untouched: the v1-labeled examples in
overview.mdandkey_management.md(they document the deprecated transport side by side with v2).🤖 Generated with Claude Code
https://claude.ai/code/session_018kz9Xo4SSaXz5Hxs57gfYJ
Generated by Claude Code
Summary by CodeRabbit