Merge pull request #604 from synonymdev/fix/all-warnings

fix: compiler warnings and code patterns

Author: ovitrif

.github/img/detekt.png

@@ -19,4 +19,4 @@ google-services.json
 *.keystore
 !debug.keystore
 keystore.*
-!*.template
+!keystore.properties.template

.gitignore

@@ -162,11 +162,12 @@ suspend fun getData(): Result<Data> = withContext(Dispatchers.IO) {
 - ALWAYS run `./gradlew detekt` after code changes to check for new lint issues and fix accordingly
 - ALWAYS ask clarifying questions to ensure an optimal plan when encountering functional or technical uncertainties in requests
 - ALWAYS when fixing lint or test failures prefer to do the minimal amount of changes to fix the issues
-- USE single-line commit messages under 50 chars; use template format: `feat: add something new`
+- USE single-line commit messages under 50 chars; use conventional commit messages template format: `feat: add something new`
 - USE `git diff HEAD sourceFilePath` to diff an uncommitted file against the last commit
+- ALWAYS run `git status` to check ALL uncommitted changes after completing any code edits, then provide exactly 3 commit message suggestions covering the ENTIRE uncommitted diff
 - ALWAYS check existing code patterns before implementing new features
 - USE existing extensions and utilities rather than creating new ones
-- ALWAYS consider applying YAGNI (You Aren't Gonna Need It) principle for new code
+- ALWAYS consider applying YAGNI (You Ain't Gonna Need It) principle for new code
 - ALWAYS reuse existing constants
 - ALWAYS ensure a method exist before calling it
 - ALWAYS remove unused code after refactors
@@ -175,14 +176,17 @@ suspend fun getData(): Result<Data> = withContext(Dispatchers.IO) {
 - ALWAYS acknowledge datastore async operations run synchronously in a suspend context
 - NEVER use `runBlocking` in suspend functions
 - ALWAYS pass the TAG as context to `Logger` calls, e.g. `Logger.debug("message", context = TAG)`
+- ALWAYS log errors at the final handling layer where the error is acted upon, not in intermediate layers that just propagate it
 - ALWAYS use the Result API instead of try-catch
 - NEVER wrap methods returning `Result<T>` in try-catch
 - PREFER to use `it` instead of explicit named parameters in lambdas e.g. `fn().onSuccess { log(it) }.onFailure { log(it) }`
 - NEVER inject ViewModels as dependencies - Only android activities and composable functions can use viewmodels
 - NEVER hardcode strings and always preserve string resources
 - ALWAYS localize in ViewModels using injected `@ApplicationContext`, e.g. `context.getString()`
 - ALWAYS use `remember` for expensive Compose computations
-- ALWAYS add modifiers to the last place in the argument list when calling `@Composable` functions
+- ALWAYS add modifiers to the last place in the argument list when calling composable functions
+- NEVER add parameters with default values BEFORE the `modifier` parameter in composable functions - modifier must be the FIRST optional parameter
+- ALWAYS prefer `VerticalSpacer`, `HorizontalSpacer`, `FillHeight` and `FillWidth` over `Spacer` when applicable
 - PREFER declaring small dependant classes, constants, interfaces or top-level functions in the same file with the core class where these are used
 - ALWAYS create data classes for state AFTER viewModel class in same file
 - ALWAYS return early where applicable, PREFER guard-like `if` conditions like `if (condition) return`
@@ -195,22 +199,23 @@ suspend fun getData(): Result<Data> = withContext(Dispatchers.IO) {
 - ALWAYS be mindful of thread safety when working with mutable lists & state
 - ALWAYS split screen composables into parent accepting viewmodel + inner private child accepting state and callbacks `Content()`
 - ALWAYS name lambda parameters in a composable function using present tense, NEVER use past tense
-- ALWAYS list 3 suggested commit messages after implementation work for the entire set of uncommitted changes
 - NEVER use `wheneverBlocking` in unit test expression body functions wrapped in a `= test {}` lambda
-- ALWAYS wrap unit tests `setUp` methods mocking suspending calls with `runBlocking`, e.g `setUp() = runBlocking { }`
+- ALWAYS wrap unit tests `setUp` methods mocking suspending calls with `runBlocking`, e.g `setUp() = runBlocking {}`
 - ALWAYS add business logic to Repository layer via methods returning `Result<T>` and use it in ViewModels
-- ALWAYS use services to wrap RUST code exposed via bindings
 - ALWAYS order upstream architectural data flow this way: `UI -> ViewModel -> Repository -> RUST` and vice-versa for downstream
 - ALWAYS add new localizable string string resources in alphabetical order in `strings.xml`
 - NEVER add string resources for strings used only in dev settings screens and previews and never localize acronyms
 - ALWAYS use template in `.github/pull_request_template.md` for PR descriptions
 - ALWAYS wrap `ULong` numbers with `USat` in arithmetic operations, to guard against overflows
-- PREFER to use one-liners with `run { }` when applicable, e.g. `override fun someCall(value: String) = run { this.value = value }`
+- PREFER to use one-liners with `run {}` when applicable, e.g. `override fun someCall(value: String) = run { this.value = value }`
 - ALWAYS add imports instead of inline fully-qualified names
+- PREFER to place `@Suppress()` annotations at the narrowest possible scope
 
 ### Architecture Guidelines
 
 - Use `LightningNodeService` to manage background notifications while the node is running
 - Use `LightningService` to wrap node's RUST APIs and manage the inner lifecycle of the node
 - Use `LightningRepo` to defining the business logic for the node operations, usually delegating to `LightningService`
 - Use `WakeNodeWorker` to manage the handling of remote notifications received via cloud messages
+- Use `*Services` to wrap rust library code exposed via bindings
+- Use CQRS pattern of Command + Handler like it's done in the `NotifyPaymentReceived` + `NotifyPaymentReceivedHandler` setup 

AGENTS.md

@@ -17,9 +17,9 @@ This repository contains a **new native Android app** which is **not ready for p
 
 #### 1. Firebase Configuration
 
-Download `google-services.json` from the Firebase Console for each build flavor:
-- **Dev/Testnet**: Place in `app/` (default location)
-- **Mainnet**: Place in `app/src/mainnet/google-services.json`
+Download `google-services.json` from the Firebase Console for each of the following build flavor groups,:
+- dev/tnet/mainnetDebug: Place in `app/google-services.json`
+- mainnetRelease: Place in `app/src/mainnetRelease/google-services.json`
 
 > **Note**: Each flavor requires its own Firebase project configuration. The mainnet flavor will fail to build without its dedicated `google-services.json` file.
 
@@ -43,23 +43,24 @@ See also:
 - [bitkit-core android bindings](https://github.com/synonymdev/bitkit-core/tree/master/bindings/android#installation)
 - [vss-rust-client-ffi android bindings](https://github.com/synonymdev/vss-rust-client-ffi/tree/master/bindings/android#installation)
 
-### Related Repositories
+### References
 
-- [bitkit-ios](https://github.com/synonymdev/bitkit-ios) - Native iOS Bitkit app
-- [bitkit-core](https://github.com/synonymdev/bitkit-core) - Shared Core Rust library with FFI bindings
-- [ldk-node](https://github.com/synonymdev/ldk-node) - Fork of ldk-node
-- [vss-server](https://github.com/synonymdev/vss-server) - Versioned Storage Service backend
-- [vss-rust-client-ffi](https://github.com/synonymdev/vss-rust-client-ffi) - FFI bindings for vss-rust-client 
-- [bitkit-e2e-tests](https://github.com/synonymdev/bitkit-e2e-tests) - End-to-end tests (WebdriverIO + Appium)
-- [bitkit-docker](https://github.com/synonymdev/bitkit-docker) - Docker setup for LNURL dev testing and local backend for integrations development
+- For LNURL dev testing see [bitkit-docker](https://github.com/synonymdev/bitkit-docker)
 
 ### Lint
 
 This project uses detekt with default ktlint and compose-rules for android code linting.
 
-Recommended Android Studio plugins:
-- EditorConfig
-- Detekt
+### IDE Plugins
+The following IDE plugins are recommended for development with Android Studio or IntelliJ IDEA:
+- [Compose Color Preview](https://plugins.jetbrains.com/plugin/21298-compose-color-preview)
+- [Compose Stability Analyzer](https://plugins.jetbrains.com/plugin/28767-compose-stability-analyzer)
+- [detekt](https://plugins.jetbrains.com/plugin/10761-detekt)
+  <details>
+  <summary>See screenshot on how to setup the Detekt plugin after installation.</summary>
+  
+  ![Detekt plugin setup][img_detekt]
+  </details>
 
 **Commands** 
 ```sh
@@ -112,16 +113,31 @@ The build config supports building 3 different apps for the 3 bitcoin networks (
 - `mainnet` flavour = mainnet
 - `tnet` flavour = testnet
 
-### Build for Mainnet
+### Build for Internal Testing
 
-To build the mainnet flavor:
+**Prerequisites**  
+Setup the signing config:
+- Add the keystore file to root dir (i.e. `internal.keystore`)
+- Setup `keystore.properties` file in root dir (`cp keystore.properties.template keystore.properties`)
+
+**Routine**
 
+Increment `versionCode` and `versionName` in `app/build.gradle.kts`, then run:
 ```sh
-./gradlew assembleMainnetDebug   # debug build
-./gradlew assembleMainnetRelease # release build (requires signing config)
+./gradlew assembleDevRelease
+# ./gradlew assembleRelease # for all flavors
 ```
 
-> **Important**: Ensure `app/src/mainnet/google-services.json` exists before building. See [Firebase Configuration](#1-firebase-configuration).
+APK is generated in `app/build/outputs/apk/_flavor_/release`. (`_flavor_` can be any of 'dev', 'mainnet', 'tnet').
+Example for dev: `app/build/outputs/apk/dev/release`
+
+### Build for Release
+
+To build the mainnet flavor for release run:
+
+```sh
+./gradlew assembleMainnetRelease
+```
 
 ### Build for E2E Testing
 
@@ -152,24 +168,6 @@ By default, geoblocking checks via API are enabled. To disable at build time, us
 GEO=false E2E=true ./gradlew assembleDevRelease
 ```
 
-### Build for Release
-
-**Prerequisites**  
-Setup the signing config:
-- Add the keystore file to root dir (i.e. `release.keystore`)
-- Setup `keystore.properties` file in root dir (`cp keystore.properties.template keystore.properties`)
-
-**Routine**
-
-Increment `versionCode` and `versionName` in `app/build.gradle.kts`, then run:
-```sh
-./gradlew assembleDevRelease
-# ./gradlew assembleRelease # for all flavors
-```
-
-APK is generated in `app/build/outputs/apk/_flavor_/release`. (`_flavor_` can be any of 'dev', 'mainnet', 'tnet').
-Example for dev: `app/build/outputs/apk/dev/release`
-
 ## Contributing
 
 ### AI Code Review with Claude
@@ -223,3 +221,5 @@ Destructive operations like `rm -rf`, `git commit`, and `git push` still require
 
 This project is licensed under the MIT License.
 See the [LICENSE](./LICENSE) file for more details.
+
+[img_detekt]: .github/img/detekt.png

README.md

@@ -9,6 +9,7 @@ import java.util.Properties
 plugins {
     alias(libs.plugins.android.application)
     alias(libs.plugins.compose.compiler)
+    alias(libs.plugins.compose.stability.analyzer)
     alias(libs.plugins.kotlin.android)
     alias(libs.plugins.kotlin.serialization)
     alias(libs.plugins.ksp)
@@ -218,6 +219,7 @@ dependencies {
     androidTestImplementation(platform(libs.compose.bom))
     implementation(libs.compose.material3)
     implementation(libs.compose.material.icons.extended)
+    implementation(libs.compose.runtime.tracing)
     implementation(libs.compose.ui)
     implementation(libs.compose.ui.graphics)
     implementation(libs.compose.ui.tooling.preview)