Step 3 - refactor(dm): parse inbound GiftWrap with mostro_core unwrap_message

[arkanoider]

Replace nostr-sdk nip59::extract_rumor for inbound trade DMs with mostro_core::unwrap_message (Ok(None) for wrong recipient). Reuse UnwrappedMessage in parse_dm_events_single / resolve_order_for_event / startup replay / listener probes; simplify rumor_cache keys to PublicKey.

Summary by CodeRabbit

Release Notes

• Refactor
  • Optimized message processing and decryption performance
  • Improved startup synchronization efficiency
  • Enhanced message selection logic for reliability

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f3ded602-0e8b-4a8b-aa47-e5684fd01140

📥 Commits

Reviewing files that changed from the base of the PR and between c98bc3b and f9fccc2.

📒 Files selected for processing (1)

• src/util/dm_utils/mod.rs

---

Walkthrough

This PR refactors GiftWrap parsing throughout src/util/dm_utils/mod.rs to centralize unwrapping logic via a shared unwrap_message helper, replacing scattered nip59 rumor extraction with a unified UnwrappedMessage type. It introduces a pre_unwrapped fast-path parameter to avoid redundant decrypt work, updates fallback decryption and startup DM replay selection, and reworks the relay notification handler's decryptability cache logic across waiter and subscription paths.

Changes

GiftWrap Unwrap Centralization

Layer / File(s)	Summary
GiftWrap Unwrap Helper & Fast-path src/util/dm_utils/mod.rs	GiftWrap parsing routes through unwrap_message(dm, pubkey) helper. parse_dm_events_single now accepts pre_unwrapped: Option<UnwrappedMessage> parameter to skip redundant decrypt/parse when message is already unwrapped.
Return Type Update src/util/dm_utils/mod.rs	resolve_order_for_event return signature changed from nip59 UnwrappedGift to UnwrappedMessage, aligning fallback decryption with centralized unwrap pattern.
Fallback Decryption Refactor src/util/dm_utils/mod.rs	Fallback decryption loop now unwraps via unwrap_message(event, &trade_keys).await, treats Ok(Some(unwrapped)) as successful match, and returns UnwrappedMessage. Comment explains reuse pattern avoids double unwrapping.
Startup DM Replay Selection src/util/dm_utils/mod.rs	Startup replay selection unwraps each relay event via unwrap_message(event, &trade_keys) instead of nip59 rumor extraction, parses with pre_unwrapped fast-path, and selects batch entry by freshest decrypted rumor timestamp.
Relay Notification Decryptability Cache src/util/dm_utils/mod.rs	Waiter and tracked-subscription paths now compute can_decrypt boolean using unwrap_message success check (Ok(Some(_))) rather than nip59 rumor extraction, centralizing decryptability detection across handler paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• MostroP2P/app#101: Refactoring directly implements migration of custom wrap/unwrap logic to mostro-core's unwrap_message/UnwrappedMessage API, replacing nip59 rumor extraction with centralized unwrap helper across all GiftWrap parsing paths.

Possibly related PRs

• MostroP2P/mostrix#64: Both PRs refactor GiftWrap handling in src/util/dm_utils/mod.rs; PR #64 updates GiftWrap construction (sending side) using mostro-core's wrap_message while this PR refactors unwrapping/decryptability (receiving side), forming complementary send/receive pipeline updates.

Suggested reviewers

• mostronatorcoder

Poem

🐰 A rabbit hops through unwrap-wrapped code,
Unifies the GiftWrap decryption road,
No more rumor tangles, the paths align,
Pre-unwrapped fast-lanes let messages shine,
The cache now speaks in one true tongue! 🎁

🚥 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 title clearly describes the main refactoring work: switching from nostr-sdk's nip59::extract_rumor to mostro_core's unwrap_message for parsing inbound GiftWrap messages.
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 docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/dm-inbound-unwrap-message

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[mostronatorcoder]

I reviewed this carefully and I do not see a blocker in the current diff.

What I checked:

• the inbound GiftWrap refactor from nostr-sdk extraction to mostro_core::unwrap_message
• the semantics of Ok(None) for non-recipient / non-decryptable events, especially in the waiter path and tracked-subscription path
• the startup replay path, where this PR now reuses UnwrappedMessage instead of re-extracting the rumor again
• the unknown-subscription fallback (resolve_order_for_event) and whether the decoded message is still reused correctly there
• the per-event decryptability cache, now keyed by PublicKey, to make sure the key space is still correct for a single incoming event

The behavior looks coherent to me:

• unwrap_message is now the single inbound decode primitive, which matches the outbound migration done in the previous step
• the Ok(None) handling is a good fit for the current routing logic, because “not for this key” is not treated as an error anymore
• startup replay / fallback paths still preserve the important optimization of avoiding a second unwrap when one already succeeded
• simplifying the cache key to PublicKey is fine here because the cache lifetime is scoped to one event handler block

Checks are green too, so this looks good from my side. Nice follow-up to the outbound NIP-59 cleanup, and it reduces another source of drift between Mostrix and mostro-core.