nigig-org/REVIEWS/PAY_CAD_IMPLEMENTATION_STATUS.md
andodeki 775eec4424
Some checks failed
Payment domain, storage, platform and UI / isolated-payment-tests (push) Waiting to run
Payment domain, storage, platform and UI / payment-ui-tests (push) Waiting to run
repo hygiene / hygiene (push) Has been cancelled
fix(pay): remove PIN capture from default builds rather than hiding it (0.3)
Asked in review: "does the latest code have the PIN input field?" It did.

## Hiding was weaker than it looked

pin_input, form_pin and the reveal toggle were all present, and the default
build hid them at runtime in on_after_new. Three problems:

1. The DSL declared the control visible and Rust hid it afterwards, so
   anything re-applying the UI definition — a hot reload, a re-instantiated
   sheet — brought it back. Defect B11 already names this class.
2. Hidden is not absent. The TextInput stayed in the widget tree, and a
   hidden input can still be focused or filled programmatically.
3. form_pin was compiled into every build, so any path reaching it could
   populate it.

Item 0.3 asks for removal, not concealment.

## The field no longer exists without `demo`

form_pin, pin_visible, the eye-toggle handler, the text-input handler, both
PIN checks, the request construction and try_build_ussd_request are all
#[cfg(feature = "demo")]. A default build has no field to write to.

The DSL now declares visible: false on the PIN input and its reveal button,
so hidden is the default state rather than a runtime correction; demo
unhides them on init. That closes the reload path in (1).

One runtime call remains as belt-and-braces: if a hot-reloaded definition
surfaces the input, the non-demo arm wipes what was typed. It has no
form_pin to clear, because there isn't one.

## Proving absence rather than asserting it

A grep for form_pin proves nothing — it passes just as happily against a
field still present behind a runtime if. tools/check-no-pin-capture.sh is a
compile probe: it references the field outside any cfg block and requires
the default build to fail with "no field `form_pin`" while demo succeeds.
The script restores the file on every exit path.

Verified both directions: passes on current code, exits 1 when the field is
re-exposed un-gated.

## Validation

  cargo check -p {nigig-pay-ui,nigig-pay,nigig-mpesa,nigig-core}  pass
  cargo check -p {nigig-pay,nigig-mpesa} --features demo          pass
  nigig-pay-ui: cargo test --lib                                  pass (61)
  domain / storage / platform / mpesa harness                     pass
  no-PIN-capture guard: verified to fail on a re-exposed field    pass

## What 0.3 still leaves open

The Java-side KEY_PIN scrubbing was already done, so 0.3 is complete for the
default product. A demo build still captures a PIN by design — that path
exists for authorised device testing and is covered by the unresolved
Play-policy decision in ADR 0007. If that decision goes against the USSD
rail, the demo path and its PIN capture are deleted with it.
2026-08-02 07:31:44 +00:00

66 KiB
Raw Permalink Blame History

Pay + CAD implementation status

Scope selected

The requested implementation scope was Nigig Pay + CAD, with broad refactoring authorised. This document records the first safe implementation tranche and the blockers that prevent falsely claiming the remaining review phases are complete.

Tranche 1: containment, domain and storage foundation

Nigig Pay phases 14: implemented foundation

  • Added a nigig-pay-storage workspace crate with transactional SQLite persistence for payment intents and append-only intent state snapshots.
  • Added PaymentStorageWorker, a single-writer worker that owns its SQLite connection off the UI thread. save_durable waits for commit acknowledgement and is the required call before any future gateway dispatch; it is intentionally not fire-and-forget.
  • Storage tests cover intent round-trip, state-event audit count, missing-intent error behavior, and cross-thread durable worker acknowledgement.
  • This is the Phase-3/4 foundation only. It is not SQLCipher/Keystore encryption and it is not yet wired into the legacy Makepad pay flow.
  • Removed the silent unwrap_or(0) fee fallback from the shared PaySheet single-payment and bulk-payment start paths. Unknown fees now block the action with a visible status instead of creating a false free quote.

Nigig Pay domain foundation

  • Added nigig-pay-domain to the workspace so the pure payment domain is a build target rather than an orphan directory.
  • Completed the missing domain module surface declared by lib.rs:
    • fee.rs: exact, checked fee/amount quote calculation; unknown fees can no longer become zero implicitly inside the domain function.
    • validation.rs: canonical Kenyan mobile-number validation, exact amount bounds, PIN shape validation that does not retain a PIN, and recipient-label validation.
  • Added checked Money::checked_add / checked_sub to prevent quote arithmetic overflow or a silently clamped fee deduction.
  • Changed the state-machine contract so a Submitted payment cannot transition to Failed merely because local confirmation did not arrive. It must use UnknownNeedsReconciliation.
  • Added unit tests to the new fee/validation modules for unknown fee, excessive fee deduction, withdrawal fee, phone forms, amount bounds and PIN shape.

Existing Pay work discovered during review

The repository already contains partial Phase-0 containment work that the review documents had described as future work:

  • nigig-pay-ui has a default-off demo feature; USSD dispatch, auto retry and bulk pay are disabled without it.
  • The fail-open biometric branch in pay_flow_handler.rs has been changed to fail closed when the biometric prompt cannot start.

This is not a declaration that Pay is safe. The thread-local coordinator, plaintext/persistent pending store, duplicate implementations, PIN/Accessibility-service work, uncorrelated events and SMS-as-evidence problems remain unresolved.

CAD assessment correction

CAD's PROGRESS.md is stale in places:

  • arch_gltf.rs already has a parallel mesh-build path above a threshold.
  • send_sync_audit.rs exists and is wired in cad/mod.rs to compile-check Solid, TriMesh and related Send/Sync requirements.
  • profile_benchmarks.rs exists.

Therefore it would be dishonest to “implement parallel mesh generation” again merely because the progress markdown still labels it deferred. The remaining CAD blocker is evidence: no benchmark output is checked in, and no Cargo toolchain is available in this execution environment to run the required tests/profiles.

Tranche 2: evidence, correlation and reconciliation

The previous tranche left Phase-2 items 2.7/2.8/2.9, Phase-3 items 3.2/3.3/3.4, Phase-7 item 7.8 and Phase-8 item 8.2 open. They are implemented here in the two pure crates, with tests, and validated by the isolated runner.

nigig-pay-domain

  • evidence.rs (new) — review items 0.4, B2, S-SMS. ObservedEvidence makes untrusted observation a type. EvidenceLog::record de-duplicates by a deterministic FNV-1a body digest, so a replayed or spoofed re-send changes nothing, and returns ConflictRequiresReconciliation when observations contradict each other or the amount disagrees with the intent. The log exposes suggests_completion/suggests_rejection hints and deliberately exposes no API that returns a settled state. Raw SMS bodies are not carried: only the digest is retained (review item 0.6).
  • reconciliation.rs (new) — review items 2.8, 6.3, U4. An explicit queue with typed ReconciliationReason, and ResolutionDecision / ResolutionAuthority so every closure is attributed to a provider API, a support agent or the account owner. SafeToRecreate clears the case without mutating the original intent, so nothing can re-dispatch it. ReconciliationItem::user_message is asserted by test to say "do not retry" and never the word "failed".
  • coordinator.rs — review items 2.7, 2.9, B3.
    • A reverse operation_id → intent index replaces the previous "whatever is current" correlation. intent_for_operation and record_observed_evidence_for_operation return None for an unknown correlation id, so an unmatched callback changes nothing.
    • A hard duplicate-dispatch guard: an intent that has reached the provider once can never call the gateway again, even if a caller walks the state machine back. TemporarilyUnavailable/Fatal are the only errors that refund the dispatch budget, because only they assert the dispatch never started; a timeout keeps it spent.
    • expire_local_confirmation_window replaces the 5-minute Submitted → Failed behaviour. It queues the intent for reconciliation and is a no-op against an already-settled payment, so no timer can disturb a confirmed transaction.

