feat: add dual send for transport v2 (Phase B)#624
Conversation
… identity proof (Phase B)
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughPhase B of Transport V2 migration extends ChangesTransport V2 Dual Send and Restore Dual-Transport (Phase B)
Sequence DiagramsequenceDiagram
participant MostroService
participant wrapForTransport
participant wrapNip44
participant _mostroSign
participant NIP44
participant NostrRelay
MostroService->>MostroService: resolve transport from node
alt transport == NIP-44 (v2)
MostroService->>wrapForTransport: wrapForTransport(v2, tradeKey, recipientPubKey, masterKey?, difficulty)
wrapForTransport->>wrapNip44: wrapNip44(tradeKey, recipientPubKey, masterKey?, difficulty)
wrapNip44->>wrapNip44: toJson(version: 2)
wrapNip44->>_mostroSign: sign message JSON with tradeKey
_mostroSign-->>wrapNip44: tradeSig
alt masterKey != null
wrapNip44->>_mostroSign: sign identity domain with masterKey
_mostroSign-->>wrapNip44: identityProof
else
wrapNip44->>wrapNip44: identityProof = null
end
wrapNip44->>NIP44: encrypt [messageMap, tradeSig, identityProof]
NIP44-->>wrapNip44: ciphertext
wrapNip44-->>wrapForTransport: kind-14 NostrEvent
else transport == Gift-wrap (v1)
wrapForTransport->>wrapForTransport: wrap (gift-wrap existing path)
end
wrapForTransport-->>MostroService: NostrEvent
MostroService->>NostrRelay: publish event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
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: d32a8624c9
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@docs/architecture/TRANSPORT_V2_MIGRATION.md`:
- Line 307: The documentation contains a contradiction where line 307 states the
kind-14 event includes "`p` and NIP-40 `expiration` tags," but lines 317-318
clarify that the expiration tag is omitted, and the actual implementation
confirms only the `p` tag is emitted. Update line 307 in the Phase B scope
description to remove the reference to "and NIP-40 `expiration` tags" and state
only that the kind-14 event includes the `p` tag, making it consistent with the
implementation notes below and the actual wrapNip44 implementation.
🪄 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: 171a4da6-2b62-4cd0-b6e7-a53da85f8809
📒 Files selected for processing (4)
docs/architecture/TRANSPORT_V2_MIGRATION.mdlib/data/models/mostro_message.dartlib/services/mostro_service.darttest/data/mostro_message_nip44_test.dart
![ermeme[bot]](https://avatars.githubusercontent.com/in/3690068?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
Verdict: Request Changes
Critical
- Restore requests now route through
wrapForTransport()forprotocol_version=2, butRestoreServicestill subscribes/extracts only kind1059gift-wrap events. On a v2-only node the restore flow will wait for a response it never listens for.
Looks Good
- The previous direct-send gap in
publishOrder()/ dispute creation is now covered by the shared transport resolver.
Please update the restore receive path to handle kind 14 before merging.
…ct) in restore flow
There was a problem hiding this comment.
Code Review Summary
Verdict: Approve
The restore flow now handles both transports, including v2 NIP-44 direct replies, and the orders subscription still resolves transport from the node protocol version. This closes the blocking restore regression from the previous head.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/data/repositories/dispute_repository.dart (1)
55-60:⚠️ Potential issue | 🟠 MajorPass
masterKeyandkeyIndexconsistent withpublishOrder.The
wrapForTransport()call is missingmasterKeyandkeyIndex. UnlikepublishOrderinmostro_service.dart, disputes will omit identity proof (tradeSigandidentityProofremainnull) when v2 transport is used, breaking the binding between trade key and master identity in reputation mode.Apply the same pattern as
publishOrder:🐛 Proposed fix to add reputation mode parameters
final event = await disputeMessage.wrapForTransport( protocolVersion: mostroInstance?.protocolVersion, tradeKey: session.tradeKey, recipientPubKey: _mostroPubkey, + masterKey: session.fullPrivacy ? null : session.masterKey, + keyIndex: session.fullPrivacy ? null : session.keyIndex, difficulty: mostroPow, );🤖 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 `@lib/data/repositories/dispute_repository.dart` around lines 55 - 60, The wrapForTransport() call in the dispute message handling is missing the masterKey and keyIndex parameters, which are required to maintain identity proof for reputation mode. Add the masterKey and keyIndex parameters to the wrapForTransport() call, following the same pattern used in the publishOrder method from mostro_service.dart. This ensures that tradeSig and identityProof are properly populated instead of remaining null, preserving the binding between the trade key and the master identity in reputation mode.
🧹 Nitpick comments (2)
test/features/restore/restore_decode_test.dart (1)
17-28: ⚡ Quick winCover the v2-native 3-tuple payload shape in restore decode tests.
Line 17 currently reuses a 2-item tuple for both transports, but v2 send-side contract is a 3-item tuple (
message,tradeSig,identityProof). Add a v2 fixture/assertion with 3 elements so restore decode is regression-tested against the actual transport-v2 envelope.Suggested test adjustment
- final restoreTuple = jsonEncode([ + final restoreTupleV1 = jsonEncode([ { 'restore': { 'version': 1, 'action': 'restore', 'payload': { 'restore_data': {'orders': [], 'disputes': []}, }, }, }, null, ]); + + final restoreTupleV2 = jsonEncode([ + { + 'restore': { + 'version': 2, + 'action': 'restore', + 'payload': { + 'restore_data': {'orders': [], 'disputes': []}, + }, + }, + }, + 'tradeSig-placeholder', + 'identityProof-placeholder', + ]);- final event = await NostrUtils.createNIP59Event( - restoreTuple, + final event = await NostrUtils.createNIP59Event( + restoreTupleV1, tempTradeKey.public, mostroKey.private, );- final event = await buildV2Reply(restoreTuple); + final event = await buildV2Reply(restoreTupleV2);- final v1 = await NostrUtils.createNIP59Event( - restoreTuple, + final v1 = await NostrUtils.createNIP59Event( + restoreTupleV1, tempTradeKey.public, mostroKey.private, ); - final v2 = await buildV2Reply(restoreTuple); + final v2 = await buildV2Reply(restoreTupleV2);Also applies to: 69-94
🤖 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 `@test/features/restore/restore_decode_test.dart` around lines 17 - 28, The restore decode tests are currently using a 2-item tuple for both transports, but the v2 transport contract specifies a 3-item tuple with the shape (message, tradeSig, identityProof). Add a separate v2-specific test fixture and assertion using jsonEncode with a 3-element array instead of the 2-element tuple shown at lines 17-28 and the related test assertions at lines 69-94, ensuring the restore decode logic is properly tested against the actual v2 transport envelope format with all three required elements.lib/data/repositories/dispute_repository.dart (1)
43-46: 💤 Low valueStale comments reference only NIP-59 gift wrap.
Lines 43 and 46 still describe "Gift Wrap protocol (NIP-59)" but the code now uses
wrapForTransportwhich may produce NIP-44 (kind 14) for v2 nodes. Consider updating these comments to reflect the dual-transport 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 `@lib/data/repositories/dispute_repository.dart` around lines 43 - 46, Update the comments on lines 43 and 46 in the dispute creation flow to accurately reflect the dual-transport capability. Instead of stating only "Gift Wrap protocol (NIP-59)", clarify that the `wrapForTransport` method dynamically selects between NIP-59 gift wrap for v1 nodes and NIP-44 (kind 14) encryption for v2 nodes. This ensures the comments accurately describe the actual behavior of the code being executed.
🤖 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 `@lib/features/restore/restore_manager.dart`:
- Around line 1093-1098: The code checks if the string content is empty but does
not validate that the decoded JSON array has elements before accessing
contentList[0], which will cause a RangeError if the array is empty. After
decoding content with jsonDecode and casting to List<dynamic> as contentList,
add a bounds check to verify that contentList is not empty before attempting to
access contentList[0], throwing an appropriate exception if the list is empty.
---
Outside diff comments:
In `@lib/data/repositories/dispute_repository.dart`:
- Around line 55-60: The wrapForTransport() call in the dispute message handling
is missing the masterKey and keyIndex parameters, which are required to maintain
identity proof for reputation mode. Add the masterKey and keyIndex parameters to
the wrapForTransport() call, following the same pattern used in the publishOrder
method from mostro_service.dart. This ensures that tradeSig and identityProof
are properly populated instead of remaining null, preserving the binding between
the trade key and the master identity in reputation mode.
---
Nitpick comments:
In `@lib/data/repositories/dispute_repository.dart`:
- Around line 43-46: Update the comments on lines 43 and 46 in the dispute
creation flow to accurately reflect the dual-transport capability. Instead of
stating only "Gift Wrap protocol (NIP-59)", clarify that the `wrapForTransport`
method dynamically selects between NIP-59 gift wrap for v1 nodes and NIP-44
(kind 14) encryption for v2 nodes. This ensures the comments accurately describe
the actual behavior of the code being executed.
In `@test/features/restore/restore_decode_test.dart`:
- Around line 17-28: The restore decode tests are currently using a 2-item tuple
for both transports, but the v2 transport contract specifies a 3-item tuple with
the shape (message, tradeSig, identityProof). Add a separate v2-specific test
fixture and assertion using jsonEncode with a 3-element array instead of the
2-element tuple shown at lines 17-28 and the related test assertions at lines
69-94, ensuring the restore decode logic is properly tested against the actual
v2 transport envelope format with all three required elements.
🪄 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: bab75273-8a14-4c26-9b40-65ab2bc9b174
📒 Files selected for processing (7)
docs/architecture/SESSION_RECOVERY_ARCHITECTURE.mddocs/architecture/TRANSPORT_V2_MIGRATION.mdlib/data/models/mostro_message.dartlib/data/repositories/dispute_repository.dartlib/features/restore/restore_manager.dartlib/services/mostro_service.darttest/features/restore/restore_decode_test.dart
✅ Files skipped from review due to trivial changes (2)
- docs/architecture/SESSION_RECOVERY_ARCHITECTURE.md
- docs/architecture/TRANSPORT_V2_MIGRATION.md
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/data/models/mostro_message.dart
There was a problem hiding this comment.
Code Review Summary
Verdict: Approve
Looks Good
- Restore now has regression coverage for both transport v1 (kind 1059) and transport v2 (kind 14).
- The restore decode path is factored cleanly into a shared helper, which keeps the transport branch testable.
- The docs now match the current dual-receive behavior.
Notes
- I did not find any blocking issues in the current head.
Reviewed by Hermes Agent
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
Strict Review — Phase B (Transport v2, Dual send)
Verdict: Approvable with minor changes. The architecture is solid, well-documented, and well-tested (the tests cryptographically verify the trade signature and identity proof, not just the tuple shape). I confirmed locally that flutter analyze reports 0 issues and all 6 new tests pass. ermeme's blocking concern (restore only received kind 1059) is resolved by the dual-receive commits, and the doc contradiction at line 307 (NIP-40 expiration tag) is already fixed.
Status of prior feedback
- ✅ Codex P2 (unconverted direct sends): resolved — there are no remaining direct
.wrap()sends inlib/; everything routes throughwrapForTransport. - ✅ ermeme (restore receive path): resolved —
_createTempSubscriptionnow subscribes to both kind 1059 and kind 14 (the latter scoped withauthors:[mostroPubkey], good). ⚠️ CodeRabbit Major (dispute missingmasterKey/keyIndex): still present and valid — see below.
Findings
1. (Medium) DisputeRepository.createDispute omits masterKey/keyIndex → in v2 reputation mode the dispute is sent without identity proof
dispute_repository.dart:55-60 calls wrapForTransport without masterKey/keyIndex. This is the actively used path (trade_detail_screen.dart:774). Consequence: in wrapNip44, when masterKey == null, both tradeSig and identityProof are null → a v2 node receives a dispute with no identity binding in reputation mode. This is inconsistent with publishOrder (mostro_service.dart:372-373), which does pass them. In v1 this is pre-existing behavior (the seal is signed by the trade key), but the PR's own docs state this path is a first-class v2 citizen, so it's worth closing. session already exposes masterKey/keyIndex/fullPrivacy, so the fix is trivial:
masterKey: session.fullPrivacy ? null : session.masterKey,
keyIndex: session.fullPrivacy ? null : session.keyIndex,2. (Low) decodeRestoreMessage — missing bounds check before contentList[0] (restore_manager.dart:~1098)
It validates content.isEmpty but not contentList.isEmpty. An empty JSON array [] would throw RangeError instead of a controlled exception. Defensive fix:
if (contentList.isEmpty) throw Exception('Restore message tuple is empty');3. (Nit) Stale comments in dispute_repository.dart:43,46 still say "Gift Wrap protocol (NIP-59)" even though the path can now emit kind 14.
4. (Verify before enabling v2 — non-blocking) Interop with mostro-core
tradeSig is computed over jsonEncode(messageMap) and the identity proof over mostro-transport-v2-identity:<tradePubkey>:<messageJSON>. This relies on the JSON key ordering matching transport.rs's re-serialization on the daemon byte-for-byte. Since it reuses the same toJson() as the v1 sign() (which already works in production), it should match, but there is no v2 node in production yet, so this is not validated against a real daemon. Recommend an end-to-end interop test before activating v2.
5. (Minor note) wrapNip44 uses createdAt: DateTime.now() with no timestamp randomization. Acceptable since the kind-14 event is signed by the trade key (already linkable), but worth being aware of.
Conclusion
None of these are blocking for the phase (no v2 nodes in production). I recommend addressing #1 before merge (identity consistency + it's the active path), with #2/#3 as quick cleanup. #4 is a pre-launch to-do for v2.
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
Follow-up: wire-format verification against mostro-core/src/transport.rs
Re-framing my earlier review per a fair point: there are no v2 nodes in production because there is no v2 client yet — this PR is the prerequisite. So the wire format isn't a "verify later" item; it's the contract that future v2 daemons will be built against. I verified wrapNip44 byte-for-byte against the reference impl (MostroP2P/mostro-core:src/transport.rs::wrap_message_nip44 + Message::sign). It matches — this implementation is spec-compliant. Details, so reviewers don't have to re-derive it:
| Aspect | mostro-core reference | This PR (wrapNip44) |
Match |
|---|---|---|---|
| Event kind | Kind::PrivateDirectMessage (14) |
kind: 14 |
✅ |
| Tuple | (Message, Option<trade_sig>, Option<(id_pub, id_sig)>) → JSON array |
[messageMap, tradeSig, identityProof] |
✅ |
| Tuple element 0 | message.as_json() = {"order"/"restore": {...}} |
{wrapperKey: toJson(version:2)} |
✅ |
| Sign scheme | schnorr(sha256_digest_raw_32B) |
sign(hex(sha256(msg))) → dart_nostr hex-decodes to the same 32B |
✅ |
| Identity domain | mostro-transport-v2-identity:<trade_pubkey_hex>:<message_json> |
mostro-transport-v2-identity:${tradeKey.public}:$messageJson |
✅ |
| identityProof shape | [identity_pubkey_hex, sig] or null |
[masterKey.public, sig] or null |
✅ |
| NIP-44 keys | encrypt(trade_secret, receiver) |
encryptNIP44(tuple, tradeKey.private, recipient) |
✅ |
| Author / event sig | signed by trade key | keyPairs: tradeKey |
✅ |
p tag |
Tag::public_key(receiver) |
['p', recipientPubKey] |
✅ |
| PoW | EventBuilder.pow(opts.pow), re-signed |
mineProofOfWork(event, difficulty, tradeKey) |
✅ |
created_at |
builder default (real time, no randomization) | DateTime.now() |
✅ (retracting my earlier note — the reference doesn't randomize either) |
| NIP-40 expiration | optional (opts.expiration) |
omitted | ✅ (valid: daemon makes it optional) |
The one cross-impl risk — that Dart's jsonEncode key ordering must equal the daemon's serde message.as_json() for tradeSig/identity-proof to verify — is already proven by the v1 path in production, which signs the exact same {wrapperKey: toJson()} and is verified the same way (Message::verify_signature over message.as_json()). v2 reuses both sides verbatim. So no new canonicalization risk.
One real consequence to fix before merge (sharpening finding #1)
Given the daemon's unwrap_message_nip44, a null identity_proof is accepted and sets identity = event.pubkey (the trade key) — i.e. it is treated as full-privacy mode. Because DisputeRepository.createDispute omits masterKey/keyIndex, a dispute raised by a reputation-mode user on a v2 node would be silently received as full-privacy: the daemon binds it to the trade key, not the user's master identity. Same omission exists in the v1 path here (pre-existing), but since we're establishing the v2 contract strictly, this is the moment to fix it so reputation disputes carry the identity proof. Fix is the 2 lines I noted (mirror publishOrder: masterKey/keyIndex gated on session.fullPrivacy).
Net
Crypto/serialization interop: verified correct. Remaining actionable items unchanged: dispute identity proof (#1, now with a concrete consequence), contentList empty-array guard (#2), stale NIP-59 comments (#3). Recommend a real end-to-end roundtrip test against a mostro-core v2 node (or a vector generated by wrap_message_nip44) added in Phase D before any v2 node ships.
Reference: mostro-core/src/transport.rs, TRANSPORT_V2_SPEC.md
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
Request Changes
The wire format is verified spec-compliant against mostro-core/src/transport.rs (see my detailed comparison comment), and the dual-receive regression is resolved. Approving is blocked only by the following, which should be addressed before merge:
Must fix
DisputeRepository.createDisputeomitsmasterKey/keyIndex, so a reputation-mode dispute on a v2 node is silently downgraded to full-privacy identity (daemon acceptsnullidentity_proof and binds the dispute to the trade key, not the master identity). MirrorpublishOrder:masterKey: session.fullPrivacy ? null : session.masterKey, keyIndex: session.fullPrivacy ? null : session.keyIndex,
Should fix
2. decodeRestoreMessage: add an empty-array guard before contentList[0] to avoid RangeError on a [] payload.
3. Stale comments in dispute_repository.dart:43,46 still say "Gift Wrap protocol (NIP-59)" though the path can now emit kind 14.
Recommended (Phase D)
- Add an end-to-end roundtrip test against a
mostro-corev2 wrap vector before any v2 node ships.
- DisputeRepository.createDispute now passes masterKey/keyIndex to wrapForTransport so reputation-mode disputes carry the v2 identity proof instead of being silently downgraded to full-privacy, matching publishOrder. - decodeRestoreMessage guards against an empty JSON tuple before indexing [0] to avoid a RangeError. - Refresh stale NIP-59-only comments in the dispute send path.
grunch
left a comment
grunch
left a comment
There was a problem hiding this comment.
Approve
Must/should-fix items from the previous review are addressed in e7bbcfdf:
- ✅
DisputeRepository.createDisputenow passesmasterKey/keyIndex(gated onsession.fullPrivacy), so reputation-mode v2 disputes carry the identity proof instead of being downgraded to full-privacy. - ✅
decodeRestoreMessageguards against an empty tuple before indexing[0]. - ✅ Stale NIP-59-only comments in the dispute send path refreshed.
Verified locally: flutter analyze clean, nip44 + restore-decode tests pass. Wire format remains spec-compliant with mostro-core/src/transport.rs.
Non-blocking follow-up for Phase D: add an end-to-end roundtrip test against a mostro-core v2 wrap vector before any v2 node ships.
* feat(transport): implement protocol v2 (NIP-44 direct) send path with identity proof (Phase B) * feat(transport): route all outbound Mostro sends through wrapForTransport (Phase B) * feat(restore): add dual-receive support for transport v2 (NIP-44 direct) in restore flow * test(restore): add dual-receive transport v2 regression tests for restore flow * fix(transport): bind dispute identity proof and guard restore tuple - DisputeRepository.createDispute now passes masterKey/keyIndex to wrapForTransport so reputation-mode disputes carry the v2 identity proof instead of being silently downgraded to full-privacy, matching publishOrder. - decodeRestoreMessage guards against an empty JSON tuple before indexing [0] to avoid a RangeError. - Refresh stale NIP-59-only comments in the dispute send path. --------- Co-authored-by: grunch <[email protected]>
…3.x (#632) * feat: add dual send for transport v2 (Phase B) (#624) * feat(transport): implement protocol v2 (NIP-44 direct) send path with identity proof (Phase B) * feat(transport): route all outbound Mostro sends through wrapForTransport (Phase B) * feat(restore): add dual-receive support for transport v2 (NIP-44 direct) in restore flow * test(restore): add dual-receive transport v2 regression tests for restore flow * fix(transport): bind dispute identity proof and guard restore tuple - DisputeRepository.createDispute now passes masterKey/keyIndex to wrapForTransport so reputation-mode disputes carry the v2 identity proof instead of being silently downgraded to full-privacy, matching publishOrder. - decodeRestoreMessage guards against an empty JSON tuple before indexing [0] to avoid a RangeError. - Refresh stale NIP-59-only comments in the dispute send path. --------- Co-authored-by: grunch <[email protected]> * fix(order): prevent order creation race with node-switch restore session reset - Add RestoreService.isOperationInProgress and awaitOperationCompletion() to expose restore/sync state - AddOrderNotifier.submitOrder now waits for any in-flight restore to finish before creating an order - Prevents session orphaning when node-switch restore's _clearAll wipes sessions mid-order-creation * fix(restore): loop awaitOperationCompletion to handle overlapping operations - Change awaitOperationCompletion from `if` to `while` to wait for any operation that starts while we're already waiting - Prevents race where a second restore begins after the first completes but before the caller proceeds - Update doc comment to clarify looping behavior and note that restore triggers are mutually exclusive with order submission * fix(restore): wait for node info before sending restore request to prevent v1/v2 protocol mismatch - Add _waitForNodeConnectivity call before _createTempSubscription to ensure mostroInstance is populated - Prevents wrapForTransport from defaulting to v1 gift wrap when mostroInstance is null at app init - Avoids protocol v2 nodes receiving kind 1059 requests they won't answer, which would leave local key index stale * fix(order): replace restore wait with session lock to close TOCTOU race in order creation - Replace awaitOperationCompletion with acquireSessionLock in AddOrderNotifier.submitOrder - Wrap session creation in try/finally with lock release to serialize with restore's _clearAll - Add _sessionLockTail future chain to RestoreService to implement critical-section lock - Hold session lock for entire restore process in initRestoreProcess - Prevents race where restore's session reset interleaves with order creation even after wait compl * fix(order): move submitOrder inside session lock to prevent session orphaning - Move mostroService.submitOrder and state update inside try/finally lock block - Prevents race where restore's _clearAll can wipe session after creation but before publish - Ensures entire create critical section (session + publish) is atomic with respect to restore * refactor(session): extract session lifecycle lock to shared provider for reuse across order flows * fix(restore): hold session lock during syncTradeIndex to prevent stale index reads - Acquire sessionLifecycleLock before updating trade index in syncTradeIndex - Prevents order/take flows from creating sessions with stale keyIndex while sync is in progress - Release lock in finally block after index update completes * fix(order): serialize range-order child session creation with restore * refactor(message): extract wrapper-key selection into a single getter - Deduplicate the restore/order wrapper-key logic repeated in sign(), serialize() and wrapNip44() into one _wrapperKey getter - The key is load-bearing for the message signature; a single source of truth prevents the three copies from drifting into mismatched signatures * docs(transport): document session lock, dispute wire change, optional expiration --------- Co-authored-by: grunch <[email protected]> Co-authored-by: Catrya <[email protected]>
2 participants
Implements Phase B — Dual send of the transport v1 → v2 migration (
docs/architecture/TRANSPORT_V2_MIGRATION.md). Builds on Phase A (#621, merged), routing outbound Mostro messages through the per-node resolved transport.Changes
MostroMessage.wrapNip44): builds the[message, tradeSig, identityProof]3-tuple withversion: 2, NIP-44 encrypts it toward the node, and emits a kind-14 event signed by the trade key with aptag. PoW is preserved (mined on the kind-14 id).mostro-core'stransport.rs: trade-key signature = existingMostroMessage.sign(SHA-256 hex digest, Schnorr); identity proof = master key signingmostro-transport-v2-identity:<tradePubkey>:<messageJSON>. Both arenullin full-privacy mode.versionthreading: derived from the resolved transport instead of the global constant (1 for gift wrap, 2 for NIP-44).MostroService.publishOrder): selectswrapNip44(v2) orwrap(v1) from the node'sprotocol_version.Scope
protocol_version=2(none in production yet).expirationtag is omitted (optional/caller-supplied per the daemon), avoiding any risk of a message expiring before processing.Verification
flutter analyze: no issues.flutter test: all pass, including a new v2 wrap → unwrap round-trip (reputation + full-privacy) that cryptographically verifies the trade signature and identity proof, plus v1 regression.Summary by CodeRabbit