Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA file descriptor (FD) monitoring system is added across iOS and Flutter. The iOS FdMonitor class periodically samples and logs FD metrics; a Flutter service wraps the native MethodChannel for cross-platform access. Additionally, login and logout flows now route users to the Wallet menu. Changes
Sequence DiagramsequenceDiagram
participant Flutter as Flutter App
participant Channel as MethodChannel
participant AppDelegate as AppDelegate
participant FdMonitor as FdMonitor
Flutter->>Channel: start(intervalSeconds: 60)
Channel->>AppDelegate: handleFdMonitorMethodCall (start)
AppDelegate->>FdMonitor: start(intervalSeconds: 60)
Note over FdMonitor: Creates DispatchSource timer<br/>Starts periodic sampling
FdMonitor-->>AppDelegate: success
AppDelegate-->>Channel: result
Channel-->>Flutter: {status: "started"}
Note over FdMonitor: Timer fires every 60s
FdMonitor->>FdMonitor: getFileDescriptorInfo()
Note over FdMonitor: Calls getdtablesize()<br/>getrlimit(), fcntl() per FD
FdMonitor->>FdMonitor: logFileDescriptorStatus()
Flutter->>Channel: getCurrentCount()
Channel->>AppDelegate: handleFdMonitorMethodCall (getCurrentCount)
AppDelegate->>FdMonitor: getCurrentCount()
FdMonitor-->>AppDelegate: FdInfo (openCount, tableSize, softLimit, ...)
AppDelegate-->>Channel: result
Channel-->>Flutter: FdMonitorStats
Flutter->>Channel: stop()
Channel->>AppDelegate: handleFdMonitorMethodCall (stop)
AppDelegate->>FdMonitor: stop()
Note over FdMonitor: Cancels DispatchSourceTimer
FdMonitor-->>AppDelegate: success
AppDelegate-->>Channel: result
Channel-->>Flutter: {status: "stopped"}
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Visit the preview URL for this PR (updated for commit 5fd4b88): https://walletrc--pull-3262-merge-bv4z56p0.web.app (expires Mon, 03 Nov 2025 09:17:29 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
lib/views/main_layout/main_layout.dart (1)
81-81: Consider extracting the duplicated navigation logic.The assignment
routingState.selectedMenu = MainMenuValue.walletappears in both the login and logout flows. While the duplication is minimal and each occurrence is contextually clear, extracting this into a helper method (e.g.,_navigateToWallet()) could improve maintainability if this navigation logic needs to evolve.Example refactor:
void _navigateToWallet() { routingState.selectedMenu = MainMenuValue.wallet; }Then use it in both places:
if (state.mode == AuthorizeMode.logIn) { QuickLoginSwitch.trackUserLoggedIn(); // Always route to Wallet on login - routingState.selectedMenu = MainMenuValue.wallet; + _navigateToWallet(); }if (state.mode == AuthorizeMode.noLogin && QuickLoginSwitch.hasBeenLoggedInThisSession) { routingState.resetOnLogOut(); QuickLoginSwitch.resetOnLogout(); // Always route to Wallet on logout - routingState.selectedMenu = MainMenuValue.wallet; + _navigateToWallet(); }Also applies to: 92-92
ios/Runner/AppDelegate.swift (2)
10-10: Gate debug-only launch logThe “🔴 didFinishLaunching…” log is noisy in release. Wrap in DEBUG or remove.
- NSLog("🔴 AppDelegate: didFinishLaunchingWithOptions REACHED") + #if DEBUG + NSLog("AppDelegate: didFinishLaunchingWithOptions reached") + #endif
26-33: Handle missing FlutterViewController more explicitlyIf window/rootVC isn’t ready yet, the channel silently isn’t created, leading to MissingPlugin errors. Log and consider retrying once the window is available (or wire via FlutterEngine if you have one).
private func setupFdMonitorChannel() { - guard let controller = window?.rootViewController as? FlutterViewController else { - return - } + guard let controller = window?.rootViewController as? FlutterViewController else { + NSLog("AppDelegate: FD Monitor channel not set up — rootViewController unavailable") + return + }ios/Runner/FdMonitor.swift (4)
1-4: Prefer explicit Darwin import and final classDarwin exposes fcntl/fstat, S_IF*, PATH_MAX consistently; final avoids subclassing overhead for a singleton.
-import Foundation -import OSLog +import Foundation +import OSLog +import Darwin - -class FdMonitor { +final class FdMonitor {
76-91: Avoid per-call ISO8601DateFormatter allocationReuse a static formatter to reduce allocations in periodic sampling.
- func getCurrentCount() -> [String: Any] { + private static let iso8601 = ISO8601DateFormatter() + + func getCurrentCount() -> [String: Any] { var result: [String: Any] = [:] queue.sync { let fdInfo = self.getFileDescriptorInfo() result = [ "openCount": fdInfo.openCount, "tableSize": fdInfo.tableSize, "softLimit": fdInfo.softLimit, "hardLimit": fdInfo.hardLimit, "percentUsed": fdInfo.percentUsed, - "timestamp": ISO8601DateFormatter().string(from: Date()) + "timestamp": FdMonitor.iso8601.string(from: Date()) ] } return result }
109-136: Tighten scan upper bound to reduce work on large tablesOpen FDs cannot exceed softLimit. Scanning up to min(tableSize, softLimit) reduces syscalls on devices where tableSize > softLimit.
- let tableSize = Int(getdtablesize()) + let tableSize = Int(getdtablesize()) var rlimit = rlimit() getrlimit(RLIMIT_NOFILE, &rlimit) let softLimit = Int(rlimit.rlim_cur) let hardLimit = Int(rlimit.rlim_max) - var openCount = 0 - for fd in 0..<tableSize { + let maxFd = min(tableSize, softLimit) + var openCount = 0 + for fd in 0..<maxFd { let fd32 = Int32(fd) errno = 0 let flags = fcntl(fd32, F_GETFD, 0) - if flags != -1 || errno != EBADF { + if flags != -1 || errno != EBADF { openCount += 1 } }
202-205: Avoid logging full file paths in release buildsFile paths can be sensitive. Gate path logging under DEBUG or omit the path outside debug.
- if logged < 20 { // Only log first 20 individual FDs to avoid spam - logger.debug(" FD \(fd): type=\(typeStr) path=\(path)") - } + if logged < 20 { + #if DEBUG + logger.debug(" FD \(fd): type=\(typeStr) path=\(path)") + #else + logger.debug(" FD \(fd): type=\(typeStr)") + #endif + }Please confirm intended logging policy for release builds.
lib/services/fd_monitor_service.dart (2)
158-166: Decode numeric fields via num to avoid int/double cast issues from platform channelsPlatform decodes may yield num; casting to int directly can fail.
factory FdMonitorStats.fromMap(Map<String, dynamic> map) { return FdMonitorStats( - openCount: map['openCount'] as int? ?? 0, - tableSize: map['tableSize'] as int? ?? 0, - softLimit: map['softLimit'] as int? ?? 0, - hardLimit: map['hardLimit'] as int? ?? 0, - percentUsed: (map['percentUsed'] as num?)?.toDouble() ?? 0.0, + openCount: (map['openCount'] as num?)?.toInt() ?? 0, + tableSize: (map['tableSize'] as num?)?.toInt() ?? 0, + softLimit: (map['softLimit'] as num?)?.toInt() ?? 0, + hardLimit: (map['hardLimit'] as num?)?.toInt() ?? 0, + percentUsed: (map['percentUsed'] as num?)?.toDouble() ?? 0.0, timestamp: map['timestamp'] as String? ?? '', ); }
95-101: Use debugPrint instead of print for errorsThrottled and consistent with Flutter logging.
- print('FD Monitor error getting count: ${e.message}'); + debugPrint('FD Monitor error getting count: ${e.message}'); ... - print('FD Monitor unexpected error: $e'); + debugPrint('FD Monitor unexpected error: $e');
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
ios/Runner/AppDelegate.swift(1 hunks)ios/Runner/FdMonitor.swift(1 hunks)lib/services/fd_monitor_service.dart(1 hunks)lib/views/main_layout/main_layout.dart(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.dart
📄 CodeRabbit inference engine (AGENTS.md)
**/*.dart: Runflutter analyzeand resolve issues before committing code
Format Dart code withdart format(on changed files) before committing
Files:
lib/views/main_layout/main_layout.dartlib/services/fd_monitor_service.dart
🧬 Code graph analysis (1)
ios/Runner/AppDelegate.swift (1)
ios/Runner/FdMonitor.swift (4)
start(23-57)stop(59-74)getCurrentCount(76-92)logDetailedStatus(94-98)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)
- GitHub Check: validate_code_guidelines
- GitHub Check: Test web-app-linux-profile
- GitHub Check: unit_tests
- GitHub Check: Test web-app-macos
- GitHub Check: Build Desktop (macos)
- GitHub Check: build_and_preview
- GitHub Check: Build Mobile (Android)
- GitHub Check: Build Desktop (linux)
- GitHub Check: Build Desktop (windows)
- GitHub Check: Build Mobile (iOS)
🔇 Additional comments (6)
lib/views/main_layout/main_layout.dart (2)
78-82: LGTM! Login navigation implemented correctly.The navigation to the Wallet page on login is implemented appropriately within the BlocConsumer listener, which is the correct place for side effects in Flutter.
86-93: ****The original concern is unfounded.
routingState.resetOnLogOut()(lines 75–83 inrouting_state.dart) does not modify theselectedMenuproperty—it only callsresetOnLogOut()on child states (walletState, fiatState, dexState, etc.). The subsequent assignmentroutingState.selectedMenu = MainMenuValue.walletat line 92 executes without interference and correctly sets the menu as intended.Likely an incorrect or invalid review comment.
ios/Runner/AppDelegate.swift (1)
36-68: Channel wiring and handler flow look goodWeak self capture, method dispatch, and JSON-safe payloads are correct.
ios/Runner/FdMonitor.swift (1)
22-57: Timer lifecycle and queue usage look correctSingle-queue mutation, start guard, weak self in handler, and proper cancel in stop().
lib/services/fd_monitor_service.dart (2)
5-139: Overall channel usage and API surface look goodSingleton wiring, error handling, and stats model are sensible. Minor nits above.
1-189: Runflutter analyzeanddart formatlocally before committingThe sandbox environment does not have
flutterordartcommand-line tools available. You must run these checks on your local machine:flutter analyze dart format lib/services/fd_monitor_service.dartAdditionally, the code uses
print()statements (lines 95 and 102 ingetCurrentCount()) instead of proper logging. Consider using a logging package (e.g.,logger) or removing debug prints for production code to align with Dart best practices.
| switch call.method { | ||
| case "start": | ||
| let intervalSeconds: TimeInterval | ||
| if let args = call.arguments as? [String: Any], | ||
| let interval = args["intervalSeconds"] as? Double { | ||
| intervalSeconds = interval | ||
| } else { | ||
| intervalSeconds = 60.0 | ||
| } | ||
| FdMonitor.shared.start(intervalSeconds: intervalSeconds) | ||
| result(["success": true, "message": "FD Monitor started with interval: \(intervalSeconds)s"]) | ||
|
|
There was a problem hiding this comment.
Validate and robustly parse intervalSeconds to avoid invalid timer schedules
Currently accepts only Double and doesn’t guard against non‑positive values. A zero/negative repeat interval can lead to undefined behavior.
case "start":
- let intervalSeconds: TimeInterval
- if let args = call.arguments as? [String: Any],
- let interval = args["intervalSeconds"] as? Double {
- intervalSeconds = interval
- } else {
- intervalSeconds = 60.0
- }
- FdMonitor.shared.start(intervalSeconds: intervalSeconds)
- result(["success": true, "message": "FD Monitor started with interval: \(intervalSeconds)s"])
+ let intervalSeconds: TimeInterval
+ if let args = call.arguments as? [String: Any] {
+ if let n = args["intervalSeconds"] as? NSNumber {
+ intervalSeconds = n.doubleValue
+ } else if let s = args["intervalSeconds"] as? String, let v = Double(s) {
+ intervalSeconds = v
+ } else {
+ intervalSeconds = 60.0
+ }
+ } else {
+ intervalSeconds = 60.0
+ }
+ guard intervalSeconds > 0 else {
+ result(FlutterError(code: "invalid_args",
+ message: "intervalSeconds must be > 0",
+ details: nil))
+ return
+ }
+ FdMonitor.shared.start(intervalSeconds: intervalSeconds)
+ result(["success": true, "message": "FD Monitor started with interval: \(intervalSeconds)s"])📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch call.method { | |
| case "start": | |
| let intervalSeconds: TimeInterval | |
| if let args = call.arguments as? [String: Any], | |
| let interval = args["intervalSeconds"] as? Double { | |
| intervalSeconds = interval | |
| } else { | |
| intervalSeconds = 60.0 | |
| } | |
| FdMonitor.shared.start(intervalSeconds: intervalSeconds) | |
| result(["success": true, "message": "FD Monitor started with interval: \(intervalSeconds)s"]) | |
| switch call.method { | |
| case "start": | |
| let intervalSeconds: TimeInterval | |
| if let args = call.arguments as? [String: Any] { | |
| if let n = args["intervalSeconds"] as? NSNumber { | |
| intervalSeconds = n.doubleValue | |
| } else if let s = args["intervalSeconds"] as? String, let v = Double(s) { | |
| intervalSeconds = v | |
| } else { | |
| intervalSeconds = 60.0 | |
| } | |
| } else { | |
| intervalSeconds = 60.0 | |
| } | |
| guard intervalSeconds > 0 else { | |
| result(FlutterError(code: "invalid_args", | |
| message: "intervalSeconds must be > 0", | |
| details: nil)) | |
| return | |
| } | |
| FdMonitor.shared.start(intervalSeconds: intervalSeconds) | |
| result(["success": true, "message": "FD Monitor started with interval: \(intervalSeconds)s"]) |
| Future<Map<String, dynamic>> start({double intervalSeconds = 60.0}) async { | ||
| try { | ||
| final result = await _channel.invokeMethod<Map<Object?, Object?>>( | ||
| 'start', | ||
| {'intervalSeconds': intervalSeconds}, | ||
| ); | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Validate intervalSeconds before invoking native
Prevent invalid values from reaching iOS and return a structured error early.
Future<Map<String, dynamic>> start({double intervalSeconds = 60.0}) async {
- try {
+ try {
+ if (intervalSeconds <= 0) {
+ return {
+ 'success': false,
+ 'message': 'intervalSeconds must be > 0',
+ };
+ }
final result = await _channel.invokeMethod<Map<Object?, Object?>>(
'start',
{'intervalSeconds': intervalSeconds},
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Future<Map<String, dynamic>> start({double intervalSeconds = 60.0}) async { | |
| try { | |
| final result = await _channel.invokeMethod<Map<Object?, Object?>>( | |
| 'start', | |
| {'intervalSeconds': intervalSeconds}, | |
| ); | |
| Future<Map<String, dynamic>> start({double intervalSeconds = 60.0}) async { | |
| try { | |
| if (intervalSeconds <= 0) { | |
| return { | |
| 'success': false, | |
| 'message': 'intervalSeconds must be > 0', | |
| }; | |
| } | |
| final result = await _channel.invokeMethod<Map<Object?, Object?>>( | |
| 'start', | |
| {'intervalSeconds': intervalSeconds}, | |
| ); |
🤖 Prompt for AI Agents
lib/services/fd_monitor_service.dart around lines 22 to 28: validate
intervalSeconds before calling the platform channel by checking it is a finite
number > 0 (reject NaN, infinite, zero or negative). If invalid, do not call
_channel.invokeMethod; instead return a structured error Map (e.g. {'success':
false, 'error': 'invalid_interval', 'message': 'intervalSeconds must be a
positive finite number'}) so callers get a clear, typed failure early. Ensure
normal flow still calls the channel when validation passes.
| if (result != null) { | ||
| _isMonitoring = true; | ||
| return Map<String, dynamic>.from(result); | ||
| } | ||
|
|
There was a problem hiding this comment.
_isMonitoring toggles true even when native start fails
Set it only on success to avoid inconsistent UI/state.
- if (result != null) {
- _isMonitoring = true;
- return Map<String, dynamic>.from(result);
- }
+ if (result != null) {
+ final map = Map<String, dynamic>.from(result);
+ _isMonitoring = map['success'] == true;
+ return map;
+ }🤖 Prompt for AI Agents
In lib/services/fd_monitor_service.dart around lines 29 to 33, _isMonitoring is
being toggled true too early; ensure it is only set after the native start call
has definitively succeeded by first verifying result is non-null and (if
available) a success flag in the returned Map, then convert the result and set
_isMonitoring = true immediately after that verification before returning the
Map; do not set _isMonitoring on null or on error paths.
| if (result != null) { | ||
| _isMonitoring = false; | ||
| return Map<String, dynamic>.from(result); | ||
| } |
There was a problem hiding this comment.
Likewise for stop(): only flip state on success
Avoid reporting monitoring stopped when native stop failed.
- if (result != null) {
- _isMonitoring = false;
- return Map<String, dynamic>.from(result);
- }
+ if (result != null) {
+ final map = Map<String, dynamic>.from(result);
+ if (map['success'] == true) {
+ _isMonitoring = false;
+ }
+ return map;
+ }🤖 Prompt for AI Agents
In lib/services/fd_monitor_service.dart around lines 59–62, the code flips
_isMonitoring to false whenever a non-null result is returned, which can
incorrectly mark monitoring stopped if the native stop call failed; change the
logic so you only set _isMonitoring = false after verifying the native result
indicates a successful stop (e.g., result != null and a success flag or status
field is true/“stopped”); if the native call returns null or a failure status,
do not change _isMonitoring and instead propagate or return the error/failed
result.
There was a problem hiding this comment.
Pull Request Overview
This PR addresses issue #3088 by ensuring the application always navigates to the Wallet page after login or logout operations, regardless of which page the user was on previously. This improves consistency and user experience across the application.
Key Changes:
- Added automatic navigation to Wallet page after successful login
- Added automatic navigation to Wallet page after logout
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Closes #3088
Though issue relates only to mobile, fix has been applied globally for consistency and UX.
To test:
Summary by CodeRabbit
Release Notes