nigig-pay-storage

  • Schema migration 2payment_intent_evidence with a UNIQUE(intent_id, body_digest) constraint so replay protection also holds at the storage layer, and payment_reconciliation_queue so the backlog is durable rather than an in-memory UI concern. schema_version() is exposed for startup checks.
  • Evidence is stored separately from the normalized intent record with only a digest of the original message (review item 3.3), and a foreign key refuses evidence for an unknown intent.
  • Legacy import now also enqueues unprovable records durably and reports queued_for_reconciliation, so an imported record is not left in a state nothing revisits.
  • The worker gained durable evidence/queue commands with the same commit-before-acknowledge contract as save_durable.
  • Fault-injection tests (review item 3.4): unwritable path, corrupt database file, worker over an unusable path, and a rollback test proving a failed audit insert leaves no partially written intent row.
  • Restart-equivalence tests (review item 8.2): intent state, amount, correlation id, timestamps and the audit trail are reproduced after reopen, and the reconciliation queue survives a restart.

Quality ratchet (review item 7.8)

Both crates now carry #![cfg_attr(not(test), deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::indexing_slicing))].

The ratchet immediately caught a real defect: PaymentStorageWorker::spawn used .expect() and would have aborted the process if the OS refused a thread. It is now fallible and returns StorageError, so a resource-exhausted device reports an error instead of losing durability silently.

Validation actually run for this tranche

Run in an isolated toolchain against a copy of the two pure crates, outside the incomplete workspace dependency graph:

cargo fmt --all --check          # clean
cargo clippy --workspace --all-targets -- -D warnings   # clean
cargo test -p nigig-pay-domain   # 43 passed
cargo test -p nigig-pay-storage  # 17 passed

All 49 workspace Cargo.toml manifests still parse. The full workspace is still not asserted buildable here: the external Makepad and remaining Robius sibling path dependencies are absent, so nigig-pay-ui and nigig-pay were not compiled.

Still not claimed complete after this tranche

  1. Wiring the coordinator, evidence log and reconciliation queue into the Makepad nigig-pay-ui flow, and deleting the legacy thread-local PayFlowHandler. That is an application-wide migration that cannot be honestly done without compiling the UI crate.
  2. SQLCipher/Android Keystore encryption at rest (Phase 3.1). The schema and single-writer boundary are ready for it; the key hierarchy is not chosen.
  3. Phase 4 (draw_walk/blocking I/O), Phase 5 (platform gateway traits and the AccessibilityService legal review), and Phase 6 (UI rebuild) — all live in crates that cannot be built or tested in this environment.
  4. Removing PIN capture and scrubbing KEY_PIN from SharedPreferences (item 0.3). This is Java/JNI work in robius-ussd, untestable here.
  5. CAD items, unchanged from the previous tranche.

Tranche 3: closing every partial phase

This tranche deliberately started no new phase. It finished the items that tranches 1 and 2 had left partially done, so that Phases 1, 3, 7 and 8 are either complete for the two pure crates or explicitly blocked on the UI/Android crates that cannot be built here.

Phase 1 — build governance, now complete for the owned crates

  • 1.1 lockfiles. crates/nigig-pay-domain/Cargo.lock (49 packages) and crates/nigig-pay-storage/Cargo.lock (66 packages) are checked in and every build now runs --locked. The runner fails outright on a missing lockfile rather than silently generating one, and CI fails if a build mutates one.
  • Toolchain drift fixed. tools/test-rust-clean.sh hardcoded 1.97.1 as a default that could silently diverge from rust-toolchain.toml. It now parses the channel out of rust-toolchain.toml, so the runner cannot drift. All results below are from the declared toolchain, rustc 1.97.1.
  • 1.2 gates. The runner and CI now run tests, fmt --check, clippy -D warnings, the SQLCipher feature build, and the benchmarks.
  • 1.4 boundary enforced in CI. A job fails the build if either payment crate declares or imports Makepad. It inspects manifests and use/extern crate lines, not prose, so doc comments that merely mention Makepad do not produce a false failure.

Moving to 1.97.1 surfaced five real lints that 1.85 had not: two manual_is_multiple_of in money.rs, a dead field, a repeat().take(), and two derivable Default impls. The two Default impls are now explicitly annotated rather than derived, because in both cases the default is a security or state-machine decision that should stay visible at the site.

Phase 3 — secure repository, now complete except key provisioning

  • 3.1 encryption at rest. New encryption.rs plus an opt-in sqlcipher feature. open_encrypted/spawn_encrypted apply PRAGMA key as the first statement on the connection and then force a read, so a wrong key fails immediately as KeyRejected rather than later as "file is not a database". DatabaseKey redacts itself in Debug and zeroes its buffer on drop. The crate never derives, generates or persists a key: the host supplies a DatabaseKeyProvider backed by Keystore/Keychain. There is no unencrypted fallback path. A test reads the raw database file and asserts the recipient MSISDN does not appear in it, so the claim is measured rather than assumed.
  • 3.5 secure preferences. New preferences.rs replaces the ANDROID_DATA-derived marker file whose mere existence was the value. Every field defaults to the safe value and load_or_safe_default returns the safe default plus a diagnostic, so an unavailable or tampered store can never enable biometric pay.
  • 3.6 redaction. New redact.rs. PaymentIntent's derived Debug was a standing PII leak — any log!("{intent:?}") would have written a customer MSISDN to the device log. It now has a manual Debug that masks phone, name and identifiers, with a regression test asserting the raw values cannot appear. StorageError::MissingIntent no longer prints a full intent id.

Phase 7 — test strategy, now complete for the owned crates

  • 7.1/7.2 already satisfied. tests/mpesa_workflow.rs does not exist in this checkout and there are no trivial enum-equality tests to delete. The review text is stale here.
  • 7.3 not applicable. No NIGIG_TEST_PAY gate exists any more.
  • 7.7 benchmarks. benches/payment_benchmarks.rs covers money formatting, phone validation, fee quoting, the intent lifecycle, evidence digesting and evidence de-duplication. It uses the stable harness, so it runs on the pinned toolchain with no nightly and no extra dependency. benches/BASELINE.md records real measured output, because a benchmark with no checked-in result is not evidence.

Phase 8 — verification

  • 8.1 simulator. New simulator.rs provides ScriptedGateway (per-call scripted responses via interior mutability) and ScriptedBiometric, with no clock, thread or I/O.
  • 8.2 property tests. The simulator enumerates every permutation of a representative event set and asserts the review's invariants hold under all of them: no ordering causes a second dispatch; untrusted events alone never reach Confirmed or Rejected; an ambiguous sequence never becomes Failed; Confirmed is terminal under any later event; a duplicate provider confirmation is idempotent; a replayed SMS cannot accumulate into a false conflict; and a failed dispatch never reaches the provider twice.

