nigig-org/REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md at main

2026-07-26 19:38:26 +03:00

54 KiB

Raw Permalink Blame History

# Nigig Pay: Consolidated Code Review & Execution Plan

This document consolidates every finding from six independent reviews into a single authoritative assessment with a unified execution plan. No finding has been redacted, softened, or omitted. Every issue is grounded in the actual source code, with file paths and line numbers verified against the workspace at the time of review.

Executive Summary
Security
Architecture
Correctness Bugs
Performance
Code Quality
Parsing & Data Integrity
Design & UX
Test Strategy
Execution Plan

1. Executive Summary

This codebase is a prototype with payment-shaped code, not a safe payment product. Do not ship it, do not use it to initiate real M-Pesa transactions, and do not continue adding features until the payment boundary is redesigned.

The project is simultaneously an M-Pesa SMS tracker/classifier, a manual expense logger, a crypto P2P exchange rate aggregator (Binance, Bybit, OKX, Remitano, Kraken, MEXC, KuCoin), a USSD automation dialer, a tax preparation scaffold, and a receipt attachment manager. None of these are done well.

The code has useful ingredients—offline state, a USSD wrapper, a parser, fee tables, a UI, and some tests—but they are assembled as duplicated, stateful UI glue rather than as a correct transaction system.

The most serious flaws:

A biometric-gated payment dispatches even when biometric authentication fails to start. The shared handler logs "dispatching USSD anyway" after authenticate() returns an error.
The M-Pesa PIN is persisted in plaintext SharedPreferences and never wiped—not in clear(), not in fail(), not in closeUssdAfterResult(). It sits on disk indefinitely after every transaction.
The transaction state machine has no trustworthy correlation model. It assigns whichever global USSD/SMS event arrives to whatever string ID is current at that moment.
A parsed SMS is treated as payment verification without authenticating its source. SMS is not a transaction authority.
Money uses f64 in the transaction ledger. This is unacceptable for financial records.
Core payment logic exists in multiple divergent copies across nigig-core, nigig-pay, nigig-pay-ui, and nigig-mpesa. You cannot know which flow is authoritative, nor safely fix one.
Automatic retry can duplicate payments. There is no provider-side idempotency or reconciliation before retry.
Classification data is written but never read back. Every classifier decision and manual user categorization is silently discarded on the next app load.
The 5-minute missing-SMS timeout marks payments Failed. This guarantees false failures in real mobile conditions.

2. Security

S1. [CRITICAL] PIN Stored in Plaintext SharedPreferences, Never Wiped

Files:

robius-ussd/src/sys/android/session.rs:57 — put_string(env, &editor, KEY_PIN, &request.pin)?;
robius-ussd/src/sys/android/session.rs:94-114 — clear() only sets KEY_PENDING=false and KEY_STATE="DONE", never removes KEY_PIN
robius-ussd/src/sys/android/UssdAccessibilityService.java:295 — finishPin() reads the PIN via prefs.getString(KEY_PIN, "") but never removes it
UssdAccessibilityService.java:334-345 — fail() sets KEY_PENDING=false and KEY_STATE=DONE, never removes KEY_PIN
UssdAccessibilityService.java:312-332 — closeUssdAfterResult() sets KEY_PENDING=false and KEY_STATE=DONE, never removes KEY_PIN

The M-Pesa PIN is written to an unencrypted Android SharedPreferences XML file on every transaction. The clear() function (called on cancel/session end) resets KEY_PENDING and KEY_STATE but never touches KEY_PIN. The Java finishPin() and fail() paths also never scrub it. The PIN sits on disk indefinitely. SharedPreferences is world-readable-to-the-app, recoverable via root, adb backup, or any file-read primitive.

This alone would fail any PCI/financial security review. A 4-6 digit mobile money PIN is one of the highest-value secrets on the device, and it is being persisted like a UI preference.

form_pin: String also lives in PaySheet and SharedPaySheet widget memory as a plain heap String (pay_sheet.rs:85, shared_pay_sheet.rs:169), cloned into UssdTransactionRequest { pin: self.form_pin.clone(), ... }, never zeroed after use.

No zeroize crate or SecretString type exists anywhere in the codebase. A grep for zeroize, Zeroize, or SecretString returns zero hits.

S2. [CRITICAL] Biometric Gate Is a UX Affordance, Not an Auth Control

File: nigig-pay-ui/src/pay_flow/pay_flow_handler.rs:165-180

if fingerprint_enabled && has_hw {
    let prompt = robius_fingerprinting::AuthPrompt::for_transaction(...);
    if let Err(e) = robius_fingerprinting::authenticate(&prompt) {
        log!("[PayFlow] biometric auth error: {e:?} — dispatching USSD anyway");
        dispatch_ussd(&mut h);  // <-- FAIL-OPEN: dispatches without authentication
    }
}

When fingerprint payments are enabled and hardware is present, if authenticate() returns an error (permission issue, platform incompatibility, hardware error), the payment dispatches without successful authentication. This is a direct fail-open authorization bypass. The biometric result is not cryptographically bound to the transaction payload. On a rooted/instrumented device the biometric check is trivially bypassable by directly calling dispatch_ussd.

S3. [CRITICAL] No Encryption at Rest for Financial Data

Files:

nigig-core/src/persistence/mpesa/store.rs:124 — transactions.psv (pipe-separated, plain text)
nigig-core/src/persistence/mpesa/pending_store.rs:254-258 — pending_transactions.json (plain JSON)
nigig-core/src/persistence/mpesa/receipts.rs — receipt files (plain)

All transaction history, pending payments, receipts, counterpart names, phone numbers, balances, and raw SMS bodies are stored in unencrypted flat files. On a rooted Android device or via an unprotected backup, this data is fully exposed.

S4. [HIGH] Accessibility Service as Payment Primitive

UssdAccessibilityService.java walks the accessibility node tree of the system dialer, injects text into fields (amount, PIN, till, etc.), and drives a state machine. This is functionally indistinguishable from the pattern Play Protect and enterprise MDMs specifically fingerprint and block. It is the same class of primitive banking trojans abuse.

Play Store policy risk: The Accessibility API is restricted to a narrow allow-list (screen readers, switch access, etc.). "Automate financial transactions" is not on it.
User trust: Users are asked to grant a permission that is a full device-compromise primitive.
Liability: If the accessibility hooks misfire and drive the wrong menu path, the liability falls on the developer.

S5. [HIGH] No Input Validation on Financial Inputs

Phone numbers, till numbers, paybill business numbers, account references, and amounts are accepted as raw strings with only trim().is_empty() checks. There is no regex validation for Kenyan formats (07xx, 01xx, 2547xx), no length limits, no amount bounds validation, and no sanitization before concatenation into USSD dial strings.

S6. [HIGH] Unencrypted Data at Rest for All Personal Data

The app processes or persists raw SMS, M-Pesa transactions, counterpart phones/names, balances, receipts, location coordinates, user/profile data, camera files, Matrix sessions, email credentials, and potentially contact data. The persistence approach is a mix of pretty JSON, hand-rolled PSV, direct files, and unknown external session DB behavior. There is no demonstrated encryption-at-rest architecture, secure key hierarchy, retention policy, consent data model, access logging, or data-erasure design.

