What the developer wrote
Authored by Julian
fix checkTrades early return
What changed, and why it matters
This commit fixes a bug in the wallet's notification service where checking one trade could accidentally stop all remaining trade checks. The function now correctly continues to the next trade instead of returning early. It also makes the trade-check function properly asynchronous so the periodic timer doesn't wait on it. This is a reliability/bug-fix change with minor security relevance: a stuck or failing trade check could have delayed or suppressed status updates for other trades, potentially hiding problems from the user.
Treat as a routine bug-fix patch. Users relying on trade notifications should update to ensure all watched trades are checked on each timer tick. No immediate security response is required, but verify that unawaited(_checkTrades()) errors are logged rather than silently dropped.
Security signals we found
Early return in a loop skipped remaining notification checks, reducing visibility into trade status
Async void method called from a periodic timer could silently swallow errors and block timer behavior
No input validation, cryptographic, or network trust-boundary changes observed
Evidence from the diff
In lib/services/notifications_service.dart, _checkTrades() is changed from a void async method to a Future
Changed components
lib/services/notifications_service.dartNotificationsService periodic timer_checkTrades()ChangeNow trade notification watcherInspect captured patch +23 / −25
diff --git a/lib/services/notifications_service.dart b/lib/services/notifications_service.dart
index 4eca542..fa7575d 100644
--- a/lib/services/notifications_service.dart
+++ b/lib/services/notifications_service.dart
@@ -109,7 +109,7 @@ class NotificationsService extends ChangeNotifier {
_timer = Timer.periodic(notificationRefreshInterval, (_) {
Logging.instance.d("Periodic notifications update check");
if (prefs.externalCalls) {
- _checkTrades();
+ unawaited(_checkTrades());
}
_checkTransactions();
});
@@ -159,21 +159,20 @@ class NotificationsService extends ChangeNotifier {
torEnabled: node.torEnabled,
clearnetEnabled: node.clearnetEnabled,
);
- final failovers =
- nodeService
- .failoverNodesFor(currency: coin)
- .map(
- (e) => ElectrumXNode(
- address: e.host,
- port: e.port,
- name: e.name,
- id: e.id,
- useSSL: e.useSSL,
- torEnabled: node.torEnabled,
- clearnetEnabled: node.clearnetEnabled,
- ),
- )
- .toList();
+ final failovers = nodeService
+ .failoverNodesFor(currency: coin)
+ .map(
+ (e) => ElectrumXNode(
+ address: e.host,
+ port: e.port,
+ name: e.name,
+ id: e.id,
+ useSSL: e.useSSL,
+ torEnabled: node.torEnabled,
+ clearnetEnabled: node.clearnetEnabled,
+ ),
+ )
+ .toList();
final client = ElectrumXClient.from(
node: eNode,
@@ -233,7 +232,7 @@ class NotificationsService extends ChangeNotifier {
}
}
- void _checkTrades() async {
+ Future<void> _checkTrades() async {
for (final notification in _watchedChangeNowTradeNotifications) {
final id = notification.changeNowId!;
@@ -243,7 +242,7 @@ class NotificationsService extends ChangeNotifier {
);
if (trades.isEmpty) {
- return;
+ continue;
}
final oldTrade = trades.first;
late final ExchangeResponse<Trade> response;
@@ -252,11 +251,11 @@ class NotificationsService extends ChangeNotifier {
final exchange = Exchange.fromName(oldTrade.exchangeName);
response = await exchange.updateTrade(oldTrade);
} catch (_) {
- return;
+ continue;
}
if (response.value == null) {
- return;
+ continue;
}
final trade = response.value!;
@@ -371,11 +370,10 @@ class NotificationsService extends ChangeNotifier {
}
Future<void> markAsRead(int id, bool shouldNotifyListeners) async {
- final model =
- DB.instance.get<NotificationModel>(
- boxName: DB.boxNameNotifications,
- key: id,
- )!;
+ final model = DB.instance.get<NotificationModel>(
+ boxName: DB.boxNameNotifications,
+ key: id,
+ )!;
await DB.instance.put<NotificationModel>(
boxName: DB.boxNameNotifications,
key: model.id,
Why this scored 33/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.