Validation actually run for this tranche

Executed against copies of the two crates on the declared toolchain (rustc 1.97.1), outside the incomplete workspace graph:

domain : test --all-targets --locked   66 passed
         fmt --check                   clean
         clippy --all-targets -D warnings   clean
         bench --locked                ran, output recorded in BASELINE.md
storage: test --all-targets --locked   20 passed
         fmt --check                   clean
         clippy --all-targets -D warnings   clean
         test --features sqlcipher     25 passed
         clippy --features sqlcipher -D warnings   clean

All 49 workspace manifests parse; git diff --check is clean; the Makepad boundary check passes.

Tranche 4: Phase 1 complete

Phase 1 is now closed. Every item is either done or has an accepted ADR recording the decision.

1.1 Build governance — done

Lockfiles were already checked in last tranche. Added here: license = "MIT" on both payment crates (cargo-deny correctly reported them as unlicensed, which would have blocked any distribution review), and a version pin on the nigig-pay-domain path dependency, since a bare path dependency is a wildcard requirement.

1.2 Quality gates — done

Added deny.toml and a CI job running cargo-deny check over both payment crates. All four checks pass: advisories, bans, licences, sources.

The configuration bans the makepad-* crates from the payment dependency graph outright, so review item 1.4 is now enforced by tooling rather than by a grep. Sources are restricted to crates.io: a git dependency would bypass the lockfile guarantee.

1.3 Canonical ownership — done, and the review here was stale

REVIEWS/adr/0002-payment-crate-ownership.md records the layering. Re-checked the A5 "fork farm" claim: the duplicates are already gone. One copy each of parser.rs, store.rs, pending_store.rs, pay_flow_handler.rs, not the three the review's table lists.

Two of the three nigig-core defects are now fixed and tested, which the previous tranche had recorded as blocked:

  • B1 — classification silently discarded. store.rs parsed category, sub_category, status and confidence from disk and then overwrote them with Default::default(). Every user categorisation was lost on reload. Fixed by reassembling the ClassifiedTransaction. This also required stable storage tokens: the old code wrote display names like "Till / Buy Goods", which are not reversible. category_storage_token/category_from_storage and the status equivalents now handle this, with a legacy fallback so rows written before the fix still load.
  • B4 — lossy PSV escaping. clean/restore mapped | to ~ and reversed every ~, so "JOHN~DOE" loaded as "JOHN|DOE". Replaced with bijective backslash escaping that also handles newlines and carriage returns, proven by a round-trip test over awkward inputs.
  • B6 — f64 money. Not fixed. It is a type change across the observation path and its UI consumers, so it belongs with Phase 6 rather than being rushed here.

The claim that these could not be tested was wrong, and worth correcting: nigig-core is unbuildable only because of git-hosted Makepad and Robius dependencies. The three files holding these defects need just serde, chrono, one log! macro and one app_data_dir() helper. tools/test-mpesa-store-clean.sh supplies those two shims in a temporary crate and runs the tests under the declared toolchain. 9 tests pass, including two that reproduced the original defects before the fix.

1.4 Boundary enforcement — done

Enforced twice: a CI check on manifests and use/extern crate lines, and the deny.toml ban list.

1.5 Migration shims — done

nigig-pay/src/lib.rs carried seven re-export shims. Three (dir, tile_service, location) had zero callers anywhere and are deleted. The remaining four carry a caller count and a named migration target each, so they have a deletion plan rather than an open-ended lifetime.

1.6 Payment scope ADR — done

REVIEWS/adr/0001-payment-scope.md, accepted: Nigig Pay is a read-only tracker and launcher, not a payment processor, until an authorised provider integration exists. This is the decision the review said had to be made before further UI investment, and it makes the remaining phases coherent — a tracker does not need Phase 5's payment gateway, but it does need Phase 6's truthful states.

Security finding raised during this work

A live Cloudflare API token was found committed in README.md, present since the initial commit and pushed to a public remote. Removed from the file and recorded as R-SEC-001 in PAYMENT_RISK_REGISTER.md. Redaction does not revoke it: it remains in git history and must be rotated.

Validation actually run

domain : test --locked, fmt, clippy -D warnings, cargo-deny, bench   all pass
storage: test --locked, fmt, clippy -D warnings, cargo-deny,
         test --features sqlcipher                                   all pass
mpesa  : 9 tests via tools/test-mpesa-store-clean.sh                 all pass

49 manifests parse; git diff --check clean.

Tranche 5: Phase 3 complete, Phase 4 started

Phase 3 — now complete

3.3 retention and erasure. New retention.rs. The hard part here is that two obligations pull opposite ways: data minimisation says delete, financial record keeping says retain. The policy resolves them explicitly —

  • untrusted evidence expires 90 days after the payment settles;
  • settled records are held for a five-year floor;
  • intents that never reached the provider expire in 30 days;
  • anything awaiting reconciliation is never swept, because deleting it destroys the only record of money that may have left the account.

erase_payment_on_request honours a user deletion request the same way. It almost always returns EvidenceErasedRecordRetained: the parsed copy of the user's inbox goes, the accounting record stays. Full erasure only once the record is settled and past the floor; refused outright while in flight. Each refusal carries a reason string that explains rather than just denies.

3.4 backup, restore and corruption reporting. integrity_check and foreign_key_check surface damage as StorageError::Corrupt instead of letting a damaged ledger be trusted silently. backup_to uses SQLite's online backup API rather than copying the file, so a concurrent writer cannot produce a torn copy, and it refuses to back up a database that fails its integrity check — a backup of corruption can overwrite a good one. restore_from validates the candidate in a separate connection before touching the live database, so a failed restore cannot destroy a working ledger. Tests prove that with a missing backup and with a corrupt one.

Storage tests: 20 → 36.

Phase 4 — started, 4.14.3 done

The confirmed defects are fixed:

  • 4.1 expenses/mod.rs called reload() — a full parse of the transaction store from disk — as the first statement of draw_walk. Scrolling that page re-read the entire ledger continuously. Data now loads once in on_after_new; an invalidate()/reload_if_stale() pair handles refreshes on the event path.
  • 4.2 The SMS inbox scan was driven by a wall-clock check inside draw_walk. A "once per 8 seconds" test there is still evaluated every frame, and the scan lands mid-paint. It now runs on a real cx.start_interval timer on the event path, with the interval named (AUTO_SCAN_INTERVAL_SECONDS) rather than inline.
  • 4.3 update_summary walked the whole filtered list and rewrote label text every frame. It is now invalidated only in refresh_filtered — the single place the list can change — and applied in handle_event. It is fully out of the draw path, not merely guarded inside it.
  • 4.4 already satisfied: PaymentStorageWorker has owned the single off-thread writer since tranche 1.

A scan across every draw_walk in nigig-pay now reports no store I/O and no recomputation.

Supporting work in the domain crate. Phase 4 asks the UI to stop computing during draw, and Phase 6 asks it to receive a view state. Both need the arithmetic to exist somewhere pure first, so view_state.rs adds PeriodSummary and TransactionsViewState. This is testable in a way the inlined widget code was not, and the tests immediately pin two behaviours the original code got wrong:

  • totals use checked arithmetic and report overflowed instead of wrapping;
  • a net outflow returns None rather than a clamped zero, so spending more than was received cannot display as a balanced period.

