What changed, and why it matters
This commit fixes a bug in how Stack Wallet sorts cryptocurrency exchange rate estimates. Previously, the sorting logic was broken: it could place failed/null estimates incorrectly and did not reliably show the best (highest) rate first. The fix ensures better rates appear at the top and failed providers are pushed to the bottom. It is a UI/ordering bug, not a direct theft-of-funds vulnerability, but it could mislead a user into picking a worse exchange rate.
Treat as a functional bug fix with low security impact. Review whether the comparator is used anywhere else with the same pattern, and consider whether the UI should visually flag failed/null providers so users do not accidentally select them. No emergency response is warranted.
Security signals we found
Broken sort comparator returning only 0 and 1
Null estimate handling that could place failed providers above valid ones
UI ordering bug that could cause users to select a suboptimal or failed exchange provider
No input validation, cryptography, or authorization changes observed
Evidence from the diff
The patch corrects a Dart comparator in sorted_exchange_providers.dart. The old comparator returned 0 or 1 in all branches, never -1, which violates sort contract expectations and produced unstable/partial ordering. It also asserted non-null reversed flags before safely handling null estimates, and treated null-vs-null as ‘greater than’ (return 1). The new comparator extracts computed rates, handles nulls symmetrically, and returns bRate compared to aRate so the highest rate sorts first; null-rate entries sort last. A unit test verifies [3, 1, null] ordering.
Changed components
lib/pages/exchange_view/sub_widgets/sorted_exchange_providers.darttest/pages/exchange_view/exchange_rate_sort_test.dartInspect captured patch +45 / −9
diff --git a/lib/pages/exchange_view/sub_widgets/sorted_exchange_providers.dart b/lib/pages/exchange_view/sub_widgets/sorted_exchange_providers.dart
index c478f9e..b0a99c7 100644
--- a/lib/pages/exchange_view/sub_widgets/sorted_exchange_providers.dart
+++ b/lib/pages/exchange_view/sub_widgets/sorted_exchange_providers.dart
@@ -58,17 +58,16 @@ class _SortedExchangeProvidersState
}
flattened.sort((a, b) {
- if (a.$2 == null && b.$2 == null) return 1;
- if (a.$2 != null && b.$2 == null) return 0;
- if (a.$2 == null && b.$2 != null) return 0;
+ if (a.$2 != null && b.$2 != null) {
+ assert(a.$2!.reversed == b.$2!.reversed);
+ }
- // or we get problems!!!
- assert(a.$2!.reversed == b.$2!.reversed);
+ final aRate = a.$2 == null ? null : _getRate(a.$2!, amount, rcvTicker);
+ final bRate = b.$2 == null ? null : _getRate(b.$2!, amount, rcvTicker);
- return _getRate(a.$2!, amount, rcvTicker) >
- _getRate(b.$2!, amount, rcvTicker)
- ? 0
- : 1;
+ if (aRate == null) return bRate == null ? 0 : 1;
+ if (bRate == null) return -1;
+ return bRate.decimal.compareTo(aRate.decimal);
});
return flattened;
diff --git a/test/pages/exchange_view/exchange_rate_sort_test.dart b/test/pages/exchange_view/exchange_rate_sort_test.dart
new file mode 100644
index 0000000..4fb9451
--- /dev/null
+++ b/test/pages/exchange_view/exchange_rate_sort_test.dart
@@ -0,0 +1,37 @@
+import 'package:decimal/decimal.dart';
+import 'package:flutter_test/flutter_test.dart';
+import 'package:stackwallet/models/exchange/response_objects/estimate.dart';
+import 'package:stackwallet/pages/exchange_view/sub_widgets/sorted_exchange_providers.dart';
+import 'package:stackwallet/services/exchange/exchange.dart';
+
+void main() {
+ test('exchange rates sort highest first with failed providers last', () {
+ final exchange = Exchange.defaultExchange;
+ final dynamic state = SortedExchangeProviders(
+ exchangees: [exchange],
+ fixedRate: false,
+ reversed: false,
+ ).createState();
+
+ Estimate estimate(int rate) => Estimate(
+ estimatedAmount: Decimal.fromInt(rate),
+ fixedRate: false,
+ reversed: false,
+ exchangeProvider: exchange.name,
+ );
+
+ state.estimates.addAll(<(Exchange, List<Estimate>?)>[
+ (exchange, [estimate(1)]),
+ (exchange, null),
+ (exchange, [estimate(3)]),
+ ]);
+
+ final result = state.transform(Decimal.one, 'BTC') as List;
+
+ expect(result.map((entry) => entry.$2?.estimatedAmount).toList(), [
+ Decimal.fromInt(3),
+ Decimal.fromInt(1),
+ null,
+ ]);
+ });
+}
Why this scored 24/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.