Replace the outdated protocol documentation with the most up-to-date version#527
Closed
Replace the outdated protocol documentation with the most up-to-date version#527
Conversation
Contributor
WalkthroughDocumentation for the Mostro P2P protocol is updated across 20+ files to standardize message formats, rename fields, update event kinds, add new protocol actions, and document dispute chat and order request mechanisms. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
docs/protocol/take_buy.md (1)
5-18:⚠️ Potential issue | 🟡 MinorAdd language specifiers to all fenced code blocks.
All eight JSON code blocks in this file are missing language identifiers, which violates the markdownlint MD040 rule. As per coding guidelines, every fenced code block in markdown files must have a language specifier.
📝 Proposed fix
Change line 5 from:
-``` +```jsonRepeat this change for all JSON code blocks at lines 22, 39, 69, 102, 120, 135, and 164.
Also applies to: 22-32, 39-64, 69-97, 102-112, 120-130, 135-156, 164-179
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/take_buy.md` around lines 5 - 18, Add the missing language specifier "json" to each fenced code block in the document where the opening fence is currently ``` so that they read ```json; update all eight JSON blocks (the ones shown in the diff and referenced in the comment) by changing their opening backticks to include the language identifier, ensuring every fenced block has a language specifier to satisfy MD040.docs/protocol/dispute.md (1)
5-17:⚠️ Potential issue | 🟡 MinorAdd language specifiers to all fenced code blocks.
All seven JSON code blocks in this file are missing language identifiers, which violates the markdownlint MD040 rule. As per coding guidelines, every fenced code block in markdown files must have a language specifier.
📝 Proposed fix
Change line 5 from:
-``` +```jsonRepeat this change for all JSON code blocks at lines 23, 41, 63, 88, 104, 135, and 155.
Also applies to: 23-37, 41-55, 63-83, 88-98, 104-129, 135-150, 155-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/dispute.md` around lines 5 - 17, Update each JSON fenced code block in the dispute protocol examples so the opening fence includes the json language specifier (i.e., change ``` to ```json) for all seven examples (the blocks that start with JSON arrays/objects such as the one beginning with [{"order": {"version": 1, "id": "<Order Id>", "action": "dispute", ...}}]). Ensure every fenced block in this file uses ```json consistently to satisfy markdownlint MD040.
🧹 Nitpick comments (2)
docs/protocol/dispute_chat.md (1)
39-39: Clarify per-party shared key implementation.The documentation states: "Each party (buyer and seller) has its own independent shared key with the admin."
However, the code at
lib/data/models/session.dart:191-203shows a single_adminSharedKeycomputed fromtradeKey.privateandadminPubkey. Since the Session is tied to a specific orderId and party (via their trade key index), this creates one shared key per session/party, which matches the documentation. But the implementation detail that the key is derived from the party's specific trade key could be made more explicit.Consider adding a clarifying note:
Each party (buyer and seller) has its own independent shared key with the admin. +The shared key is derived using each party's trade key, ensuring that messages +from the buyer and seller remain separate and private.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/dispute_chat.md` at line 39, Update the docs to explicitly state that the admin shared key is derived per-session from the party's trade key by the Session model: the Session computes _adminSharedKey using tradeKey.private and adminPubkey, and because Session is bound to a specific orderId and party (via the trade key index), each party (buyer/seller) ends up with its own independent admin shared key; add a short clarifying sentence in dispute_chat.md explaining this derivation and that a session can therefore hold both a peer shared key and an admin shared key simultaneously.docs/protocol/dispute.md (1)
132-132: Minor grammar correction.The phrase "different than" is considered colloquial. The preferred formal usage is "different from."
✏️ Proposed fix
-Then mostrod send messages to each trade participant, the buyer and seller for them to know the pubkey of the admin who took the dispute, that way the client can start listening events from that specific pubkey, by default clients should discard any messages received from any pubkey different than Mostro node or dispute solver, the message looks like this: +Then mostrod send messages to each trade participant, the buyer and seller for them to know the pubkey of the admin who took the dispute, that way the client can start listening events from that specific pubkey, by default clients should discard any messages received from any pubkey different from Mostro node or dispute solver, the message looks like this:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/protocol/dispute.md` at line 132, Update the sentence in the dispute protocol docs to use formal phrasing: replace "different than Mostro node or dispute solver" with "different from Mostro node or dispute solver" in the sentence that begins "Then mostrod send messages to each trade participant..." so the line reads that clients should discard any messages received from any pubkey different from Mostro node or dispute solver; ensure only that wording change is made.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/protocol/dispute_chat.md`:
- Around line 29-31: The fenced code blocks containing the ECDH formulas (e.g.,
"Shared Key = ECDH(tradeKey.private, adminPubkey)" and "Shared Key =
ECDH(adminPrivateKey, tradeKey.public)" and the similar block at lines 35-37)
are missing language specifiers; update each fence to include a language tag
(suggest "text") so each block becomes ```text ... ``` to satisfy MD040 and the
repo markdown guideline.
In `@docs/protocol/dispute.md`:
- Line 75: Dispute.fromNostrEvent currently only extracts "d" and "s" tags so
the initiator tag from events is dropped; update the Dispute model to add an
initiator field and modify Dispute.fromNostrEvent to parse the ["initiator",
<value>] tag (same convention used in docs/protocol/dispute.md) and assign it to
the new field, ensuring the field is added to the Dispute class (and
constructor/serialization if applicable) so it aligns with RestoredDispute which
already preserves initiator.
In `@docs/protocol/message_suggestions_for_actions.md`:
- Around line 148-149: Add support for the documented too-many-requests CantDo
reason: update the CantDoReason enum (lib/data/models/enums/cant_do_reason.dart)
to include a tooManyRequests variant, ensure CantDoReason.fromString()
recognizes the "too_many_requests" string, and add a corresponding entry for
'too_many_requests' in CantDoNotificationMapper._messageMap
(lib/shared/widgets/notification_listener_widget.dart) so CantDo.fromJson()
won't throw ArgumentError/FormatException; alternatively, if you prefer to
remove the reason, delete references to "too-many-requests"/"too_many_requests"
from the docs and any server contracts so the enum and mapper remain unchanged.
In `@docs/protocol/new_buy_order.md`:
- Around line 62-63: Update the test fixtures in
test/notifiers/add_order_notifier_test.dart to use the current protocol field
names buyer_trade_pubkey and seller_trade_pubkey instead of the deprecated
master_buyer_pubkey and master_seller_pubkey (replace the occurrences around the
existing fixtures at the two places mentioned), keeping the same values
(including null) so Order.fromJson continues to parse the fixtures correctly;
after replacing, run the tests to confirm no behavior changes.
In `@docs/protocol/new_sell_order.md`:
- Around line 3-27: The example shows a bare object but the client serializes
new-order messages as an array envelope; update the example to match
MostroMessage serialization by wrapping the object in an array of two elements
(the body object and a signature or null), i.e., mirror the serialization logic
used in lib/data/models/mostro_message.dart for "new-order" messages so the
request example matches the client and the confirmation example later in the
doc.
In `@docs/protocol/restore_session.md`:
- Around line 9-20: Several fenced code blocks in the restore_session.md
document that contain JSON examples (for example the block showing the array
with the "restore" object containing "version", "action": "restore-session", and
"payload") are missing language specifiers; update each JSON fenced block by
replacing the opening triple backticks (```) with language-marked backticks
(```json) for all JSON examples in the file so they comply with MD040. Ensure
you change every JSON example block (the three blocks currently without
specifiers) to start with ```json.
---
Outside diff comments:
In `@docs/protocol/dispute.md`:
- Around line 5-17: Update each JSON fenced code block in the dispute protocol
examples so the opening fence includes the json language specifier (i.e., change
``` to ```json) for all seven examples (the blocks that start with JSON
arrays/objects such as the one beginning with [{"order": {"version": 1, "id":
"<Order Id>", "action": "dispute", ...}}]). Ensure every fenced block in this
file uses ```json consistently to satisfy markdownlint MD040.
In `@docs/protocol/take_buy.md`:
- Around line 5-18: Add the missing language specifier "json" to each fenced
code block in the document where the opening fence is currently ``` so that they
read ```json; update all eight JSON blocks (the ones shown in the diff and
referenced in the comment) by changing their opening backticks to include the
language identifier, ensuring every fenced block has a language specifier to
satisfy MD040.
---
Nitpick comments:
In `@docs/protocol/dispute_chat.md`:
- Line 39: Update the docs to explicitly state that the admin shared key is
derived per-session from the party's trade key by the Session model: the Session
computes _adminSharedKey using tradeKey.private and adminPubkey, and because
Session is bound to a specific orderId and party (via the trade key index), each
party (buyer/seller) ends up with its own independent admin shared key; add a
short clarifying sentence in dispute_chat.md explaining this derivation and that
a session can therefore hold both a peer shared key and an admin shared key
simultaneously.
In `@docs/protocol/dispute.md`:
- Line 132: Update the sentence in the dispute protocol docs to use formal
phrasing: replace "different than Mostro node or dispute solver" with "different
from Mostro node or dispute solver" in the sentence that begins "Then mostrod
send messages to each trade participant..." so the line reads that clients
should discard any messages received from any pubkey different from Mostro node
or dispute solver; ensure only that wording change is made.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 171697f0-fd69-4bc7-a3eb-dded50a58d5a
📒 Files selected for processing (28)
docs/protocol/SUMMARY.mddocs/protocol/admin_add_solver.mddocs/protocol/admin_cancel_order.mddocs/protocol/admin_settle_order.mddocs/protocol/cancel.mddocs/protocol/dispute.mddocs/protocol/dispute_chat.mddocs/protocol/key_management.mddocs/protocol/last_trade_index.mddocs/protocol/list_disputes.mddocs/protocol/message_suggestions_for_actions.mddocs/protocol/new_buy_order.mddocs/protocol/new_buy_order_ln_address.mddocs/protocol/new_sell_order.mddocs/protocol/new_sell_range_order.mddocs/protocol/order_event.mddocs/protocol/orders.mddocs/protocol/other_events.mddocs/protocol/overview.mddocs/protocol/release.mddocs/protocol/restore_session.mddocs/protocol/seller_pay_hold_invoice.mddocs/protocol/take_buy.mddocs/protocol/take_buy_range_order.mddocs/protocol/take_sell.mddocs/protocol/take_sell_ln_address.mddocs/protocol/take_sell_range_order.mddocs/protocol/user_rating.md
Member
|
I think we should remove this documentation from here and not spend time maintaining two repositories with the same info |
Member
Author
Agree, I'll remove that documentation and reference it in one of the existing ones. |
Member
|
great closing it |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
In this PR, all the protocol documentation is updated to match exactly what is in https://github.com/MostroP2P/protocol.
Summary by CodeRabbit