Money also gained a Default (zero), written out rather than derived because an unset amount being nothing is a deliberate statement.

Domain tests: 66 → 75.

Validation actually run

domain : test --locked, fmt, clippy -D warnings, cargo-deny, bench   pass (75)
storage: test --locked, fmt, clippy -D warnings, cargo-deny,
         test --features sqlcipher                                   pass (36)
mpesa  : 7 tests via tools/test-mpesa-store-clean.sh                 pass

49 manifests parse; git diff --check clean.

Not verified: the nigig-pay widget changes for 4.14.3 are not compiled. That crate needs the git-hosted Makepad fork, which is not available here. The edits are small and local — a moved call, a dirty flag, a timer — but they are unbuilt, and a cargo check -p nigig-pay on a full developer checkout is required before they should be trusted.

Not falsely marked complete

After three tranches, the following remain open. Each is blocked on a crate that cannot be compiled in this environment, not on unwritten design.

  1. Phase 1 is complete. What remains of the old 1.3 item is pointing the surviving pay_flow_handler.rs at PaymentCoordinator and deleting the thread-local, which is Phase 6 work and requires compiling nigig-pay-ui.
  2. Phase 2.5 / 6.x wiring. The coordinator, evidence log and reconciliation queue are complete and tested, but no Makepad widget consumes them yet.
  3. Phase 3.1 key provisioning. SQLCipher works and is tested; the Android Keystore DatabaseKeyProvider implementation is Android-side work.
  4. Phase 4 — remaining work. 4.1-4.4 are implemented but the nigig-pay edits are uncompiled. Phase 4 cannot be called complete until cargo check -p nigig-pay passes on a full checkout.
  5. Phase 5 — platform adapters and the AccessibilityService legal review. The traits exist; the Android adapters and the Play-policy review do not.
  6. Phase 7.3/7.5 — UI tests in CI and an adversarial SMS corpus. Both need the UI crate and a consent-safe corpus that does not exist in this repository.
  7. Item 0.3 — PIN capture removal and KEY_PIN scrubbing. Java/JNI work in robius-ussd.
  8. B6 in nigig-core. B1 and B4 are fixed and tested (tranche 4). B6, the nine f64 money sites, is a type change that ripples into the UI consumers and belongs with Phase 6.
  9. CAD items, unchanged from tranche 1.

Repository path repair after upstream crate import

After pulling upstream commit e9616c328816eb9fcae44880e589a5b10fe9245c, the newly vendored Robius crates lived under crates/, while consumers still pointed four directories upward to sibling repositories. The workspace member list also lacked commas and duplicated robius-trigger.

This tranche repairs every Cargo.toml dependency path for the newly local:

  • robius-sms
  • robius-ussd
  • robius-fingerprinting
  • robius-contacts
  • robius-trigger

and makes all five workspace members explicit exactly once. A manifest parser check covers all 48 manifests, and a path-resolution check confirms every dependency on those five local Robius crates resolves to an in-repository Cargo.toml.

The full workspace is still not asserted buildable in this isolated checkout: it retains intentionally external makepad and robius/crates/{location,file-picker,directories,...} path dependencies. Those need to be vendored or provided by the documented super-workspace before a full cargo check --workspace claim is honest.

Tranche 6: Phase 4 verified, Phase 5 implemented

Phase 4 — now actually verified, not just written

Tranche 5 implemented 4.14.3 and then said plainly that the nigig-pay widget edits were unbuilt, because the crate needs the git-hosted Makepad fork and the Linux GUI system libraries.

That blocker was environmental, not architectural. Installing the packages tools/makepad-native-libs.sh already lists (libwayland-dev, libxkbcommon-dev, libx11-dev, libxi-dev, libxcursor-dev, libxrandr-dev, libgl1-mesa-dev, libpolkit-gobject-1-dev) makes the whole UI graph compile. Both commands the previous tranche listed as required now pass:

cargo check -p nigig-pay      pass (1m 00s, warnings only)
cargo check -p nigig-pay-ui   pass (8.97s, warnings only)

Phase 4 is therefore complete and verified, including the draw_walk changes. Nothing about the code changed in this tranche; what changed is that the claim is now evidence rather than assertion.

A note for whoever runs this next: the --check mode of tools/makepad-native-libs.sh is the fastest way to find out whether a checkout can build the UI crates at all.

Phase 5 — implemented in a new nigig-pay-platform crate

ADR 0002 reserved this crate and marked it "not yet created". It exists now, and ADR 0007 records the design and the one item that engineering cannot close.

5.1 traits and adapters. The crate is the only place in the payment stack that may name a platform SDK. CI enforces the boundary in both directions: the payment crates may not import Makepad, and domain/storage may not import jni, robius_ussd, robius_sms or robius_fingerprinting.

5.3 correlated events. SessionRegistry is single-flight. Every event is Accepted, Duplicate or Ignored, and PlatformEvent cannot be built without a CorrelationId. Closed session ids are retired permanently, so the scenario that motivated the module — an abandoned session's confirmation arriving while a different payment is on screen — is refused by construction. There is a test named after exactly that.

2.8 / 5.3 fail-closed classification. This is the part worth reading closely. classify_failure takes the failure and the DispatchProgress reached before it, and progress is the authority. The same TemporarilyUnavailable is safely retryable before the dial and ambiguous once the menu is being driven. The old code could not tell those apart, which is defect B3's mechanism. A property test asserts the invariant across the whole failure space: nothing that may have reached the provider ever authorises a fresh attempt.

5.4 no fakes in production. MockGateway is cfg-gated, is a compile_error! in a release build unless allow-mock-in-release is named explicitly, and stamps every session id with MOCK-. CI asserts the release build genuinely fails rather than trusting the comment.

5.5 unsafe and threading. The crate is #![forbid(unsafe_code)]. UssdGateway is !Send/!Sync by construction, so the JNI main-thread requirement is a compile error rather than a comment. The adapter leaves the pin field empty and a test asserts it.

5.6 the web claim is withdrawn. WebGateway refuses every call and maps to Fatal — "never sent" — which is accurate and owes no reconciliation. The alternative in the review was to implement real web adapters; there is no browser API that can drive USSD, so the claim goes instead.

7.5 adversarial SMS corpus. StrictMpesaSms is the payment-boundary reader, separate from nigig-core's permissive tracker parser on purpose — ADR 0007 explains why that is not the duplication ADR 0002 forbids. The corpus covers sender spoofing (MPESA-REFUNDS, FAKE-MPESA, MPESA.INFO, bare MSISDNs), forged code shapes, fractional and out-of-range amounts, oversized bodies, unicode and NUL injection, and replay. The closing test asserts the honest limit of all of it: a well-crafted forgery is still admitted as evidence, because the output type has no settled state to reach.

5.2 — not done, and not closeable here

The AccessibilityService legal/policy review is a business decision. ADR 0007 records it as blocking, states the Play-policy exposure and the termination risk, and names what has to happen before the rail is enabled: a human accepts the risk in writing, or the rail is replaced by an authorised provider API. USSD dispatch stays behind the default-off demo feature meanwhile.

If the review fails, ADR 0001's tracker/launcher position applies and the domain, storage and platform work all survive it. Only the dispatch adapter is discarded.

Validation actually run for this tranche

