From 6b12fd648182d45f955b0d53b963e0a133319c10 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 10:52:40 +0200 Subject: [PATCH 1/9] feat(system-health): add NTP and HTTP header time providers --- .../providers/http_head_time_provider.dart | 109 +++++++++++++ .../providers/http_time_provider.dart | 149 ++++++++++++++++++ .../providers/ntp_time_provider.dart | 85 ++++++++++ .../providers/time_provider.dart | 15 ++ .../providers/time_provider_registry.dart | 51 ++++++ .../system_clock_repository.dart | 148 +++++++---------- 6 files changed, 467 insertions(+), 90 deletions(-) create mode 100644 lib/bloc/system_health/providers/http_head_time_provider.dart create mode 100644 lib/bloc/system_health/providers/http_time_provider.dart create mode 100644 lib/bloc/system_health/providers/ntp_time_provider.dart create mode 100644 lib/bloc/system_health/providers/time_provider.dart create mode 100644 lib/bloc/system_health/providers/time_provider_registry.dart diff --git a/lib/bloc/system_health/providers/http_head_time_provider.dart b/lib/bloc/system_health/providers/http_head_time_provider.dart new file mode 100644 index 0000000000..d04e1a6e79 --- /dev/null +++ b/lib/bloc/system_health/providers/http_head_time_provider.dart @@ -0,0 +1,109 @@ +import 'dart:async' show TimeoutException; +import 'dart:io'; + +import 'package:http/http.dart' as http; +import 'package:logging/logging.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; + +/// A time provider that fetches time from server 'Date' headers via HEAD requests +class HttpHeadTimeProvider extends TimeProvider { + HttpHeadTimeProvider({ + this.servers = const [ + 'https://www.google.com/', + 'https://www.alibaba.com/', + 'https://www.vk.com/', + 'https://www.cloudflare.com/', + 'https://www.microsoft.com/', + ], + http.Client? httpClient, + this.timeout = const Duration(seconds: 2), + this.maxRetries = 2, + Logger? logger, + }) : _httpClient = httpClient ?? http.Client(), + _logger = logger ?? Logger('HttpHeadTimeProvider'); + + /// The name of the provider (for logging and identification) + final Logger _logger; + + /// List of servers to query via HEAD requests + final List servers; + + /// Timeout for HTTP requests + final Duration timeout; + + /// Maximum retries per server + final int maxRetries; + + final http.Client _httpClient; + + @override + String get name => 'HttpHead'; + + @override + Future getCurrentUtcTime() async { + for (final server in servers) { + int retries = 0; + + while (retries < maxRetries) { + try { + final serverTime = await _fetchServerTime(server); + if (serverTime != null) { + _logger.fine('Successfully retrieved time from $server'); + return serverTime; + } + } on Exception catch (e) { + _logger.severe('Error with $server: $e'); + } + retries++; + } + } + + _logger + .severe('Failed to get time from any server after $maxRetries retries'); + return null; + } + + /// Fetches server time from the 'date' header of an HTTP HEAD response + Future _fetchServerTime(String url) async { + try { + final response = await _httpClient.head( + Uri.parse(url), + headers: { + HttpHeaders.userAgentHeader: 'Komodo-Wallet/1.0', + }, + ).timeout(timeout); + + if (response.statusCode != 200) { + _logger.warning('HTTP error from $url: ${response.statusCode}'); + return null; + } + + final dateHeader = response.headers['date']; + if (dateHeader == null) { + _logger.warning('No Date header in response from $url'); + return null; + } + + final parsed = HttpDate.parse(dateHeader); + return parsed.toUtc(); // Ensure it's UTC + } on SocketException catch (e) { + _logger.warning('Socket error with $url: ${e.message}'); + return null; + } on TimeoutException catch (e) { + _logger.warning('Timeout with $url: ${e.message}'); + return null; + } on FormatException catch (e) { + _logger.severe('Failed to parse Date header from $url: $e'); + return null; + } on Exception catch (e) { + _logger.severe('Error fetching time from $url: $e'); + return null; + } + } + + /// Disposes the HTTP client when done + @override + void dispose() { + _httpClient.close(); + } +} diff --git a/lib/bloc/system_health/providers/http_time_provider.dart b/lib/bloc/system_health/providers/http_time_provider.dart new file mode 100644 index 0000000000..1ffded9296 --- /dev/null +++ b/lib/bloc/system_health/providers/http_time_provider.dart @@ -0,0 +1,149 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:http/http.dart' as http; +import 'package:logging/logging.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; + +/// A time provider that fetches time from an HTTP API +class HttpTimeProvider extends TimeProvider { + HttpTimeProvider({ + required this.url, + required this.timeFieldPath, + required this.timeFormat, + required String providerName, + http.Client? httpClient, + Duration? apiTimeout, + Logger? logger, + }) : _httpClient = httpClient ?? http.Client(), + _apiTimeout = apiTimeout ?? const Duration(seconds: 2), + name = providerName, + _logger = logger ?? Logger(providerName); + + /// The URL of the time API + final String url; + + /// The field path in the JSON response that contains the time. + /// + /// Separate nested fields with dots (e.g., "time.current") + final String timeFieldPath; + + /// The format of the time string in the response + final TimeFormat timeFormat; + + /// The name of the provider (for logging and identification) + @override + final String name; + + final Logger _logger; + + final http.Client _httpClient; + final Duration _apiTimeout; + + @override + Future getCurrentUtcTime() async { + try { + final response = await _httpGet(url); + + if (response.statusCode != 200) { + _logger + .warning('API request failed with status ${response.statusCode}'); + return null; + } + + final jsonResponse = json.decode(response.body) as Map; + final DateTime? parsedTime = await _parseTimeFromJson(jsonResponse); + + if (parsedTime == null) { + _logger.warning('Failed to parse time from response'); + return null; + } + + return parsedTime; + } on Exception catch (e) { + _logger.severe('Error fetching time: $e'); + return null; + } + } + + Future _httpGet(String url) async { + try { + return await _httpClient.get(Uri.parse(url)).timeout(_apiTimeout); + } on Exception catch (e) { + return http.Response('Error: $e', HttpStatus.internalServerError); + } + } + + Future _parseTimeFromJson( + Map jsonResponse) async { + try { + final fieldParts = timeFieldPath.split('.'); + dynamic value = jsonResponse; + + for (final part in fieldParts) { + if (value is! Map) { + _logger.warning('JSON path error: expected Map at $part'); + return null; + } + value = value[part]; + if (value == null) { + _logger.warning('JSON path error: null value at $part'); + return null; + } + } + + final timeStr = value.toString(); + if (timeStr.isEmpty) { + _logger.warning('Empty time string'); + return null; + } + + return await _parseDateTime(timeStr); + } on Exception catch (e) { + _logger.severe('JSON parsing error: $e'); + return null; + } + } + + Future _parseDateTime(String timeStr) async { + try { + String formattedTime = timeStr; + + switch (timeFormat) { + case TimeFormat.iso8601: + if (formattedTime.endsWith('+00:00')) { + formattedTime = formattedTime.replaceAll('+00:00', 'Z'); + } else if (!formattedTime.endsWith('Z')) { + formattedTime += 'Z'; + } + + case TimeFormat.custom: + throw const FormatException('Custom time format not supported'); + } + + final dateTime = DateTime.parse(formattedTime); + if (!dateTime.isUtc) { + throw const FormatException('Time is not in UTC'); + } + + return dateTime; + } on Exception catch (e) { + _logger.severe('Date parsing error: $e'); + return null; + } + } + + @override + void dispose() { + _httpClient.close(); + } +} + +/// Enum representing the format of time returned by the API +enum TimeFormat { + /// ISO8601 format (e.g. "2023-05-07T12:34:56Z") + iso8601, + + /// Custom format that may require special parsing + custom +} diff --git a/lib/bloc/system_health/providers/ntp_time_provider.dart b/lib/bloc/system_health/providers/ntp_time_provider.dart new file mode 100644 index 0000000000..f6e096285b --- /dev/null +++ b/lib/bloc/system_health/providers/ntp_time_provider.dart @@ -0,0 +1,85 @@ +import 'dart:async'; +import 'dart:io'; + +import 'package:logging/logging.dart'; +import 'package:ntp/ntp.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; + +/// A time provider that fetches accurate time from NTP servers +class NtpTimeProvider extends TimeProvider { + NtpTimeProvider({ + this.ntpServers = const [ + 'pool.ntp.org', + 'time.google.com', + 'time.cloudflare.com', + 'time.apple.com', + ], + this.lookupTimeout = const Duration(seconds: 1), + this.maxRetries = 3, + Logger? logger, + }) : _logger = logger ?? Logger('NtpTimeProvider'); + + /// The name of the provider (for logging and identification) + final Logger _logger; + + /// List of NTP servers to query + final List ntpServers; + + /// Timeout for NTP lookup + final Duration lookupTimeout; + + /// Maximum number of retries per server + final int maxRetries; + + @override + String get name => 'NTP'; + + @override + Future getCurrentUtcTime() async { + for (final server in ntpServers) { + DateTime? time; + int retries = 0; + + while (time == null && retries < maxRetries) { + try { + final int offset = await NTP.getNtpOffset( + localTime: DateTime.now(), + lookUpAddress: server, + timeout: lookupTimeout, + ); + + final now = DateTime.now(); + time = now.add(Duration(milliseconds: offset)); + + final utcTime = DateTime.utc( + time.year, + time.month, + time.day, + time.hour, + time.minute, + time.second, + time.millisecond, + time.microsecond, + ); + + _logger.fine('Successfully retrieved time from $server'); + return utcTime; + } on SocketException catch (e) { + _logger.warning('Socket error with $server: ${e.message}'); + retries++; + } on TimeoutException catch (e) { + _logger.warning('Timeout with $server: ${e.message}'); + retries++; + } on Exception catch (e) { + _logger.severe('Error with $server: $e'); + retries++; + } + } + } + + _logger.severe( + 'Failed to get time from any NTP server after $maxRetries retries', + ); + return null; + } +} diff --git a/lib/bloc/system_health/providers/time_provider.dart b/lib/bloc/system_health/providers/time_provider.dart new file mode 100644 index 0000000000..766289686c --- /dev/null +++ b/lib/bloc/system_health/providers/time_provider.dart @@ -0,0 +1,15 @@ +/// Base interface for all time providers +abstract class TimeProvider { + /// Returns the current UTC time from an external source + /// + /// Returns null if the provider failed to get the time + Future getCurrentUtcTime(); + + /// Returns a descriptive name for the provider + String get name; + + /// Dispose of any resources used by the provider + void dispose() { + // Default implementation does nothing + } +} diff --git a/lib/bloc/system_health/providers/time_provider_registry.dart b/lib/bloc/system_health/providers/time_provider_registry.dart new file mode 100644 index 0000000000..197cd11e23 --- /dev/null +++ b/lib/bloc/system_health/providers/time_provider_registry.dart @@ -0,0 +1,51 @@ +import 'package:http/http.dart' as http; +import 'package:web_dex/bloc/system_health/providers/http_head_time_provider.dart'; +import 'package:web_dex/bloc/system_health/providers/http_time_provider.dart'; +import 'package:web_dex/bloc/system_health/providers/ntp_time_provider.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; + +/// Registry of all available time providers +class TimeProviderRegistry { + TimeProviderRegistry({ + List? providers, + http.Client? httpClient, + Duration? apiTimeout, + }) : _httpClient = httpClient ?? http.Client(), + _apiTimeout = apiTimeout ?? const Duration(seconds: 2) { + _providers = providers ?? _createDefaultProviders(); + } + + final http.Client _httpClient; + final Duration _apiTimeout; + late final List _providers; + + /// Returns all registered time providers + List get providers => _providers; + + /// Creates the default time providers + List _createDefaultProviders() { + return [ + NtpTimeProvider(), + HttpHeadTimeProvider( + httpClient: _httpClient, + timeout: _apiTimeout, + ), + HttpTimeProvider( + url: 'https://timeapi.io/api/time/current/zone?timeZone=UTC', + timeFieldPath: 'currentDateTime', + timeFormat: TimeFormat.iso8601, + providerName: 'TimeAPI', + httpClient: _httpClient, + apiTimeout: _apiTimeout, + ) + ]; + } + + /// Disposes all providers that need cleanup + /// Necessary for providers that manage resources like sockets or streams + void dispose() { + for (final provider in _providers) { + provider.dispose(); + } + } +} diff --git a/lib/bloc/system_health/system_clock_repository.dart b/lib/bloc/system_health/system_clock_repository.dart index 21cc0c6954..7a680a31d7 100644 --- a/lib/bloc/system_health/system_clock_repository.dart +++ b/lib/bloc/system_health/system_clock_repository.dart @@ -1,117 +1,85 @@ -// lib/repositories/system_clock_repository.dart -import 'dart:convert'; -import 'dart:io'; - import 'package:http/http.dart' as http; -import 'package:web_dex/shared/utils/utils.dart'; +import 'package:logging/logging.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider_registry.dart'; class SystemClockRepository { SystemClockRepository({ + TimeProviderRegistry? providerRegistry, http.Client? httpClient, Duration? maxAllowedDifference, Duration? apiTimeout, - }) : _httpClient = httpClient ?? http.Client(), - _maxAllowedDifference = + Logger? logger, + }) : _maxAllowedDifference = maxAllowedDifference ?? const Duration(seconds: 60), - _apiTimeout = apiTimeout ?? const Duration(seconds: 2); - - static const _utcWorldTimeApis = [ - 'https://worldtimeapi.org/api/timezone/UTC', - 'https://timeapi.io/api/time/current/zone?timeZone=UTC', - 'http://worldclockapi.com/api/json/utc/now', - ]; + _httpClient = httpClient ?? http.Client(), + _providerRegistry = providerRegistry ?? + TimeProviderRegistry( + httpClient: httpClient, + apiTimeout: apiTimeout, + ), + _logger = logger; final Duration _maxAllowedDifference; - final Duration _apiTimeout; final http.Client _httpClient; + final TimeProviderRegistry _providerRegistry; + final Logger? _logger; - /// Queries the available 3rd party APIs to validate the system clock validity - /// returning true if the system clock is within allowed difference of the API - /// time, false otherwise. Uses the first successful response - Future isSystemClockValid({ - List timeApiUrls = _utcWorldTimeApis, - }) async { - try { - final futures = timeApiUrls.map((url) => _httpGet(url)); - - final responses = await Future.wait( - futures, - eagerError: false, - ); + Logger? get logger => _logger; - for (final response in responses) { - if (response.statusCode != 200) { - continue; + /// Queries the available time providers to validate the system clock validity + /// returning true if the system clock is within allowed difference of the + /// first provider that responds, false otherwise. Returns true in case of + /// errors to avoid blocking app usage. + Future isSystemClockValid() async { + try { + final providers = _providerRegistry.providers; + bool receivedValidResponse = false; + + for (final provider in providers) { + try { + final apiTime = await provider.getCurrentUtcTime(); + + if (apiTime != null) { + receivedValidResponse = true; + final localTime = DateTime.timestamp(); + final Duration difference = apiTime.difference(localTime).abs(); + + final isValid = difference < _maxAllowedDifference; + if (isValid) { + await _log('System clock validated by ${provider.name} provider'); + } else { + await _log( + 'System clock differs by ${difference.inSeconds}s from ' + '${provider.name} provider'); + } + + return isValid; + } + } on Exception catch (e) { + await _log('Provider ${provider.name} failed: $e'); } + } - final jsonResponse = json.decode(response.body) as Map; - final DateTime apiTime = _parseUtcDateTimeString(jsonResponse); - final localTime = DateTime.timestamp(); - final Duration difference = apiTime.difference(localTime).abs(); - - return difference < _maxAllowedDifference; + if (!receivedValidResponse) { + await _log('All time providers failed to provide a time'); } - // Log error if no successful responses - log('All time API requests failed').ignore(); + // Default to allowing usage when no provider responded + return true; + } on Exception catch (e) { + await _log('Failed to validate system clock: $e'); + // Don't block usage of dex if the time provider fetch fails return true; - } catch (e) { - log('Failed to validate system clock: $e').ignore(); - return true; // Don't block usage - } - } - - Future _httpGet(String url) async { - try { - return await _httpClient.get(Uri.parse(url)).timeout(_apiTimeout); - } catch (e) { - return http.Response('Error: $e', HttpStatus.internalServerError); - } - } - - DateTime _parseUtcDateTimeString(Map jsonResponse) { - dynamic apiTimeStr = jsonResponse['datetime'] ?? // worldtimeapi.org - jsonResponse['dateTime'] ?? // worldclockapi.com - jsonResponse['currentDateTime']; // timeapi.io - - if (apiTimeStr == null) { - throw Exception('API response does not contain datetime field'); - } - - if (apiTimeStr is! String || apiTimeStr.isEmpty) { - throw const FormatException('API datetime field is not a string'); - } - - // Convert +00:00 format to Z format if needed - if (apiTimeStr.endsWith('+00:00')) { - apiTimeStr = apiTimeStr.replaceAll('+00:00', 'Z'); - } else if (!apiTimeStr.endsWith('Z')) { - apiTimeStr += 'Z'; // Add UTC timezone indicator if missing - } - - final apiTime = DateTime.parse(apiTimeStr); - if (!apiTime.isUtc) { - throw const FormatException('API time is not in UTC'); } - return apiTime; } - /// Checks if there are enough active seeders to indicate valid system clock - Future hasActiveSeeders() async { - // TODO: Implement seeder check logic onur suggested - few seeders - // implies that the user's clock is invalid and being rejected by seeders - throw UnimplementedError('Not implemented yet'); - } - - /// Combines multiple clock validation methods - Future isClockValidWithAllChecks() async { - final apiCheck = await isSystemClockValid(); - final seederCheck = await hasActiveSeeders(); - - return apiCheck && seederCheck; + Future _log(String message) async { + (logger ?? Logger('SystemClockRepository')) + .info('[SystemClockRepository] $message'); } void dispose() { + _providerRegistry.dispose(); _httpClient.close(); } } From 9d6019954f92efe5646236fff53e3bd7d4090353 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 10:53:38 +0200 Subject: [PATCH 2/9] test(system-health): add unit tests for system clock providers --- test_units/main.dart | 13 ++ .../http_head_time_provider_test.dart | 118 ++++++++++++++++ .../http_time_provider_test.dart | 76 +++++++++++ .../system_health/ntp_time_provider_test.dart | 19 +++ .../system_clock_repository_test.dart | 126 ++++++++++++++++++ .../time_provider_registry_test.dart | 41 ++++++ 6 files changed, 393 insertions(+) create mode 100644 test_units/tests/system_health/http_head_time_provider_test.dart create mode 100644 test_units/tests/system_health/http_time_provider_test.dart create mode 100644 test_units/tests/system_health/ntp_time_provider_test.dart create mode 100644 test_units/tests/system_health/system_clock_repository_test.dart create mode 100644 test_units/tests/system_health/time_provider_registry_test.dart diff --git a/test_units/main.dart b/test_units/main.dart index e5008f3126..55955f9458 100644 --- a/test_units/main.dart +++ b/test_units/main.dart @@ -24,6 +24,11 @@ import 'tests/helpers/update_sell_amount_test.dart'; import 'tests/password/validate_password_test.dart'; import 'tests/password/validate_rpc_password_test.dart'; import 'tests/sorting/sorting_test.dart'; +import 'tests/system_health/http_head_time_provider_test.dart'; +import 'tests/system_health/http_time_provider_test.dart'; +import 'tests/system_health/ntp_time_provider_test.dart'; +import 'tests/system_health/system_clock_repository_test.dart'; +import 'tests/system_health/time_provider_registry_test.dart'; import 'tests/utils/convert_double_to_string_test.dart'; import 'tests/utils/convert_fract_rat_test.dart'; import 'tests/utils/double_to_string_test.dart'; @@ -86,4 +91,12 @@ void main() { testProfitLossRepository(); testGenerateDemoData(); }); + + group('SystemHealth: ', () { + testHttpHeadTimeProvider(); + testSystemClockRepository(); + testHttpTimeProvider(); + testNtpTimeProvider(); + testTimeProviderRegistry(); + }); } diff --git a/test_units/tests/system_health/http_head_time_provider_test.dart b/test_units/tests/system_health/http_head_time_provider_test.dart new file mode 100644 index 0000000000..c807da45a1 --- /dev/null +++ b/test_units/tests/system_health/http_head_time_provider_test.dart @@ -0,0 +1,118 @@ +import 'dart:async' show TimeoutException; +import 'dart:io'; + +import 'package:http/http.dart' as http; +import 'package:test/test.dart'; +import 'package:web_dex/bloc/system_health/providers/http_head_time_provider.dart'; + +void testHttpHeadTimeProvider() { + group('HttpHeadTimeProvider', () { + late HttpHeadTimeProvider provider; + late MockClient mockClient; + + setUp(() { + mockClient = MockClient(); + provider = HttpHeadTimeProvider( + httpClient: mockClient, + timeout: const Duration(seconds: 1), + ); + }); + + tearDown(() { + provider.dispose(); + }); + + test('returns DateTime when header is valid', () async { + // RFC 1123 date format used in HTTP headers + const dateHeader = 'Wed, 07 May 2025 12:34:56 GMT'; + final expectedDateTime = HttpDate.parse(dateHeader); + + mockClient.mockResponse = http.Response( + '', + 200, + headers: {'date': dateHeader}, + ); + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNotNull); + expect(result, equals(expectedDateTime)); + }); + + test('returns null when date header is missing', () async { + mockClient.mockResponse = http.Response('', 200, headers: {}); + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + + test('returns null when response status is not 200', () async { + mockClient.mockResponse = http.Response('', 404); + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + + test('returns null when all servers fail', () async { + mockClient.mockResponse = http.Response('', 500); + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + + test('handles timeout exceptions', () async { + mockClient.shouldThrowTimeout = true; + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + + test('handles socket exceptions', () async { + mockClient.shouldThrowSocketException = true; + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + + test('handles format exceptions with invalid date', () async { + mockClient.mockResponse = http.Response( + '', + 200, + headers: {'date': 'invalid-date-format'}, + ); + + final result = await provider.getCurrentUtcTime(); + + expect(result, isNull); + }); + }); +} + +/// Simple mock HTTP client for testing +class MockClient extends http.BaseClient { + http.Response mockResponse = http.Response('', 200); + bool shouldThrowTimeout = false; + bool shouldThrowSocketException = false; + + @override + Future send(http.BaseRequest request) async { + if (shouldThrowTimeout) { + throw TimeoutException('Timeout'); + } + + if (shouldThrowSocketException) { + throw const SocketException('Socket error'); + } + + return http.StreamedResponse( + Stream.value(mockResponse.bodyBytes), + mockResponse.statusCode, + headers: mockResponse.headers, + ); + } +} diff --git a/test_units/tests/system_health/http_time_provider_test.dart b/test_units/tests/system_health/http_time_provider_test.dart new file mode 100644 index 0000000000..64d085839d --- /dev/null +++ b/test_units/tests/system_health/http_time_provider_test.dart @@ -0,0 +1,76 @@ +import 'dart:convert' show jsonEncode; +import 'dart:io' show HttpException; + +import 'package:http/http.dart' as http; +import 'package:test/test.dart'; +import 'package:web_dex/bloc/system_health/providers/http_time_provider.dart'; + +void testHttpTimeProvider() { + group('HttpTimeProvider', () { + late HttpTimeProvider provider; + late MockClient mockClient; + + setUp(() { + mockClient = MockClient(); + provider = HttpTimeProvider( + url: 'http://example.com', + timeFieldPath: 'currentDateTime', + timeFormat: TimeFormat.iso8601, + providerName: 'TestProvider', + httpClient: mockClient, + ); + }); + + tearDown(() { + provider.dispose(); + }); + + test('returns DateTime when response is valid', () async { + final now = DateTime.utc(2025, 5, 7, 12, 34, 56); + mockClient.mockResponse = http.Response( + jsonEncode({'currentDateTime': now.toIso8601String()}), 200); + final result = await provider.getCurrentUtcTime(); + expect(result, equals(now)); + }); + + test('returns null on non-200 response', () async { + mockClient.mockResponse = http.Response('error', 500); + final result = await provider.getCurrentUtcTime(); + expect(result, isNull); + }); + + test('returns null if field missing', () async { + mockClient.mockResponse = + http.Response(jsonEncode({'other': 'value'}), 200); + final result = await provider.getCurrentUtcTime(); + expect(result, isNull); + }); + + test('returns null on invalid date format', () async { + mockClient.mockResponse = + http.Response(jsonEncode({'currentDateTime': 'not-a-date'}), 200); + final result = await provider.getCurrentUtcTime(); + expect(result, isNull); + }); + + test('returns null on exception', () async { + mockClient.shouldThrow = true; + final result = await provider.getCurrentUtcTime(); + expect(result, isNull); + }); + }); +} + +class MockClient extends http.BaseClient { + http.Response mockResponse = http.Response('', 200); + bool shouldThrow = false; + @override + Future send(http.BaseRequest request) async { + if (shouldThrow) throw const HttpException('error'); + return http.StreamedResponse( + Stream.value(mockResponse.bodyBytes), + mockResponse.statusCode, + headers: mockResponse.headers, + ); + } +} diff --git a/test_units/tests/system_health/ntp_time_provider_test.dart b/test_units/tests/system_health/ntp_time_provider_test.dart new file mode 100644 index 0000000000..be69aa81c1 --- /dev/null +++ b/test_units/tests/system_health/ntp_time_provider_test.dart @@ -0,0 +1,19 @@ +import 'package:test/test.dart'; +import 'package:web_dex/bloc/system_health/providers/ntp_time_provider.dart'; + +void testNtpTimeProvider() { + group('NtpTimeProvider', () { + test('returns null if all servers fail', () async { + final provider = NtpTimeProvider( + ntpServers: ['bad.ntp.server'], + lookupTimeout: const Duration(milliseconds: 10), + maxRetries: 1, + ); + // This will likely fail due to bad server + final result = await provider.getCurrentUtcTime(); + expect(result, isNull); + }); + // Note: A true success test would require a real NTP server and is best as + // an integration test. + }); +} diff --git a/test_units/tests/system_health/system_clock_repository_test.dart b/test_units/tests/system_health/system_clock_repository_test.dart new file mode 100644 index 0000000000..9c6cd2aa0b --- /dev/null +++ b/test_units/tests/system_health/system_clock_repository_test.dart @@ -0,0 +1,126 @@ +import 'package:http/http.dart' as http; +import 'package:test/test.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider_registry.dart'; +import 'package:web_dex/bloc/system_health/system_clock_repository.dart'; + +void testSystemClockRepository() { + group('SystemClockRepository', () { + late SystemClockRepository repository; + late MockTimeProviderRegistry mockRegistry; + + setUp(() { + mockRegistry = MockTimeProviderRegistry(); + repository = SystemClockRepository( + providerRegistry: mockRegistry, + httpClient: http.Client(), + maxAllowedDifference: const Duration(seconds: 30), + ); + }); + + tearDown(() { + repository.dispose(); + }); + + test('returns true when first provider returns valid time', () async { + mockRegistry.mockProviders = [ + MockTimeProvider( + name: 'ValidProvider', + returnTime: DateTime.now(), + ), + ]; + + final result = await repository.isSystemClockValid(); + + expect(result, isTrue); + }); + + test('returns false when first provider time differs too much', () async { + // This time is significantly different from local time + mockRegistry.mockProviders = [ + MockTimeProvider( + name: 'InvalidTimeProvider', + returnTime: DateTime.utc(2030), + ), + ]; + + final result = await repository.isSystemClockValid(); + + expect(result, isFalse); + }); + + test('returns true when no providers respond', () async { + mockRegistry.mockProviders = [ + MockTimeProvider(name: 'FailingProvider'), + ]; + + final result = await repository.isSystemClockValid(); + + expect(result, isTrue); + }); + + test('returns true when provider throws exception', () async { + mockRegistry.mockProviders = [ + MockTimeProvider(name: 'ExceptionProvider', shouldThrow: true), + ]; + + final result = await repository.isSystemClockValid(); + + expect(result, isTrue); + }); + + test('uses provider order from registry', () async { + final validProvider = MockTimeProvider( + name: 'ValidProvider', + returnTime: DateTime.timestamp().toUtc(), + ); + final failingProvider = MockTimeProvider(name: 'FailingProvider'); + + mockRegistry.mockProviders = [validProvider, failingProvider]; + await repository.isSystemClockValid(); + + expect(validProvider.callCount, 1); + expect(failingProvider.callCount, 0); + }); + }); +} + +class MockTimeProviderRegistry implements TimeProviderRegistry { + List mockProviders = []; + + @override + List get providers => mockProviders; + + @override + void dispose() {} + + @override + dynamic noSuchMethod(Invocation invocation) { + return null; + } +} + +class MockTimeProvider extends TimeProvider { + MockTimeProvider({ + required String name, + this.returnTime, + this.shouldThrow = false, + }) : _name = name; + + final String _name; + final DateTime? returnTime; + final bool shouldThrow; + int callCount = 0; + + @override + String get name => _name; + + @override + Future getCurrentUtcTime() async { + callCount++; + if (shouldThrow) { + throw Exception('Test exception'); + } + return returnTime; + } +} diff --git a/test_units/tests/system_health/time_provider_registry_test.dart b/test_units/tests/system_health/time_provider_registry_test.dart new file mode 100644 index 0000000000..22ecf55cec --- /dev/null +++ b/test_units/tests/system_health/time_provider_registry_test.dart @@ -0,0 +1,41 @@ +import 'package:test/test.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider_registry.dart'; + +void testTimeProviderRegistry() { + group('TimeProviderRegistry', () { + test('returns default providers', () { + final registry = TimeProviderRegistry(); + expect(registry.providers, isNotEmpty); + expect(registry.providers.first.name, isNotEmpty); + }); + + test('accepts custom providers', () { + final custom = _MockTimeProvider(); + final registry = TimeProviderRegistry(providers: [custom]); + expect(registry.providers.length, 1); + expect(registry.providers.first, custom); + }); + + test('dispose calls dispose on all providers', () { + final disposed = []; + final p1 = _MockTimeProvider(onDispose: () => disposed.add(true)); + final p2 = _MockTimeProvider(onDispose: () => disposed.add(true)); + TimeProviderRegistry(providers: [p1, p2]).dispose(); + expect(disposed.length, 2); + }); + }); +} + +class _MockTimeProvider extends TimeProvider { + _MockTimeProvider({this.onDispose}); + final void Function()? onDispose; + @override + String get name => 'mock'; + @override + Future getCurrentUtcTime() async => null; + @override + void dispose() { + onDispose?.call(); + } +} From cde26620c30c54d7833f2d837bec39e05405e9b6 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 17:09:47 +0200 Subject: [PATCH 3/9] chore(deps): add ntp package dependency and reorder imports --- lib/shared/ui/clock_warning_banner.dart | 3 +-- lib/views/bridge/bridge_exchange_form.dart | 1 - .../dex/simple/form/maker/maker_form_trade_button.dart | 1 - lib/views/dex/simple/form/taker/taker_form_content.dart | 1 - .../add_market_maker_bot_trade_button.dart | 1 - pubspec.lock | 8 ++++++++ pubspec.yaml | 1 + 7 files changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/shared/ui/clock_warning_banner.dart b/lib/shared/ui/clock_warning_banner.dart index dac7b0058e..d98c1932e7 100644 --- a/lib/shared/ui/clock_warning_banner.dart +++ b/lib/shared/ui/clock_warning_banner.dart @@ -1,10 +1,9 @@ +import 'package:easy_localization/easy_localization.dart'; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:web_dex/app_config/app_config.dart'; import 'package:web_dex/bloc/system_health/system_health_bloc.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/generated/codegen_loader.g.dart'; -import 'package:easy_localization/easy_localization.dart'; class ClockWarningBanner extends StatelessWidget { const ClockWarningBanner({Key? key}) : super(key: key); diff --git a/lib/views/bridge/bridge_exchange_form.dart b/lib/views/bridge/bridge_exchange_form.dart index 71fbbf51c0..6bea57f2e6 100644 --- a/lib/views/bridge/bridge_exchange_form.dart +++ b/lib/views/bridge/bridge_exchange_form.dart @@ -9,7 +9,6 @@ import 'package:web_dex/bloc/bridge_form/bridge_bloc.dart'; import 'package:web_dex/bloc/bridge_form/bridge_event.dart'; import 'package:web_dex/bloc/bridge_form/bridge_state.dart'; import 'package:web_dex/bloc/system_health/system_health_bloc.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/generated/codegen_loader.g.dart'; import 'package:web_dex/shared/widgets/connect_wallet/connect_wallet_wrapper.dart'; import 'package:web_dex/views/bridge/bridge_group.dart'; diff --git a/lib/views/dex/simple/form/maker/maker_form_trade_button.dart b/lib/views/dex/simple/form/maker/maker_form_trade_button.dart index 2f728c1a4f..d0dbe8f6f8 100644 --- a/lib/views/dex/simple/form/maker/maker_form_trade_button.dart +++ b/lib/views/dex/simple/form/maker/maker_form_trade_button.dart @@ -5,7 +5,6 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:komodo_ui_kit/komodo_ui_kit.dart'; import 'package:web_dex/bloc/auth_bloc/auth_bloc.dart'; import 'package:web_dex/bloc/system_health/system_health_bloc.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/blocs/maker_form_bloc.dart'; import 'package:web_dex/generated/codegen_loader.g.dart'; diff --git a/lib/views/dex/simple/form/taker/taker_form_content.dart b/lib/views/dex/simple/form/taker/taker_form_content.dart index 42fd7f6dbb..f66068b443 100644 --- a/lib/views/dex/simple/form/taker/taker_form_content.dart +++ b/lib/views/dex/simple/form/taker/taker_form_content.dart @@ -6,7 +6,6 @@ import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:komodo_ui_kit/komodo_ui_kit.dart'; import 'package:web_dex/bloc/coins_bloc/coins_repo.dart'; import 'package:web_dex/bloc/system_health/system_health_bloc.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/bloc/taker_form/taker_bloc.dart'; import 'package:web_dex/bloc/taker_form/taker_event.dart'; import 'package:web_dex/bloc/taker_form/taker_state.dart'; diff --git a/lib/views/market_maker_bot/add_market_maker_bot_trade_button.dart b/lib/views/market_maker_bot/add_market_maker_bot_trade_button.dart index 56a1fd1767..0c81ff781a 100644 --- a/lib/views/market_maker_bot/add_market_maker_bot_trade_button.dart +++ b/lib/views/market_maker_bot/add_market_maker_bot_trade_button.dart @@ -3,7 +3,6 @@ import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:komodo_ui_kit/komodo_ui_kit.dart'; import 'package:web_dex/bloc/system_health/system_health_bloc.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/generated/codegen_loader.g.dart'; class AddMarketMakerBotTradeButton extends StatelessWidget { diff --git a/pubspec.lock b/pubspec.lock index 6918b79260..e48fd1ae13 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -890,6 +890,14 @@ packages: url: "https://pub.dev" source: hosted version: "2.0.2" + ntp: + dependency: "direct main" + description: + name: ntp + sha256: "198db73e5059b334b50dbe8c626011c26576778ee9fc53f4c55c1d89d08ed2d2" + url: "https://pub.dev" + source: hosted + version: "2.0.0" package_config: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index 2099b77e30..3bce361b41 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -161,6 +161,7 @@ dependencies: path: packages/komodo_ui ref: dev feedback: ^3.1.0 + ntp: ^2.0.0 dev_dependencies: integration_test: # SDK From dd49c34671d7141621840d24c74ff3eedbdf5cba Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 17:11:10 +0200 Subject: [PATCH 4/9] refactor(system-health): rename events & move logging init to bootstrap --- lib/bloc/app_bloc_root.dart | 5 +- .../system_clock_repository.dart | 26 ++++------ .../system_health/system_health_bloc.dart | 47 +++++++++++++++---- .../system_health/system_health_event.dart | 11 ++++- .../system_health/system_health_state.dart | 6 ++- lib/main.dart | 14 +++--- .../initializer/app_bootstrapper.dart | 3 +- 7 files changed, 74 insertions(+), 38 deletions(-) diff --git a/lib/bloc/app_bloc_root.dart b/lib/bloc/app_bloc_root.dart index 488079ef87..44fd535572 100644 --- a/lib/bloc/app_bloc_root.dart +++ b/lib/bloc/app_bloc_root.dart @@ -284,7 +284,7 @@ class AppBlocRoot extends StatelessWidget { ), ), BlocProvider( - create: (_) => SystemHealthBloc(SystemClockRepository(), mm2Api), + create: (_) => SystemHealthBloc(SystemClockRepository(), mm2Api)..add(SystemHealthPeriodicCheckStarted()), ), BlocProvider( create: (context) => TrezorInitBloc( @@ -300,7 +300,8 @@ class AppBlocRoot extends StatelessWidget { ), ), BlocProvider( - create: (context) => FaucetBloc(kdfSdk: context.read()), + create: (context) => + FaucetBloc(kdfSdk: context.read()), ) ], child: _MyAppView(), diff --git a/lib/bloc/system_health/system_clock_repository.dart b/lib/bloc/system_health/system_clock_repository.dart index 7a680a31d7..602a665600 100644 --- a/lib/bloc/system_health/system_clock_repository.dart +++ b/lib/bloc/system_health/system_clock_repository.dart @@ -17,14 +17,12 @@ class SystemClockRepository { httpClient: httpClient, apiTimeout: apiTimeout, ), - _logger = logger; + _logger = logger ?? Logger('SystemClockRepository'); final Duration _maxAllowedDifference; final http.Client _httpClient; final TimeProviderRegistry _providerRegistry; - final Logger? _logger; - - Logger? get logger => _logger; + final Logger _logger; /// Queries the available time providers to validate the system clock validity /// returning true if the system clock is within allowed difference of the @@ -46,38 +44,34 @@ class SystemClockRepository { final isValid = difference < _maxAllowedDifference; if (isValid) { - await _log('System clock validated by ${provider.name} provider'); + _logger + .info('System clock validated by ${provider.name} provider'); } else { - await _log( + _logger.warning( 'System clock differs by ${difference.inSeconds}s from ' '${provider.name} provider'); } return isValid; } - } on Exception catch (e) { - await _log('Provider ${provider.name} failed: $e'); + } on Exception catch (e, s) { + _logger.severe('Provider ${provider.name} failed', e, s); } } if (!receivedValidResponse) { - await _log('All time providers failed to provide a time'); + _logger.warning('All time providers failed to provide a time'); } // Default to allowing usage when no provider responded return true; - } on Exception catch (e) { - await _log('Failed to validate system clock: $e'); + } on Exception catch (e, s) { + _logger.shout('Failed to validate system clock', e, s); // Don't block usage of dex if the time provider fetch fails return true; } } - Future _log(String message) async { - (logger ?? Logger('SystemClockRepository')) - .info('[SystemClockRepository] $message'); - } - void dispose() { _providerRegistry.dispose(); _httpClient.close(); diff --git a/lib/bloc/system_health/system_health_bloc.dart b/lib/bloc/system_health/system_health_bloc.dart index 7635960f57..c59e5716bf 100644 --- a/lib/bloc/system_health/system_health_bloc.dart +++ b/lib/bloc/system_health/system_health_bloc.dart @@ -1,31 +1,58 @@ import 'dart:async'; import 'package:bloc/bloc.dart'; +import 'package:bloc_concurrency/bloc_concurrency.dart'; import 'package:web_dex/bloc/system_health/system_clock_repository.dart'; -import 'package:web_dex/bloc/system_health/system_health_event.dart'; -import 'package:web_dex/bloc/system_health/system_health_state.dart'; import 'package:web_dex/mm2/mm2_api/mm2_api.dart'; import 'package:web_dex/mm2/mm2_api/rpc/directly_connected_peers/get_directly_connected_peers.dart'; +part 'system_health_event.dart'; +part 'system_health_state.dart'; + class SystemHealthBloc extends Bloc { SystemHealthBloc(this._systemClockRepository, this._api) : super(SystemHealthInitial()) { - on(_onCheckSystemClock); - _startPeriodicCheck(); + on( + _onSystemHealthCheckRequested, + transformer: restartable(), + ); + on(_onSystemHealthPeriodicCheckStarted); + on( + _onSystemHealthPeriodicCheckCancelled, + ); + add(SystemHealthPeriodicCheckStarted()); } Timer? _timer; final SystemClockRepository _systemClockRepository; final Mm2Api _api; - void _startPeriodicCheck() { + void _cancelPeriodicCheck() { + _timer?.cancel(); + _timer = null; + } + + Future _onSystemHealthPeriodicCheckStarted( + SystemHealthPeriodicCheckStarted event, + Emitter emit, + ) async { + _cancelPeriodicCheck(); + + add(SystemHealthCheckRequested()); _timer = Timer.periodic(const Duration(seconds: 60), (timer) { - add(CheckSystemClock()); + add(SystemHealthCheckRequested()); }); } - Future _onCheckSystemClock( - CheckSystemClock event, + Future _onSystemHealthPeriodicCheckCancelled( + SystemHealthPeriodicCheckCancelled event, + Emitter emit, + ) async { + _cancelPeriodicCheck(); + } + + Future _onSystemHealthCheckRequested( + SystemHealthCheckRequested event, Emitter emit, ) async { emit(SystemHealthLoadInProgress()); @@ -36,7 +63,7 @@ class SystemHealthBloc extends Bloc { final bool isSystemHealthy = systemClockValid || connectedPeersHealthy; emit(SystemHealthLoadSuccess(isSystemHealthy)); - } catch (_) { + } on Exception catch (_) { emit(SystemHealthLoadFailure()); } } @@ -56,7 +83,7 @@ class SystemHealthBloc extends Bloc { @override Future close() { - _timer?.cancel(); + _cancelPeriodicCheck(); return super.close(); } } diff --git a/lib/bloc/system_health/system_health_event.dart b/lib/bloc/system_health/system_health_event.dart index 3caba2ca8e..f5813a11f1 100644 --- a/lib/bloc/system_health/system_health_event.dart +++ b/lib/bloc/system_health/system_health_event.dart @@ -1,3 +1,12 @@ +part of 'system_health_bloc.dart'; + abstract class SystemHealthEvent {} -class CheckSystemClock extends SystemHealthEvent {} +/// Event to request a system health check (past tense, as per conventions) +class SystemHealthCheckRequested extends SystemHealthEvent {} + +/// Event to start the periodic check timer +class SystemHealthPeriodicCheckStarted extends SystemHealthEvent {} + +/// Event to cancel the periodic check timer +class SystemHealthPeriodicCheckCancelled extends SystemHealthEvent {} diff --git a/lib/bloc/system_health/system_health_state.dart b/lib/bloc/system_health/system_health_state.dart index 60857c9b6d..3af24e42e7 100644 --- a/lib/bloc/system_health/system_health_state.dart +++ b/lib/bloc/system_health/system_health_state.dart @@ -1,3 +1,5 @@ +part of 'system_health_bloc.dart'; + abstract class SystemHealthState {} class SystemHealthInitial extends SystemHealthState {} @@ -5,9 +7,9 @@ class SystemHealthInitial extends SystemHealthState {} class SystemHealthLoadInProgress extends SystemHealthState {} class SystemHealthLoadSuccess extends SystemHealthState { - final bool isValid; - SystemHealthLoadSuccess(this.isValid); + + final bool isValid; } class SystemHealthLoadFailure extends SystemHealthState {} diff --git a/lib/main.dart b/lib/main.dart index 3a04775f16..179a5358a0 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -2,7 +2,7 @@ import 'dart:async'; import 'package:easy_localization/easy_localization.dart'; import 'package:feedback/feedback.dart'; -import 'package:flutter/foundation.dart' show kIsWeb, kIsWasm; +import 'package:flutter/foundation.dart' show kIsWasm, kIsWeb; import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; import 'package:flutter_web_plugins/url_strategy.dart'; @@ -12,7 +12,6 @@ import 'package:komodo_cex_market_data/komodo_cex_market_data.dart'; import 'package:komodo_defi_sdk/komodo_defi_sdk.dart'; import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; -import 'package:web_dex/services/feedback/custom_feedback_form.dart'; import 'package:web_dex/app_config/app_config.dart'; import 'package:web_dex/app_config/package_information.dart'; import 'package:web_dex/bloc/app_bloc_observer.dart'; @@ -30,6 +29,7 @@ import 'package:web_dex/mm2/mm2_api/mm2_api.dart'; import 'package:web_dex/mm2/mm2_api/mm2_api_trezor.dart'; import 'package:web_dex/model/stored_settings.dart'; import 'package:web_dex/performance_analytics/performance_analytics.dart'; +import 'package:web_dex/services/feedback/custom_feedback_form.dart'; import 'package:web_dex/services/logger/get_logger.dart'; import 'package:web_dex/services/storage/get_storage.dart'; import 'package:web_dex/shared/constants.dart'; @@ -55,8 +55,14 @@ Future main() async { catchUnhandledExceptions(details.exception, details.stack); }; + // Foundational dependencies / setup - everything else builds on these 3. + // The current focus is migrating mm2Api to the new sdk, so that the sdk + // is the only/primary API/repository for KDF final KomodoDefiSdk komodoDefiSdk = await mm2.initialize(); + final mm2Api = Mm2Api(mm2: mm2, sdk: komodoDefiSdk); + await AppBootstrapper.instance.ensureInitialized(komodoDefiSdk, mm2Api); + // Strange inter-dependencies here that should ideally not be the case. final trezorRepo = TrezorRepo( api: Mm2ApiTrezor(mm2.call), kdfSdk: komodoDefiSdk, @@ -67,16 +73,12 @@ Future main() async { mm2: mm2, trezorBloc: trezor, ); - final mm2Api = Mm2Api(mm2: mm2, sdk: komodoDefiSdk); final walletsRepository = WalletsRepository( komodoDefiSdk, mm2Api, getStorage(), ); - await AppBootstrapper.instance.ensureInitialized(komodoDefiSdk); - await initializeLogger(mm2Api); - runApp( EasyLocalization( supportedLocales: localeList, diff --git a/lib/services/initializer/app_bootstrapper.dart b/lib/services/initializer/app_bootstrapper.dart index e1fe0f40c0..f3bfde5134 100644 --- a/lib/services/initializer/app_bootstrapper.dart +++ b/lib/services/initializer/app_bootstrapper.dart @@ -9,13 +9,14 @@ final class AppBootstrapper { bool _isInitialized = false; - Future ensureInitialized(KomodoDefiSdk kdfSdk) async { + Future ensureInitialized(KomodoDefiSdk kdfSdk, Mm2Api mm2Api) async { if (_isInitialized) return; GetIt.I.registerSingleton(kdfSdk); final timer = Stopwatch()..start(); await logger.init(); + await initializeLogger(mm2Api); log('AppBootstrapper: Log initialized in ${timer.elapsedMilliseconds}ms'); timer.reset(); From dada2cd8b902a55aa57f9a6917c843f5fa485ad9 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 17:12:10 +0200 Subject: [PATCH 5/9] fix(system-health): add binance time provider for web CORS rules block accessing the `date` header on web from other domains unless it's been explicitly allowed. --- .../providers/binance_time_provider.dart | 93 +++++++++++++++++++ .../providers/http_head_time_provider.dart | 82 +++++++--------- .../providers/time_provider_registry.dart | 32 ++++++- 3 files changed, 157 insertions(+), 50 deletions(-) create mode 100644 lib/bloc/system_health/providers/binance_time_provider.dart diff --git a/lib/bloc/system_health/providers/binance_time_provider.dart b/lib/bloc/system_health/providers/binance_time_provider.dart new file mode 100644 index 0000000000..1cdcc1bfc9 --- /dev/null +++ b/lib/bloc/system_health/providers/binance_time_provider.dart @@ -0,0 +1,93 @@ +import 'dart:async' show TimeoutException; +import 'dart:convert'; +import 'dart:io'; + +import 'package:http/http.dart' as http; +import 'package:logging/logging.dart'; +import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; + +/// A time provider that fetches time from the Binance API +class BinanceTimeProvider extends TimeProvider { + BinanceTimeProvider({ + this.url = 'https://api.binance.com/api/v3/time', + http.Client? httpClient, + this.timeout = const Duration(seconds: 2), + this.maxRetries = 3, + Logger? logger, + }) : _httpClient = httpClient ?? http.Client(), + _logger = logger ?? Logger('BinanceTimeProvider'); + + /// The URL of the Binance time API + final String url; + + /// Timeout for HTTP requests + final Duration timeout; + + /// Maximum retries + final int maxRetries; + + /// Logger instance + final Logger _logger; + + /// HTTP client for making requests + final http.Client _httpClient; + + @override + String get name => 'Binance'; + + @override + Future getCurrentUtcTime() async { + int retries = 0; + + while (retries < maxRetries) { + try { + final serverTime = await _fetchServerTime(); + _logger.fine('Successfully retrieved time from Binance API'); + return serverTime; + } on SocketException catch (e, s) { + _logger.warning('Socket error with Binance API', e, s); + } on TimeoutException catch (e, s) { + _logger.warning('Timeout with Binance API', e, s); + } on FormatException catch (e, s) { + _logger.severe('Failed to parse response from Binance API', e, s); + } on Exception catch (e, s) { + _logger.severe('Error fetching time from Binance API', e, s); + } + retries++; + } + + _logger.severe( + 'Failed to get time from Binance API after $maxRetries retries', + ); + return null; + } + + /// Fetches server time from the Binance API + Future _fetchServerTime() async { + final response = await _httpClient.get(Uri.parse(url)).timeout(timeout); + + if (response.statusCode != 200) { + _logger.warning('HTTP error from $url: ${response.statusCode}'); + throw HttpException( + 'HTTP error from $url: ${response.statusCode}', + uri: Uri.parse(url), + ); + } + + final jsonData = jsonDecode(response.body) as Map; + final serverTime = jsonData['serverTime'] as int?; + + if (serverTime == null) { + throw const FormatException( + 'No serverTime field in Binance API response', + ); + } + + return DateTime.fromMillisecondsSinceEpoch(serverTime, isUtc: true); + } + + @override + void dispose() { + _httpClient.close(); + } +} diff --git a/lib/bloc/system_health/providers/http_head_time_provider.dart b/lib/bloc/system_health/providers/http_head_time_provider.dart index d04e1a6e79..c6112040cc 100644 --- a/lib/bloc/system_health/providers/http_head_time_provider.dart +++ b/lib/bloc/system_health/providers/http_head_time_provider.dart @@ -9,15 +9,15 @@ import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; class HttpHeadTimeProvider extends TimeProvider { HttpHeadTimeProvider({ this.servers = const [ - 'https://www.google.com/', - 'https://www.alibaba.com/', - 'https://www.vk.com/', - 'https://www.cloudflare.com/', - 'https://www.microsoft.com/', + 'https://alibaba.com/', + 'https://google.com/', + 'https://cloudflare.com/', + 'https://microsoft.com/', + 'https://github.com/', ], http.Client? httpClient, this.timeout = const Duration(seconds: 2), - this.maxRetries = 2, + this.maxRetries = 3, Logger? logger, }) : _httpClient = httpClient ?? http.Client(), _logger = logger ?? Logger('HttpHeadTimeProvider'); @@ -41,18 +41,22 @@ class HttpHeadTimeProvider extends TimeProvider { @override Future getCurrentUtcTime() async { - for (final server in servers) { + for (final serverUrl in servers) { int retries = 0; while (retries < maxRetries) { try { - final serverTime = await _fetchServerTime(server); - if (serverTime != null) { - _logger.fine('Successfully retrieved time from $server'); - return serverTime; - } - } on Exception catch (e) { - _logger.severe('Error with $server: $e'); + final serverTime = await _fetchServerTime(serverUrl); + _logger.fine('Successfully retrieved time from $serverUrl'); + return serverTime; + } on SocketException catch (e, s) { + _logger.warning('Socket error with $serverUrl', e, s); + } on TimeoutException catch (e, s) { + _logger.warning('Timeout with $serverUrl', e, s); + } on FormatException catch (e, s) { + _logger.severe('Failed to parse Date header from $serverUrl', e, s); + } on Exception catch (e, s) { + _logger.severe('Error fetching time from $serverUrl', e, s); } retries++; } @@ -64,41 +68,25 @@ class HttpHeadTimeProvider extends TimeProvider { } /// Fetches server time from the 'date' header of an HTTP HEAD response - Future _fetchServerTime(String url) async { - try { - final response = await _httpClient.head( - Uri.parse(url), - headers: { - HttpHeaders.userAgentHeader: 'Komodo-Wallet/1.0', - }, - ).timeout(timeout); - - if (response.statusCode != 200) { - _logger.warning('HTTP error from $url: ${response.statusCode}'); - return null; - } - - final dateHeader = response.headers['date']; - if (dateHeader == null) { - _logger.warning('No Date header in response from $url'); - return null; - } + Future _fetchServerTime(String url) async { + final response = await _httpClient.head(Uri.parse(url)).timeout(timeout); + + if (response.statusCode != 200) { + _logger.warning('HTTP error from $url: ${response.statusCode}'); + throw HttpException( + 'HTTP error from $url: ${response.statusCode}', + uri: Uri.parse(url), + ); + } - final parsed = HttpDate.parse(dateHeader); - return parsed.toUtc(); // Ensure it's UTC - } on SocketException catch (e) { - _logger.warning('Socket error with $url: ${e.message}'); - return null; - } on TimeoutException catch (e) { - _logger.warning('Timeout with $url: ${e.message}'); - return null; - } on FormatException catch (e) { - _logger.severe('Failed to parse Date header from $url: $e'); - return null; - } on Exception catch (e) { - _logger.severe('Error fetching time from $url: $e'); - return null; + final dateHeader = response.headers['date']; + if (dateHeader == null) { + _logger.warning('No Date header in response from $url'); + throw FormatException('No Date header in response from $url'); } + + final parsed = HttpDate.parse(dateHeader); + return parsed.toUtc(); } /// Disposes the HTTP client when done diff --git a/lib/bloc/system_health/providers/time_provider_registry.dart b/lib/bloc/system_health/providers/time_provider_registry.dart index 197cd11e23..4bd089a5d8 100644 --- a/lib/bloc/system_health/providers/time_provider_registry.dart +++ b/lib/bloc/system_health/providers/time_provider_registry.dart @@ -1,4 +1,6 @@ +import 'package:flutter/foundation.dart'; import 'package:http/http.dart' as http; +import 'package:web_dex/bloc/system_health/providers/binance_time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/http_head_time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/http_time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/ntp_time_provider.dart'; @@ -25,11 +27,27 @@ class TimeProviderRegistry { /// Creates the default time providers List _createDefaultProviders() { return [ - NtpTimeProvider(), - HttpHeadTimeProvider( + // NTP is not supported on web with the existing flutter packages, + // so we use HTTP time providers instead via REST APIs that correctly + // configure the CORS headers to allow all origins + if (!kIsWeb && !kIsWasm) NtpTimeProvider(), + + // CORS errors on web block head requests to external servers, so HTTP + // header time providers are not available. We use REST APIs instead. + if (!kIsWeb && !kIsWasm) + HttpHeadTimeProvider( + httpClient: _httpClient, + timeout: _apiTimeout, + ), + + // Web fallback to NTP and HTTP head providers before trying the REST APIs + BinanceTimeProvider( httpClient: _httpClient, timeout: _apiTimeout, ), + + // REST APIs that return the current UTC time + // NOTE: these are prone to change, outages, and rate limits. HttpTimeProvider( url: 'https://timeapi.io/api/time/current/zone?timeZone=UTC', timeFieldPath: 'currentDateTime', @@ -37,7 +55,15 @@ class TimeProviderRegistry { providerName: 'TimeAPI', httpClient: _httpClient, apiTimeout: _apiTimeout, - ) + ), + HttpTimeProvider( + url: 'https://worldtimeapi.org/api/timezone/Etc/UTC', + timeFieldPath: 'utc_datetime', + timeFormat: TimeFormat.iso8601, + providerName: 'WorldTimeAPI', + httpClient: _httpClient, + apiTimeout: _apiTimeout, + ), ]; } From c630f869d068b454e54b23e4a4d838491a9ea901 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 21:45:18 +0200 Subject: [PATCH 6/9] refactor: change method return type and implement ai review suggestions --- .../providers/binance_time_provider.dart | 12 +- .../providers/http_head_time_provider.dart | 24 ++-- .../providers/http_time_provider.dart | 123 +++++++----------- .../providers/ntp_time_provider.dart | 24 ++-- .../providers/time_provider.dart | 4 +- .../system_clock_repository.dart | 30 ++--- .../system_health/system_health_bloc.dart | 11 +- .../initializer/app_bootstrapper.dart | 2 +- .../http_head_time_provider_test.dart | 59 ++++----- .../http_time_provider_test.dart | 33 +++-- .../system_health/ntp_time_provider_test.dart | 9 +- .../system_clock_repository_test.dart | 17 ++- .../time_provider_registry_test.dart | 2 +- 13 files changed, 168 insertions(+), 182 deletions(-) diff --git a/lib/bloc/system_health/providers/binance_time_provider.dart b/lib/bloc/system_health/providers/binance_time_provider.dart index 1cdcc1bfc9..1f82d8c947 100644 --- a/lib/bloc/system_health/providers/binance_time_provider.dart +++ b/lib/bloc/system_health/providers/binance_time_provider.dart @@ -36,7 +36,7 @@ class BinanceTimeProvider extends TimeProvider { String get name => 'Binance'; @override - Future getCurrentUtcTime() async { + Future getCurrentUtcTime() async { int retries = 0; while (retries < maxRetries) { @@ -54,12 +54,20 @@ class BinanceTimeProvider extends TimeProvider { _logger.severe('Error fetching time from Binance API', e, s); } retries++; + + // Calculate exponential backoff: 100ms, 200ms, 400ms, 800ms... + if (retries < maxRetries) { + final delayDuration = Duration(milliseconds: 100 * (1 << retries)); + await Future.delayed(delayDuration); + } } _logger.severe( 'Failed to get time from Binance API after $maxRetries retries', ); - return null; + throw TimeoutException( + 'Failed to get time from Binance API after $maxRetries retries', + ); } /// Fetches server time from the Binance API diff --git a/lib/bloc/system_health/providers/http_head_time_provider.dart b/lib/bloc/system_health/providers/http_head_time_provider.dart index c6112040cc..9c6cab0d6b 100644 --- a/lib/bloc/system_health/providers/http_head_time_provider.dart +++ b/lib/bloc/system_health/providers/http_head_time_provider.dart @@ -1,5 +1,6 @@ import 'dart:async' show TimeoutException; import 'dart:io'; +import 'dart:math' show Random; import 'package:http/http.dart' as http; import 'package:logging/logging.dart'; @@ -40,8 +41,14 @@ class HttpHeadTimeProvider extends TimeProvider { String get name => 'HttpHead'; @override - Future getCurrentUtcTime() async { - for (final serverUrl in servers) { + Future getCurrentUtcTime() async { + // Randomize the order of servers to avoid overloading any single server + // and to provide a more even distribution of requests. + // This also avoid a single server being a single point of failure. + final shuffledServers = List.from(servers)..shuffle(Random()); + _logger.fine('Randomized server order for time retrieval'); + + for (final serverUrl in shuffledServers) { int retries = 0; while (retries < maxRetries) { @@ -53,25 +60,24 @@ class HttpHeadTimeProvider extends TimeProvider { _logger.warning('Socket error with $serverUrl', e, s); } on TimeoutException catch (e, s) { _logger.warning('Timeout with $serverUrl', e, s); - } on FormatException catch (e, s) { - _logger.severe('Failed to parse Date header from $serverUrl', e, s); - } on Exception catch (e, s) { - _logger.severe('Error fetching time from $serverUrl', e, s); - } + } retries++; } } _logger .severe('Failed to get time from any server after $maxRetries retries'); - return null; + throw TimeoutException( + 'Failed to get time from any server after $maxRetries retries', + ); } /// Fetches server time from the 'date' header of an HTTP HEAD response Future _fetchServerTime(String url) async { final response = await _httpClient.head(Uri.parse(url)).timeout(timeout); - if (response.statusCode != 200) { + // Treat any successful or redirect status as acceptable. + if (response.statusCode < 200 || response.statusCode >= 400) { _logger.warning('HTTP error from $url: ${response.statusCode}'); throw HttpException( 'HTTP error from $url: ${response.statusCode}', diff --git a/lib/bloc/system_health/providers/http_time_provider.dart b/lib/bloc/system_health/providers/http_time_provider.dart index 1ffded9296..dfdedeb061 100644 --- a/lib/bloc/system_health/providers/http_time_provider.dart +++ b/lib/bloc/system_health/providers/http_time_provider.dart @@ -41,96 +41,69 @@ class HttpTimeProvider extends TimeProvider { final Duration _apiTimeout; @override - Future getCurrentUtcTime() async { - try { - final response = await _httpGet(url); - - if (response.statusCode != 200) { - _logger - .warning('API request failed with status ${response.statusCode}'); - return null; - } - - final jsonResponse = json.decode(response.body) as Map; - final DateTime? parsedTime = await _parseTimeFromJson(jsonResponse); - - if (parsedTime == null) { - _logger.warning('Failed to parse time from response'); - return null; - } - - return parsedTime; - } on Exception catch (e) { - _logger.severe('Error fetching time: $e'); - return null; + Future getCurrentUtcTime() async { + final response = await _httpClient.get(Uri.parse(url)).timeout(_apiTimeout); + + if (response.statusCode != 200) { + _logger.warning('API request failed with status ${response.statusCode}'); + throw HttpException( + 'API request failed with status ${response.statusCode}', + uri: Uri.parse(url), + ); } - } - Future _httpGet(String url) async { - try { - return await _httpClient.get(Uri.parse(url)).timeout(_apiTimeout); - } on Exception catch (e) { - return http.Response('Error: $e', HttpStatus.internalServerError); - } + final jsonResponse = json.decode(response.body) as Map; + final parsedTime = await _parseTimeFromJson(jsonResponse); + + return parsedTime; } - Future _parseTimeFromJson( - Map jsonResponse) async { - try { - final fieldParts = timeFieldPath.split('.'); - dynamic value = jsonResponse; + Future _parseTimeFromJson(Map jsonResponse) async { + final fieldParts = timeFieldPath.split('.'); + dynamic value = jsonResponse; - for (final part in fieldParts) { - if (value is! Map) { - _logger.warning('JSON path error: expected Map at $part'); - return null; - } - value = value[part]; - if (value == null) { - _logger.warning('JSON path error: null value at $part'); - return null; - } + for (final part in fieldParts) { + if (value is! Map) { + _logger.warning('JSON path error: expected Map at $part'); + throw FormatException('JSON path error: expected Map at $part'); } - - final timeStr = value.toString(); - if (timeStr.isEmpty) { - _logger.warning('Empty time string'); - return null; + value = value[part]; + if (value == null) { + _logger.warning('JSON path error: null value at $part'); + throw FormatException('JSON path error: null value at $part'); } + } - return await _parseDateTime(timeStr); - } on Exception catch (e) { - _logger.severe('JSON parsing error: $e'); - return null; + final timeStr = value.toString(); + if (timeStr.isEmpty) { + _logger.warning('Empty time string'); + throw const FormatException('Empty time string'); } - } - Future _parseDateTime(String timeStr) async { - try { - String formattedTime = timeStr; + return _parseDateTime(timeStr); + } - switch (timeFormat) { - case TimeFormat.iso8601: - if (formattedTime.endsWith('+00:00')) { - formattedTime = formattedTime.replaceAll('+00:00', 'Z'); - } else if (!formattedTime.endsWith('Z')) { - formattedTime += 'Z'; - } + Future _parseDateTime(String timeStr) async { + String formattedTime = timeStr; - case TimeFormat.custom: - throw const FormatException('Custom time format not supported'); - } + switch (timeFormat) { + case TimeFormat.iso8601: + if (formattedTime.endsWith('+00:00')) { + formattedTime = formattedTime.replaceAll('+00:00', 'Z'); + } else if (!formattedTime.endsWith('Z')) { + formattedTime += 'Z'; + } - final dateTime = DateTime.parse(formattedTime); - if (!dateTime.isUtc) { - throw const FormatException('Time is not in UTC'); - } + case TimeFormat.custom: + throw const FormatException('Custom time format not supported'); + } - return dateTime; - } on Exception catch (e) { - _logger.severe('Date parsing error: $e'); - return null; + final dateTime = DateTime.parse(formattedTime); + if (!dateTime.isUtc) { + throw const FormatException('Time is not in UTC'); } + + return dateTime; } @override diff --git a/lib/bloc/system_health/providers/ntp_time_provider.dart b/lib/bloc/system_health/providers/ntp_time_provider.dart index f6e096285b..27ab08d886 100644 --- a/lib/bloc/system_health/providers/ntp_time_provider.dart +++ b/lib/bloc/system_health/providers/ntp_time_provider.dart @@ -35,32 +35,22 @@ class NtpTimeProvider extends TimeProvider { String get name => 'NTP'; @override - Future getCurrentUtcTime() async { + Future getCurrentUtcTime() async { for (final server in ntpServers) { DateTime? time; int retries = 0; while (time == null && retries < maxRetries) { try { + final localNow = DateTime.now(); final int offset = await NTP.getNtpOffset( - localTime: DateTime.now(), + localTime: localNow, lookUpAddress: server, timeout: lookupTimeout, ); - final now = DateTime.now(); - time = now.add(Duration(milliseconds: offset)); - - final utcTime = DateTime.utc( - time.year, - time.month, - time.day, - time.hour, - time.minute, - time.second, - time.millisecond, - time.microsecond, - ); + time = localNow.add(Duration(milliseconds: offset)); + final utcTime = time.toUtc(); _logger.fine('Successfully retrieved time from $server'); return utcTime; @@ -80,6 +70,8 @@ class NtpTimeProvider extends TimeProvider { _logger.severe( 'Failed to get time from any NTP server after $maxRetries retries', ); - return null; + throw TimeoutException( + 'Failed to get time from any NTP server after $maxRetries retries', + ); } } diff --git a/lib/bloc/system_health/providers/time_provider.dart b/lib/bloc/system_health/providers/time_provider.dart index 766289686c..25b100a22c 100644 --- a/lib/bloc/system_health/providers/time_provider.dart +++ b/lib/bloc/system_health/providers/time_provider.dart @@ -1,9 +1,7 @@ /// Base interface for all time providers abstract class TimeProvider { /// Returns the current UTC time from an external source - /// - /// Returns null if the provider failed to get the time - Future getCurrentUtcTime(); + Future getCurrentUtcTime(); /// Returns a descriptive name for the provider String get name; diff --git a/lib/bloc/system_health/system_clock_repository.dart b/lib/bloc/system_health/system_clock_repository.dart index 602a665600..3eaa91e9ce 100644 --- a/lib/bloc/system_health/system_clock_repository.dart +++ b/lib/bloc/system_health/system_clock_repository.dart @@ -11,7 +11,6 @@ class SystemClockRepository { Logger? logger, }) : _maxAllowedDifference = maxAllowedDifference ?? const Duration(seconds: 60), - _httpClient = httpClient ?? http.Client(), _providerRegistry = providerRegistry ?? TimeProviderRegistry( httpClient: httpClient, @@ -20,7 +19,6 @@ class SystemClockRepository { _logger = logger ?? Logger('SystemClockRepository'); final Duration _maxAllowedDifference; - final http.Client _httpClient; final TimeProviderRegistry _providerRegistry; final Logger _logger; @@ -36,24 +34,21 @@ class SystemClockRepository { for (final provider in providers) { try { final apiTime = await provider.getCurrentUtcTime(); + receivedValidResponse = true; - if (apiTime != null) { - receivedValidResponse = true; - final localTime = DateTime.timestamp(); - final Duration difference = apiTime.difference(localTime).abs(); + final localTime = DateTime.timestamp(); + final Duration difference = apiTime.difference(localTime).abs(); - final isValid = difference < _maxAllowedDifference; - if (isValid) { - _logger - .info('System clock validated by ${provider.name} provider'); - } else { - _logger.warning( - 'System clock differs by ${difference.inSeconds}s from ' - '${provider.name} provider'); - } - - return isValid; + final isValid = difference < _maxAllowedDifference; + if (isValid) { + _logger.info('System clock validated by ${provider.name} provider'); + } else { + _logger.warning( + 'System clock differs by ${difference.inSeconds}s from ' + '${provider.name} provider'); } + + return isValid; } on Exception catch (e, s) { _logger.severe('Provider ${provider.name} failed', e, s); } @@ -74,6 +69,5 @@ class SystemClockRepository { void dispose() { _providerRegistry.dispose(); - _httpClient.close(); } } diff --git a/lib/bloc/system_health/system_health_bloc.dart b/lib/bloc/system_health/system_health_bloc.dart index c59e5716bf..82a5e6449e 100644 --- a/lib/bloc/system_health/system_health_bloc.dart +++ b/lib/bloc/system_health/system_health_bloc.dart @@ -10,8 +10,12 @@ part 'system_health_event.dart'; part 'system_health_state.dart'; class SystemHealthBloc extends Bloc { - SystemHealthBloc(this._systemClockRepository, this._api) - : super(SystemHealthInitial()) { + SystemHealthBloc( + this._systemClockRepository, + this._api, { + Duration checkInterval = const Duration(seconds: 60), + }) : _checkInterval = checkInterval, + super(SystemHealthInitial()) { on( _onSystemHealthCheckRequested, transformer: restartable(), @@ -24,6 +28,7 @@ class SystemHealthBloc extends Bloc { } Timer? _timer; + final Duration _checkInterval; final SystemClockRepository _systemClockRepository; final Mm2Api _api; @@ -39,7 +44,7 @@ class SystemHealthBloc extends Bloc { _cancelPeriodicCheck(); add(SystemHealthCheckRequested()); - _timer = Timer.periodic(const Duration(seconds: 60), (timer) { + _timer = Timer.periodic(_checkInterval, (timer) { add(SystemHealthCheckRequested()); }); } diff --git a/lib/services/initializer/app_bootstrapper.dart b/lib/services/initializer/app_bootstrapper.dart index f3bfde5134..f2649aa4e7 100644 --- a/lib/services/initializer/app_bootstrapper.dart +++ b/lib/services/initializer/app_bootstrapper.dart @@ -16,7 +16,7 @@ final class AppBootstrapper { final timer = Stopwatch()..start(); await logger.init(); - await initializeLogger(mm2Api); + await initializeLogger(mm2Api); log('AppBootstrapper: Log initialized in ${timer.elapsedMilliseconds}ms'); timer.reset(); diff --git a/test_units/tests/system_health/http_head_time_provider_test.dart b/test_units/tests/system_health/http_head_time_provider_test.dart index c807da45a1..734493e35c 100644 --- a/test_units/tests/system_health/http_head_time_provider_test.dart +++ b/test_units/tests/system_health/http_head_time_provider_test.dart @@ -39,56 +39,54 @@ void testHttpHeadTimeProvider() { expect(result, equals(expectedDateTime)); }); - test('returns null when date header is missing', () async { + test('throws FormatException when date header is missing', () async { mockClient.mockResponse = http.Response('', 200, headers: {}); - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('returns null when response status is not 200', () async { + test('throws HttpException when response status is not 200', () async { mockClient.mockResponse = http.Response('', 404); - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('returns null when all servers fail', () async { + test('throws HttpException when all servers fail', () async { mockClient.mockResponse = http.Response('', 500); - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('handles timeout exceptions', () async { + test('throws TimeoutException on timeout', () async { mockClient.shouldThrowTimeout = true; - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); - }); - - test('handles socket exceptions', () async { - mockClient.shouldThrowSocketException = true; - - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('handles format exceptions with invalid date', () async { + // HttpDate.parse is used, which throws HttpException + test('throws HttpException with invalid date', () async { mockClient.mockResponse = http.Response( '', 200, headers: {'date': 'invalid-date-format'}, ); - final result = await provider.getCurrentUtcTime(); - - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); }); } @@ -97,7 +95,6 @@ void testHttpHeadTimeProvider() { class MockClient extends http.BaseClient { http.Response mockResponse = http.Response('', 200); bool shouldThrowTimeout = false; - bool shouldThrowSocketException = false; @override Future send(http.BaseRequest request) async { @@ -105,10 +102,6 @@ class MockClient extends http.BaseClient { throw TimeoutException('Timeout'); } - if (shouldThrowSocketException) { - throw const SocketException('Socket error'); - } - return http.StreamedResponse( Stream.value(mockResponse.bodyBytes), mockResponse.statusCode, diff --git a/test_units/tests/system_health/http_time_provider_test.dart b/test_units/tests/system_health/http_time_provider_test.dart index 64d085839d..c873c17a55 100644 --- a/test_units/tests/system_health/http_time_provider_test.dart +++ b/test_units/tests/system_health/http_time_provider_test.dart @@ -1,3 +1,4 @@ +import 'dart:async'; import 'dart:convert' show jsonEncode; import 'dart:io' show HttpException; @@ -33,30 +34,38 @@ void testHttpTimeProvider() { expect(result, equals(now)); }); - test('returns null on non-200 response', () async { + test('throws HttpException on non-200 response', () async { mockClient.mockResponse = http.Response('error', 500); - final result = await provider.getCurrentUtcTime(); - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('returns null if field missing', () async { + test('throws FormatException if field missing', () async { mockClient.mockResponse = http.Response(jsonEncode({'other': 'value'}), 200); - final result = await provider.getCurrentUtcTime(); - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('returns null on invalid date format', () async { + test('throws FormatException on invalid date format', () async { mockClient.mockResponse = http.Response(jsonEncode({'currentDateTime': 'not-a-date'}), 200); - final result = await provider.getCurrentUtcTime(); - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); - test('returns null on exception', () async { + test('throws HttpException on exception', () async { mockClient.shouldThrow = true; - final result = await provider.getCurrentUtcTime(); - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); }); } diff --git a/test_units/tests/system_health/ntp_time_provider_test.dart b/test_units/tests/system_health/ntp_time_provider_test.dart index be69aa81c1..f303a7ac33 100644 --- a/test_units/tests/system_health/ntp_time_provider_test.dart +++ b/test_units/tests/system_health/ntp_time_provider_test.dart @@ -1,17 +1,20 @@ +import 'dart:async'; import 'package:test/test.dart'; import 'package:web_dex/bloc/system_health/providers/ntp_time_provider.dart'; void testNtpTimeProvider() { group('NtpTimeProvider', () { - test('returns null if all servers fail', () async { + test('throws TimeoutException if all servers fail', () async { final provider = NtpTimeProvider( ntpServers: ['bad.ntp.server'], lookupTimeout: const Duration(milliseconds: 10), maxRetries: 1, ); // This will likely fail due to bad server - final result = await provider.getCurrentUtcTime(); - expect(result, isNull); + expect( + () => provider.getCurrentUtcTime(), + throwsA(isA()), + ); }); // Note: A true success test would require a real NTP server and is best as // an integration test. diff --git a/test_units/tests/system_health/system_clock_repository_test.dart b/test_units/tests/system_health/system_clock_repository_test.dart index 9c6cd2aa0b..11d9b7aec9 100644 --- a/test_units/tests/system_health/system_clock_repository_test.dart +++ b/test_units/tests/system_health/system_clock_repository_test.dart @@ -51,7 +51,7 @@ void testSystemClockRepository() { test('returns true when no providers respond', () async { mockRegistry.mockProviders = [ - MockTimeProvider(name: 'FailingProvider'), + MockTimeProvider(name: 'FailingProvider', returnTime: DateTime.now()), ]; final result = await repository.isSystemClockValid(); @@ -61,7 +61,11 @@ void testSystemClockRepository() { test('returns true when provider throws exception', () async { mockRegistry.mockProviders = [ - MockTimeProvider(name: 'ExceptionProvider', shouldThrow: true), + MockTimeProvider( + name: 'ExceptionProvider', + shouldThrow: true, + returnTime: DateTime.now(), + ), ]; final result = await repository.isSystemClockValid(); @@ -74,7 +78,8 @@ void testSystemClockRepository() { name: 'ValidProvider', returnTime: DateTime.timestamp().toUtc(), ); - final failingProvider = MockTimeProvider(name: 'FailingProvider'); + final failingProvider = + MockTimeProvider(name: 'FailingProvider', returnTime: DateTime.now()); mockRegistry.mockProviders = [validProvider, failingProvider]; await repository.isSystemClockValid(); @@ -103,12 +108,12 @@ class MockTimeProviderRegistry implements TimeProviderRegistry { class MockTimeProvider extends TimeProvider { MockTimeProvider({ required String name, - this.returnTime, + required this.returnTime, this.shouldThrow = false, }) : _name = name; final String _name; - final DateTime? returnTime; + final DateTime returnTime; final bool shouldThrow; int callCount = 0; @@ -116,7 +121,7 @@ class MockTimeProvider extends TimeProvider { String get name => _name; @override - Future getCurrentUtcTime() async { + Future getCurrentUtcTime() async { callCount++; if (shouldThrow) { throw Exception('Test exception'); diff --git a/test_units/tests/system_health/time_provider_registry_test.dart b/test_units/tests/system_health/time_provider_registry_test.dart index 22ecf55cec..63250bc1f8 100644 --- a/test_units/tests/system_health/time_provider_registry_test.dart +++ b/test_units/tests/system_health/time_provider_registry_test.dart @@ -33,7 +33,7 @@ class _MockTimeProvider extends TimeProvider { @override String get name => 'mock'; @override - Future getCurrentUtcTime() async => null; + Future getCurrentUtcTime() async => DateTime.now(); @override void dispose() { onDispose?.call(); From 3d66ca6eae64e1662f7cac9b908d41a9375cc335 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 20:54:11 +0200 Subject: [PATCH 7/9] fix(system-health): show banner if clock invalid even if peers >= 2 previous check was a regression that wouldn't show the banner for incorrect clock if there were still peers connected --- lib/bloc/system_health/system_health_bloc.dart | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/bloc/system_health/system_health_bloc.dart b/lib/bloc/system_health/system_health_bloc.dart index 82a5e6449e..367f5aead5 100644 --- a/lib/bloc/system_health/system_health_bloc.dart +++ b/lib/bloc/system_health/system_health_bloc.dart @@ -64,15 +64,18 @@ class SystemHealthBloc extends Bloc { try { final bool systemClockValid = await _systemClockRepository.isSystemClockValid(); - final bool connectedPeersHealthy = await _arePeersConnected(); - final bool isSystemHealthy = systemClockValid || connectedPeersHealthy; - emit(SystemHealthLoadSuccess(isSystemHealthy)); + emit(SystemHealthLoadSuccess(systemClockValid)); } on Exception catch (_) { emit(SystemHealthLoadFailure()); } } + // TODO: add an additional state or banner if there are no peers connected + // the current system health check indicates an out-of-sync clock message + // to the user, which might not be the reason for too few peers + // final bool connectedPeersHealthy = await _arePeersConnected(); + // ignore: unused_element Future _arePeersConnected() async { try { final directlyConnectedPeers = From 8b3d95cbe7ce7811fd5b1d79d6cfe3db427f8016 Mon Sep 17 00:00:00 2001 From: Francois Date: Thu, 8 May 2025 22:22:05 +0200 Subject: [PATCH 8/9] refactor: simplify datetime parsing and revert exception regression --- .../providers/http_head_time_provider.dart | 4 +++ .../providers/http_time_provider.dart | 27 +++++++------------ .../http_head_time_provider_test.dart | 18 +++++++------ 3 files changed, 24 insertions(+), 25 deletions(-) diff --git a/lib/bloc/system_health/providers/http_head_time_provider.dart b/lib/bloc/system_health/providers/http_head_time_provider.dart index 9c6cab0d6b..58e7e8b27e 100644 --- a/lib/bloc/system_health/providers/http_head_time_provider.dart +++ b/lib/bloc/system_health/providers/http_head_time_provider.dart @@ -60,6 +60,10 @@ class HttpHeadTimeProvider extends TimeProvider { _logger.warning('Socket error with $serverUrl', e, s); } on TimeoutException catch (e, s) { _logger.warning('Timeout with $serverUrl', e, s); + } on HttpException catch (e, s) { + _logger.warning('HTTP error with $serverUrl', e, s); + } on FormatException catch (e, s) { + _logger.warning('Date header parse error with $serverUrl', e, s); } retries++; } diff --git a/lib/bloc/system_health/providers/http_time_provider.dart b/lib/bloc/system_health/providers/http_time_provider.dart index dfdedeb061..5aa1b5f9c8 100644 --- a/lib/bloc/system_health/providers/http_time_provider.dart +++ b/lib/bloc/system_health/providers/http_time_provider.dart @@ -52,7 +52,14 @@ class HttpTimeProvider extends TimeProvider { ); } - final jsonResponse = json.decode(response.body) as Map; + final dynamic decoded = json.decode(response.body); + if (decoded is! Map) { + _logger.warning( + 'Expected top-level JSON object, got ${decoded.runtimeType}', + ); + throw const FormatException('Invalid JSON structure – object expected'); + } + final Map jsonResponse = decoded; final parsedTime = await _parseTimeFromJson(jsonResponse); return parsedTime; @@ -83,27 +90,13 @@ class HttpTimeProvider extends TimeProvider { return _parseDateTime(timeStr); } - Future _parseDateTime(String timeStr) async { - String formattedTime = timeStr; - + DateTime _parseDateTime(String timeStr) { switch (timeFormat) { case TimeFormat.iso8601: - if (formattedTime.endsWith('+00:00')) { - formattedTime = formattedTime.replaceAll('+00:00', 'Z'); - } else if (!formattedTime.endsWith('Z')) { - formattedTime += 'Z'; - } - + return DateTime.parse(timeStr).toUtc(); case TimeFormat.custom: throw const FormatException('Custom time format not supported'); } - - final dateTime = DateTime.parse(formattedTime); - if (!dateTime.isUtc) { - throw const FormatException('Time is not in UTC'); - } - - return dateTime; } @override diff --git a/test_units/tests/system_health/http_head_time_provider_test.dart b/test_units/tests/system_health/http_head_time_provider_test.dart index 734493e35c..bef4db0658 100644 --- a/test_units/tests/system_health/http_head_time_provider_test.dart +++ b/test_units/tests/system_health/http_head_time_provider_test.dart @@ -39,30 +39,32 @@ void testHttpHeadTimeProvider() { expect(result, equals(expectedDateTime)); }); - test('throws FormatException when date header is missing', () async { + // Timeout expected because other servers should be tried rather than + // exiting + test('throws TimeoutException when date header is missing', () async { mockClient.mockResponse = http.Response('', 200, headers: {}); expect( () => provider.getCurrentUtcTime(), - throwsA(isA()), + throwsA(isA()), ); }); - test('throws HttpException when response status is not 200', () async { + test('throws TimeoutException when response status is not 200', () async { mockClient.mockResponse = http.Response('', 404); expect( () => provider.getCurrentUtcTime(), - throwsA(isA()), + throwsA(isA()), ); }); - test('throws HttpException when all servers fail', () async { + test('throws TimeoutException when all servers fail', () async { mockClient.mockResponse = http.Response('', 500); expect( () => provider.getCurrentUtcTime(), - throwsA(isA()), + throwsA(isA()), ); }); @@ -76,7 +78,7 @@ void testHttpHeadTimeProvider() { }); // HttpDate.parse is used, which throws HttpException - test('throws HttpException with invalid date', () async { + test('throws TimeoutException with invalid date', () async { mockClient.mockResponse = http.Response( '', 200, @@ -85,7 +87,7 @@ void testHttpHeadTimeProvider() { expect( () => provider.getCurrentUtcTime(), - throwsA(isA()), + throwsA(isA()), ); }); }); From 3b6196ece87498f47589278cc71efc508efa8702 Mon Sep 17 00:00:00 2001 From: Francois Date: Fri, 9 May 2025 16:14:40 +0200 Subject: [PATCH 9/9] refactor: remove shared http client to avoid issues with multiple closes --- lib/bloc/app_bloc_root.dart | 3 ++- .../providers/time_provider_registry.dart | 23 ++++--------------- .../system_clock_repository.dart | 3 --- .../system_clock_repository_test.dart | 2 -- 4 files changed, 7 insertions(+), 24 deletions(-) diff --git a/lib/bloc/app_bloc_root.dart b/lib/bloc/app_bloc_root.dart index 44fd535572..409bc8619e 100644 --- a/lib/bloc/app_bloc_root.dart +++ b/lib/bloc/app_bloc_root.dart @@ -284,7 +284,8 @@ class AppBlocRoot extends StatelessWidget { ), ), BlocProvider( - create: (_) => SystemHealthBloc(SystemClockRepository(), mm2Api)..add(SystemHealthPeriodicCheckStarted()), + create: (_) => SystemHealthBloc(SystemClockRepository(), mm2Api) + ..add(SystemHealthPeriodicCheckStarted()), ), BlocProvider( create: (context) => TrezorInitBloc( diff --git a/lib/bloc/system_health/providers/time_provider_registry.dart b/lib/bloc/system_health/providers/time_provider_registry.dart index 4bd089a5d8..8dbd4484b7 100644 --- a/lib/bloc/system_health/providers/time_provider_registry.dart +++ b/lib/bloc/system_health/providers/time_provider_registry.dart @@ -1,5 +1,4 @@ import 'package:flutter/foundation.dart'; -import 'package:http/http.dart' as http; import 'package:web_dex/bloc/system_health/providers/binance_time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/http_head_time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/http_time_provider.dart'; @@ -10,14 +9,11 @@ import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; class TimeProviderRegistry { TimeProviderRegistry({ List? providers, - http.Client? httpClient, Duration? apiTimeout, - }) : _httpClient = httpClient ?? http.Client(), - _apiTimeout = apiTimeout ?? const Duration(seconds: 2) { + }) : _apiTimeout = apiTimeout ?? const Duration(seconds: 2) { _providers = providers ?? _createDefaultProviders(); } - final http.Client _httpClient; final Duration _apiTimeout; late final List _providers; @@ -32,28 +28,20 @@ class TimeProviderRegistry { // configure the CORS headers to allow all origins if (!kIsWeb && !kIsWasm) NtpTimeProvider(), - // CORS errors on web block head requests to external servers, so HTTP + // CORS errors on web block head requests to external servers, so HTTP // header time providers are not available. We use REST APIs instead. - if (!kIsWeb && !kIsWasm) - HttpHeadTimeProvider( - httpClient: _httpClient, - timeout: _apiTimeout, - ), + if (!kIsWeb && !kIsWasm) HttpHeadTimeProvider(timeout: _apiTimeout), // Web fallback to NTP and HTTP head providers before trying the REST APIs - BinanceTimeProvider( - httpClient: _httpClient, - timeout: _apiTimeout, - ), + BinanceTimeProvider(timeout: _apiTimeout), // REST APIs that return the current UTC time - // NOTE: these are prone to change, outages, and rate limits. + // NOTE: these are prone to change, outages, and rate limits. HttpTimeProvider( url: 'https://timeapi.io/api/time/current/zone?timeZone=UTC', timeFieldPath: 'currentDateTime', timeFormat: TimeFormat.iso8601, providerName: 'TimeAPI', - httpClient: _httpClient, apiTimeout: _apiTimeout, ), HttpTimeProvider( @@ -61,7 +49,6 @@ class TimeProviderRegistry { timeFieldPath: 'utc_datetime', timeFormat: TimeFormat.iso8601, providerName: 'WorldTimeAPI', - httpClient: _httpClient, apiTimeout: _apiTimeout, ), ]; diff --git a/lib/bloc/system_health/system_clock_repository.dart b/lib/bloc/system_health/system_clock_repository.dart index 3eaa91e9ce..425b7b0065 100644 --- a/lib/bloc/system_health/system_clock_repository.dart +++ b/lib/bloc/system_health/system_clock_repository.dart @@ -1,11 +1,9 @@ -import 'package:http/http.dart' as http; import 'package:logging/logging.dart'; import 'package:web_dex/bloc/system_health/providers/time_provider_registry.dart'; class SystemClockRepository { SystemClockRepository({ TimeProviderRegistry? providerRegistry, - http.Client? httpClient, Duration? maxAllowedDifference, Duration? apiTimeout, Logger? logger, @@ -13,7 +11,6 @@ class SystemClockRepository { maxAllowedDifference ?? const Duration(seconds: 60), _providerRegistry = providerRegistry ?? TimeProviderRegistry( - httpClient: httpClient, apiTimeout: apiTimeout, ), _logger = logger ?? Logger('SystemClockRepository'); diff --git a/test_units/tests/system_health/system_clock_repository_test.dart b/test_units/tests/system_health/system_clock_repository_test.dart index 11d9b7aec9..aa4647c28f 100644 --- a/test_units/tests/system_health/system_clock_repository_test.dart +++ b/test_units/tests/system_health/system_clock_repository_test.dart @@ -1,4 +1,3 @@ -import 'package:http/http.dart' as http; import 'package:test/test.dart'; import 'package:web_dex/bloc/system_health/providers/time_provider.dart'; import 'package:web_dex/bloc/system_health/providers/time_provider_registry.dart'; @@ -13,7 +12,6 @@ void testSystemClockRepository() { mockRegistry = MockTimeProviderRegistry(); repository = SystemClockRepository( providerRegistry: mockRegistry, - httpClient: http.Client(), maxAllowedDifference: const Duration(seconds: 30), ); });