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.
66 KiB
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 1–4: implemented foundation
- Added a
nigig-pay-storageworkspace 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_durablewaits 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-domainto 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_subto prevent quote arithmetic overflow or a silently clamped fee deduction. - Changed the state-machine contract so a
Submittedpayment cannot transition toFailedmerely because local confirmation did not arrive. It must useUnknownNeedsReconciliation. - 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-uihas a default-offdemofeature; USSD dispatch, auto retry and bulk pay are disabled without it.- The fail-open biometric branch in
pay_flow_handler.rshas 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.rsalready has a parallel mesh-build path above a threshold.send_sync_audit.rsexists and is wired incad/mod.rsto compile-checkSolid,TriMeshand related Send/Sync requirements.profile_benchmarks.rsexists.
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.ObservedEvidencemakes untrusted observation a type.EvidenceLog::recordde-duplicates by a deterministic FNV-1a body digest, so a replayed or spoofed re-send changes nothing, and returnsConflictRequiresReconciliationwhen observations contradict each other or the amount disagrees with the intent. The log exposessuggests_completion/suggests_rejectionhints 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 typedReconciliationReason, andResolutionDecision/ResolutionAuthorityso every closure is attributed to a provider API, a support agent or the account owner.SafeToRecreateclears the case without mutating the original intent, so nothing can re-dispatch it.ReconciliationItem::user_messageis 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 → intentindex replaces the previous "whatever is current" correlation.intent_for_operationandrecord_observed_evidence_for_operationreturnNonefor 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/Fatalare the only errors that refund the dispatch budget, because only they assert the dispatch never started; a timeout keeps it spent. expire_local_confirmation_windowreplaces the 5-minuteSubmitted → Failedbehaviour. It queues the intent for reconciliation and is a no-op against an already-settled payment, so no timer can disturb a confirmed transaction.
- A reverse
nigig-pay-storage
- Schema migration 2 —
payment_intent_evidencewith aUNIQUE(intent_id, body_digest)constraint so replay protection also holds at the storage layer, andpayment_reconciliation_queueso 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
- Wiring the coordinator, evidence log and reconciliation queue into the
Makepad
nigig-pay-uiflow, and deleting the legacy thread-localPayFlowHandler. That is an application-wide migration that cannot be honestly done without compiling the UI crate. - 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.
- 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.
- Removing PIN capture and scrubbing
KEY_PINfrom SharedPreferences (item 0.3). This is Java/JNI work inrobius-ussd, untestable here. - 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) andcrates/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.shhardcoded1.97.1as a default that could silently diverge fromrust-toolchain.toml. It now parses the channel out ofrust-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 cratelines, 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.rsplus an opt-insqlcipherfeature.open_encrypted/spawn_encryptedapplyPRAGMA keyas the first statement on the connection and then force a read, so a wrong key fails immediately asKeyRejectedrather than later as "file is not a database".DatabaseKeyredacts itself inDebugand zeroes its buffer on drop. The crate never derives, generates or persists a key: the host supplies aDatabaseKeyProviderbacked 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.rsreplaces theANDROID_DATA-derived marker file whose mere existence was the value. Every field defaults to the safe value andload_or_safe_defaultreturns 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 derivedDebugwas a standing PII leak — anylog!("{intent:?}")would have written a customer MSISDN to the device log. It now has a manualDebugthat masks phone, name and identifiers, with a regression test asserting the raw values cannot appear.StorageError::MissingIntentno 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.rsdoes 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_PAYgate exists any more. - 7.7 benchmarks.
benches/payment_benchmarks.rscovers 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.mdrecords real measured output, because a benchmark with no checked-in result is not evidence.
Phase 8 — verification
- 8.1 simulator. New
simulator.rsprovidesScriptedGateway(per-call scripted responses via interior mutability) andScriptedBiometric, 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
ConfirmedorRejected; an ambiguous sequence never becomesFailed;Confirmedis 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.rsparsedcategory,sub_category,statusandconfidencefrom disk and then overwrote them withDefault::default(). Every user categorisation was lost on reload. Fixed by reassembling theClassifiedTransaction. 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_storageand the status equivalents now handle this, with a legacy fallback so rows written before the fix still load. - B4 — lossy PSV escaping.
clean/restoremapped|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 —
f64money. 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.1–4.3 done
The confirmed defects are fixed:
- 4.1
expenses/mod.rscalledreload()— a full parse of the transaction store from disk — as the first statement ofdraw_walk. Scrolling that page re-read the entire ledger continuously. Data now loads once inon_after_new; aninvalidate()/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 realcx.start_intervaltimer on the event path, with the interval named (AUTO_SCAN_INTERVAL_SECONDS) rather than inline. - 4.3
update_summarywalked the whole filtered list and rewrote label text every frame. It is now invalidated only inrefresh_filtered— the single place the list can change — and applied inhandle_event. It is fully out of the draw path, not merely guarded inside it. - 4.4 already satisfied:
PaymentStorageWorkerhas 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
overflowedinstead of wrapping; - a net outflow returns
Nonerather 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.1–4.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.
- Phase 1 is complete. What remains of the old 1.3 item is pointing the
surviving
pay_flow_handler.rsatPaymentCoordinatorand deleting the thread-local, which is Phase 6 work and requires compilingnigig-pay-ui. - Phase 2.5 / 6.x wiring. The coordinator, evidence log and reconciliation queue are complete and tested, but no Makepad widget consumes them yet.
- Phase 3.1 key provisioning. SQLCipher works and is tested; the Android
Keystore
DatabaseKeyProviderimplementation is Android-side work. - Phase 4 — remaining work. 4.1-4.4 are implemented but the
nigig-payedits are uncompiled. Phase 4 cannot be called complete untilcargo check -p nigig-paypasses on a full checkout. - Phase 5 — platform adapters and the AccessibilityService legal review. The traits exist; the Android adapters and the Play-policy review do not.
- 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.
- Item 0.3 — PIN capture removal and
KEY_PINscrubbing. Java/JNI work inrobius-ussd. - B6 in
nigig-core. B1 and B4 are fixed and tested (tranche 4). B6, the ninef64money sites, is a type change that ripples into the UI consumers and belongs with Phase 6. - 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-smsrobius-ussdrobius-fingerprintingrobius-contactsrobius-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.1–4.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
- Phase 5.2 — the legal review. Blocking, recorded in ADR 0007.
- 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.rsstill owns athread_local!. This is the next phase, and it is the invasive one. - Phase 3.1 key provisioning — SQLCipher works and is tested; the Android
Keystore
DatabaseKeyProvideris Android-side work. - Item 0.3 — PIN capture removal and
KEY_PINscrubbing in the Java AccessibilityService. The Rust adapter now refuses to carry a PIN, which narrows but does not close it. - B6 in
nigig-core— the ninef64money sites. Belongs with Phase 6 because the type change ripples into the UI consumers. - Phase 7.3 — UI tests in CI. Needs a headless Makepad harness.
- 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.
- Bulk quotes silently under-charged.
compute_bulk_costsusedfilter_mapover 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 incontacts_without_a_fee, andis_complete()gates the quote. - 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 reportsamounts_overflowed. - The cost preview showed unknown fees as free. Two
unwrap_or(0)calls inrefresh_cost_previewrendered an unpriceable amount as a zero-fee one. Both now propagateNoneto 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
- Phase 6 wiring is partial.
PaymentViewStateexists, is tested, and the confirmation path usesConfirmationSummary. What is not done is deleting thethread_local!PayFlowHandlerand routing the sheet's status text throughPaymentViewStateend 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. - 6.6 bulk — still demo-gated. It needs an idempotent rail first, which is Phase 5.2's outcome.
- Phase 3.1 key provisioning — Android Keystore work.
- Item 0.3 —
KEY_PINscrubbing in the Java service. - B6 in
nigig-core— the ninef64money sites. This tranche documented one of its symptoms rather than fixing the cause. - 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_amountwas checked exhaustively across every value from0.00to2000.00against an exact integer reference: zero mismatches. It also rounds1234.567and2.675correctly. 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
f64silently 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
- The
thread_local!PayFlowHandlerstill exists. The status vocabulary is now domain-owned, which was the part that carried the correctness risk, butPayFlowHandlerstill holds flow state in a thread-local andPaymentCoordinatoris still not what the widgets talk to. That remains the Phase 6 architectural item. - B6 elsewhere. The parser's
MpesaTransaction::amount,balanceandcostare stillf64on disk. Changing the stored representation is a migration, not an edit, and belongs with the SQLite move. - 6.6 bulk — demo-gated pending an idempotent rail.
- 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:
finishedis notsucceeded.BatchProgress::confirmedcounts 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_recreatereturns onlyNotSentandRejecteditems. Replaying an item that may already have settled is defect B3 at batch scope. - Cancelling does not rewrite history.
cancel_remainingmarks 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
PayFlowHandler→PaymentCoordinator. The last Phase 6 architectural item, scoped above.- 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.
- B6 on disk — the parser's stored
f64amount/balance/cost. A migration, not an edit. - 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:
authenticatemust 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 — andauthorize_and_dispatchthen 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
PayFlowHandler→PaymentCoordinator. Unblocked in design, pending a coordinator API change and device testing.- 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
- B6 on disk — the parser's stored
f64fields. A migration. - 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.
PromptShownplusSensorEngagedis 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
AwaitingUserAuthorizationrather than transitioned toFailed. - 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 thenext_eventpump); - 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
- USSD session ownership — the remaining half of the coordinator migration, scoped above. Needs device testing.
- 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
- B6 on disk — the parser's stored
f64fields. A migration. - 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
f64round 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 confidentKSh 0row. - And
NaNgets in."NaN"and"inf"both parse successfully asf64, and the writer emits them back out verbatim, so they survive a round trip. One corrupt SMS poisons every total it enters, permanently — once aNaNis 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-finite —
NaN/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
f64cannot 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
- USSD session ownership — the remaining half of the coordinator migration. Needs ADR 0007's device matrix.
- 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
- B6 stored representation — a data migration, scoped above.
- 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:
- 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.
- Hidden is not absent. The
TextInputstayed in the widget tree, and a hidden input can still be focused or filled programmatically. - The field existed to be filled.
form_pinwas 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
- Phase 5.2 — the AccessibilityService legal review. Blocking, ADR 0007.
- USSD session ownership — the remaining half of the coordinator migration. Needs ADR 0007's device matrix.
- 6.6 bulk UI — accounting is correct; per-item review, limits, pause/resume and audit export are not built.
- B6 stored representation — a data migration.
- 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-ui — now 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.