platform: test --locked (56), test --features ussd (64), fmt,
          clippy -D warnings (default and ussd), cargo-deny,
          mock-in-release guard asserted to fail                 pass
domain  : test --locked                                          pass (75)
storage : test --locked                                          pass (36)
nigig-pay / nigig-pay-ui : cargo check                           pass

cargo-deny on the platform crate reports advisories ok, bans ok, licenses ok, sources ok; the remaining output is duplicate-version warnings from the transitive Android/Windows dependency tree, which the other payment crates also carry.

The isolated runner gained a platform target, so this is reproducible with TEST_TARGET=platform ./tools/test-rust-clean.sh and runs in CI on every push that touches the crate.

Still not claimed complete after this tranche

  1. Phase 5.2 — the legal review. Blocking, recorded in ADR 0007.
  2. Phase 6 — the UI rebuild. The coordinator, evidence log, reconciliation queue and now the platform adapters all exist, but no Makepad widget consumes them yet and pay_flow_handler.rs still owns a thread_local!. This is the next phase, and it is the invasive one.
  3. Phase 3.1 key provisioning — SQLCipher works and is tested; the Android Keystore DatabaseKeyProvider is Android-side work.
  4. Item 0.3 — PIN capture removal and KEY_PIN scrubbing in the Java AccessibilityService. The Rust adapter now refuses to carry a PIN, which narrows but does not close it.
  5. B6 in nigig-core — the nine f64 money sites. Belongs with Phase 6 because the type change ripples into the UI consumers.
  6. Phase 7.3 — UI tests in CI. Needs a headless Makepad harness.
  7. CAD items, unchanged.

Tranche 7: Phase 6 truthful states, and Phase 7.3

The rule, made into a type

Phase 6's exit criterion is a sentence:

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

Prose cannot enforce that, and the previous code is the proof: the sheet decided its own status inline, in about a dozen places, from whatever local signal was nearest. That is how "no SMS within five minutes" became a red ✗ Payment failed for transfers that had actually settled (review item U4).

payment_view_state.rs makes the rule a type. PaymentPresentation has seven variants and no Success that untrusted evidence can reach, and no Failed that a missing SMS can reach:

Durable state Presented as May show success May offer retry
Confirmed + provider code Confirmed yes
Rejected Rejected no yes
Failed{DispatchNotStarted} NotSent no yes
Failed{UserCancelled} NotSent no yes
Failed{VerificationExpired} PendingConfirmation no no
Failed{Unknown} PendingConfirmation no no
UnknownNeedsReconciliation PendingConfirmation no no

The last three are the interesting rows. A widget now has to go through may_show_as_success() / may_show_as_failure(), so a dishonest label would have to be introduced in that one file, in front of its tests, rather than anywhere in 2,300 lines of widget code.

Two defence-in-depth cases are covered because they are cheap: a Confirmed intent carrying no provider reference reads as unknown rather than settled, and PaymentFailureReason::Unknown reads as pending rather than failed — an unclassified error is not evidence that nothing was sent.

6.2 confirmation. ConfirmationSummary makes every term the review lists a mandatory field, so a confirmation cannot be constructed with the fee or the total missing. It is built from the same compute_adjusted_amount quote that will be dispatched, so the screen cannot disagree with what is sent. The modal previously showed only recipient and amount; it now shows the rail, transaction fee, withdrawal fee where charged, what the recipient receives, what the payer pays, and how the payment is authorised.

material_digest() implements the re-display requirement. Consent is recorded against the exact terms shown, and re-checked when the user taps OK: if anything material moved in between, the authorisation is void and the screen is shown again rather than dispatched on. Cancelling clears it.

6.3 no auto-close. must_stay_open() is true for anything in flight or pending. The status line for a pending payment begins with the exact required wording, which a test asserts: Pending confirmation — do not retry.

6.5 receipt and user rights. PaymentViewState::evidence exposes stored observations, each labelled untrusted, and the action set for a pending payment offers ViewReceipt, ReportProblem and RequestDataDeletion (the storage side of deletion already exists in retention.rs).

Three real defects found and fixed while wiring this

These were not on the review's list. They were found by writing the tests.

  1. Bulk quotes silently under-charged. compute_bulk_costs used filter_map over the fee lookup, so any contact whose amount falls outside the published tariff was dropped from the fee total — the user saw a total missing that fee. It now reports them in contacts_without_a_fee, and is_complete() gates the quote.
  2. The batch total could overflow. contacts.iter().map(..).sum() panics in debug and wraps in release. A wrapped total is a quote for the wrong amount. It now saturates and reports amounts_overflowed.
  3. The cost preview showed unknown fees as free. Two unwrap_or(0) calls in refresh_cost_preview rendered an unpriceable amount as a zero-fee one. Both now propagate None to the label, which reads — (no published fee for this amount).

Item 3 is review defect B5 resurfacing in the preview path after tranche 1 fixed it in the dispatch path. Worth noting for whoever audits next: fixing a defect at one call site is not the same as fixing the defect.

A pre-existing test failure, diagnosed rather than deleted

money::tests::ksh_rounds_to_nearest_cent asserted format_ksh(1.005) == "1.01" and had been failing on every run. Confirmed pre-existing by stashing this tranche's changes and re-running against a clean tree.

The expectation is impossible, not the formatter wrong. 1.005 is not representable in binary floating point; the nearest f64 is 1.00499999999999989…, so the correctly rounded result really is 1.00.

This is review defect B6 (f64 for money) in miniature: the type cannot hold the value the caller meant, and no rounding logic recovers it. The test is rewritten to state that, alongside a companion test showing nigig_pay_domain::Money handling the same case exactly. Deleting the test would have hidden a live argument for finishing B6.

Phase 7.3 — UI tests now run in CI

The review asked for UI tests in CI and the removal of the NIGIG_TEST_PAY gate. The gate is already gone. What blocked CI was the Makepad link step, and the cause turned out to be a gap in tools/makepad-native-libs.sh: it listed the libraries needed to compile, but not libasound2-dev, libpulse-dev and libssl-dev, which are needed to link a test binary. The failure mode is confusing — cargo check succeeds and then unable to find library -lasound appears much later.

The helper now installs and checks all of them, and a new payment-ui-tests CI job runs cargo test -p nigig-pay-ui --lib plus cargo check on nigig-pay-ui, nigig-pay and nigig-mpesa. Every step was run locally before being committed.

Validation actually run for this tranche

domain  : test --locked (95), fmt, clippy -D warnings, bench     pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy         pass
platform: test --locked (56), --features ussd (64), fmt, clippy,
          mock-in-release guard                                  pass
nigig-pay-ui : cargo test --lib                                  pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa : cargo check             pass

Domain tests 75 → 95. UI tests 55 → 61, with the long-standing failure fixed.

Still not claimed complete after this tranche

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. Phase 6 wiring is partial. PaymentViewState exists, is tested, and the confirmation path uses ConfirmationSummary. What is not done is deleting the thread_local! PayFlowHandler and routing the sheet's status text through PaymentViewState end to end. The type now exists to make that mechanical, but it is a large edit and claiming it here would be the kind of overstatement this document exists to avoid.
  3. 6.6 bulk — still demo-gated. It needs an idempotent rail first, which is Phase 5.2's outcome.
  4. Phase 3.1 key provisioning — Android Keystore work.
  5. Item 0.3KEY_PIN scrubbing in the Java service.
  6. B6 in nigig-core — the nine f64 money sites. This tranche documented one of its symptoms rather than fixing the cause.
  7. CAD items, unchanged.