S7. [HIGH] Header Forgery for Exchange APIs

File: nigig-pay/src/payments_frame/pages/mpesa/exchange/api.rs (via request_json_post_with_bybit_headers)

req.set_header("origin".into(), "https://www.bybit.com".into());
req.set_header("referer".into(), "https://www.bybit.com/".into());

Hardcodes fake origin and referer headers to impersonate a browser session. This is a terms-of-service violation, potentially a Computer Fraud and Abuse Act issue, and a liability for any user of this software.

S8. [HIGH] No Certificate Pinning

All exchange API calls (fetch_binance_p2p, fetch_bybit_p2p, etc.) use raw HTTP without pinning. MITM attacks on public Wi-Fi could manipulate P2P rates or inject malicious JSON.

S9. [MEDIUM] Clipboard Exposure

cx.copy_to_clipboard(&self.recipient_name) in SharedPaySheet copies transaction details to the global clipboard, accessible to any other application on the device.

S10. [MEDIUM] User-Agent Exposure

req.set_header("User-Agent".into(), "Robrix/1.0 Makepad".into()) exposes internal framework names to external exchanges, increasing attack surface fingerprinting.

S11. [MEDIUM] Matrix Session Persistence Without Encryption

persistence/matrix_state.rs obtains and saves access_token, refresh_token, homeserver, and passphrase into a stored session. The code points to sessions.sqlite but nothing in the supplied layer establishes that it is encrypted with an OS-keystore-protected key. A stolen session token is account takeover.

S12. [MEDIUM] Email Configuration as Credential Exfiltration and SSRF Risk

SmtpConfig derives Serialize and contains SMTP server, username, and plaintext password. On WASM, the client sends the entire configuration—including password—to a configurable email API URL. On native, caller-controlled server/port configuration makes the app a potential internal-network connection primitive.

S13. [MEDIUM] Generic Persistence Helper Permits Path Escape

persistence/json.rs resolves a requested relative path with base.join(rel) but does not reject absolute paths or parent traversal. In Rust, an absolute RHS can replace the base; .. can escape it. This is a path traversal footgun.

S14. [MEDIUM] Fingerprint Preference Storage

SharedPaySheet stores fingerprint enablement as existence of a marker file derived from ANDROID_DATA, otherwise .. It is neither a user-protected secure setting nor reliably app-private. An attacker with local storage access can change it.

S15. [LOW] Unsafe Trait Assertions in Location

nigig-core/src/location.rs:100-101:

unsafe impl Send for ManagerWrapper {}
unsafe impl Sync for ManagerWrapper {}

These assert cross-thread correctness to the compiler without a documented safety proof. The underlying location manager's thread-safety is not established.

3. Architecture

A1. God Widgets Doing Everything

PayMpesaPage (in transact.rs, ~1200 lines) handles SMS scanning, USSD dispatch, biometric gating, file I/O, modal management, camera handling, receipt attachment, transaction classification, date filtering, and drawing. PaySheet (~1180 lines) mixes Rust widget logic with inline Makepad DSL layout. SharedPaySheet (~1500 lines) contains payment form fields, permission flags, biometric state, networking indicator, bulk CSV state, fee toggles, and status display state.

These are entire applications stuffed into Widget impls. There is no separation between view, view-model, service, and persistence layers.

A2. Thread-Local Singleton Anti-Pattern

File: nigig-pay-ui/src/pay_flow/pay_flow_handler.rs:18-20

thread_local! {
    pub static HANDLER: RefCell<PayFlowHandler> = RefCell::new(PayFlowHandler::with_loaded_store());
}

Financial transaction state is stored in a thread-local RefCell. This is catastrophic for:

Testability: Impossible to mock or run tests in parallel.
State inspection: No way to observe transaction flow for debugging.
Reentrancy: USSD callbacks + UI events can trigger borrow panics.
Lifecycle: No explicit owner, dependency injection, or lifecycle management.
Recovery: App restart rehydrates JSON but not a real in-flight USSD session.

A3. Four Incompatible State Paradigms Simultaneously

