Phase 1 of NIGIG_PAY_CONSOLIDATED_REVIEW.md is now closed.
1.1 build governance:
- Declare license = "MIT" on both payment crates. cargo-deny correctly
reported them as unlicensed, which would block any distribution review.
- Version-pin the nigig-pay-domain path dependency; a bare path dependency
is a wildcard requirement.
1.2 quality gates:
- Add deny.toml and a CI job running cargo-deny over both payment crates.
Advisories, bans, licences and sources all pass. The config bans the
makepad-* crates outright and restricts sources to crates.io.
1.3 canonical ownership (ADR 0002):
- Record the domain/storage/platform/UI layering and its one-way deps.
- The review's A5 "fork farm" table is stale: one copy each of parser.rs,
store.rs, pending_store.rs and pay_flow_handler.rs, not three.
- Fix B1: store.rs parsed category, sub_category, status and confidence
from disk then overwrote them with Default::default(), losing every user
categorisation on reload. Persistence also wrote display names, which are
not reversible, so this adds stable storage tokens with a legacy-display
fallback so existing rows still load.
- Fix B4: clean/restore mapped '|' to '~' and reversed every '~', so
"JOHN~DOE" loaded as "JOHN|DOE". Replaced with bijective backslash
escaping covering the separator, newlines and carriage returns.
- B6 (f64 money) deliberately deferred to Phase 6: it is a type change that
ripples into UI consumers.
These were previously recorded as untestable because nigig-core is not a
workspace member. That was wrong: the three files involved need only serde,
chrono, one log! macro and one app_data_dir() helper. The new
tools/test-mpesa-store-clean.sh supplies those shims in a throwaway crate
and runs 9 tests, two of which reproduced the defects before the fix.
1.4 boundary: enforced twice, by a CI manifest/import check and by the
deny.toml ban list.
1.5 shims: three re-exports in nigig-pay/src/lib.rs had zero callers and are
deleted. The remaining four carry a caller count and a named migration
target so they have a deletion plan rather than an open-ended lifetime.
1.6 scope (ADR 0001): accepted that 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 required before further UI work.
SECURITY: a live Cloudflare API token was found committed in README.md,
present since the initial commit and pushed to a public remote. Removed and
recorded as R-SEC-001 in PAYMENT_RISK_REGISTER.md. Redaction does not revoke
it; it remains in history and must be rotated by the owner.
Validated on rustc 1.97.1: domain and storage each pass test --locked, fmt,
clippy -D warnings and cargo-deny; storage also passes --features sqlcipher;
domain benches run; 9 M-Pesa store tests pass. 49 manifests parse.
No new phase was started. This finishes every item tranches 1-2 left
partial, so Phases 1, 3, 7 and 8 are complete for the two pure crates or
explicitly blocked on crates that cannot be built here.
Phase 1 (build governance):
- Check in Cargo.lock for both payment crates and build with --locked.
.gitignore excluded them, which would have made item 1.1 a false claim.
- tools/test-rust-clean.sh now reads the channel from rust-toolchain.toml
instead of a hardcoded default that had already drifted, and gates on
fmt, clippy -D warnings, the sqlcipher feature and the benchmarks.
- CI enforces lockfile presence/freshness and a Makepad-boundary check that
inspects manifests and use/extern lines rather than prose (1.4).
Building on the declared 1.97.1 toolchain surfaced five lints 1.85 missed;
all are fixed. The two Default impls are annotated rather than derived
because each encodes a security or state-machine decision.
Phase 3 (secure repository):
- 3.1 encryption at rest: new encryption.rs and an opt-in sqlcipher feature.
PRAGMA key is applied first and verified by a forced read, so a wrong key
fails as KeyRejected rather than as corruption. DatabaseKey redacts its
Debug and zeroes on drop. No key is ever derived or persisted here, and
there is no unencrypted fallback. A test asserts the recipient MSISDN is
absent from the raw database bytes.
- 3.5: preferences.rs replaces the ANDROID_DATA marker file whose existence
was the value; every field fails safe.
- 3.6: redact.rs. PaymentIntent's derived Debug leaked a customer MSISDN
into any log line; it now masks phone, name and ids, with a regression
test. StorageError no longer prints a full intent id.
Phase 7:
- 7.7 benchmarks over money, validation, fees, the intent lifecycle and
evidence handling, using the stable harness so they run on the pinned
toolchain. BASELINE.md records measured output.
- 7.1/7.2/7.3 verified already satisfied; the review text is stale.
Phase 8:
- 8.1 simulator.rs with a scripted gateway and biometric, no clock or I/O.
- 8.2 property tests over every permutation of a representative event set:
no ordering dispatches twice, untrusted events never settle a payment,
ambiguity never becomes failure, Confirmed is terminal, duplicate
confirmations are idempotent, and replayed SMS cannot fake a conflict.
Validated on rustc 1.97.1 outside the incomplete workspace graph:
domain 66 tests, storage 20 tests, storage+sqlcipher 25 tests, fmt clean,
clippy -D warnings clean on both crates and both feature sets, benches run.
49 manifests parse; git diff --check clean.
Still unclaimed and blocked on uncompilable crates: UI wiring, Keystore key
provisioning, Phase 4 draw_walk I/O, Phase 5 adapters and legal review, the
Java PIN scrub, and B1/B4/B6 in nigig-core.
Implements the review phases left open by the previous tranche in the two
pure crates, per REVIEWS/NIGIG_PAY_CONSOLIDATED_REVIEW.md.
nigig-pay-domain:
- evidence.rs: ObservedEvidence/EvidenceLog make SMS and USSD output an
untrusted observation that can raise a reconciliation flag but can never
settle a payment (0.4, B2). Replays are de-duplicated by a deterministic
body digest; only the digest is retained, never the raw body (0.6).
- reconciliation.rs: explicit queue with typed reasons and attributed
resolutions (2.8, 6.3). SafeToRecreate never mutates or re-dispatches the
original intent. The user-facing message says "do not retry" and never
"failed" (U4).
- coordinator.rs: operation_id -> intent index so an unmatched callback
changes nothing (2.7); hard duplicate-dispatch guard where only a
never-started dispatch refunds the budget (2.9, B3);
expire_local_confirmation_window replaces Submitted -> Failed on timeout
and cannot disturb a settled payment.
nigig-pay-storage:
- schema migration 2 adds payment_intent_evidence with a unique digest
constraint and a durable payment_reconciliation_queue (3.2, 3.3).
- legacy import now durably queues unprovable records.
- fault-injection tests for unwritable path, corrupt file, unusable worker
path and audit-insert rollback (3.4); restart-equivalence tests (8.2).
Both crates deny unwrap/expect/panic/indexing outside tests (7.8). That
ratchet caught PaymentStorageWorker::spawn aborting on thread-spawn
failure; it is now fallible and returns StorageError.
Validated in an isolated toolchain over copies of the two pure crates:
cargo fmt --all --check clean, cargo clippy --workspace --all-targets
-D warnings clean, 43 domain tests and 17 storage tests passing, and all
49 workspace manifests parse. The full workspace is not asserted buildable
here: external Makepad/Robius sibling path dependencies are absent, so the
Makepad UI wiring, SQLCipher/Keystore encryption and the Java PIN scrub
remain explicitly unclaimed.