Tranche 8: the last false-success labels, and B6 where it matters

Two labels that still claimed settlement

Tranche 7 built PaymentViewState and said plainly that the sheet's remaining status strings were still composed inline. Two of them were not merely untidy — they were the Phase 6 violation itself, still shipping:

PayFlowEvent::ObservedEvidence  => "✓ SMS received — ref: {code} …"
PayFlowEvent::BulkItemObserved  => "✓ {current}/{total} sent"

A tick is a settlement claim. The first put one next to an SMS, which proves nothing and can be forged. The second counted dispatches as though they were confirmations, so a batch of five unknown outcomes rendered as ✓ 5/5 sent.

The vocabulary now lives in FlowStage, beside the presentation types, with one rule: a tick requires a provider confirmation. Untrusted evidence gets "Message received (ref X) — not yet confirmed. Do not send again until this is resolved." A finished batch reads "Batch finished — N sent, awaiting confirmation", because that is what actually happened.

Nine call sites in shared_pay_sheet.rs now derive their text instead of composing it. The two remaining in that file are fingerprint-sensor feedback, which is a real local event and not a settlement claim.

A CI guard backs this, and it was tested against the old code to confirm it fails when the regression is reintroduced — a guard that cannot fail is decoration.

B6, scoped to where it is real

The review lists nine f64 money sites. Before changing them all, each was measured, because "replace f64 everywhere" is easy to claim and easy to get wrong.

  • format_amount was checked exhaustively across every value from 0.00 to 2000.00 against an exact integer reference: zero mismatches. It also rounds 1234.567 and 2.675 correctly. This function is not the defect, and rewriting it would have been churn.
  • Accumulation is the defect. Summing 10,000 realistic amounts showed no visible drift, so the everyday risk is low — but f64 silently discards additions past 2^53 and offers no overflow signal at all. A corrupt or hostile record therefore produces a confident wrong total with nothing to indicate it.

So the fix went to the accumulators, not the formatter. update_summary in both nigig-mpesa and nigig-pay — an exact duplicate of each other, review item A5 — now builds a nigig_pay_domain::PeriodSummary from exact minor units, with checked arithmetic, and renders rather than a wrapped figure when overflowed is set. Conversion from the stored f64 is explicit and validated; a non-finite or negative stored amount is skipped rather than allowed to poison the total.

store.rs::totals() returns (f64, f64, usize) and has no callers. It is now #[deprecated] with the reason, rather than deleted, because nigig-core has no dependency on the domain crate and that is a separate change.

Two regression tests state the case executably: one asserts that f64 silently swallows 2^53 + 1 while the exact path keeps both entries, and one asserts that a genuinely impossible total is reported rather than shown.

Validation actually run for this tranche

domain  : test --locked (104), fmt, clippy -D warnings, bench    pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy         pass
platform: test --locked (56 + 64 ussd), fmt, clippy, mock guard  pass
nigig-pay-ui : cargo test --lib                                  pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core : cargo check pass
settlement-tick CI guard : verified to fail on the old code      pass

Domain tests 95 → 104.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. The thread_local! PayFlowHandler still exists. The status vocabulary is now domain-owned, which was the part that carried the correctness risk, but PayFlowHandler still holds flow state in a thread-local and PaymentCoordinator is still not what the widgets talk to. That remains the Phase 6 architectural item.
  3. B6 elsewhere. The parser's MpesaTransaction::amount, balance and cost are still f64 on disk. Changing the stored representation is a migration, not an edit, and belongs with the SQLite move.
  4. 6.6 bulk — demo-gated pending an idempotent rail.
  5. Phase 3.1 key provisioning, item 0.3 KEY_PIN, CAD — unchanged.

Tranche 9: batch accounting made bounded

A progress bar that could read 3/2

While working toward retiring the thread_local! handler, the bulk accounting turned out to hold a defect worth fixing on its own merits.

bulk_done was a bare usize incremented at eight independent match arms, none of them bounded, and bulk_current_idx added one more while an item was in flight. Nothing tied the counter to the batch's actual contents.

The USSD event pump does not guarantee one terminal event per item: an Error can be followed by a SessionEnded for the same session, and each arm increments separately. Two lines of arithmetic reproduce the result:

one item, two terminal events -> bulk_done=2/2
BulkItemDispatched reports        3/2   <-- exceeds total

That was reproduced before any code was changed, so the fix is answering a demonstrated defect rather than a suspected one.

PaymentBatch

The count is now a function of the batch's contents rather than of how many code paths happened to run. Each item carries exactly one BatchItemOutcome; settle refuses a second one instead of counting it. Progress is derived by walking the items, so finished > total is unrepresentable rather than merely unlikely.

Every former h.bulk_done += 1 now routes through one settle_bulk_item helper. A CI guard rejects direct increments, and was tested against the reintroduced bug to confirm it fails — the same discipline used for the settlement-tick guard in tranche 8.

Three further properties came out of putting this in the domain, where it can be tested at all:

  • finished is not succeeded. BatchProgress::confirmed counts only provider confirmations. A batch that finished with unknown outcomes reports "Batch finished — N confirmed, M awaiting confirmation. Do not re-send: check your M-Pesa statement first." and never carries a tick. Only an all-confirmed batch may be ticked. This is review item U7.
  • A pending item is never offered for re-run. safe_to_recreate returns only NotSent and Rejected items. Replaying an item that may already have settled is defect B3 at batch scope.
  • Cancelling does not rewrite history. cancel_remaining marks only items that never got an outcome. An already-dispatched item keeps its pending status, because cancelling a batch does not recall a request the provider may already hold.

A stray callback for an unknown item id is refused rather than counted, which is the batch-level counterpart of the correlation rule in ADR 0007.

Validation actually run for this tranche

domain  : test --locked (117), fmt, clippy -D warnings, bench    pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy         pass
platform: test --locked (56 + 64 ussd), fmt, clippy, mock guard  pass
nigig-pay-ui : cargo test --lib                                  pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core : check      pass
batch-counter guard : verified to fail on the reintroduced bug   pass
settlement-tick guard : still passing                            pass

Domain tests 104 → 117.

On the thread_local! handler, again

It is still there, and this tranche did not remove it.

What changed is that the state which carried the correctness risk — the batch accounting — no longer lives in it. PayFlowHandler now delegates to PaymentBatch for every outcome, so the thread-local holds queue plumbing and platform handles rather than the numbers a user is shown.

That is a smaller and more honest claim than "Phase 6 wiring complete". The remaining work is replacing PayFlowHandler itself with PaymentCoordinator, which changes the ownership of the USSD session and the biometric prompt, not just where a counter lives. It should be done as its own change with the device matrix in ADR 0007's exit criterion, not folded into a tranche that also touches accounting.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. PayFlowHandlerPaymentCoordinator. The last Phase 6 architectural item, scoped above.
  3. 6.6 bulk UI — the accounting is now correct and testable, but per-item review, limits, pause/resume and audit export are not built, and bulk stays demo-gated pending an idempotent rail.
  4. B6 on disk — the parser's stored f64 amount/balance/cost. A migration, not an edit.
  5. Phase 3.1 key provisioning, item 0.3 KEY_PIN, CAD — unchanged.

