feat: handle deep links from different Mostro instances#552
Merged
Conversation
Implements #541 When a deep link contains mostro=<pubkey> targeting a different Mostro instance, the app shows a confirmation dialog before switching. ## Changes - parseMostroUrl: extract optional 'mostro' query parameter - OrderInfo: add mostroPubkey field - DeepLinkHandler: compare link pubkey with current settings, show switch dialog if different, call updateMostroInstance on confirm - l10n: add EN/ES strings for the switch dialog - Tests: URL parsing with/without mostro param - Docs: DEEP_LINK_MOSTRO_SWITCH.md Backward compatible: links without mostro= param work as before. Closes #541
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughParses optional Changes
Sequence DiagramsequenceDiagram
participant User
participant App as DeepLinkHandler
participant Parser as DeepLinkService/NostrUtils
participant Settings as SettingsProvider
participant Dialog as MostroSwitchDialog
participant Router as GoRouter
User->>App: Open deep link (orderId, relays, optional mostroPubkey)
App->>Parser: parseMostroUrl(url) / processMostroLink(...)
Parser-->>App: OrderInfo(orderId, relays, mostroPubkey)
App->>Settings: read current mostroPublicKey
Settings-->>App: currentPubkey
alt mostroPubkey missing or equals currentPubkey
App->>Router: Navigate to order using relays/orderId
else mostroPubkey differs
App->>Dialog: Show MostroSwitchDialog(linkPubkey, currentPubkey)
Dialog->>User: Prompt "Switch & View" or "Cancel"
alt User confirms
User->>Dialog: Confirm
Dialog-->>App: true
App->>Settings: updateMostroInstance(mostroPubkey)
Settings-->>App: updated
App->>Router: Navigate to order
else User cancels
User->>Dialog: Cancel
Dialog-->>App: false
App-->>App: Abort navigation
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/deep_link_service.dart (1)
91-113:⚠️ Potential issue | 🟠 MajorDon't drop the relay list after this lookup.
The PR objective explicitly says relays from the URL must be honored, but here
relaysare only used to resolve the initial order type. The returnedOrderInfostill doesn't carry them, andDeepLinkHandlerlater only navigates with that model, so the order screen has no way to keep using the relay list from the link. A valid deep link can therefore resolve successfully here and still fail once the destination screen refetches against the current settings relays.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 91 - 113, The code currently discards the relays parsed from the deep link after calling _fetchOrderInfoById; ensure the relay list from the URL is preserved in the returned model so downstream navigation uses the link relays. Update the logic around fetchedOrderInfo (from _fetchOrderInfoById) to attach or merge the parsed relays (relays variable) and mostroPubkey into the returned OrderInfo (or whichever model DeepLinkHandler receives) so that DeepLinkHandler and the order screen can continue using the link-provided relay list instead of falling back to current settings.
🧹 Nitpick comments (2)
lib/core/deep_link_handler.dart (1)
167-205: Open this dialog on the next frame.This new
showDialogside effect is invoked directly from the handler, right after popping the loading dialog. The repo convention is to schedule dialogs and snackbars from a post-frame callback to avoid lifecycle/build races, which would also keep this flow aligned with the deferred navigation below.As per coding guidelines,
**/{lib,test}/**/*.dart: Use post-frame callbacks for side effects like SnackBars and dialogs to avoid state management issues.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/core/deep_link_handler.dart` around lines 167 - 205, The dialog is being shown synchronously; change the showDialog call so it runs inside a post-frame callback (e.g. WidgetsBinding.instance.addPostFrameCallback or SchedulerBinding.instance.addPostFrameCallback) to match project convention and avoid lifecycle/build races — wrap the existing showDialog<bool>(context: context, builder: ...) invocation (the AlertDialog that references truncatedLink, truncatedCurrent, s, dialogContext, etc.) inside that post-frame callback and keep the same actions and returned bool, adding the needed import if missing.test/shared/utils/deep_link_parsing_test.dart (1)
63-88: These tests don't exercise the production switch logic.Lines 67-86 only restate Dart
==andnullsemantics, so they stay green even if the real deep-link comparison flow regresses. Replace them with coverage around a real helper/predicate or the handler path so malformed or normalized pubkeys are actually tested.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/shared/utils/deep_link_parsing_test.dart` around lines 63 - 88, Replace the three trivial equality/null tests with tests that exercise the actual deep-link parsing/comparison logic: use DeepLinkService (or the parsing/handler that constructs OrderInfo) to parse a link into an OrderInfo and compare its mostroPubkey against a current pubkey (and null) so normalization/malformed variants are covered; specifically, call the real parser/handler that returns OrderInfo (e.g., DeepLinkService.parse or OrderInfo.fromDeepLink) and assert that mostroPubkey equality behaves correctly for identical hex, different hex, normalized (mixed-case/leading zeros) and null values rather than using direct `==` on raw strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/core/deep_link_handler.dart`:
- Around line 183-190: The dialog currently always shows truncated pubkeys
(using truncatedLink and truncatedCurrent) but should show node labels when
available; change the UI to accept two new string inputs (e.g., targetName and
currentName) or resolve them before building the Text widgets and render
"${s.deepLinkDifferentMostroFrom}\n${targetName.isNotEmpty ? targetName :
truncatedLink}" and
"${s.deepLinkDifferentMostroCurrent}\n${currentName.isNotEmpty ? currentName :
truncatedCurrent}"; update the caller(s) that build this dialog to pass the
resolved node labels (or empty string/null) so the Text widgets fallback to
truncatedLink/truncatedCurrent only when no label exists.
In `@lib/l10n/intl_en.arb`:
- Around line 1508-1512: Add the five missing deep-link localization keys to the
Italian ARB by inserting entries for deepLinkDifferentMostroTitle,
deepLinkDifferentMostroBody, deepLinkDifferentMostroFrom,
deepLinkDifferentMostroCurrent, and deepLinkSwitchAndView with appropriate
Italian translations (matching the English meanings) so Italian falls back no
longer; ensure each key uses the same key names and string values in Italian and
follows the existing ARB formatting/conventions.
In `@lib/services/deep_link_service.dart`:
- Around line 177-179: The code maps any non-'sell' k tag to OrderType.buy,
which misclassifies unknown k values; change the parsing so that when
orderTypeValue is not exactly 'sell' or 'buy' the function returns null (or
otherwise fails fast) instead of defaulting to OrderType.buy. Locate the
assignment using orderTypeValue and OrderType (in deep_link_service.dart) and
implement a strict branch that maps 'sell' -> OrderType.sell, 'buy' ->
OrderType.buy, and returns null on any other value (mirroring the behavior of
NostrService.fetchOrderInfoByEventId()).
In `@lib/shared/utils/nostr_utils.dart`:
- Around line 177-181: Validate and normalize the extracted mostro pubkey before
adding it to result: in the block that reads uri.queryParameters['mostro']
(variable mostroPubkey) only accept a 64-character hexadecimal string (e.g.
apply a /^[0-9a-f]{64}$/i check after trimming and removing any leading 0x, then
normalize to lowercase) and set result['mostroPubkey'] to that normalized value;
otherwise ignore the parameter (do not propagate malformed values) so downstream
code like updateMostroInstance(...) never receives invalid keys.
---
Outside diff comments:
In `@lib/services/deep_link_service.dart`:
- Around line 91-113: The code currently discards the relays parsed from the
deep link after calling _fetchOrderInfoById; ensure the relay list from the URL
is preserved in the returned model so downstream navigation uses the link
relays. Update the logic around fetchedOrderInfo (from _fetchOrderInfoById) to
attach or merge the parsed relays (relays variable) and mostroPubkey into the
returned OrderInfo (or whichever model DeepLinkHandler receives) so that
DeepLinkHandler and the order screen can continue using the link-provided relay
list instead of falling back to current settings.
---
Nitpick comments:
In `@lib/core/deep_link_handler.dart`:
- Around line 167-205: The dialog is being shown synchronously; change the
showDialog call so it runs inside a post-frame callback (e.g.
WidgetsBinding.instance.addPostFrameCallback or
SchedulerBinding.instance.addPostFrameCallback) to match project convention and
avoid lifecycle/build races — wrap the existing showDialog<bool>(context:
context, builder: ...) invocation (the AlertDialog that references
truncatedLink, truncatedCurrent, s, dialogContext, etc.) inside that post-frame
callback and keep the same actions and returned bool, adding the needed import
if missing.
In `@test/shared/utils/deep_link_parsing_test.dart`:
- Around line 63-88: Replace the three trivial equality/null tests with tests
that exercise the actual deep-link parsing/comparison logic: use DeepLinkService
(or the parsing/handler that constructs OrderInfo) to parse a link into an
OrderInfo and compare its mostroPubkey against a current pubkey (and null) so
normalization/malformed variants are covered; specifically, call the real
parser/handler that returns OrderInfo (e.g., DeepLinkService.parse or
OrderInfo.fromDeepLink) and assert that mostroPubkey equality behaves correctly
for identical hex, different hex, normalized (mixed-case/leading zeros) and null
values rather than using direct `==` on raw strings.
🪄 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: 88974119-1f47-4ad9-b1be-448320ab6d68
📒 Files selected for processing (7)
docs/DEEP_LINK_MOSTRO_SWITCH.mdlib/core/deep_link_handler.dartlib/l10n/intl_en.arblib/l10n/intl_es.arblib/services/deep_link_service.dartlib/shared/utils/nostr_utils.darttest/shared/utils/deep_link_parsing_test.dart
…n, dialog - Validate mostro pubkey: must be 64-char hex, normalized to lowercase (rejects malformed/short/non-hex values silently) - Strict k-tag parsing: unknown order types return null instead of defaulting to OrderType.buy - Add l10n strings for IT, DE, FR (deep link switch dialog) - Dialog: accept optional targetName/currentName for node labels, fall back to truncated pubkeys. Uses post-frame callback to avoid lifecycle/build races. - Tests: pubkey validation (short, non-hex, uppercase normalization)
Tests now exercise NostrUtils.parseMostroUrl for instance comparison: - Same pubkey matches current instance - Different pubkey does not match - Mixed-case normalized and matches lowercase - Absent mostro param treated as same instance - Malformed pubkey silently dropped (treated as same instance)
Catrya
requested changes
Mar 30, 2026
Member
Catrya
left a comment
Catrya
left a comment
There was a problem hiding this comment.
At the moment I tap “Switch & View,” a red error screen appears for a few seconds with the message “Null check operator used on a null value.” Then, after switching to Mostro and viewing the order, it gets stuck on “Loading order...” and does not disappear unless the user manually presses the back button. I’m sharing all the logs.
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/deep_link_service.dart (1)
93-113:⚠️ Potential issue | 🟠 MajorBind the link's Mostro pubkey to the fetched event.
mostroPubkeyis passed through from the query string, but_fetchOrderInfoById()still selectsevents.firstpurely by#d. A crafted or ambiguous link can therefore resolve order A, switch the app to node B, and then open the order under the wrong instance. WhenmostroPubkeyis present, only accept an event from that author, or reject the link on mismatch.Also applies to: 166-190
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 93 - 113, The fetched order selection must respect the optional mostroPubkey from the query string: update _fetchOrderInfoById (and any secondary fetch path used around the other block) to, when a non-null mostroPubkey is provided, only accept an event whose author/pubkey equals mostroPubkey rather than unconditionally using events.first; if no matching event exists return DeepLinkResult.error(S.of(context)!.deepLinkInvalidOrderId) (or the existing error for mismatch) and log the mismatch. Locate usages of mostroPubkey and events.first in _fetchOrderInfoById (and the follow-up fetch path referenced in the other block) and add the author check/filter, rejecting the link when the author does not match.
🧹 Nitpick comments (1)
test/shared/utils/deep_link_parsing_test.dart (1)
69-79: Lock in the full pubkey-normalization contract.The parser also trims whitespace and strips a leading
0xbefore validating the pubkey, but this suite only locks lowercase normalization. Please add one case like" mostro=0xABC... "so those behaviors do not regress silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/shared/utils/deep_link_parsing_test.dart` around lines 69 - 79, Add a new unit test for NostrUtils.parseMostroUrl that asserts the parser trims surrounding whitespace and strips a leading "0x" before validating/normalizing the pubkey: create a test (e.g., 'normalizes pubkey with 0x and whitespace to lowercase') that calls NostrUtils.parseMostroUrl with a URL containing mostro=" 0xABC... " (uppercased, with spaces and 0x) and assert result!['mostroPubkey'] equals the lowercase hex string without the 0x prefix; this ensures the trimming and 0x-stripping behavior is locked in alongside the existing lowercase normalization test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 88-89: The _CountdownWidget is being rendered for all orders —
change the rendering so _CountdownWidget(order: order) is only included when
order.status indicates pending (e.g., OrderStatus.pending or equivalent) and for
non-pending waiting orders render the separate waiting countdown system (or
nothing) as per guidelines; also add a pending-status guard inside
_buildCountDownTime so it returns a meaningful value only for pending orders
(and use the alternate countdown logic for waiting orders); apply the same
change where similar countdowns are rendered in the code block around the other
mentioned section (the block referenced by the reviewer).
- Around line 258-259: After awaiting showDialog<int>(...) in the
TakeOrderScreen state, guard any post-await UI updates by checking the widget is
still mounted; specifically, before calling setState(...) after the await, add
`if (!mounted) return;` to avoid calling setState on a disposed State. Apply the
same mounted check for the other post-await update block around the showDialog
result handling (the code that reads enteredAmount and then calls setState) so
all BuildContext/use of setState after async awaits in this State are protected.
- Around line 137-141: The code is doing brittle English-only surgery by calling
replaceAll(' Sats', '') on localized titles; update localization to add
dedicated keys with placeholders for amount (e.g., someoneIsSellingWithAmount
and someoneIsBuyingWithAmount) and use those keys with the amount plus the
hardcoded "Sats" suffix when rendering (referencing
S.of(context)!.someoneIsSellingWithAmount / someoneIsBuyingWithAmount,
widget.orderType and order.amount) so you avoid manipulating localized strings
directly and preserve correct translations across locales.
- Around line 66-67: The UI currently shows a spinner whenever order == null,
which hides error/not-found states; update the body to only show
CircularProgressIndicator when the provider indicates a loading state (e.g.,
check eventProvider.isLoading or AsyncValue.isLoading), and render an
error/empty-state widget when order == null but provider is in error or
not-found state; specifically modify the conditional around the order variable
in take_order_screen.dart to use eventProvider's status (or
AsyncValue.hasError/whenData/when) instead of just order == null so loading,
error, and empty are handled separately.
In `@lib/l10n/intl_de.arb`:
- Around line 1508-1512: The new German strings use formal "Sie" and the word
"Bestellung" which is inconsistent with the repo's informal tone and existing
term "Order"; update the keys deepLinkDifferentMostroBody,
deepLinkDifferentMostroFrom, deepLinkDifferentMostroCurrent (and any related
deepLinkDifferentMostroTitle / deepLinkSwitchAndView if needed) to use informal
"du"/"dich" phrasing and replace "Bestellung" with the project's established
German term "Order" so the dialog matches the rest of intl_de.arb.
---
Outside diff comments:
In `@lib/services/deep_link_service.dart`:
- Around line 93-113: The fetched order selection must respect the optional
mostroPubkey from the query string: update _fetchOrderInfoById (and any
secondary fetch path used around the other block) to, when a non-null
mostroPubkey is provided, only accept an event whose author/pubkey equals
mostroPubkey rather than unconditionally using events.first; if no matching
event exists return DeepLinkResult.error(S.of(context)!.deepLinkInvalidOrderId)
(or the existing error for mismatch) and log the mismatch. Locate usages of
mostroPubkey and events.first in _fetchOrderInfoById (and the follow-up fetch
path referenced in the other block) and add the author check/filter, rejecting
the link when the author does not match.
---
Nitpick comments:
In `@test/shared/utils/deep_link_parsing_test.dart`:
- Around line 69-79: Add a new unit test for NostrUtils.parseMostroUrl that
asserts the parser trims surrounding whitespace and strips a leading "0x" before
validating/normalizing the pubkey: create a test (e.g., 'normalizes pubkey with
0x and whitespace to lowercase') that calls NostrUtils.parseMostroUrl with a URL
containing mostro=" 0xABC... " (uppercased, with spaces and 0x) and assert
result!['mostroPubkey'] equals the lowercase hex string without the 0x prefix;
this ensures the trimming and 0x-stripping behavior is locked in alongside the
existing lowercase normalization test.
🪄 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: 8ddb978c-0ce7-409f-abba-651a5a88d732
📒 Files selected for processing (8)
lib/core/deep_link_handler.dartlib/features/order/screens/take_order_screen.dartlib/l10n/intl_de.arblib/l10n/intl_fr.arblib/l10n/intl_it.arblib/services/deep_link_service.dartlib/shared/utils/nostr_utils.darttest/shared/utils/deep_link_parsing_test.dart
🚧 Files skipped from review as they are similar to previous changes (2)
- lib/shared/utils/nostr_utils.dart
- lib/core/deep_link_handler.dart
- Distinguish loading vs error/not-found state when order == null (show spinner only while loading, icon when not found) - Guard _CountdownWidget to only render for pending orders - Add mounted check before setState after awaited showDialog - Fix German locale to use informal du/dich and 'Order' instead of 'Bestellung' to match existing intl_de.arb tone and terminology
- Filter fetched NIP-69 events by event.pubkey when mostroPubkey is present in the deep link - Reject the link if no event from that specific Mostro node is found - Log a warning with event count from other nodes for easier debugging - Prevents a crafted link from switching the app to a fraudulent Mostro instance by exploiting a legitimate order published by a different node - Falls back to existing behavior (events.first) when mostroPubkey is absent from the link
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
lib/features/order/screens/take_order_screen.dart (1)
145-149:⚠️ Potential issue | 🟡 MinorStop editing translated titles with
replaceAll(' Sats', '').This only works for the current English copy. Please add dedicated localized templates for the amount variants instead of mutating an already-translated title at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 145 - 149, The code is mutating translated titles with replaceAll(' Sats', ''); instead add dedicated localized templates and use them when hasFixedSatsAmount is true: create new i18n entries (e.g., someoneIsSellingWithAmount and someoneIsBuyingWithAmount) that accept an amount parameter, then update the Text call in take_order_screen.dart to call S.of(context)!.someoneIsSellingWithAmount(order.amount) for OrderType.sell (and someoneIsBuyingWithAmount for the buy case) instead of manipulating someoneIsSellingTitle/someoneIsBuyingTitle with replaceAll; keep the existing titles when hasFixedSatsAmount is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 47-58: The listener currently only clears _isSubmitting for
dialog-cancel and actions.Action.cantDo, leaving the CTA disabled for other
terminal outcomes; update the
ref.listen(mostroMessageStreamProvider(widget.orderId)) handler to reset
_isSubmitting for every terminal submit path by checking msg.action against all
terminal outcomes (not just actions.Action.cantDo) and calling setState(() =>
_isSubmitting = false) when a terminal action is observed (use _lastSeenAction
to avoid duplicates), and also add finally blocks or catch/finally handling to
the takeBuyOrder and takeSellOrder call sites to ensure _isSubmitting is cleared
when those futures complete or throw. Ensure you reference and update the same
symbols: ref.listen, mostroMessageStreamProvider, _lastSeenAction,
_isSubmitting, actions.Action.cantDo, takeBuyOrder, and takeSellOrder.
In `@lib/services/deep_link_service.dart`:
- Around line 172-176: The warning message in deep_link_service.dart is escaping
the interpolation so it logs the literal "\${events.length}" instead of the
actual count; update the logger.w call (the block that checks
candidateEvents.isEmpty && mostroPubkey != null) to use real string
interpolation for events.length (e.g., include ${events.length}) so the log
shows how many mismatched-node events were seen for orderId and mostroPubkey.
- Around line 165-170: Filter candidate events only after verifying signatures:
when building candidateEvents (using mostroPubkey, events, and e.pubkey) first
filter events by event.isVerified() and then apply the mostroPubkey equality
check so you only trust verified authors; update the logic that assigns
candidateEvents to use events.where((e) => e.isVerified() && (mostroPubkey ==
null || e.pubkey == mostroPubkey)). Also fix the log interpolation by changing
the escaped string "\${events.length}" to use real interpolation
"${events.length}" so the event count is printed.
---
Duplicate comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 145-149: The code is mutating translated titles with replaceAll('
Sats', ''); instead add dedicated localized templates and use them when
hasFixedSatsAmount is true: create new i18n entries (e.g.,
someoneIsSellingWithAmount and someoneIsBuyingWithAmount) that accept an amount
parameter, then update the Text call in take_order_screen.dart to call
S.of(context)!.someoneIsSellingWithAmount(order.amount) for OrderType.sell (and
someoneIsBuyingWithAmount for the buy case) instead of manipulating
someoneIsSellingTitle/someoneIsBuyingTitle with replaceAll; keep the existing
titles when hasFixedSatsAmount is false.
🪄 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: 916c46ba-0904-4f7f-9e77-90072dd9c9d7
📒 Files selected for processing (3)
lib/features/order/screens/take_order_screen.dartlib/l10n/intl_de.arblib/services/deep_link_service.dart
✅ Files skipped from review due to trivial changes (1)
- lib/l10n/intl_de.arb
… state
- Add isVerified() check per event before accepting it as a candidate
in _fetchOrderInfoById; log a warning on failure but do not reject
the event, consistent with how mostro_nodes_notifier handles kind-0
events (known dart_nostr limitation)
- Fix escaped string interpolation in logger.w so ${events.length}
now correctly prints the actual event count instead of the literal text
- Expand the terminal-action set in mostroMessageStreamProvider listener
to include canceled, paymentFailed and holdInvoicePaymentCanceled
alongside cantDo so _isSubmitting is cleared for every terminal outcome
- Wrap takeBuyOrder and takeSellOrder call sites in try/catch blocks to
ensure _isSubmitting is reset when those futures throw, preventing the
CTA button from being permanently disabled after an error
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
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/services/deep_link_service.dart (1)
148-188:⚠️ Potential issue | 🟠 MajorRun the default-relay fallback after the Mostro filter, not before it.
The retry is still gated by
events.isEmpty. WithmostroPubkeypresent, a non-empty batch of same-devents from other nodes skips the fallback and returnsnullimmediately, even though the correct order may still exist on the app's default relays. This should retry whencandidateEventsis empty too.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 148 - 188, The fallback to default relays is currently conditioned on events.isEmpty which skips retry when events exist but none match mostroPubkey; change the flow so that after calling nostrService.fetchEvents with specific relays and computing candidateEvents (the filtered list using mostroPubkey), if candidateEvents.isEmpty and default relays are available then call nostrService.fetchEvents(filter) to fetch defaultEvents, merge into events, and recompute candidateEvents (applying the same mostroPubkey filter and verification logging) before returning or logging the final warning for orderId; use the existing symbols events, candidateEvents, relays, mostroPubkey, nostrService.fetchEvents, filter and logger to locate and implement this adjustment.
♻️ Duplicate comments (3)
lib/services/deep_link_service.dart (1)
165-180:⚠️ Potential issue | 🔴 CriticalReject unverified events before trusting
e.pubkey.
mostro=only protects this flow if the matching event is authenticated. Right now a failedisVerified()just logs and the event still qualifies, so an attacker-controlled relay can fabricate a matching pubkey and drive the wrong Mostro-switch path. If thisdart_nostrversion does false-negative on valid events, this path needs a stronger verification source rather than a silent bypass.🔒 Suggested guard
- final candidateEvents = mostroPubkey != null - ? events.where((e) { - if (!e.isVerified()) { - logger.w( - 'Event ${e.id} from pubkey ${e.pubkey} failed signature ' - 'verification (possible dart_nostr limitation).', - ); - } - return e.pubkey == mostroPubkey; - }).toList() + final candidateEvents = mostroPubkey != null + ? events.where((e) { + final verified = e.isVerified(); + if (!verified) { + logger.w( + 'Ignoring unverified event ${e.id} for order $orderId ' + 'from pubkey ${e.pubkey}.', + ); + } + return verified && e.pubkey == mostroPubkey; + }).toList() : events;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 165 - 180, The current filter for candidateEvents lets events with failing e.isVerified() pass as long as e.pubkey matches mostroPubkey; update the predicate used in events.where so that unverified events are rejected before comparing e.pubkey (i.e., if !e.isVerified() then log a warning and return false), ensuring candidateEvents only includes events where e.isVerified() is true and e.pubkey == mostroPubkey; touch the filter closure that builds candidateEvents and keep the existing logger.w call but move it to the rejection branch.lib/features/order/screens/take_order_screen.dart (2)
47-66:⚠️ Potential issue | 🟠 Major
_isSubmittingcan still stick on uncovered terminal actions.This listener now handles more cases, but the inline set still only covers four outcomes.
lib/data/models/enums/action.dart, Lines 1-44 contains other terminal states likeadminCanceled,adminSettled,cooperativeCancelAccepted,released, andrateReceived; if any of those end the flow on this screen, the CTA stays disabled. This should be centralized instead of maintained as a partial set here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 47 - 66, The listener in TakeOrderScreen (the ref.listen block using mostroMessageStreamProvider and the _isSubmitting flag) only resets _isSubmitting for a hard-coded four-action set, leaving other terminal actions in actions.Action unhandled; add a centralized predicate (e.g., Action.isTerminal or a helper function isTerminalAction(Action a) placed with the enum in lib/data/models/enums/action.dart) that returns true for all terminal outcomes (adminCanceled, adminSettled, cooperativeCancelAccepted, released, rateReceived, cantDo, canceled, paymentFailed, holdInvoicePaymentCanceled, etc.), then replace the inline terminalActions.contains check in the listener with that predicate so all terminal states automatically reset _isSubmitting.
153-159:⚠️ Potential issue | 🟡 MinorDon't splice localized titles with English text.
replaceAll(' Sats', '')depends on the current English translation shape. A locale change or copy edit will either do nothing or remove the wrong text. Add dedicated l10n keys with an amount placeholder and append the hardcodedSatssuffix instead of rewriting localized output at runtime.Based on learnings: Do not localize the term 'sats' (satoshis) in the Mostro mobile app. Treat 'sats' as a universal Bitcoin unit and keep it as hardcoded text in Dart UI code.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 153 - 159, The current code splices localized strings with English text by using replaceAll(' Sats', '') on S.of(context)!.someoneIsSellingTitle / someoneIsBuyingTitle; instead add new l10n keys with an amount placeholder (e.g., someoneIsSellingWithAmount and someoneIsBuyingWithAmount) that accept the amount, use those via S.of(context)!.someoneIsSellingWithAmount(order.amount) / someoneIsBuyingWithAmount(order.amount) when hasFixedSatsAmount is true, and then append the hardcoded " Sats" suffix in the UI (do not attempt to localize or strip "Sats"); remove any replaceAll usage and keep the branch using widget.orderType and order.amount to select the correct localized key.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 75-84: The empty-state fires too early because you only check
order == null and orderEventsAsync.isLoading; instead show the search icon only
after the stream/provider has actually emitted and confirmed "no value". Change
the ternary in take_order_screen (the body that reads order and
orderEventsAsync) to display the loading spinner while the provider/stream has
not yet emitted, and only show the search Icon when the stream has
emitted/settled with no order (e.g. replace the isLoading check with a guard
like !orderEventsAsync.hasValue or !orderEventsAsync.hasData / snapshot.hasData
depending on your provider API), so switching Mostro via updateMostroInstance()
won’t trigger the empty-state prematurely.
---
Outside diff comments:
In `@lib/services/deep_link_service.dart`:
- Around line 148-188: The fallback to default relays is currently conditioned
on events.isEmpty which skips retry when events exist but none match
mostroPubkey; change the flow so that after calling nostrService.fetchEvents
with specific relays and computing candidateEvents (the filtered list using
mostroPubkey), if candidateEvents.isEmpty and default relays are available then
call nostrService.fetchEvents(filter) to fetch defaultEvents, merge into events,
and recompute candidateEvents (applying the same mostroPubkey filter and
verification logging) before returning or logging the final warning for orderId;
use the existing symbols events, candidateEvents, relays, mostroPubkey,
nostrService.fetchEvents, filter and logger to locate and implement this
adjustment.
---
Duplicate comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 47-66: The listener in TakeOrderScreen (the ref.listen block using
mostroMessageStreamProvider and the _isSubmitting flag) only resets
_isSubmitting for a hard-coded four-action set, leaving other terminal actions
in actions.Action unhandled; add a centralized predicate (e.g.,
Action.isTerminal or a helper function isTerminalAction(Action a) placed with
the enum in lib/data/models/enums/action.dart) that returns true for all
terminal outcomes (adminCanceled, adminSettled, cooperativeCancelAccepted,
released, rateReceived, cantDo, canceled, paymentFailed,
holdInvoicePaymentCanceled, etc.), then replace the inline
terminalActions.contains check in the listener with that predicate so all
terminal states automatically reset _isSubmitting.
- Around line 153-159: The current code splices localized strings with English
text by using replaceAll(' Sats', '') on S.of(context)!.someoneIsSellingTitle /
someoneIsBuyingTitle; instead add new l10n keys with an amount placeholder
(e.g., someoneIsSellingWithAmount and someoneIsBuyingWithAmount) that accept the
amount, use those via S.of(context)!.someoneIsSellingWithAmount(order.amount) /
someoneIsBuyingWithAmount(order.amount) when hasFixedSatsAmount is true, and
then append the hardcoded " Sats" suffix in the UI (do not attempt to localize
or strip "Sats"); remove any replaceAll usage and keep the branch using
widget.orderType and order.amount to select the correct localized key.
In `@lib/services/deep_link_service.dart`:
- Around line 165-180: The current filter for candidateEvents lets events with
failing e.isVerified() pass as long as e.pubkey matches mostroPubkey; update the
predicate used in events.where so that unverified events are rejected before
comparing e.pubkey (i.e., if !e.isVerified() then log a warning and return
false), ensuring candidateEvents only includes events where e.isVerified() is
true and e.pubkey == mostroPubkey; touch the filter closure that builds
candidateEvents and keep the existing logger.w call but move it to the rejection
branch.
🪄 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: 9bcac224-7dd8-43d1-9e5b-8edd235d197c
📒 Files selected for processing (2)
lib/features/order/screens/take_order_screen.dartlib/services/deep_link_service.dart
…re empty-state - Add Action.isTerminal getter to the Action enum covering all terminal outcomes (cantDo, canceled, adminCanceled, adminSettled, cooperativeCancelAccepted, released, rateReceived, paymentFailed, holdInvoicePaymentCanceled, holdInvoicePaymentSettled, purchaseCompleted) so the UI never needs to maintain a partial hard-coded set - Replace inline terminalActions set in TakeOrderScreen listener with msg.action.isTerminal for automatic coverage of future terminal actions - Fix relay fallback logic in _fetchOrderInfoById: retry with default relays when candidateEvents is empty (not just when events is empty), which previously skipped the fallback when link relays returned events from other Mostro nodes that did not match the requested mostroPubkey - Fix premature empty-state icon after Mostro switch by checking !orderEventsAsync.hasValue instead of orderEventsAsync.isLoading; the StreamProvider does not re-enter loading when the repository updates in place via updateMostroInstance so isLoading was unreliable
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/services/deep_link_service.dart (1)
162-171:⚠️ Potential issue | 🔴 CriticalReject unverified events before matching the linked Mostro.
Line 165 logs a failed signature check but still falls through to
e.pubkey == mostroPubkey. A forged event from a link relay can then satisfy the newmostrogate and suppress the default-relay fallback, so this flow can resolve or switch against an unverified author. IfisVerified()has false negatives in the currentdart_nostrversion, that needs a separate fix; this filter should fail closed here.🐛 Proposed fix
List<NostrEvent> buildCandidates(List<NostrEvent> raw) { if (mostroPubkey == null) return raw; return raw.where((e) { if (!e.isVerified()) { logger.w( - 'Event ${e.id} from pubkey ${e.pubkey} failed signature ' - 'verification (possible dart_nostr limitation).', + 'Ignoring event ${e.id} from pubkey ${e.pubkey}: ' + 'signature verification failed.', ); + return false; } return e.pubkey == mostroPubkey; }).toList(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 162 - 171, buildCandidates currently logs failed signature checks but still returns events where e.pubkey == mostroPubkey, allowing unverified/forged events to pass; change buildCandidates to immediately reject events where !e.isVerified() (i.e., filter them out before comparing e.pubkey to mostroPubkey) so only verified events are considered for the mostroPubkey match; keep the existing logger.w call for telemetry but do not let unverified events reach the e.pubkey == mostroPubkey check in buildCandidates.
🧹 Nitpick comments (1)
lib/features/order/screens/take_order_screen.dart (1)
430-435: Consider logging caught exceptions.The caught exception is silently discarded. Adding a log statement would aid debugging without changing user-facing behavior.
♻️ Suggested improvement
+import 'package:mostro_mobile/services/logger_service.dart'; ... } catch (e) { + logger.e('Failed to take order', error: e); if (!mounted) return; setState(() { _isSubmitting = false; }); }Apply the same pattern to the other catch block at lines 463-468.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/features/order/screens/take_order_screen.dart` around lines 430 - 435, The catch blocks that only check mounted and setState to clear _isSubmitting swallow exceptions; change both catch clauses to capture the exception and stack trace (e.g., catch (e, stackTrace)) and log them before calling setState—use an existing logger or Flutter's debugPrint/developer.log to print a clear message including the exception and stackTrace; update both occurrences around the setState({_isSubmitting = false}) in the TakeOrderScreen so errors are recorded while preserving the current user-facing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Line 7: The code uses msg.action.isTerminal but only imports status.dart
(which defines Status); add the missing import for the Action enum so the
compiler can resolve Action and its isTerminal getter by importing the file that
defines it (import the package defining Action, i.e., the enums/action.dart
module) so that references like msg.action.isTerminal compile.
---
Duplicate comments:
In `@lib/services/deep_link_service.dart`:
- Around line 162-171: buildCandidates currently logs failed signature checks
but still returns events where e.pubkey == mostroPubkey, allowing
unverified/forged events to pass; change buildCandidates to immediately reject
events where !e.isVerified() (i.e., filter them out before comparing e.pubkey to
mostroPubkey) so only verified events are considered for the mostroPubkey match;
keep the existing logger.w call for telemetry but do not let unverified events
reach the e.pubkey == mostroPubkey check in buildCandidates.
---
Nitpick comments:
In `@lib/features/order/screens/take_order_screen.dart`:
- Around line 430-435: The catch blocks that only check mounted and setState to
clear _isSubmitting swallow exceptions; change both catch clauses to capture the
exception and stack trace (e.g., catch (e, stackTrace)) and log them before
calling setState—use an existing logger or Flutter's debugPrint/developer.log to
print a clear message including the exception and stackTrace; update both
occurrences around the setState({_isSubmitting = false}) in the TakeOrderScreen
so errors are recorded while preserving the current user-facing behavior.
🪄 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: 711c657b-b904-418a-805a-4edd89c2536c
📒 Files selected for processing (3)
lib/data/models/enums/action.dartlib/features/order/screens/take_order_screen.dartlib/services/deep_link_service.dart
added 2 commits
March 31, 2026 16:50
- Re-add import for action.dart (without alias) so Action.isTerminal resolves correctly; the previous commit removed it when dropping the unused 'as actions' prefix but isTerminal still requires the import - Add logger import from logger_service.dart - Capture exception and stack trace in both catch blocks around takeBuyOrder and takeSellOrder so errors are logged via logger.e before clearing _isSubmitting, aiding future debugging
The Action type is resolved transitively through MostroMessage, which already imports action.dart. A direct import in take_order_screen.dart is unnecessary and triggers an unused_import warning that fails CI.
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
lib/services/deep_link_service.dart (1)
157-172:⚠️ Potential issue | 🔴 CriticalReject unverified events before trusting
e.pubkey.Line 171 still accepts an event after Line 165 reports that signature verification failed. That lets a relay spoof
mostroPubkeywith an invalid event and bypass the new node-isolation path.🔒 Minimal hardening
List<NostrEvent> buildCandidates(List<NostrEvent> raw) { if (mostroPubkey == null) return raw; return raw.where((e) { - if (!e.isVerified()) { + final isVerified = e.isVerified(); + if (!isVerified) { logger.w( - 'Event ${e.id} from pubkey ${e.pubkey} failed signature ' - 'verification (possible dart_nostr limitation).', + 'Ignoring unverified event ${e.id} from pubkey ${e.pubkey}.', ); + return false; } return e.pubkey == mostroPubkey; }).toList(); }For the `dart_nostr` version used in MostroP2P/mobile, what does `NostrEvent.isVerified()` validate, and is there any documented reason to accept an event after `isVerified()` returns false?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 157 - 172, The buildCandidates function currently logs failed signature verification but still accepts events (allowing a relay to spoof mostroPubkey); change the filter so events are only accepted when e.isVerified() is true and e.pubkey == mostroPubkey (i.e., short-circuit reject unverified events), keep the existing logger.w for failures, and ensure you reference the same symbols (buildCandidates, mostroPubkey, NostrEvent.isVerified) when updating the predicate so unverified events cannot bypass node-isolation.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/data/models/enums/action.dart`:
- Around line 68-80: The isTerminal getter currently omits Action.release which
causes optimistic local emits to not be treated as terminal; update the const
set in the isTerminal getter to include Action.release, and also ensure the
status-mapping logic that consumes Action values treats both Action.release and
Action.released as Status.settledHoldInvoice (e.g., update the mapping function
that converts Action -> Status) so submit/loading state clears correctly for
both optimistic outgoing and confirmed events.
---
Duplicate comments:
In `@lib/services/deep_link_service.dart`:
- Around line 157-172: The buildCandidates function currently logs failed
signature verification but still accepts events (allowing a relay to spoof
mostroPubkey); change the filter so events are only accepted when e.isVerified()
is true and e.pubkey == mostroPubkey (i.e., short-circuit reject unverified
events), keep the existing logger.w for failures, and ensure you reference the
same symbols (buildCandidates, mostroPubkey, NostrEvent.isVerified) when
updating the predicate so unverified events cannot bypass node-isolation.
🪄 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: 2a6022ff-d0c6-4c0a-b534-0947c1d2e057
📒 Files selected for processing (3)
lib/data/models/enums/action.dartlib/features/order/screens/take_order_screen.dartlib/services/deep_link_service.dart
✅ Files skipped from review due to trivial changes (1)
- lib/features/order/screens/take_order_screen.dart
- Remove isTerminal getter from Action enum: it contained actions that cannot occur for an order in pending status (released, paymentFailed, holdInvoicePaymentCanceled, adminCanceled, cooperativeCancelAccepted, etc.), making it semantically incorrect and leaking UI logic into the domain enum - Per the Mostro protocol, only cantDo and canceled can arrive in TakeOrderScreen — cooperative cancel, disputes and admin actions require the order to be active/in-progress, not pending - Replace msg.action.isTerminal with an explicit two-value check (Action.cantDo || Action.canceled) that reflects the actual protocol - Add direct import for action.dart since Action values are now referenced explicitly instead of via a getter on an inferred type
Member
|
@CodeRabbit review |
Contributor
✅ Actions performedReview triggered.
|
… Action flutter/material.dart also exports an Action class (keyboard shortcuts). Import mostro action.dart with 'as mostro_action' alias so the compiler resolves Action.cantDo and Action.canceled unambiguously.
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
…fing Events that fail isVerified() are now rejected (return false) instead of being accepted when pubkey matches. This prevents an attacker from publishing a fake event with the target Mostro pubkey and bypassing the signature check.
Member
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
lib/services/deep_link_service.dart (1)
166-169:⚠️ Potential issue | 🟡 MinorFix the escaped string interpolations in the warning log.
\${e.id}and\${e.pubkey}will be logged literally instead of showing the actual event ID and pubkey. Remove the backslashes.🐛 Proposed fix
logger.w( - 'Event \${e.id} from pubkey \${e.pubkey} failed signature ' + 'Event ${e.id} from pubkey ${e.pubkey} failed signature ' 'verification — rejecting to prevent spoofing.', );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 166 - 169, The warning log in deep_link_service.dart is escaping string interpolation so it prints "\${e.id}" and "\${e.pubkey}" literally; update the logger.w call (the statement using logger.w) to remove the backslashes so it uses Dart interpolation with ${e.id} and ${e.pubkey} to log the actual event id and pubkey values. Ensure the message string still spans the two concatenated parts and references e.id and e.pubkey directly in the interpolations.
🧹 Nitpick comments (1)
lib/services/deep_link_service.dart (1)
159-161: Comment contradicts the actual code behavior.The comment states events failing
isVerified()are "not treated as hard rejections," but the code at lines 165-171 returnsfalse, which is a rejection. Per the commit message, this rejection is the intended security fix. Update the comment to reflect the actual behavior.📝 Suggested comment fix
- // accepted. isVerified() failures are logged but not treated as hard - // rejections due to a known dart_nostr limitation (consistent with - // how mostro_nodes_notifier.dart handles kind-0 events). + // accepted. Events failing isVerified() are now rejected to prevent + // spoofing attacks where an attacker publishes a fake event with the + // target Mostro pubkey.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/services/deep_link_service.dart` around lines 159 - 161, Update the misleading comment in lib/services/deep_link_service.dart near the isVerified() check to state that verification failures are treated as hard rejections (the code returns false), rather than being merely logged; reference the isVerified() call and the surrounding rejection-return behavior so the comment accurately documents the security fix implemented by that return false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@lib/services/deep_link_service.dart`:
- Around line 166-169: The warning log in deep_link_service.dart is escaping
string interpolation so it prints "\${e.id}" and "\${e.pubkey}" literally;
update the logger.w call (the statement using logger.w) to remove the
backslashes so it uses Dart interpolation with ${e.id} and ${e.pubkey} to log
the actual event id and pubkey values. Ensure the message string still spans the
two concatenated parts and references e.id and e.pubkey directly in the
interpolations.
---
Nitpick comments:
In `@lib/services/deep_link_service.dart`:
- Around line 159-161: Update the misleading comment in
lib/services/deep_link_service.dart near the isVerified() check to state that
verification failures are treated as hard rejections (the code returns false),
rather than being merely logged; reference the isVerified() call and the
surrounding rejection-return behavior so the comment accurately documents the
security fix implemented by that return false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 31bfebd6-c9c7-4b87-9d29-bab2380ecec7
📒 Files selected for processing (1)
lib/services/deep_link_service.dart
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
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.
Implements #541
Summary
When a
mostro:deep link includes amostro=<pubkey>parameter identifying a different Mostro instance than the currently connected one, the app now shows a confirmation dialog before switching.How It Works
parseMostroUrlextracts the optionalmostroquery parameterDeepLinkHandlercompares the link pubkey withsettings.mostroPublicKeyupdateMostroInstance(newPubkey)then navigate to orderChanges
lib/shared/utils/nostr_utils.dartmostroquery param inparseMostroUrllib/services/deep_link_service.dartmostroPubkeyfield toOrderInfo, pass throughlib/core/deep_link_handler.dartlib/l10n/intl_en.arblib/l10n/intl_es.arbtest/shared/utils/deep_link_parsing_test.dartdocs/DEEP_LINK_MOSTRO_SWITCH.mdWhat This Does NOT Change
mostro=param work exactly as before (backward compatible)updateMostroInstance)Testing
mostroparammostro=param handled (treated as absent)Closes #541
Summary by CodeRabbit
New Features
UI Changes
Bug Fixes
Localization
Documentation
Tests