Makepad #[rust] widget fields (e.g., PaySheet with 20+ String fields) for ephemeral form state.
Thread-local RefCell singletons (pay_flow_handler.rs) for payment orchestration.
Global Mutex<Vec<...>> statics (api.rs's PENDING_FIAT, PENDING_P2P) for HTTP request tracking.
Makepad's action bus (PaySheetRequest, SharedPaySheetAction) for cross-widget signaling.

A single payment flow touches thread-local storage, global mutexes, widget fields, and the action bus before it dials USSD. There is no single source of truth, no transaction boundaries, and no recoverable state machine.

A4. No Domain Layer

There is no Domain or UseCase layer. The MpesaTransaction struct is passed directly from the SMS parser through the classifier into the UI, with no intermediate representation. Business logic is smeared across #[rust] fields inside Makepad widgets. Stores are accessed directly from UI event handlers.

A5. Massive Code Duplication Across Crates

The same concepts are implemented in multiple places:

Component	Copies	Locations
`MpesaTransaction` (with `f64` amounts)	3	`nigig-core/src/persistence/mpesa/parser.rs:18-30`, `nigig-pay/.../parser.rs:16-28`, `nigig-mpesa/.../parser.rs:16-28`
`MpesaTransactionStore`	3	`nigig-core/src/persistence/mpesa/store.rs:16`, `nigig-pay/.../store.rs:14`, `nigig-mpesa/.../store.rs:14`
`PendingTransactionStore`	3+	`nigig-core/src/persistence/mpesa/pending_store.rs:127`, `nigig-pay/.../pending_store.rs:126`, `nigig-pay-ui/.../pending_store.rs:127`
`fn parse` (SMS parser)	3	`nigig-core/.../parser.rs:51`, `nigig-pay/.../parser.rs:49`, `nigig-mpesa/.../parser.rs:49`
`PayFlowHandler` (thread-local)	3	`nigig-pay-ui/.../pay_flow_handler.rs:18`, `nigig-pay/.../pay_flow_handler.rs:10`, `nigig-mpesa/.../pay_flow_handler.rs:10`
`form_pin` handling	8+	Across `nigig-pay`, `nigig-pay-ui`, `nigig-mpesa`, plus 5 feature branch copies
`KEY_PIN` in `robius-ussd`	5	Across `apps/code/robius-mpesa-pay-feature*/robius-ussd/`

This is a staged migration that never finished and is now a fork farm. The result is semantic drift: one caller can write a record type/state path that another UI never reads.

A6. Migration Debris Is Now Production Architecture

The codebase contains compatibility re-exports in nigig-pay/src/lib.rs that reach into core/uikit old paths (e.g., pub mod dir { pub use nigig_core::dir::*; }). These shims were acceptable during a short migration with deletion dates. Here they hide dependency direction and let feature/UI code couple to internals indefinitely.

A7. Platform Coupling

#[cfg(target_os = "android")] blocks are scattered throughout UI code (transact.rs, pay_sheet.rs). Platform abstractions are non-existent. The app cannot be tested on desktop without compiling out core features.

A8. `nigig-core` Is Not Core

nigig-core directly depends on makepad-widgets, Matrix, SMS, USSD, directories, camera/image behavior, email/SMTP, location, networking, UI login/settings modules, and platform async implementation. A "core" package that imports UI framework types and platform services is not portable business core; it is an application catch-all.

A9. Global Mutable State as Unofficial Service Locator

Examples:

TOKIO_RUNTIME, REQUEST_SENDER, CLIENT, SYNC_SERVICE, SSO state in Matrix syncing
LATEST_LOCATION, permission sender, location-request sender
LAST_STATUS / network status
Thread-local payment HANDLER
Platform JNI callback/event channels

Global state causes hidden lifecycle requirements, races, test pollution, inability to run multiple accounts/sessions, and ambiguous behavior during logout/backgrounding/restart.

A10. Anomalous Files in Source Tree

nigig-pay-ui/src/shared_pay_sheet1.rs0 — extension .rs0 (accidental backup)
nigig-pay-ui/src/robius-mpesa-pay-feature.tar.gz — binary tarball in src/
nigig-pay-ui/src/.DS_Store — macOS artifact
nigig-core/src/persistence/app_state/mod.rs0 — extension .rs0
apps/code/robius-mpesa-pay-feature*/ — 5 near-identical copies of robius-ussd

4. Correctness Bugs

B1. [CRITICAL] Classification Data Written but Never Read Back

File: nigig-core/src/persistence/mpesa/store.rs

save_to_disk() (lines 127-164) writes 14 pipe-delimited fields including parts[10..=13] (category, subcategory, status, confidence):

let line = format!(
    "{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}|{}\n",
    ..., clean(MpesaClassifier::category_display_name(&tx.classification.category)),
    clean(&tx.classification.sub_category),
    clean(&format!("{:?}", tx.classification.status)),
    tx.classification.confidence,
);

load_from_disk() (lines 166-198) only parses parts[0..=9] and hardcodes:

classification: Default::default(),   // <-- parts[10..=13] never read

Every classifier decision and every manual user categorization is silently discarded on the next app load. Additionally, save_to_disk never writes classification.party_name or party_identifier at all, so even fixing the read side loses those two fields permanently.

B2. [CRITICAL] Verification Cross-Matching Can Wire Two Pending Transactions

File: nigig-pay-ui/src/pay_flow/pay_flow_handler.rs:337-341

UssdSessionEvent::ResultText(text) => {
    if let Some(id) = h.current.clone() {
        out.push(PayFlowEvent::UssdResultReceived { id, text });
    }
}

Global USSD/SMS events are assigned to h.current. There is no provider session correlation ID. If payment A is dispatched and payment B becomes current before A's USSD result arrives, A's result is incorrectly attributed to B.

The pending store's expire_stale() (line 228-252) marks all Dispatched records older than 5 minutes as Failed with reason "verification window expired". Two pending transactions of the same amount (very common—e.g., two KSh 500 sends) will both get marked Verified by one confirmation SMS, one of them incorrectly, if the fallback matching path is used.

B3. [CRITICAL] Automatic Retry Can Duplicate Payments

File: nigig-pay-ui/src/pay_flow/pay_flow_handler.rs:415-421, 434-471

The handler retries robius_ussd::begin_transaction with exponential backoff (1s, 2s, 4s, 8s, 16s, capped at 30s) up to 5 times for TemporarilyUnavailable / SessionTimeout. There is no provider idempotency key, no submitted-operation identifier, no reconciliation check before retry, and no guarantee that a "temporary" error did not still start a USSD operation.

For money movement, retrying a dispatch after an ambiguous outcome can pay twice.

B4. [CRITICAL] Unsafe Custom PSV Escaping (Lossy and Non-Bijective)

File: nigig-core/src/persistence/mpesa/store.rs:201-206

fn clean(value: &str) -> String {
    value.replace('|', "~").replace('\n', " ")
}
fn restore(value: &str) -> String {
    value.replace('~', "|")
}

This is not a bijective escape. If a party name, raw SMS body, or merchant string legitimately contains a literal ~, restore() will silently reintroduce a | and misalign every subsequent column on that row. Given raw_message stores full SMS text verbatim, a ~ in an SMS (not rare) will corrupt that record's fields on next load. There is no test covering this.

B5. [HIGH] Fee Lookup Failures Are Silently Free

Files: nigig-pay/src/pay/pay_sheet.rs (8 calls), nigig-pay-ui/src/shared_pay_sheet.rs (9 calls), nigig-pay-ui/src/bulk_pay.rs (7 calls)

let fee = robius_ussd::mpesa_fee_for_kind_amount(self.payment_kind, amount).unwrap_or(0);

This pattern is repeated 6+ times. If the fee lookup fails (unknown band, parsing error, future tariff change not in the table), the user is silently charged zero fee in the cost preview and the adjusted amount calculation. In a payments app, "silently treat unknown cost as free" is exactly backwards.

B6. [HIGH] `f64` for Money

Files:

nigig-core/src/persistence/mpesa/parser.rs:21,24,25 — amount: f64, balance: Option<f64>, cost: Option<f64>
nigig-core/src/persistence/mpesa/store.rs:93-108 — totals() sums f64
nigig-core/src/persistence/mpesa/store.rs:184 — amount: parts[2].parse().unwrap_or(0.0)

Binary floating point will create rounding and equality problems even if Kenyan shilling values usually have two decimal places. The verification matching code (t.amount as f64 == parsed.amount) performs float equality on money, which is fragile if either side ever computes the value instead of parsing it literally.

File: nigig-pay/src/payments_frame/pages/mpesa/transactions/transact.rs:1266

MpesaFilterMode::Month => current + Duration::days(31 * delta),

January 31 + 31 days = March 3, not February. The code is not navigating months; it is adding 31-day chunks. A proper NaiveDate month/year arithmetic is needed.

B8. [HIGH] `expect("validated by caller")` — Live Panic Risk

File: nigig-pay/src/payments_frame/pages/mpesa/pay/transaction_flow.rs:115

create_pending() does request.kind.expect("validated by caller"), but on_biometric_succeeded() builds a fresh UssdTransactionRequest and dispatches without re-validating. One future refactor away from a runtime panic on the hot payment path.

B9. [HIGH] No Concurrency Control on Flat-File Stores

Every mutation is: full read → mutate in memory → full write. Two writers (SMS-arrival thread and UI thread) racing lose an update silently. There is no file lock, no versioning, no merge strategy. For financial records, that is a data-integrity bug, not a cosmetic one.

B10. [MEDIUM] Custom JSON Parser Is Broken

File: nigig-pay/src/payments_frame/pages/mpesa/exchange/api.rs:35 (and nigig-mpesa copy)

A hand-rolled enum J recursive-descent JSON parser exists despite serde_json being a dependency. Bugs:

Unicode escapes: \u0041 is not parsed as A. The escape handler falls through to c => s.push(c as char), pushing literal characters u, 0, 0, 4, 1.
Number parsing: parse_num accepts malformed numbers like 1.2.3 or 1e2e3 because the loop accepts ./e/E/+/- without validation, then passes to f64::parse which will fail and abort.
Object validation: parse_obj does not verify that keys are strings; it blindly calls parse_str, which returns None on any malformed key and aborts.
Runs on the UI thread. Allocates a Vec<(String, J)> for every JSON object and a new String for every key.

B11. [MEDIUM] State Reset on DSL Reload

tabs_initialized, initialized, and other #[rust] flags in PaySheet and MpesaExchangePage reset to false when Makepad reloads the live DSL. The UI will re-initialize, re-fetch data, and jump tabs unexpectedly during development or hot-reload scenarios.

B12. [MEDIUM] Timestamp Normalization Edge Cases

normalize_sms_timestamp_ms uses arbitrary thresholds (10_000_000_000_000 and 100_000_000_000) to guess the unit. A microseconds timestamp slightly below 10 trillion would be misidentified. This is guesswork, not engineering.

B13. [MEDIUM] Double Action Posting

PayActionBar posts actions twice:

cx.action(action.clone());
Cx::post_action(action);

If the parent screen catches both, it may process the navigation twice, causing stack corruption or duplicate page pushes.

B14. [MEDIUM] `pop_pending` Is LIFO, Not FIFO

File: nigig-pay/src/payments_frame/pages/mpesa/exchange/api.rs

Pending requests are popped from the end of a Vec, meaning out-of-order HTTP responses will map to wrong exchange parsers.

B15. [MEDIUM] `Mutex::unwrap()` Poisoning Risk

File: nigig-pay/src/payments_frame/pages/mpesa/exchange/api.rs (lines 312, 331, 347, 364, 380, 391)

PENDING_FIAT.lock().unwrap() and PENDING_P2P.lock().unwrap() will panic if any thread panics while holding the lock, crashing the UI process.

B16. [LOW] Silent Data Loss on Write Failure

PendingTransactionStore::save() (lines 144-160):

if let Ok(mut f) = fs::File::create(&tmp) {
    let _ = f.write_all(s.as_bytes());
    let _ = f.sync_all();
    drop(f);
    let _ = fs::rename(&tmp, &path);
}

If disk is full, battery dies mid-write, or app is backgrounded, the user's pending transaction record vanishes without an error message. The same pattern infects MpesaTransactionStore, ReceiptStore, and UserCategoryStore.

B17. [LOW] `store_dir()` Calls `create_dir_all` as Side Effect

File: nigig-core/src/persistence/mpesa/store.rs:117-121

fn store_dir() -> PathBuf {
    let dir = app_data_dir().join("mpesa_tracker");
    let _ = fs::create_dir_all(&dir);  // <-- side effect in a path getter
    dir
}

Called on every load() and save_to_disk(), creating unnecessary syscall overhead on a hot path.

5. Performance

P1. [CRITICAL] Disk I/O on the Draw Thread

File: nigig-pay/src/payments_frame/pages/mpesa/expenses/mod.rs:161-183

fn draw_walk(&mut self, cx: &mut Cx2d, scope: &mut Scope, walk: Walk) -> DrawStep {
    self.reload(cx.cx);   // <-- calls MpesaTransactionStore::load() from disk on EVERY frame
    ...
}

reload() invokes MpesaTransactionStore::load(), which performs fs::read_to_string from disk on every 16ms frame boundary. On a low-end Android device, this will drop frames and get the app killed by the ANR watchdog.

P2. [HIGH] Blocking I/O on the UI Thread for Persistence

MpesaTransactionStore::save_to_disk(), PendingTransactionStore::save(), ReceiptStore::save(), and UserCategoryStore::save() are all called synchronously from UI event handlers. Every time a transaction status changes, the entire store is rewritten to disk, blocking the UI thread.

P3. [HIGH] SMS Scanning in draw_walk on 8-Second Timer

File: nigig-pay/src/payments_frame/pages/mpesa/transactions/transact.rs:949

if now - self.last_auto_scan_at > 8.0 {
    self.last_auto_scan_at = now;
    self.scan_sms(cx.cx);  // <-- reads SMS content provider (blocking IPC) during draw
}

On a device with 2,000+ SMS messages, this is a multi-hundred-millisecond jank spike.

P4. [MEDIUM] Busy-Loop Polling Architecture

File: nigig-pay/src/payments_frame/payments.rs (referenced)

self.flow_timer = cx.start_interval(0.1); // Polls biometric + USSD state every 100ms

This drains battery and creates race conditions between native callbacks and UI state.

P5. [MEDIUM] No Caching of Store Results

Widgets that need transaction data call Store::load() fresh rather than holding a shared, invalidated-on-write cache. Anything that touches the ledger (classifier re-runs, totals recompute) pays full-file-parse cost repeatedly per frame/interaction.

P6. [MEDIUM] PortalList Text Label Mutation Every Frame

In PayMpesaPage, PortalList item drawing mutates text labels via set_text every frame, invalidating Makepad's glyph cache and causing re-layout.

P7. [MEDIUM] `update_summary` and `update_period_tab_styles` Called from draw_walk

These mutate widget state during the render pass—a violation of retained-mode UI hygiene.

P8. [LOW] Redundant String Allocations

Repeated .to_string(), .clone(), format!(), .collect() throughout the codebase. For example, exchange.clone(), crypto.clone(), fiat.clone() appear constantly. Not catastrophic at current scale but unnecessary.

6. Code Quality

Q1. [HIGH] ~41 `let _ =` Patterns Discarding Errors in Production

nigig-pay (24 occurrences):

transactions/store.rs:117,135,159,161 — FS operations (create_dir_all, writeln!, write_all, rename)
transactions/receipts.rs:135,145 — FS operations
pay/pending_store.rs:147,151,152,154,255 — FS operations (create, write, sync, rename)
pay/pay_flow_handler.rs:153 — robius_ussd::cancel_session()
pay/pay_sheet.rs:120-121 — robius_fingerprinting::init() / robius_ussd::init()
transactions/transact.rs:1156,1554,1555,1556 — permissions, init, prefetch
exchange/api.rs:443 — let _ = cx (suppressing unused variable)

nigig-pay-ui (17 occurrences):

shared_pay_sheet.rs:124,126,369,429,776 — FS write/remove, permission request, file dialog, fingerprint cancel
pay_flow/pay_flow_handler.rs:485,486,534,535,568,569 — cancel_session + cancel_authentication (3 pairs)
pay_flow/pending_store.rs:148,152,153,155,256 — FS operations

Silent swallowing at JNI/file-IO boundaries is exactly where you'll need the error message most during a production incident and currently have none.

Q2. [HIGH] 162 `unwrap()` + 27 `expect()` Calls in Payment/USSD/SMS Hot Paths

Production unwrap() calls of concern:

exchange/api.rs: 9 Mutex lock unwraps
pay_flow_handler.rs (both crates): 1 unwrap on pending_request.as_ref().unwrap()
transaction_flow.rs: 1 expect on a pre-validated field

In a UI thread process, any of these panicking mid-transaction takes down the whole app, potentially leaving Dispatched transactions stuck forever un-swept (no panic-safety/cleanup hook resets in-flight state).

Q3. [HIGH] 32+ `dbg!`/`println!` Debug Outputs in Shipping Code

Present in what appears to be code intended to ship.

Q4. [MEDIUM] Trivial Enum-Equality Tests Provide Negative Value

Unit tests in classifier.rs, parser.rs, and data.rs are mostly "does TradeSide::Buy equal TradeSide::Buy?" tests. They test the language, not the logic.

Q5. [MEDIUM] Duplicated Test File

tests/mpesa_workflow.rs literally duplicates 500+ lines of parser and classifier logic instead of importing them. When the production parser changes, the tests will lie to you by testing obsolete code. This is worse than no tests.

Q6. [MEDIUM] Tests Skip Themselves Unless Env Var Is Set

tests/ui.rs contains tests that skip unless NIGIG_TEST_PAY is set. They are dead code in CI and provide zero confidence.

Q7. [MEDIUM] String-Heavy Architecture

Fixed values like exchange names, currency codes, trade sides, API statuses are compared as raw strings ("Personal", "Cash", "BUY", "SEND_MONEY") rather than enums at the UI boundary. This makes refactoring unsafe.

Q8. [MEDIUM] Copy-Paste Page Templates

send.rs, receive.rs, and history.rs are 95% identical scaffold code. The developer doesn't understand Makepad's component model or is too lazy to abstract StackNavigation pages.

Q9. [MEDIUM] Giant Match Statements

PaySheet::handle_event() contains 30+ independent if blocks checking button clicks. This should be a state machine or command pattern.

Q10. [MEDIUM] Magic Numbers Everywhere

0.55 seconds for double-tap
5 * 60 * 1000 ms verification window
250 symbol limit in parse_mexc_catalog
8.0 seconds for auto-scan in draw_walk
72.0 pixels for pull-to-refresh
10_000_000_000_000 and 100_000_000_000 for timestamp guessing

Q11. [LOW] Comments Overpromise Implementation Status

Comments claim stores are "local source of truth," writes are "atomic," fields have "UUID v4 generated," but implementation does not establish those claims. No shown UUID dependency/generator in the pending store. Persistence errors are swallowed. JSON parse failures become an empty store. Multiple "source of truth" stores exist.

Q12. [LOW] Commented-Out Dead Code

transaction_content_detail.rs contains commented-out impl ScriptHook blocks. This indicates the developer doesn't trust version control.

Q13. [LOW] Cross-Platform Claims Are Misleading

platform.rs says WASM uses std::fs::read/write and that blocking is acceptable on single-threaded WASM. In a browser, ordinary std::fs is not a browser persistence API. The WASM timeout implementation simply awaits forever and returns Ok.

7. Parsing & Data Integrity

D1. [HIGH] SMS Parser Is Heuristic and Fragile

is_mpesa_message accepts sender/body strings containing "MPESA", "M-PESA", or "SAFARICOM". Extraction functions parse human text heuristically. The extract_transaction_code function assumes the first word is always the code. This will break on any SMS format variation.

Match strings ("KSH", "TRANSACTION COST, KSH", " ON ", " AT ") are hardcoded English literals. A Safaricom copy change (which has happened historically) breaks parsing for 100% of users simultaneously with no fallback.

When is_mpesa_message(...) == true and parse(...).is_none(), no telemetry or log event is emitted. Real transactions can go missing from the ledger with zero visibility.

D2. [HIGH] SMS Sender Spoofing

is_mpesa_message trusts the sender string containing "MPESA"/"SAFARICOM". SMS sender IDs are trivially spoofable on Android. This mostly matters if parsed SMS ever auto-triggers anything beyond passive ledger entries. Worth explicitly documenting the trust boundary.

D3. [MEDIUM] Parser Only Recognizes Limited Templates

The parser only recognizes "received", "sent", "withdrawn" phrasing. Real M-Pesa SMS traffic includes Fuliza (overdraft) messages, airtime purchases, reversals, and paybill-specific templates with different wording. None appear in the parser or its tests.

D4. [MEDIUM] Locale/Copy Fragility

Every match string is a hardcoded English literal. A Safaricom copy change breaks parsing for all users simultaneously.

8. Design & UX

U1. [HIGH] Feature Bloat Masquerading as MVP

The app suffers from acute identity disorder. It is simultaneously:

An M-Pesa SMS tracker and classifier
A manual expense logger
A crypto P2P exchange rate aggregator (Binance, Bybit, OKX, Remitano, Kraken, MEXC, KuCoin)
A USSD automation dialer
A tax preparation scaffold ("VAT / withholding / income summaries coming next")
A receipt attachment manager (camera, audio, location, documents)

None of these are done well.

U2. [HIGH] Fee Logic Duplicated in Four Places

refresh_cost_preview, on_pay_cta, start_payment_flow, and on_biometric_succeeded all re-implement the same "subtract fee, add withdrawal fee" arithmetic. Any future fee-model change requires editing four call sites correctly—a guaranteed source of drift bugs.

U3. [HIGH] Offline-First Claims Are Theater

seed_exchange_catalog writes hardcoded metadata and immediately posts an action. There is no cache eviction, no TTL, and no reconciliation logic. The offline store is an append-only dump of JSON files.

U4. [MEDIUM] 5-Minute Missing-SMS Timeout Marks Payments Failed

expire_stale() in pending_store.rs:228-252 marks Dispatched records as Failed after 5 minutes if no SMS arrives. In real mobile conditions (network delays, message filtering, Do Not Disturb mode), this guarantees false failures that may make users retry a transaction that actually succeeded.

U5. [MEDIUM] Failure Semantics Are Dishonest

The app marks records Failed on user cancellation, biometric cancellation, USSD error, or SMS timeout. Some of those mean "we know it did not happen"; others mean "we do not know." Combining them causes users to retry payments that may already have gone through.

U6. [MEDIUM] Network Status Is a False Health Signal

The network module uses TCP connect to 8.8.8.8:53, which proves neither DNS resolution, HTTPS reachability, Matrix reachability, nor payment-provider availability. It can be blocked while internet works, allowed in a captive portal, or violate local network assumptions. A green "connected" indicator based on this is operationally misleading.

U7. [MEDIUM] Bulk Pay Is Operationally Dangerous

Bulk queues add automatic retries, progress counts, and sequential dispatch without:

Per-recipient review and explicit approval
Recipient normalization/deduplication
Per-item limits/funds checks
Rate limiting/carrier pacing
Idempotency/reconciliation
Pause/resume/cancel semantics that preserve ambiguous items
Post-batch audit export

Bulk USSD payment must be disabled until there is an authorized, idempotent provider rail and reconciliation model.

U8. [LOW] Platform "Fake Success" Paths Can Contaminate Production Assumptions

The non-Android platform code advertises fake success/failure events for desktop development. Test doubles are valid, but they must be behind an explicit test-only/mock feature and impossible to select in production builds.

U9. [LOW] Hard-Coded 2024 Fee Table

Fee schedules are constants labelled "effective Jan 2024" inside the USSD crate. A payment app operating in 2026 cannot quietly display or use a two-year-old static tariff as authoritative.

9. Test Strategy

T1. [HIGH] Tests Are Not Sufficient Evidence

There are roughly 387 #[test] markers, but the snapshot does not prove execution. Missing or not demonstrated are:

State-machine transition/property tests
Crash points around intent persistence and dispatch
Delayed/out-of-order/duplicate platform event tests
Ambiguous outcome/retry tests proving no duplicate dispatch
Parser adversarial corpus and SMS spoof/replay tests
Storage fault injection (disk full, crash mid-write, rename failure, corruption)
Android device/integration tests
End-to-end tests against an authorized sandbox/provider
Security tests for logs, files, and permissions
UI accessibility and payment confirmation tests

T2. [HIGH] Duplicated Test Code Tests Itself, Not Production

tests/mpesa_workflow.rs duplicates the parser and classifier enums/functions instead of importing them. When the production parser changes, the tests will lie.

T3. [MEDIUM] No Integration Tests for Payment Flow

There are zero integration tests for the payment flow, biometric gate, or USSD dispatch. The specific bugs above (classification persistence round-trip, PSV special-character escaping, concurrent-write race, duplicate-amount SMS matching) are not covered.

T4. [MEDIUM] Missing Storage Failure Injection Tests

No tests for disk-full, crash-mid-write, rename-failure, or corruption scenarios.

T5. [LOW] Test the Dangerous Realities, Not Just Parsing Happy-Path Messages

The test suite covers basic SMS parsing and enum equality. It does not cover adversarial SMS formats, spoofed senders, concurrent events, or payment state machine edge cases.

10. Execution Plan

Phase 0: Immediate Containment (Days 1-3)

Goal: Prevent the prototype from moving real money incorrectly.

#	Action	Rationale
0.1	Disable production transaction dispatch, auto-retry, and bulk pay behind a hard compile-time `demo`/`sandbox` feature. Default production build must not invoke USSD automation.	P0 safety
0.2	Remove the fail-open biometric branch in `pay_flow_handler.rs:171-173`. Authentication failure/unavailability must block dispatch.	P0 security
0.3	Remove custom PIN capture from the app flow unless an approved provider contract requires it. Never log/store it. Scrub `KEY_PIN` from SharedPreferences in `clear()`, `fail()`, and `closeUssdAfterResult()`.	P0 security
0.4	Stop declaring SMS/USSD parse output as `Verified`. Rename to `ObservedEvidence` until reconciliation exists.	P0 correctness
0.5	Remove static desktop "success" behavior from production code; put fakes in test-only adapters.	P0 safety
0.6	Stop persisting raw SMS by default. If necessary for a user-approved troubleshooting mode, encrypt, redact, and expire it.	P0 privacy
0.7	Archive the current branch/tag it. Create a clean recovery branch from a pinned workspace/toolchain.	Engineering discipline
0.8	Create a threat model and payment-risk register before more UI work. Include lost/duplicate/forged events, device compromise, SMS spoofing, storage theft, carrier/menu changes, and ambiguous provider outcome.	P0 safety

Exit criterion: No release build can dispatch money. No biometric error can bypass authorization. All customer-visible payment status labels distinguish confirmed from unknown.

Phase 1: Establish Build, Ownership, and Quality Gates (Week 1)

#	Action	Rationale
1.1	Turn this into a real Cargo workspace with checked-in `Cargo.lock`, MSRV/toolchain file, Android build instructions, and CI.	Build governance
1.2	Run mandatory gates: `cargo fmt --check`, Clippy with `unwrap_used` denied for owned crates, unit/integration tests, dependency audit/deny, license checks, and Android build smoke test.	Quality gates
1.3	Delete the duplicate payment/parser/pending-store implementations. Pick canonical owners: `nigig-pay-domain` (pure types/state machine/validation), `nigig-pay-storage` (repositories/migrations/encryption), `nigig-pay-platform` (trait interfaces and Android adapters), `nigig-pay-app`/Makepad UI (presentation only).	Eliminate fork farm
1.4	Ban core/domain dependencies on Makepad. `makepad_widgets` must not appear in payment domain or storage crates.	Enforce boundaries
1.5	Eliminate compatibility shims with an explicit migration PR sequence and deletion issue/date.	Remove migration debris
1.6	Add an ADR for payment scope: read-only finance tracker vs provider-authorized payments. If the business cannot obtain an authorized M-Pesa integration, position this as a tracker/launcher, not a payment processor.	Strategic clarity

Exit criterion: One parser, one ledger repository, one payment coordinator, no duplicated source of truth, reproducible CI build.

Phase 2: Extract the Domain Layer (Weeks 2-4)

#	Action	Rationale
2.1	Create `nigig-pay-domain` crate: pure Rust, no Makepad. Contains entities, traits, services.	Architecture
2.2	Define exact domain types: `PaymentIntentId`, `Money { currency, minor }`, `Recipient`, `FeeQuote`, `PaymentIntent`, `ProviderOperationId`, `PaymentEvidence`.	Type safety
2.3	Replace `f64` entirely. Migrate ledger values to exact minor units; write migration/import tests.	Financial correctness
2.4	Implement one explicit state machine with transition validation: `Draft → AwaitingUserAuthorization → Dispatching → Submitted → Confirmed/Rejected/UnknownNeedsReconciliation`.	Payment correctness
2.5	Make `TransactionFlow` a normal struct that takes a `UssdProvider` and `BiometricProvider` via constructor injection. Kill the `thread_local!` singleton.	Testability
2.6	Persist intent before dispatch in the same durable transaction as the audit event. Persist every state transition and platform attempt.	Durability
2.7	Give each gateway operation a correlation/session ID. Reject events that do not match the currently valid operation/state.	Event correlation
2.8	Separate outcomes: `DispatchNotStarted` (safe retry), `SubmittedUnknown` (no automatic retry), `ProviderRejected`, `Confirmed`, `NeedsReconciliation`.	Retry safety
2.9	Delete automatic retries for ambiguous USSD outcomes. Reconciliation must decide before a retry.	Duplicate payment prevention
2.10	Extract fee/amount-adjustment math into one pure, unit-tested function: `compute_adjusted_amount(kind, amount, deduct_fee, include_withdrawal) -> Result<AdjustedAmount, FeeError>`. Call it from all current duplicate sites.	DRY

Exit criterion: 100% of state transitions are unit/property tested. Test sequences with duplicate, delayed, and reordered events cannot mark the wrong intent confirmed or produce a second dispatch.

Phase 3: Replace Persistence with a Secure Repository (Weeks 3-6)

#	Action	Rationale
3.1	Use SQLite with migrations. Use SQLCipher (`rusqlite` with `sqlcipher` feature flag) for encryption. Derive the key from Android Keystore.	Storage correctness + encryption
3.2	Run one storage writer/transaction boundary. Return `Result` from every operation; do not swallow errors.	Error handling
3.3	Store minimum necessary data. Split raw external evidence from normalized records; hash/deduplicate evidence; encrypt sensitive content; define retention/deletion.	Data minimization
3.4	Implement crash-safe migrations, backup/recovery, corruption reporting, and storage fault injection tests.	Reliability
3.5	Add secure app-private preference storage for biometric opt-in. Do not use `ANDROID_DATA`/`.` marker files.	Security
3.6	Redact PII/secrets from logs and user-visible error messages.	Privacy

Exit criterion: Disk full, permission denied, corrupted DB, and crash-mid-transition tests produce a safe, diagnosable state—not silent data loss or false failure.

Phase 4: Remove Blocking I/O from draw_walk and UI Thread (Week 3)

#	Action	Rationale
4.1	Delete `reload()` calls from `draw_walk` in `expenses/mod.rs`. Load data once on init and on explicit refresh actions only.	Performance
4.2	Move the 8-second `scan_sms` timer from `draw_walk` in `transact.rs` to a background thread or explicit user action.	Performance
4.3	Move `update_summary` and `update_period_tab_styles` out of draw_walk.	Performance
4.4	Wrap all store `save()` operations in a background thread or async channel.	Performance

Phase 5: Harden Platform Gateways (Weeks 5-8)

#	Action	Rationale
5.1	Define traits: `PaymentGateway`, `BiometricAuthorizer`, `SmsEvidenceSource`, `ContactSource`. Implement Android behind adapters; keep JNI out of domain/UI crates.	Architecture
5.2	Get a legal/policy review on the AccessibilityService-driven USSD automation against current Google Play policy before further investment.	Existential risk
5.3	Ensure every Android callback is mapped to a typed event with an operation/session correlation ID.	Correctness
5.4	Use explicit `MockGateway` only in tests/dev builds. No fake completion in production target configuration.	Safety
5.5	Conduct memory/JNI/threading review for every `unsafe` boundary. Add safe wrappers with documented ownership, callback lifetime, and thread requirements.	Safety
5.6	Implement actual web adapters or remove the web claim. Browser storage must use IndexedDB/WebCrypto; timeout/cancellation semantics must be real.	Honesty

Exit criterion: Device/integration test matrix proves permission denial, cancellation, backgrounding, app restart, and out-of-order callbacks cannot bypass authorization or corrupt an intent.

Phase 6: Rebuild UX Around Truthful States (Weeks 7-10)

#	Action	Rationale
6.1	UI receives `PaymentViewState`; it does not own transaction state or call platform APIs directly.	Architecture
6.2	Require an immutable final confirmation screen showing recipient, exact amount, estimated fee, total, rail, and authorization method. Re-display on any material change.	UX safety
6.3	Never automatically close the sheet and call a payment failed due to missing SMS. Show `Pending confirmation — do not retry` with reconciliation/help actions.	Correct UX
6.4	Make accessibility/permission instructions separate from payment confirmation. Never conflate a successful prompt with settlement.	UX clarity
6.5	Add user-accessible receipt/evidence display, correction/reporting flow, and data deletion controls.	User rights
6.6	Build bulk only after official/API-backed idempotency/reconciliation exists; add per-item review, limits, pause/resume, and audit export.	Safety

Exit criterion: UI cannot describe a transaction as successful absent trusted confirmation, or failed absent known rejection.

Phase 7: Fix the Test Strategy (Week 11)

#	Action	Rationale
7.1	Delete `tests/mpesa_workflow.rs`. It tests duplicated code.	Harmful tests
7.2	Delete all trivial enum-equality tests. They provide negative value.	Noise
7.3	Make UI tests run in CI. Remove the `NIGIG_TEST_PAY` gate. Use Makepad's headless test runner or screenshot diffing.	CI confidence
7.4	Write unit tests against `domain/services/`. You should be able to test SMS parsing, classification, and transaction flow without importing `makepad_widgets`.	Testability
7.5	Add adversarial parser corpus and SMS spoof/replay tests.	Security
7.6	Add storage fault injection tests (disk full, crash mid-write, rename failure, corruption).	Reliability
7.7	Add benchmarks: `store_merge_1000_transactions`, `parse_100_sms_messages`, `classify_random_messages`.	Performance
7.8	Add `#![deny(clippy::unwrap_used)]` ratchet to payment/USSD/SMS-parsing crates.	Quality ratchet

Phase 8: Verification, Operations, and Release Discipline (Ongoing)

#	Action	Rationale
8.1	Build a deterministic simulator for all gateway event sequences.	Correctness
8.2	Add property tests for state machine invariants: confirmed is terminal; no intent dispatches twice without explicit new approval; unmatched event changes nothing; ambiguous result never becomes failed automatically; restart produces equivalent state.	Correctness
8.3	Add parser fuzzing and a curated, consent-safe SMS corpus; strict parser outputs remain evidence only.	Security
8.4	Add security review: Android permissions, local encryption/key handling, logs, JNI, dependency audit, SBOM, privacy policy, and third-party licensing.	Compliance
8.5	Add operational telemetry with redacted correlation IDs, state transition metrics, and reconciliation queues—not raw SMS or recipient details.	Observability
8.6	Pilot only against an authorized sandbox/limited internal test cohort with manual reconciliation. Do not treat a UI demo as production validation.	Release safety

Priority Defect Summary

Priority	Defect	Section	Required Action
P0	Biometric error dispatches USSD anyway	S2	Fail closed; block/re-authenticate/explicit fallback approval
P0	PIN stored in plaintext SharedPreferences, never wiped	S1	Remove PIN persistence; use short-lived in-memory channel; scrub on all terminal paths
P0	SMS/USSD heuristics mark payment verified	B2	Treat as untrusted evidence; provider reconciliation required
P0	Automatic retry of ambiguous USSD without idempotency	B3	Remove; use idempotency/reconciliation
P0	`f64` money ledger	B6	Replace with exact minor-unit money type and migrate
P0	Plaintext raw SMS/payment records, swallowed write errors	S3, B16	Secure database/repository and explicit errors
P0	Multiple payment flow/store/parser copies	A5	Delete duplicates and enforce ownership boundaries
P0	Classification data written but never read back	B1	Fix the read side; migrate to SQLite where this class of bug doesn't exist
P1	Global `thread_local` transaction coordinator	A2	Explicit durable coordinator with correlated events
P1	5-minute no-SMS → Failed	U4	Add `UnknownNeedsReconciliation` state
P1	Hard-coded 2024 fee table	U9	Versioned policy, estimates vs provider final fee
P1	Fee lookup failures silently free	B5	Propagate as `Result` to UI
P1	Bulk USSD flow	U7	Disable until official/idempotent rail exists
P1	Disk I/O on draw_walk	P1	Remove; load once on init/refresh
P1	Blocking persistence on UI thread	P2	Background thread/async channel
P1	No input validation on financial inputs	S5	Phone regex, amount bounds, PIN length
P1	Broken month navigation	B7	Proper `NaiveDate` arithmetic
P1	PSV escaping is lossy and non-bijective	B4	Migrate to SQLite or proper escaping
P1	No concurrency control on flat-file stores	B9	Mutex/single-writer or SQLite
P1	Duplicated test file tests itself, not production	T2	Delete; test real modules
P2	Full-file PSV/JSON persistence	B9	SQLite/encrypted storage, migrations, single writer
P2	UI manages domain/platform lifecycle	A1	Unidirectional command/event architecture
P2	Missing privacy/compliance controls	S6	Consent, retention, encryption, legal/provider review
P2	Custom JSON parser	B10	Replace with `serde_json` and proper structs
P2	Header forgery for exchange APIs	S7	Remove; use official public APIs
P2	No certificate pinning	S8	Implement for all exchange APIs
P2	User-Agent exposure	S10	Remove framework-identifiable strings
P2	Copy-paste page templates	Q8	Abstract into shared component
P2	Giant match statements in handle_event	Q9	State machine or command pattern
P2	String-heavy architecture	Q7	Replace with enums for fixed values
P2	Magic numbers	Q10	Named constants
P2	`expect("validated by caller")` panic risk	B8	Internal validation with `Result`
P3	Matrix session persistence without encryption	S11	Audit and encrypt
P3	Email config credential risk	S12	Secure storage; no web-client relay
P3	Path traversal in generic persistence	S13	Validate paths are relative
P3	32+ debug outputs in shipping code	Q3	Route through `log!`/`error!`
P3	Trivial tests providing negative value	Q4	Delete
P3	Tests skip themselves unless env var set	Q6	Make unconditional or remove

Hard Truth

This code was written by someone who understands Makepad DSL syntax but does not understand Rust software architecture. It will work as a demo, but it will not survive real users, real devices, or a real team.

The good news: the business logic is salvageable. The parser, the fee tables, the state machine concepts, and the domain knowledge (M-Pesa flows, fee bands, crypto exchange APIs) are all real and valuable.

The bad news: roughly 40% of the UI code needs to be deleted and rewritten with proper separation of concerns. The security posture is not just weak—it is actively hostile to the user's financial safety. The persistence model guarantees data loss under realistic failure conditions. The test strategy provides false confidence.

Do not ship this to users with real money. Execute Phase 0 immediately, then Phase 1, then Phase 2, before considering a beta release. If the rebuilding work is not funded, do not market or expose this as a payment system. Position it as a read-only tracker/launcher until the payment path is rebuilt.

54 KiB Raw Permalink Blame History

Table of Contents

1. Executive Summary

2. Security

S1. [CRITICAL] PIN Stored in Plaintext SharedPreferences, Never Wiped

S2. [CRITICAL] Biometric Gate Is a UX Affordance, Not an Auth Control

S3. [CRITICAL] No Encryption at Rest for Financial Data

S4. [HIGH] Accessibility Service as Payment Primitive

S5. [HIGH] No Input Validation on Financial Inputs

S6. [HIGH] Unencrypted Data at Rest for All Personal Data

S7. [HIGH] Header Forgery for Exchange APIs

S8. [HIGH] No Certificate Pinning

S9. [MEDIUM] Clipboard Exposure

S10. [MEDIUM] User-Agent Exposure

S11. [MEDIUM] Matrix Session Persistence Without Encryption

S12. [MEDIUM] Email Configuration as Credential Exfiltration and SSRF Risk

S13. [MEDIUM] Generic Persistence Helper Permits Path Escape

S14. [MEDIUM] Fingerprint Preference Storage

S15. [LOW] Unsafe Trait Assertions in Location

3. Architecture

A1. God Widgets Doing Everything

A2. Thread-Local Singleton Anti-Pattern

A3. Four Incompatible State Paradigms Simultaneously

A4. No Domain Layer

A5. Massive Code Duplication Across Crates

A6. Migration Debris Is Now Production Architecture

A7. Platform Coupling

A8. nigig-core Is Not Core

A9. Global Mutable State as Unofficial Service Locator

A10. Anomalous Files in Source Tree

4. Correctness Bugs

B1. [CRITICAL] Classification Data Written but Never Read Back

B2. [CRITICAL] Verification Cross-Matching Can Wire Two Pending Transactions

B3. [CRITICAL] Automatic Retry Can Duplicate Payments

B4. [CRITICAL] Unsafe Custom PSV Escaping (Lossy and Non-Bijective)

B5. [HIGH] Fee Lookup Failures Are Silently Free

B6. [HIGH] f64 for Money

B7. [HIGH] Broken Date Navigation

B8. [HIGH] expect("validated by caller") — Live Panic Risk

B9. [HIGH] No Concurrency Control on Flat-File Stores

B10. [MEDIUM] Custom JSON Parser Is Broken

B11. [MEDIUM] State Reset on DSL Reload

B12. [MEDIUM] Timestamp Normalization Edge Cases

B13. [MEDIUM] Double Action Posting

B14. [MEDIUM] pop_pending Is LIFO, Not FIFO

B15. [MEDIUM] Mutex::unwrap() Poisoning Risk

B16. [LOW] Silent Data Loss on Write Failure

B17. [LOW] store_dir() Calls create_dir_all as Side Effect

5. Performance

P1. [CRITICAL] Disk I/O on the Draw Thread

P2. [HIGH] Blocking I/O on the UI Thread for Persistence

P3. [HIGH] SMS Scanning in draw_walk on 8-Second Timer

P4. [MEDIUM] Busy-Loop Polling Architecture

P5. [MEDIUM] No Caching of Store Results

P6. [MEDIUM] PortalList Text Label Mutation Every Frame

P7. [MEDIUM] update_summary and update_period_tab_styles Called from draw_walk

P8. [LOW] Redundant String Allocations

6. Code Quality

Q1. [HIGH] ~41 let _ = Patterns Discarding Errors in Production

Q2. [HIGH] 162 unwrap() + 27 expect() Calls in Payment/USSD/SMS Hot Paths

Q3. [HIGH] 32+ dbg!/println! Debug Outputs in Shipping Code

Q4. [MEDIUM] Trivial Enum-Equality Tests Provide Negative Value

Q5. [MEDIUM] Duplicated Test File

Q6. [MEDIUM] Tests Skip Themselves Unless Env Var Is Set

Q7. [MEDIUM] String-Heavy Architecture

Q8. [MEDIUM] Copy-Paste Page Templates

Q9. [MEDIUM] Giant Match Statements

Q10. [MEDIUM] Magic Numbers Everywhere

Q11. [LOW] Comments Overpromise Implementation Status

Q12. [LOW] Commented-Out Dead Code

Q13. [LOW] Cross-Platform Claims Are Misleading

7. Parsing & Data Integrity

D1. [HIGH] SMS Parser Is Heuristic and Fragile

D2. [HIGH] SMS Sender Spoofing

D3. [MEDIUM] Parser Only Recognizes Limited Templates

D4. [MEDIUM] Locale/Copy Fragility

8. Design & UX

U1. [HIGH] Feature Bloat Masquerading as MVP

U2. [HIGH] Fee Logic Duplicated in Four Places

U3. [HIGH] Offline-First Claims Are Theater

54 KiB

Raw Permalink Blame History

A8. `nigig-core` Is Not Core

B6. [HIGH] `f64` for Money

B8. [HIGH] `expect("validated by caller")` — Live Panic Risk

B14. [MEDIUM] `pop_pending` Is LIFO, Not FIFO

B15. [MEDIUM] `Mutex::unwrap()` Poisoning Risk

B17. [LOW] `store_dir()` Calls `create_dir_all` as Side Effect

P7. [MEDIUM] `update_summary` and `update_period_tab_styles` Called from draw_walk

Q1. [HIGH] ~41 `let _ =` Patterns Discarding Errors in Production

Q2. [HIGH] 162 `unwrap()` + 27 `expect()` Calls in Payment/USSD/SMS Hot Paths

Q3. [HIGH] 32+ `dbg!`/`println!` Debug Outputs in Shipping Code