Tranche 10: the biometric gate the coordinator could not provide

Attempting the migration found a defect in the plan

The stated next step was replacing PayFlowHandler with PaymentCoordinator. Doing that means writing a BiometricAuthorizer adapter for Android. It cannot be written correctly, and finding out why is the substance of this tranche.

The trait's contract is blocking:

authenticate must be a blocking call that waits for user action. Returns Ok(()) on success, Err on failure/cancel.

robius_fingerprinting::authenticate issues a JNI call that shows the prompt and returns Ok(()) immediately. Verified by reading through sys/android/prompt.rs: it calls the Java authenticate static method and returns; the user's answer arrives later via next_event() as an AuthResult.

So an adapter can either:

  • return Ok(()) because the prompt opened — and authorize_and_dispatch then sends money before the user has touched the sensor. That is defect S2, the fail-open biometric, reintroduced through the type system; or
  • block, and deadlock the thread that must pump the callback.

Had the migration been done without noticing this, the result would have been a correct-looking refactor that silently reopened the review's most serious security finding.

AuthorizationAttempt

Authorization is a state machine, not a function call: Requested → Prompting → Granted | Denied, advanced by inbound signals. One rule, enforced by the type: only an explicit success grants dispatch.

  • A displayed prompt does not authorise. A touched sensor does not authorise. A non-match keeps the prompt up and stays retryable rather than denying.
  • A grant is bound to one intent, so a callback for an abandoned payment cannot authorise the current one.
  • A grant is spent on use, so one fingerprint cannot authorise two dispatches (defect B3), and a replayed success cannot re-arm it.
  • A late success arriving after a cancel is ignored rather than resurrecting the payment.
  • Backgrounding mid-prompt denies; it never silently allows.

dispatch_ussd now consumes a grant before dispatching and refuses without one. auth == None means no biometric gate was configured, which is deliberately distinct from an ungranted one.

A CI guard asserts the gate exists, and was tested against a version with the check disabled to confirm it fails.

The trait keeps its blocking contract for synchronous authorizers and test doubles, and now documents that Android must not use it. ADR 0007 carries the amendment.

Validation actually run for this tranche

domain  : test --locked (129), fmt, clippy -D warnings, bench    pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy         pass
platform: test --locked (56 + 64 ussd), fmt, clippy, mock guard  pass
nigig-pay-ui : cargo test --lib                                  pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core : check      pass
authorization guard : verified to fail with the gate removed     pass
batch-counter and settlement-tick guards : still passing         pass

Domain tests 117 → 129.

Where the coordinator migration now stands

The blocker is no longer unknown. PaymentCoordinator needs an authorization path that does not assume a blocking authorizer; AuthorizationAttempt is that path, and it is built and tested. What remains is a coordinator entry point that takes an already-granted attempt instead of calling biometric.authenticate() itself, and then moving the USSD session ownership across.

That is a coordinator API change. It should be designed against ADR 0007's device matrix — permission denial, cancellation, backgrounding, app restart, out-of-order callbacks — and none of that can be exercised here. Doing the API change blind and calling Phase 6 complete would be exactly the overstatement this document exists to prevent.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. PayFlowHandlerPaymentCoordinator. Unblocked in design, pending a coordinator API change and device testing.
  3. 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
  4. B6 on disk — the parser's stored f64 fields. A migration.
  5. Phase 3.1 key provisioning, item 0.3 KEY_PIN, CAD — unchanged.

Tranche 11: the coordinator accepts an external grant

The API change tranche 10 scoped

Tranche 10 established that Android cannot implement BiometricAuthorizer without reopening defect S2, and built AuthorizationAttempt as the replacement. It named the remaining work precisely:

a coordinator entry point that takes an already-granted attempt instead of calling biometric.authenticate() itself

That entry point now exists:

coordinator.dispatch_with_authorization(&id, &mut attempt)

It does not prompt, does not wait, and does not consult the injected BiometricAuthorizer at all. That last point is asserted rather than assumed: one test injects an authorizer that reports no hardware and returns an error, and shows that a granted attempt still dispatches. If the coordinator ever fell back to the injected authorizer, that test fails.

Three refusals are enforced, each with a test that fails when the gate is removed:

  • An unanswered prompt does not dispatch. PromptShown plus SensorEngaged is not consent. This is the exact shape the Android adapter would have produced.
  • A grant belongs to one payment. A grant naming a different intent is refused before anything else, so a stray callback cannot even fail the payment on screen — the victim intent is left untouched in AwaitingUserAuthorization rather than transitioned to Failed.
  • A grant is spent on use. One authorization, one dispatch (defect B3).

Denied, still-prompting and already-spent attempts all fail the intent closed and never reach the gateway.

Verifying the tests can fail

The gate was removed (if !attempt.consume()if false) and the suite re-run: 5 tests failed. That is the check that matters — a fail-closed test that passes against fail-open code is worthless.

Worth recording: a_grant_cannot_dispatch_twice still passed with the gate removed, because the coordinator's existing duplicate-dispatch budget caught it independently. Two unrelated mechanisms both refuse the second dispatch. That is defence in depth working as intended, and it is the reason that particular test is not sufficient evidence on its own.

Validation actually run for this tranche

domain  : test --locked (137), fmt, clippy -D warnings, bench    pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy         pass
platform: test --locked (56 + 64 ussd), fmt, clippy, mock guard  pass
nigig-pay-ui : cargo test --lib                                  pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core : check      pass
authorization / batch / settlement-tick guards                   pass
fail-open injection: 5 tests fail with the gate removed          pass

Domain tests 129 → 137.

What remains of the migration, precisely

The authorization half is done and verified. PayFlowHandler still owns:

  • the USSD session lifecycle (begin_transaction, cancel_session, and the next_event pump);
  • the pending-store writes that shadow the coordinator's repository;
  • the bulk queue plumbing.

Moving those means the coordinator owns the gateway session, which changes who cancels on teardown and who observes an out-of-order callback. ADR 0007's exit criterion asks for a device matrix — permission denial, cancellation, backgrounding, app restart, out-of-order callbacks — before that can be called done, and none of it can be exercised here.

The honest position: the part that could be built and proven without a device has been. The part that cannot is still open, and is smaller and better specified than it was two tranches ago.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. USSD session ownership — the remaining half of the coordinator migration, scoped above. Needs device testing.
  3. 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
  4. B6 on disk — the parser's stored f64 fields. A migration.
  5. Phase 3.1 key provisioning, item 0.3 KEY_PIN, CAD — unchanged.

Tranche 12: B6 at the persistence boundary

Why this and not the migration

B6 is the last unfinished P0 on the review's priority table. The obvious reading is "change f64 to Money on disk", which is a data migration and belongs with the SQLite move — it cannot be done safely here.

But the stored representation is not where the damage is. Measuring first, as in tranche 8:

  • The f64 round trip is exact. Every value tested — 1500.0, 1500.5, 0.1 + 0.2, 1e20, 12345678.995 — writes and re-reads bit-identically. The format is not losing money.
  • The read path is where the damage is. parts[2].parse().unwrap_or(0.0) turned any unreadable amount into a confident KSh 0 row.
  • And NaN gets in. "NaN" and "inf" both parse successfully as f64, and the writer emits them back out verbatim, so they survive a round trip. One corrupt SMS poisons every total it enters, permanently — once a NaN is in a sum, every comparison against that sum is false.

