diff --git a/.gitignore b/.gitignore index 4882a49..c2281e1 100644 --- a/.gitignore +++ b/.gitignore @@ -68,4 +68,4 @@ lib/logic/backup/ 00-FULL-REPORT.md server full picture.txt technical_debt_1_answer.md -tehcnical_debt_1.md \ No newline at end of file +technical_debt_1.md \ No newline at end of file diff --git a/technical_debt_1.md b/technical_debt_1.md deleted file mode 100644 index f59f139..0000000 --- a/technical_debt_1.md +++ /dev/null @@ -1,614 +0,0 @@ -# PaladinVPN β€” Technical Audit & UX Bug Report - -**Date:** 2025-07-15 -**Scope:** Full codebase β€” Flutter client (`lib/`), Android native layer (`android/`, `local_packages/wireguard_flutter/`), Python Hivemind server (`server_hivemind_5_0.py`) - ---- - -## πŸ”΄ Critical Blockers -*Things that will crash the app, break the VPN, or get the app rejected by Google Play.* - ---- - -### 1. Conflicting `build.gradle.kts` β€” Dual `applicationId` Declaration - -**Files:** `android/build.gradle.kts` (root) vs `android/app/build.gradle.kts` (app-level) - -The **root** `build.gradle.kts` incorrectly applies `com.android.application` and declares: -- `namespace = "com.esefcloud.vpn"` -- `applicationId = "com.esefcloud.vpn"` - -The **app-level** `build.gradle.kts` correctly declares: -- `namespace = "com.paladinvpn.app"` -- `applicationId = "com.paladinvpn.app"` - -The root project should NOT apply `com.android.application`. This causes merge conflicts, duplicate `AndroidManifest` merging, and can unpredictably override the `applicationId`. If the package name doesn't match your Play Console listing, Google Play **will reject** the upload. AdMob is also tied to the application ID. - -**Fix:** Remove the `com.android.application` plugin and the `android {}` block from the root `build.gradle.kts`. Those belong only in `android/app/build.gradle.kts`. - ---- - -### 2. Missing Foreground Service Type Declaration (Android 14+) β€” **REJECTION RISK** - -**Files:** `android/app/src/main/AndroidManifest.xml`, `local_packages/wireguard_flutter/android/src/main/AndroidManifest.xml` - -Your manifest declares: -```xml - - -``` - -But there is **no `` element** declaring `android:foregroundServiceType`. Android 14 (API 34) **requires** every foreground service to declare a type (e.g., `dataSync`, `connectedDevice`, `specialUse`). The AmneziaWG library starts a VPN foreground service internally β€” without the type declaration in the merged manifest, the service **will crash on launch** on Android 14+ devices. - -Additionally, `FOREGROUND_SERVICE_SPECIAL_USE` triggers a Google Play review requirement: you must submit a written justification. VPN apps typically don't need this permission β€” the `VpnService` API includes its own persistent notification mechanism that is exempt from the foreground service type requirement. Consider removing `FOREGROUND_SERVICE_SPECIAL_USE` entirely. - -The wireguard_flutter plugin's manifest at `local_packages/wireguard_flutter/android/src/main/AndroidManifest.xml` is **completely empty** β€” it must include the VpnService declaration or ensure the AmneziaWG AAR ships its own. - -**Fix:** Verify whether the AmneziaWG AAR declares the VpnService in its own manifest. If not, add it to the plugin manifest with the proper `foregroundServiceType`. - ---- - -### 3. AdMob SSV Completely Bypassed β€” Both Client and Server - -**Files:** `lib/logic/hivemind_service.dart` (lines 21–29), `server_hivemind_5_0.py` (lines 424–438) - -**Client-side** β€” `hivemind_service.dart` fires a **fake AdMob ping** on every connection: -```dart -// ── TEMPORARY AD BYPASS HACK FOR TESTING ── -final fakeAdmobPingUrl = Uri.parse( - '${AppConfig.hivemindApiBase}/admob/callback?signature=test&key_id=test&custom_data=...'); -await http.get(fakeAdmobPingUrl).timeout(const Duration(seconds: 8)); -``` -The error is swallowed (`catch (_) {}`), so even if the server is unreachable the app proceeds to poll. - -**Server-side** β€” `server_hivemind_5_0.py` has the **entire** ECDSA signature verification block commented out: -```python -# ⚠️ ADMOB BYPASS (TEMPORARY FOR APP REVIEW) -# DELETE THIS ENTIRE BLOCK WHEN YOU WANT TO ENABLE REAL ADS -``` -The `/api/admob/callback` endpoint processes **all** requests unconditionally. - -**Impact:** Anyone can `curl https://paladinvpn.duckdns.org/api/admob/callback?custom_data=...` and create unlimited 60-minute/2GB sessions without watching an ad. Google Play reviewers or automated testing will detect that the ad flow is non-functional. This is grounds for **immediate rejection** under the "Deceptive Behavior" policy. - -**Fix:** Remove both bypasses. Enable real SSV signature verification on the server. Remove the fake ping from `hivemind_service.dart` β€” the actual connection flow must wait for Google's server-to-server callback, not a client-side fake. - ---- - -### 4. `app_config.dart` Contains Production Secrets β€” Verify Git Tracking - -**File:** `lib/logic/app_config.dart` - -The `.gitignore` lists `lib/logic/app_config.dart`, and an example template exists at `app_config.example.dart`. However, the **real file exists in the working tree** with: -- Production server IP: `204.168.246.88` -- Production AdMob unit ID: `ca-app-pub-5671224884648691/9198987725` -- Production AdMob app ID: `ca-app-pub-5671224884648691~1007152454` (in AndroidManifest) -- Production DuckDNS domain: `paladinvpn.duckdns.org` - -**Run immediately:** -```powershell -git ls-files lib/logic/app_config.dart -``` -If it returns the file, it IS tracked β€” meaning these secrets are in your git history forever. You would need to: -1. Rotate the AdMob IDs (create new ad units) -2. Change the server IP or accept it's public -3. Use `git filter-branch` or `BFG Repo-Cleaner` to scrub history - ---- - -### 5. `/api/session/start` Endpoint Does Not Exist β€” Bonus Ads Are Broken - -**Files:** `lib/logic/session_timer.dart` β†’ `addBonusTime()`, `server_hivemind_5_0.py` - -The `addBonusTime()` method in SessionTimer does an HTTP POST to: -``` -POST ${AppConfig.hivemindApiBase}/session/start -``` - -The server has **no such route**. The server only has: -- `GET /api/session/status` β€” status polling -- `POST /api/session/stop` β€” session cleanup -- `GET /api/admob/callback` β€” SSV (bypassed) - -The intended flow is: user watches ad β†’ Google calls your server via SSV β†’ server extends the session. But since SSV is bypassed AND the endpoint is missing, **bonus ads do absolutely nothing**. The user watches a 30-second ad, the `showAd()` returns `true`, the snackbar says "+30 minutes added!", but the server session is never extended. After the current session expires, the user is throttled/disconnected regardless. - -**Fix:** Either add the `/api/session/start` endpoint to the server, or β€” the correct approach β€” enable real SSV and have the server extend sessions inside the `/api/admob/callback` handler (which it already does for both `main_ad` and `bonus_ad` types). Then `addBonusTime()` only needs to re-sync, not create a session. - ---- - -### 6. Server Public Key Fallback Is a Hardcoded Invalid String - -**File:** `server_hivemind_5_0.py` (~line 480) - -When the server can't fetch the WireGuard public key from `awg0`: -```python -SERVER_PUBKEY = SERVER_PUBKEY_FALLBACK or "server key=" -``` - -The string `"server key="` is not a valid base64 WireGuard key. If this fallback is ever hit, every client connection will fail with an invalid config. The `SERVER_PUBKEY_FALLBACK` is `None` by default, so the fallback-to-fallback string is always used. Either set a real fallback key or fail gracefully with a clear error log. - ---- - -## 🟑 Warnings β€” Architecture & Reliability -*Race conditions, edge cases, and architecture flaws that could cause a bad user experience.* - ---- - -### 7. Race Condition: Optimistic Client State Before Server Confirmation - -**File:** `lib/logic/session_timer.dart` β†’ `start()` - -```dart -_remainingSeconds = 3600; // optimistic start -_quotaBytes = 2 * 1024 * 1024 * 1024; -``` - -The UI immediately renders "01:00:00" and "2.00 GB" before the first server sync completes (~1–3 seconds). If the server allocated a different duration (e.g., 45 minutes from a previous session that wasn't properly cleaned), the UI flashes wrong data then jumps to the correct value. Users perceive this as a glitch. - -**Fix:** Set initial values to 0 or show a loading state until the first sync returns. Display "Calculating…" or a shimmer placeholder until real data arrives. - ---- - -### 8. Race Condition: In-Flight HTTP Callback During Disconnect - -**File:** `lib/logic/session_timer.dart` - -`_syncWithHivemind()` fires on a `Timer.periodic` every 1 second (sync every 3rd tick). When the user disconnects: -1. `vpn.disconnect()` is called -2. `timer.stop()` cancels the timer -3. But an in-flight `_syncWithHivemind()` HTTP request may still be pending -4. When it completes, it sees `!active` β†’ calls `stop()` and `vpnConnection.disconnect()` again - -The double-disconnect is mostly harmless since `disconnect()` guards against re-entry, but it could cause the cleanup HTTP call (`disconnectAndCleanup()`) to fire twice, and the notification to be cancelled/restored in a flicker. - -**Fix:** Add an `_isDisconnecting` flag checked at the top of `_syncWithHivemind()`. - ---- - -### 9. No Graceful Recovery From Temporary Network Drops - -**File:** `lib/logic/session_timer.dart` β†’ `_tick()` and `_syncWithHivemind()` - -The local `_tick` decrements `_remainingSeconds` **every second regardless of sync status**. If the server is unreachable for 60 seconds: -- The UI countdown hits `00:00:00` -- The user sees "expired" -- But the server session is still valid (it has its own expiry clock) -- When the network returns, the next sync jumps the timer back up β€” confusing the user - -The catch block silently logs and does nothing: -```dart -} catch (e) { - debugPrint('Hivemind sync error: $e'); - // If we can't reach the server, just let the local timer run -} -``` - -This is **not** "just letting the local timer run" β€” the local timer is actively decrementing, creating a growing drift from reality. - -**Fix:** Track consecutive sync failures. After 3 failures, pause the local countdown and show "Reconnecting…" in the UI. Resume when sync succeeds. - ---- - -### 10. Permission Dialog + Immediate Failure = Broken UX - -**File:** `local_packages/wireguard_flutter/.../WireguardFlutterPlugin.kt` β†’ `connect()` - -```kotlin -if (!havePermission) { - checkPermission() // launches system VPN permission dialog - throw Exception("Permissions are not given") // immediately fails -} -``` - -`checkPermission()` fires an intent that shows the Android VPN permission dialog. But the code **immediately throws an exception** β€” before the user even sees the dialog. The Flutter side receives the error, sets status to `VpnStatus.error` with "Tunnel error", and the user sees a failure message. Meanwhile, the system permission dialog is still on screen. The user grants permission, returns to the app, sees "Tunnel error", and must tap Connect again. - -**Fix:** Make `connect()` wait for the permission result (use `startActivityForResult` with a callback/completer pattern) instead of throwing. - ---- - -### 11. 3-Second Polling Per Client β€” Server Scalability Issue - -**File:** `lib/logic/session_timer.dart` β†’ `_syncWithHivemind()` - -Every connected client hits your Flask server every 3 seconds. With 100 concurrent users, that's ~33 requests/second. Flask's development server is single-threaded β€” it will queue requests and time out. Even with gunicorn, this is expensive. - -**Fix:** Increase the poll interval to 5–10 seconds. Better: use Server-Sent Events (SSE) or a lightweight WebSocket so the server pushes updates rather than the client polling. - ---- - -### 12. `isVpnChecked` Flag β€” Single-Use, State Desync on App Restoration - -**File:** `local_packages/wireguard_flutter/.../WireguardFlutterPlugin.kt` - -The `isVpnChecked` flag is set only in the `"start"` method and reset only in `onDetachedFromEngine`. If the Flutter engine is killed and recreated (process death), the flag is `false` but the VPN may still be running. On restart, `VpnConnection._init()` calls `_wireguard.stage()` to detect the running tunnel β€” this path works, but the native plugin's internal state tracking is fragile. - ---- - -### 13. Server: `management_loop` Blocks on Subprocess Calls - -**File:** `server_hivemind_5_0.py` β†’ `management_loop()` - -The loop calls `get_wireguard_stats()` every 60 seconds, which spawns two Docker exec subprocesses: -```python -transfer_out = run_cmd("awg show awg0 transfer") -allowed_out = run_cmd("awg show awg0 allowed-ips") -``` - -Each `run_cmd` calls `subprocess.run()` synchronously. If Docker is slow or the system is under load, these calls can take seconds. The management loop is single-threaded β€” during this time, expired sessions are not cleaned up and the Flask server (if running single-process) may be blocked on the GIL. - -**Fix:** Use `subprocess.Popen` with a timeout, or cache stats more aggressively. - ---- - -### 14. Server: `tc filter` Cleanup Is Never Done on Expiry β€” Possible tc Rule Leak - -**File:** `server_hivemind_5_0.py` β†’ `management_loop()` - -When a session expires in the management loop, the code does: -```python -expired_devices.append(device_id) -``` -Then later: -```python -for device_id in expired_devices: - del active_sessions[device_id] -``` - -But the associated `tc` throttle rules are **never removed** on expiry. The `remove_throttle()` + `remove_wg_peer()` + `release_ip()` calls only happen in `session_stop()` (explicit disconnect from the app). If the app crashes or the user force-stops, expired sessions leave orphaned tc filter/class rules. Over weeks, the tc qdisc accumulates hundreds of dead rules, degrading network performance. - -**Fix:** In the expiry cleanup block, call `remove_throttle()`, `remove_wg_peer()`, and `release_ip()` for each expired device. - ---- - -### 15. No ProGuard/R8 Rules for AmneziaWG Native Library - -**File:** `android/app/build.gradle.kts` - -There is no `minifyEnabled`, no ProGuard configuration, and no keep rules for the `org.amnezia.awg` native library. Release builds may strip JNI bindings, causing `UnsatisfiedLinkError` crashes. - -**Fix:** Add a `proguard-rules.pro` with keep rules for `org.amnezia.awg.backend.*` and `org.amnezia.awg.crypto.*`, and enable minification in `buildTypes { release { ... } }`. - ---- - -## 🟒 Polish & Best Practices -*Minor code cleanliness, structural improvements, and maintenance issues.* - ---- - -### 16. `build.gradle.kts` Root Duplicates `flutter {}` and Kotlin Config - -The root `build.gradle.kts` redundantly declares `flutter { source = "../.." }` and `kotlin { compilerOptions { ... } }` which are already in the app-level file. This is dead config that causes confusion. - ---- - -### 17. `AppConfig` Is Instantiable Despite Being Static-Only - -**File:** `lib/logic/app_config.dart` - -```dart -class AppConfig { - static const String serverIp = '...'; - // ... -} -``` - -Nothing prevents `AppConfig()` from being instantiated. Mark it: -```dart -abstract final class AppConfig { - AppConfig._(); - // ... -} -``` - ---- - -### 18. Debug Banner Disabled Unconditionally - -**File:** `lib/main.dart` - -```dart -debugShowCheckedModeBanner: false, -``` - -This disables the debug banner even during development. Consider: -```dart -debugShowCheckedModeBanner: false, // acceptable for this app's dark theme -``` - -Not a bug, but the debug banner is useful during development for catching accidental release-mode testing. - ---- - -### 19. Privacy Policy Links to GitHub Raw `.md` β€” May Render Poorly - -**File:** `lib/components/privacy_policy_button.dart` - -```dart -final Uri url = Uri.parse('https://github.com/esefxdz/PaladinVPN/blob/main/PRIVACY_POLICY.md'); -``` - -Google Play requires your privacy policy to be a properly hosted URL. GitHub raw links are generally accepted, but a `.md` file renders as raw text in a browser. Use GitHub Pages or a simple HTML page. - ---- - -### 20. `url_launcher` on Android 11+ Requires `` β€” Already Done - -Your manifest correctly includes the `` block for `intent.action.VIEW`. No issue here β€” confirming compliance. - ---- - -### 21. Hardcoded "Helsinki, Finland" Location - -**File:** `lib/components/status_text.dart` - -The server location is hardcoded. If you ever add more servers or move the server, the UI lies. Move this to `AppConfig` or fetch it from the server. - ---- - -### 22. Zero Tests - -**Files:** No `test/` directory, no test files anywhere. - -Despite `flutter_test` being in `dev_dependencies`, there are zero unit tests, widget tests, or integration tests. For a VPN app handling: -- Cryptographic key generation and persistence -- Session state machine transitions -- Network timeout and retry logic -- Ad reward flow - -At minimum, unit-test the crypto service, session timer logic, and config parsing. - ---- - -### 23. `Server Full Picture.txt` in `.gitignore` But Present in Working Tree - -The file `server full picture.txt` is in `.gitignore` but exists locally. It contains sensitive server configuration (IP addresses, Docker setup, iptables rules). Verify it's not tracked by git: -```powershell -git ls-files "server full picture.txt" -``` - ---- - -## πŸ‘€ User Experience Bugs -*Things that create an unpleasant, confusing, or glitchy experience for real users.* - ---- - -### UX-1. 5-Second Cooldown Triggers Even on Instant Failure - -**File:** `lib/components/connect_button.dart` β†’ `_handleTap()` - -```dart -setState(() => _inCooldown = true); -Future.delayed(const Duration(seconds: 5), () { - if (mounted) setState(() => _inCooldown = false); -}); -``` - -The cooldown is set **before** attempting the connection. If the connection fails instantly (no network, server unreachable, permission denied), the user sees: -- "PLEASE WAIT" with an hourglass icon -- A greyed-out, unresponsive button -- For **5 full seconds** - -This is infuriating. The user knows the connection failed, but the app locks them out anyway. - -**Fix:** Only apply the cooldown on **successful** connections (to prevent rapid disconnect/reconnect spam). On failure, reset immediately or use a much shorter cooldown (1 second). - ---- - -### UX-2. Countdown Timer Jumps Backward When Sync Arrives Late - -**File:** `lib/logic/session_timer.dart` - -The optimistic start sets `_remainingSeconds = 3600`. The `_tick()` decrements it every second. When the first server sync arrives (1–3 seconds later), the server returns the *real* `expires_in_seconds` β€” which could be anything from 0 to 3600. The timer visibly jumps: - -``` -01:00:00 β†’ 00:59:59 β†’ 00:59:58 β†’ [sync] β†’ 00:45:23 ← user sees a jump -``` - -Even worse if the sync fails for 10 seconds β€” the tick keeps decrementing local time while the server still shows 3600. When sync resumes, the timer jumps UP: - -``` -00:59:50 β†’ [sync] β†’ 01:00:00 ← "Wait, did I just gain time?" -``` - -**Fix:** Don't start the local countdown until the first successful sync. Show "Syncing…" or a blurred placeholder until real data arrives. - ---- - -### UX-3. "TAP TO CONNECT" Is Always Visible β€” Even When Nothing Will Happen - -**File:** `lib/components/connect_button.dart` - -When tapping has no possible effect (no network, ad not loaded, cooldown active), the button still says "TAP TO CONNECT". The only indication that tapping won't work is a subtle color change (grey vs muted blue). Users will tap repeatedly and wonder why nothing happens. - -**Fix:** Show contextual text: -- No network: "NO CONNECTION" -- Ad loading: "LOADING AD…" -- Cooldown: "PLEASE WAIT" (already done) -- Ready: "TAP TO CONNECT" - ---- - -### UX-4. No Feedback When Ad Fails to Load - -**File:** `lib/logic/ad_manager.dart` - -If `_preloadAd()` fails, `_isAdLoaded` stays `false`. The `ConnectButton` doesn't check `AdManager.isAdLoaded` at all β€” it proceeds to call `vpn.connect()` which calls `hivemind_service.fetchConfigWithPolling()`, which fires the fake SSV ping (see Critical #3), which polls. The user sees "Establishing tunnel…" for up to 30 seconds (15 polls Γ— 2 seconds) then gets a cryptic "Session not activated" error. - -**Fix:** Check `adManager.isAdLoaded` before attempting connection. If no ad is ready, show "Ad not ready, please wait…" and trigger a reload. - ---- - -### UX-5. Snackbar Says "+30 minutes added!" Even When Bonus Fails - -**File:** `lib/components/watch_ad_button.dart` - -```dart -final success = await context.read().showAd('bonus_ad'); -if (success && context.mounted) { - context.read().addBonusTime(); - ScaffoldMessenger.of(context).showSnackBar( - SnackBar(content: const Text('+30 minutes added!'), ...), - ); -} -``` - -The snackbar shows **regardless of whether `addBonusTime()` succeeded**. The `addBonusTime()` method silently catches all errors. So the user sees "+30 minutes added!" but the server never got the request (missing endpoint, see Critical #5). The timer stays unchanged. The user thinks the app is broken or lying. - -**Fix:** `addBonusTime()` should return a `bool`. Only show the success snackbar if the API call succeeded. Show an error snackbar otherwise. - ---- - -### UX-6. Intro Splash Screen β€” 2.5 Seconds of Wasted Time, Every Launch - -**File:** `lib/screens/intro.dart` - -```dart -Timer(const Duration(milliseconds: 2500), () { - if (mounted) { - Navigator.of(context).pushReplacement(...); - } -}); -``` - -Every time the user opens the app, they stare at a logo and spinner for 2.5 seconds. This is a placeholder delay, not a loading screen β€” nothing is being loaded. For an app that users open frequently to check their VPN status, this adds up to minutes of wasted time per week. - -**Fix:** Remove the artificial delay. If the app genuinely needs startup time (e.g., checking for a running tunnel), make the intro screen reflect actual loading progress and transition as soon as `_init()` completes. - ---- - -### UX-7. Notification Shows "0.00 KB/s" on Slow Connections - -**File:** `lib/logic/session_timer.dart` β†’ `_syncWithHivemind()` - -```dart -if (_lastUsedBytes > 0 && deltaBytes > 0) { - _currentSpeedKbps = (deltaBytes / 3) / 1024; -} else { - _currentSpeedKbps = 0.0; -} -``` - -The speed calculation uses `_lastUsedBytes > 0` as a guard. On the first sync, `_lastUsedBytes` is 0, so speed shows `0.00 KB/s`. If the user isn't actively transferring data, the delta is 0 and speed shows `0.00 KB/s`. The persistent notification displays "0.00 KB/s" which looks like the VPN is broken. Users associate "0.00 KB/s" with "not working." - -**Fix:** Show "Idle" or "Connected" instead of "0.00 KB/s" when speed is effectively zero. - ---- - -### UX-8. Persistent Notification Is Re-Posted Every 3 Seconds - -**File:** `lib/components/notification.dart` β†’ `showOrUpdateStatus()` - -The notification is updated every 3 seconds (every sync cycle). On some Android skins (Xiaomi, OnePlus, Samsung), frequent notification updates cause the notification to "flicker" in the shade β€” it briefly disappears and reappears. This is distracting and looks buggy. - -**Fix:** Only update the notification when the displayed values actually change (time string differs from last posted, speed changes by >10%). - ---- - -### UX-9. No Visual Indicator That Bonus Ad Loading Failed - -**File:** `lib/components/watch_ad_button.dart`, `lib/logic/ad_manager.dart` - -The `WatchAdButton` always renders β€” even if no ad is loaded. Tapping it calls `showAd('bonus_ad')` which internally calls `_preloadAd()` and waits. If loading fails, `showAd()` returns `false` and… nothing happens. No error message. No snackbar. The user taps "Support +30m", waits a moment, and… nothing. They'll tap again. And again. - -**Fix:** Show the ad button with a loading state ("Loading ad…") when no ad is ready. Show a brief error if loading fails after a tap. - ---- - -### UX-10. Disconnect Clears Timer Instantly β€” No "Tearing Down" Visual - -**File:** `lib/logic/session_timer.dart` β†’ `stop()` - -```dart -void stop() { - _timer?.cancel(); - _timer = null; - _remainingSeconds = 0; - ... -} -``` - -`stop()` sets the timer to zero immediately, but the `VpnConnection` goes through a `disconnecting β†’ disconnected` transition. The ConnectButton circle shows a spinner during disconnection, but the timer text inside it jumps to `00:00:00` prematurely. - -**Fix:** Don't zero the timer in `stop()`. Let `VpnConnection` control the disconnect flow and only zero the timer when the status reaches `disconnected`. - ---- - -### UX-11. Error Messages Are Generic and Unhelpful - -**File:** `lib/logic/vpn_connection.dart` β†’ `connect()` - -```dart -_errorMessage = 'Tunnel failed to start.\nTry reconnecting.'; -``` - -Every failure produces the same error message. The user doesn't know if: -- The server is down -- Their network has no internet -- The ad wasn't watched -- The config was invalid -- Android denied VPN permission - -This makes debugging impossible for users and will generate 1-star reviews saying "doesn't work." - -**Fix:** Surface specific error messages from each failure branch. Map them to user-friendly strings. - ---- - -### UX-12. Connected Timer Text Is Small and Hard to Read at a Glance - -**File:** `lib/components/clock_display.dart` - -The countdown text is 40px inside a 260px circle. For a quick-glance status, many users will pick up their phone and squint at "01:00:00" in white on a dark background. The speed text below it (12px, grey) is nearly invisible in sunlight. - -**Fix:** Consider increasing font weight, adding a subtle background contrast behind the timer, or showing remaining time in the notification as a large-format text for at-a-glance viewing. - ---- - -### UX-13. No Haptic Feedback on Connect/Disconnect - -**File:** `lib/components/connect_button.dart` - -The connect/disconnect action has no haptic feedback. For a button this important, a brief vibration on successful connection gives tactile confirmation. Many premium VPN apps do this. - ---- - -### UX-14. Intro Screen "PALADIN VPN" Text Has No Loading Context - -**File:** `lib/screens/intro.dart` - -The intro shows a logo, "PALADIN VPN", and a spinning circle for 2.5 seconds. It doesn't say "Loading…" or "Preparing…" β€” it's just a spinner. Users may think the app froze. If the startup takes longer (slow device), 2.5 seconds feels like an eternity with no text indicating progress. - ---- - -### UX-15. Status Bar Icon Brightness Is Always Light - -**File:** `lib/main.dart` - -```dart -statusBarIconBrightness: Brightness.light, -``` - -This is correct for the dark theme β€” light icons on a transparent/dark status bar. No bug here. Confirming compliance. - ---- - -### UX-16. App Crashes If WireGuard Plugin Is Not Available - -**File:** `lib/logic/vpn_connection.dart` β†’ `_init()` - -The `try/catch` around initialization catches the error, but `_wireguard` remains half-initialized. If the AmneziaWG native library fails to load (wrong architecture, missing .so files), subsequent calls to `startVpn()` or `stopVpn()` will throw unhandled exceptions that crash the app. - -**Fix:** Set a `_isAvailable` flag after successful init. Guard all VPN operations behind it. Show "VPN service unavailable" in the UI if init failed. - ---- - -## Summary - -| Category | Count | Top Priority Items | -|----------|-------|-------------------| -| πŸ”΄ Critical Blockers | 6 | Conflicting `applicationId`, missing `foregroundServiceType`, SSV bypasses, secrets in git, broken bonus ads, invalid server key fallback | -| 🟑 Warnings | 9 | Optimistic state flash, disconnect race, no network-drop recovery, permission dialog UX, polling scale, tc rule leak, ProGuard missing | -| 🟒 Polish | 9 | Static class, no tests, hardcoded location, privacy policy URL, debug banner, dead gradle config | -| πŸ‘€ UX Bugs | 16 | Cooldown on failure, timer jumps, broken bonus feedback, 2.5s artificial delay, generic errors, notification flicker, 0.00 KB/s display | - -**Before Google Play submission, fix items 1–6 in Critical Blockers. Items UX-1 through UX-7 will directly prevent 1-star reviews.** diff --git a/technical_debt_1_answer.md b/technical_debt_1_answer.md deleted file mode 100644 index 1263c22..0000000 --- a/technical_debt_1_answer.md +++ /dev/null @@ -1,102 +0,0 @@ -# Thoughts and Answers to Technical Audit - -Here are my thoughts and proposed actions for every single point raised in the technical audit report. The agent did a fantastic job pointing out native Android issues and server-side edge cases that are crucial for a Google Play launch. - ---- - -## πŸ”΄ Critical Blockers - -### 1. Conflicting `build.gradle.kts` -**Thoughts:** The agent is 100% correct. Applying `com.android.application` in the root `build.gradle` is an anti-pattern in Flutter and will cause massive headaches during the release build. -**Action:** Remove the `android {}` block and plugin from the root `build.gradle.kts`. - -### 2. Missing Foreground Service Type Declaration -**Thoughts:** Android 14+ is incredibly strict about Foreground Services. However, `VpnService` is a special case in Android that generally provides its own system-level notification. If we are using `FOREGROUND_SERVICE_SPECIAL_USE`, Google Play will definitely flag us for a manual review. -**Action:** We need to inspect `wireguard_flutter` and potentially remove our manual `FOREGROUND_SERVICE` permissions if the AmneziaWG AAR handles it, or correctly classify it to avoid Play Store rejection. - -### 3. AdMob SSV Completely Bypassed -**Thoughts:** We knowingly bypassed this for your rapid testing phase, so this isn't a surprise. But the agent is right: if we ship this to Google Play, anyone can bypass the ads, and Google might reject it for deceptive ad behavior. -**Action:** When you are ready to enable real ads, we must remove the client-side fake ping in `hivemind_service.dart` and uncomment the ECDSA signature verification in the Python server. - -### 4. `app_config.dart` Contains Production Secrets -**Thoughts:** AdMob App IDs are public anyway (they are embedded in the APK), but tracking IP addresses and configurations in Git is bad practice. -**Action:** We should ensure `app_config.dart` is untracked (`git rm --cached`) so future changes don't pollute the repository. - -### 5. `/api/session/start` Endpoint Does Not Exist -**Thoughts:** Excellent catch by the agent! Our `addBonusTime()` method in Flutter posts to a `/session/start` endpoint that the Python server doesn't even have. -**Action:** We need to refactor `addBonusTime()`. It shouldn't try to start a session. Instead, watching the ad should trigger the real SSV callback on the server (which already correctly handles `bonus_ad` to extend time), and the client should simply re-sync the timer. - -### 6. Server Public Key Fallback Is a Hardcoded Invalid String -**Thoughts:** Passing `"server key="` when `awg0` fails to read the pubkey will guarantee the Flutter client crashes with an invalid WireGuard config. -**Action:** We should make the server throw a proper HTTP 500 error instead of silently passing garbage to the client. - ---- - -## 🟑 Warnings β€” Architecture & Reliability - -### 7. Optimistic Client State Before Server Confirmation -**Thoughts:** Showing 01:00:00 before the server confirms the real time causes a glitchy UI jump. -**Action:** We will add a "Calculating..." or "Syncing..." state in the UI until the first `_syncWithHivemind` succeeds. - -### 8. Race Condition: In-Flight HTTP Callback During Disconnect -**Thoughts:** If the 3-second sync finishes right after the user hits disconnect, it triggers a double-disconnect. -**Action:** Easy fix. Add a `bool _isDisconnecting` guard. - -### 9. No Graceful Recovery From Temporary Network Drops -**Thoughts:** Very important for mobile. If the user drives through a tunnel and loses 4G, the local timer shouldn't blindly tick down to 0 while the server sync fails. -**Action:** Pause the local timer decrement if the HTTP sync throws an error, and resume when it reconnects. - -### 10. Permission Dialog + Immediate Failure = Broken UX -**Thoughts:** Throwing an exception in Kotlin while the Android permission dialog is still on screen is a bad bug in `wireguard_flutter`. -**Action:** We need to edit `WireguardFlutterPlugin.kt` to await the `onActivityResult` before returning to Flutter. - -### 11. 3-Second Polling Per Client β€” Server Scalability Issue -**Thoughts:** Polling every 3 seconds is ~33 req/sec for 100 users. Flask might struggle with this under heavy load. -**Action:** We can increase this to 10 seconds. The local Dart timer is accurate enough to cover the gap. - -### 12. `isVpnChecked` Flag β€” Single-Use -**Thoughts:** We actually solved most of the state restoration in our previous chat, but the native Kotlin plugin state tracking is still a bit messy. -**Action:** Minor cleanup in the Kotlin plugin. - -### 13. Server: `management_loop` Blocks on Subprocess Calls -**Thoughts:** `subprocess.run("awg show ...")` is synchronous and blocks the Python GIL. -**Action:** We can optimize the server to run this in a thread or use a timeout. - -### 14. Server: `tc filter` Cleanup Is Never Done on Expiry -**Thoughts:** This is a **massive server bug**. If a user's session expires naturally, we delete them from memory but leave their bandwidth throttling rules active in the Linux kernel forever. Over time, the server will choke on dead rules. -**Action:** We must call `remove_throttle()` and `remove_wg_peer()` in the expiry loop inside `server_hivemind_5_0.py`. - -### 15. No ProGuard/R8 Rules for AmneziaWG Native Library -**Thoughts:** Standard Android release builds strip unused code. If it strips AmneziaWG, the release APK will crash on launch. -**Action:** Add ProGuard rules for `org.amnezia.awg.**`. - ---- - -## 🟒 Polish & Best Practices - -**16-23.** The agent's points here are all solid minor improvements. We should host the Privacy Policy on GitHub Pages instead of a raw `.md` file, make `AppConfig` private, and untrack `server full picture.txt`. - ---- - -## πŸ‘€ User Experience Bugs - -The agent highlighted 16 UX bugs. Here are the most impactful ones that we should fix: - -* **UX-1 (5-Second Cooldown on Failure):** We shouldn't lock the connect button for 5 seconds if the connection fails instantly. -* **UX-3 ("TAP TO CONNECT" Always Visible):** We should change the text dynamically (e.g., "NO NETWORK"). -* **UX-4 & UX-9 (No Ad Load Feedback):** We need to prevent the user from connecting if the ad isn't loaded yet, and show them a "Loading ad..." state. -* **UX-5 (Fake "+30 mins" Snackbar):** We must only show this snackbar if the server actually extended the time. -* **UX-6 (2.5s Splash Delay):** The artificial `Future.delayed` in `intro.dart` should be removed so the app opens instantly. -* **UX-7 (Notification "0.00 KB/s"):** We should show "Idle" instead of "0.00 KB/s" to look more professional. -* **UX-10 (Timer clears instantly on Disconnect):** We should let the timer stay visible until the disconnection finishes. -* **UX-13 (No Haptic Feedback):** Adding `HapticFeedback.lightImpact()` on the connect button will make the app feel much more premium. - ---- - -### Conclusion - -The agent provided an incredibly valuable roadmap. -I recommend we tackle these in batches: -1. **Server Fixes:** Fix the `tc filter` memory leak (Critical) and the AdMob SSV / bonus time endpoints. -2. **Android Native Fixes:** Fix the `build.gradle.kts` conflict, ProGuard rules, and the Kotlin permission dialog bug. -3. **Flutter UX Polish:** Fix the fake snackbars, loading states, and haptic feedback.