So the fix went to the boundary, not the format.

What changed

validate_money refuses three classes of value, on parse, on load and on save:

  • non-finiteNaN/inf, for the reason above;
  • negative — direction is carried by TransactionType, so a negative amount is a contradiction, not an outflow;
  • beyond 2^53 — past that an f64 cannot represent consecutive shillings, so arithmetic silently loses value.

A row whose amount cannot be trusted is now skipped with a log line rather than zeroed. Dropping a row is visible and recoverable by rescanning the inbox; a silent KSh 0 is neither. balance and cost are optional context rather than the ledger figure, so they degrade to None instead of discarding the row — but they can no longer be NaN either.

The writer refuses to persist an invalid amount at all, which is what stops a poisoned value becoming permanent.

One smaller parse fix: Ksh ,5 used to read as 5. A grouping separator before any digit means the text is not the expected shape, and guessing is worse than declining to read it.

The parser had no tests

Worth stating plainly: parser.rs — the file that decides what every observed amount is — had zero tests before this tranche. It now has 13, covering validation, extraction, end-to-end parsing, and unicode/NUL bodies that must not panic on a byte-index slice.

M-Pesa harness tests: 7 → 24.

Verifying the tests can fail

validate_money was reduced to Some(value) and the harness re-run: 4 tests failed. As in tranches 8 and 11, a guard that cannot fail is decoration.

Validation actually run for this tranche

mpesa harness : 24 tests via tools/test-mpesa-store-clean.sh       pass
domain  : test --locked (137), fmt, clippy -D warnings, bench      pass
storage : test --locked (36 + 41 sqlcipher), fmt, clippy           pass
platform: test --locked (56 + 64 ussd), fmt, clippy, mock guard    pass
nigig-pay-ui : cargo test --lib                                    pass (61)
nigig-pay-ui / nigig-pay / nigig-mpesa / nigig-core : check        pass
authorization / batch / settlement-tick guards                     pass
defect injection: 4 tests fail with validation removed             pass

What B6 still leaves open

The stored representation is unchanged: MpesaTransaction::amount is still f64 on disk. That is deliberate and now bounded — nothing untrustworthy can enter or leave the store, so the remaining exposure is precision within the validated range, which for whole-shilling M-Pesa amounts under 2^53 is none.

Converting the field to Money means rewriting the PSV format and migrating existing files. ADR 0002 is updated to record B6 as partially addressed rather than open, with the migration explicitly deferred to the SQLite move.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. USSD session ownership — the remaining half of the coordinator migration. Needs ADR 0007's device matrix.
  3. 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
  4. B6 stored representation — a data migration, scoped above.
  5. Phase 3.1 key provisioning, item 0.3 KEY_PIN, CAD — unchanged.

Tranche 13: item 0.3, PIN capture removed rather than hidden

Asked in review: "does the latest code have the PIN input field?"

It did. pin_input, form_pin and the reveal toggle were all present, and the default build hid them at runtime in on_after_new:

if !cfg!(feature = "demo") {
    self.form_pin.clear();
    self.view.view(cx, ids!(pin_visibility_row)).set_visible(cx, false);
}

That is weaker than it looks, and item 0.3 asks for removal rather than concealment:

  1. The DSL declared the control visible. Rust hid it afterwards. Anything that re-applies the UI definition — a hot reload, a re-instantiated sheet — brings it back. Defect B11 already names this class.
  2. Hidden is not absent. The TextInput stayed in the widget tree, and a hidden input can still be focused or filled programmatically.
  3. The field existed to be filled. form_pin was compiled into every build, so any path that reached it could populate it.

Now: the field does not exist unless demo is enabled

form_pin, pin_visible, the eye-toggle handler, the text-input handler, both PIN checks, the request construction and the try_build_ussd_request accessor are all #[cfg(feature = "demo")]. A default build has no field to write to, so nothing can populate it — not a stale UI definition, not a hot reload, not a hidden-but-present widget.

The DSL now also declares visible: false on the PIN input and its reveal button, so hidden is the default state rather than a runtime correction; a demo build unhides them on init. That closes the reload path in (1).

The one remaining runtime call is a deliberate belt-and-braces: if a hot-reloaded definition somehow surfaces the input, the non-demo arm wipes whatever was typed. It has no form_pin to clear, because there isn't one.

Proving absence rather than asserting it

A grep for form_pin proves nothing — it would pass just as happily against a field still present behind a runtime if. tools/check-no-pin-capture.sh is a compile probe instead: it inserts a reference to the field outside any cfg block and requires the default build to fail with no field `form_pin` while the demo build succeeds. The script restores the file on any exit path.

Verified in both directions: it passes on current code, and re-exposing the field un-gated makes it exit 1.

Validation actually run for this tranche

cargo check -p {nigig-pay-ui,nigig-pay,nigig-mpesa,nigig-core}   pass
cargo check -p {nigig-pay,nigig-mpesa} --features demo           pass
nigig-pay-ui : cargo test --lib                                  pass (61)
domain / storage / platform / mpesa harness                      pass
no-PIN-capture guard: verified to fail on a re-exposed field     pass

What item 0.3 still leaves open

0.3 has two halves. The Rust-side capture UI is now removed from default builds, which is this tranche. The Java-side KEY_PIN scrubbing in UssdAccessibilityService.java was already done in earlier work (remove(KEY_PIN) on completion, failure and handoff-disabled paths), so 0.3 is complete for the default product.

What remains is that a demo build still captures a PIN, by design — that path exists only for authorised device testing and is covered by the unresolved Play-policy decision in ADR 0007. If that decision goes against the USSD rail, the demo path and its PIN capture are deleted with it.

Still not claimed complete

  1. Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
  2. USSD session ownership — the remaining half of the coordinator migration. Needs ADR 0007's device matrix.
  3. 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
  4. B6 stored representation — a data migration.
  5. Phase 3.1 key provisioning, CAD — unchanged.

Required next commands

The two pure crates are fully validated by the isolated runner, which installs the toolchain declared in rust-toolchain.toml, runs --locked builds, and deletes its temporary environment on any exit:

TEST_TARGET=domain ./tools/test-rust-clean.sh
TEST_TARGET=storage ./tools/test-rust-clean.sh
TEST_TARGET=platform ./tools/test-rust-clean.sh
./tools/test-mpesa-store-clean.sh

From a complete developer checkout with all sibling Makepad/Robius dependencies available, the remaining crates still need:

cargo test -p nigig-build cad::send_sync_audit -- --nocapture
cargo test -p nigig-build cad::profile_benchmarks -- --nocapture --test-threads=1

The two Pay commands that were listed here as blockers — cargo check -p nigig-pay and cargo check -p nigig-pay-uinow pass and are recorded under tranche 6. Reproducing them needs the Makepad native libraries:

./tools/makepad-native-libs.sh --check     # reports what is missing
./tools/makepad-native-libs.sh --install   # opt-in, apt-based
cargo check -p nigig-pay
cargo check -p nigig-pay-ui
cargo test  -p nigig-pay-ui --lib          # runs in CI as of tranche 7

The CAD commands above remain unrun. Phase 6 is the next Pay phase.