Repository navigation
Feat/bmoni payments - #20
Merged
Merged
Conversation
BMONI takes two signatures with opposite methods: owner proof signs challenge text WITH the EIP-191 prefix, a proposal signs a raw 32-byte digest WITHOUT it. Signing a proposal the message way is the failure worth building against. The signature is structurally valid, decodes cleanly, carries a correct v byte, and is accepted and recorded — it just recovers to a different address, never matches the signer snapshot, and the transfer silently never executes. For Aide that is the worst shape a bug can take: it would have every reason to tell a worker their wages were sent, and no way to find out otherwise. BMONI has no endpoint that validates a signature without submitting it, so the published vector is the only proof available before real money depends on it. The module was written first with the WRONG method, so the red test was the actual bug — it recovered to 0x5e474E09 instead of 0xf39Fd6e5. It now matches BMONI's expected bytes exactly. Two wrong paths, not one. The docs name signMessage(getBytes(hash)) -> 0xC69f336b. Passing the hash as a string instead gives a THIRD address, 0x5e474E09, undocumented and failing the same silent way. Both are pinned, along with the digest regenerated from its preimage, so the comments cannot quietly stop being true. signProposalDigest is sync. There is nothing to await, and a Promise would imply a round trip that does not happen.
Aide holds the owner key for each user's smart wallet. BMONI's SDK would hold it in the Keystore or Secure Enclave, but it also puts a 4-to-6 digit PIN on screen for every signature — the interaction the spoken security phrase exists to replace, and one a blind user cannot complete. Aide is a web app besides, so the SDK was never available. The cost of that trade is that this key is ours to protect: whoever holds it can sign a transfer out of a worker's wallet. So it is AES-256-GCM sealed before it goes near Convex. GCM specifically, for the authentication rather than the secrecy. A tampered row must fail loudly, not decrypt to a DIFFERENT key — a different key signs proposals that recover to an address BMONI does not recognise, which it accepts, records, and never executes. That is the same silent failure as the signing module, one layer down. The random IV matters too: identical ciphertexts would reveal that two users share a key and let a stolen row be replayed onto another. BMONI_KEY_SECRET is read lazily so importing this cannot break a build that runs with no secrets, but there is no default and no fallback — without it, sealing and opening both refuse rather than quietly using a key that ships in the repository.
Four pieces, all of which exist because BMONI hands back plain strings and numbers with nothing to distinguish a unit or a state. amounts.ts — BMONI mixes decimal strings, minor-unit strings and plain numbers, none type-checked. Parsing fails closed: parseFloat takes the leading digits of "100abc", expands "1e3" to a thousand, and rounds "100.005" up to a kobo that never existed. Each is a plausible number Aide would then say out loud. Verified those against the real parseFloat rather than assuming them; the naive round-trip I expected to break turned out to be fine, so the tests pin the faults that are actually there. bmoni-client.ts — the transport rule, and it is deliberately asymmetric. BMONI has NO idempotency keys anywhere; wallet creation has no uniqueness guard at all. So reads may time out and retry, and money-moving calls may do neither. Aborting a slow transfer does not cancel it — it only destroys our record of having asked, which is how a "failed" withdrawal becomes a second one. Both money rules were written before their tests, so the tests were proved by mutation rather than trusted: adding a retry failed the retry test, adding a timeout failed the abort test, and the implementation was restored after each. bmoni.ts — operations in lifecycle order. Note the wallet precedes KYC, which is backwards from most providers: start-nigeria needs the wallet address. proposal-status.ts — an unknown status must never mean "completed". Exact and case-sensitive: "SUCCESS", "SETTLED" and lowercase "completed" all resolve to unknown, because a status we have not seen documented resolving to "your wages arrived" is the one error the listener cannot catch. Gap worth recording: BMONI's docs recover a 409 with findUserByEmail, but publish no such endpoint. There is no way to ask which user collided, so bmoniUserId must be persisted by us at creation and a 409 recovered from our own store.
Completes the money path: provision a user and smart wallet, activate the NGN rail, and pay out to a Nigerian bank — behind a seam that keeps Monnify the default. The double-payment guard is the important part. A payout is four calls (create, approve, fetch payload, sign) and BMONI has no idempotency keys, so a failure anywhere after the first leaves a REAL payout sitting at BMONI. Starting over would pay the same wages twice. The proposal id is therefore claimed in Convex the instant it exists, a later attempt resumes that proposal rather than creating another, and the claim refuses instead of overwriting when a different one is already in flight. The guard is released only on a terminal status. An UNKNOWN status deliberately does not release it: not knowing what happened to the last payout is exactly when a second must stay blocked. Provisioning is resumable for the same reason. bmoniUserId is persisted before anything depends on it, because a 409 on a repeat names the field that collided but gives no way to ask which user it was — and BMONI publishes no lookup endpoint, so losing that id spends the phone number for good. Wallet creation has no uniqueness guard at all, so if one may already exist the balances are read first and the flow refuses rather than making a second wallet. Both guards were written before their tests, so both were proved by mutation rather than trusted: removing the resume failed the resume test, clearing on unknown failed the fail-closed test. Two honest gaps, recorded in code rather than papered over. listInbound returns [] on BMONI — there is no wallet-level inbound history endpoint, only card history, and synthesising a credit from a balance delta would be Aide announcing a payment it cannot see. Provisioning refuses without a first/last name and phone, which Aide has never collected; splitting one free-text name would be a guess about someone's legal identity that fails KYC undebuggably later. The seam itself is explicit opt-in: anything that is not exactly "bmoni" resolves to Monnify, so a typo cannot move real wages onto a sandbox. lib/monnify.ts and lib/store/payments.ts are untouched.
The greeting only mentioned money when the balance was above zero, so a worker with nothing heard the same thing as a worker whose balance had not come back yet: nothing at all. Those are different facts and only one of them means "stop waiting for your wages". A sighted user resolves the ambiguity by glancing at the screen. Ours cannot, which is the case this whole product is built for. Zero is now stated plainly, and not described as ready to withdraw. Silence in the money line now means only that the figure missed the 2.5s budget the greeting allows so Aide can start speaking promptly — still deliberate, since announcing a failure on every slow morning would worry people about something routine. Both lines now format through formatNaira, so kobo are spoken rather than rounded away: a worker owed 12,000.50 who hears "twelve thousand naira" has been told a number that is not theirs. Extracted as a pure function so what Aide says can be asserted without standing up a route. Proved by mutation — restoring the skip-zero behaviour fails two of the five tests.
Probed the live sandbox rather than trusting the documented client types. Three responses did not match, and every mismatch was silent — the field came back undefined, got persisted, and would have surfaced later as a 404 on a path containing the word "undefined". - POST /v1/users wraps the user, and carries two uuids. `id` is the partner record; `bmoniUserId` is what every other endpoint paths on. Reading the top level gave undefined; picking `id` gives "User not found". - create-managed returns `id`/`walletAddress`, not `smartWalletId`/`address`. A wallet requested as CNGN comes back as NGN. - balances wraps in `balances`, `balance` is a decimal string, and each entry carries its own `error` inside an HTTP 200. - nigerian-banks wraps in `banks` with bankName/bankCode, and the codes are NIBSS institution codes (Wema 000017), not the NIP codes the payments page uses (Wema 035). The two lists are not interchangeable. - onboarding/status has no `status`: one field per rail, all independent. - onramp/vba/nigeria requires a bankAccountId UUID. Which one is still unknown, so it is a required argument rather than a guess. The deposit-account parser refuses BMONI's pooled house account. It is a real, payable NUBAN returned to any user without their own virtual account, and nothing in the response says it is not theirs — Aide would have read it aloud as the worker's own and an employer would have paid wages into a shared pool with no reference tying them to anyone. Nothing defaults. A balance parser that fell back to zero would tell a worker with money that they have none, out loud, with no screen to check.
… fix the sandbox forced
Walked Bunch Dillon (worker persona) through the whole lifecycle on the
shared sandbox: user, wallet, start-nigeria, virtual account, VBA link.
Nigerian onboarding does NOT require the full KYC wizard — kyc/readiness
still reports address and biometrics missing, but start-nigeria returns
200 and anchorStatus goes active, which issues a real per-user account:
1442873873 at PROVIDUS BANK in the worker's own name.
That run also disproved how the deposit parser picked an account. It took
accounts[0]. Live, the worker's own account is issued asynchronously — for
the first few seconds the list holds ONLY the pooled Bkey house account,
and afterwards it holds both with the worker's first. So accounts[0] is
sometimes right and sometimes the pooled account, with nothing in the
response distinguishing them. It now filters by identity instead.
Cards: POST /v1/users/{userId}/cards exists after all. Creation is on the
USER path while listing is on the SMART-WALLET path, which is why posting
to the wallet path 404s. Issuance is proven end to end and stops only at
E503 "Insufficient balance to cover the card fee" — the ₦1,000 fee against
a zero wallet, which needs BMONI to credit the sandbox balance.
The panel renders the real state read from BMONI, and keeps four states
apart: not switched on, no wallet, no card, and card being issued. Folding
"being issued" into "no card" would tell a worker mid-issuance that nothing
happened, and they would request a second card and be charged twice.
No card number is rendered or held. BMONI returns the PAN only from the
sensitive-data call it documents as never-log and never-cache, and
publishes no masked-PAN field on the list route — so any digits on the
card face would be invented ones.
The seam existed but nothing routed through it, so every number on the payments page still came from Monnify. The default is inverted now: BMONI unless AIDE_PAYMENT_PROVIDER explicitly says monnify, which stays only as a way back if the sandbox goes down mid-demo. - getBalance and ensureWallet in lib/store/payments.ts go through paymentProvider(). The store no longer knows which rail it is on. - Added getBalanceKobo to the seam, because the two providers know the figure in genuinely different ways. Monnify has no balance for a reserved account, so its adapter derives one by subtracting Aide's withdrawal ledger from confirmed credits. BMONI holds the wallet and has already debited every payout — subtracting the ledger again would understate a worker's money by everything they had ever withdrawn. - Added canListInbound. BMONI publishes a balance but no wallet-level inbound history, and returning [] would have rendered as "No payments received yet" — a claim that nobody has paid this worker, which is exactly what they cannot check for themselves. The page now says it can see the total but not the individual payments. - The demo worker is Bunch Dillon, the sandbox persona, and their wallet points at the BMONI virtual account rather than the Monnify one. Live: balance ₦0.00 from BMONI, account 4534076021 at PROVIDUS BANK, and the greeting says the zero out loud. The Monnify-mocking tests now name their rail instead of relying on the default. New tests cover the BMONI balance path end to end, including the two ways it must fail rather than answer zero: an unreachable provider, and a per-entry error arriving inside an HTTP 200.
The payments page was showing "I can see your balance, but I cannot list
the individual payments behind it yet", on my claim that BMONI publishes
no wallet-level history. That was wrong. The SDK documents an
EmbeddedWalletTransactionsResponse, which meant something had to serve it,
and it does:
GET /v1/users/{userId}/smart-wallets/{smartWalletId}/transactions
→ { transactions, page, perPage, total, pageCount, hasNextPage, hasPreviousPage }
So listInbound is real for BMONI, canListInbound is deleted along with the
message, and an empty history now honestly means no money has arrived.
The envelope is verified against the sandbox. The ITEM field names are not
— no wallet on the shared sandbox has a transaction yet, so they come from
the SDK's EmbeddedWalletTransaction model. That is why the parser throws on
a row it cannot read rather than skipping it: a skipped row is a payment
the worker is never told about, and they have no screen on which to notice
the list came back short. If the names are wrong the page says it could not
list the payments, which is recoverable; a quietly short list is not.
Only completed incoming credits count. Pending has not arrived and reversed
has left again, and Aide announces money as having landed.
….cookie The switch button did nothing at all. It wrote document.cookie = "aide-user=demo-employer" and reloaded, but aide-user is HMAC-signed and HttpOnly, so a hand-written value fails verification and the request stays whoever it already was. tests/unit/session.test.ts already covers this exact case under "refuses a hand-written cookie naming an account" — the cookie was doing its job, the button was asking it not to. POST /api/account/switch already existed and issues a correctly signed cookie. The buttons call it now. The reload also only happens if the switch succeeded, and a failure is shown. Reloading unconditionally is what made a broken switch look exactly like a working one: the page came back, the name had not changed, and there was nothing to tell you which of those two things had happened.
…personas The employer's payments page was still failing outright — no BMONI wallet existed for demo-employer, so the balance came back unavailable and the card panel said no-wallet. ClearVoice Media is Samson Jabo, the second sandbox persona, and now has a real virtual account: 3463455722 at PROVIDUS BANK, provisioned through the same lifecycle as the worker. The identities are seeded rather than hand-patched. Both demo accounts now carry the persona's first/last name, E.164 phone and BVN, so a fresh Convex deployment provisions itself instead of needing someone to know which UUIDs to paste in. seedDefaults backfills these onto accounts that predate the fields, and never overwrites a value that is already there — a real user who filled these in keeps what they entered. The BVN is now stripped by publicAccount, alongside the password hash. Adding it to Account had quietly put it in every /api/account response. It is not a profile field: it is the identifier every Nigerian bank uses to tie a person to their accounts, it cannot be reissued if it leaks, and it is enough on its own to begin an identity fraud. Nothing in the browser needs it — it exists so BMONI can run KYC, server-side. tests/unit/public-account fails if a sensitive field is added to Account without that decision being made again.
The "Account name" row rendered `summary.name` — the Aide profile. BMONI holds a different name for the same account: "Jabo Samson Joe" against a profile reading "ClearVoice Media", and "Dillon Bunch" against "Bunch Dillon". Whoever pays in sees the bank's name during their own name enquiry. Being told the account belongs to ClearVoice Media and then shown Jabo Samson Joe reads exactly like a mistyped account number, and abandoning the transfer is the sensible response — so the wages never arrive, for a reason nobody involved can diagnose. The worker cannot check the screen either; Aide would simply be stating a name no bank agrees with. accountName now comes from the deposit-account response, is stored on the wallet, and is what the page and the summary route present as the account holder. The parser throws if BMONI omits it rather than falling back to the profile name, because falling back is the bug. Optional on setProvisioned so the Monnify path, which has no separate bank-held name, is unchanged.
…h the seam The bank dropdown hardcoded thirteen NIP codes. Those are Monnify's vocabulary. Verified against the sandbox: bankCode 035 (Wema, NIP) returns "We could not verify this account", while 000017 (Wema, NIBSS) resolves. The failure names the ACCOUNT NUMBER, so the user retries a different account instead of a different bank, and the withdrawal never works. The list is now fetched from the provider — GET /api/payments/banks, 302 banks with the codes that rail actually accepts, and no hardcoded fallback, because a fallback of wrong codes is the bug. Nothing is pre-selected any more. With thirteen banks a default of GTBank was harmless; with 302 the default landed on AGOSASA MICROFINANCE BANK, a bank nobody chose, sitting under an account number someone had typed. On a screen you would catch that. Name enquiry also no longer runs until a bank is chosen, so it cannot verify against a default. Another silent shape mismatch: verify-nigerian-account returns the holder as `accountName`, not `accountHolderName`. Name enquiry was returning undefined — the one check a blind user has before approving where their wages go. Also moved onto the seam: the payout itself (lib/payments.ts still called Monnify's singleTransfer directly, so every withdrawal went to the wrong rail), destination resolution, beneficiary saving, the validate-account route, and the agent's save_beneficiary. Added a list_banks tool so the model looks codes up instead of recalling them. lib/store/events.ts is the one that would have hurt most. It is the poller that lets Aide say "₦5,000 just landed", and it was polling Monnify's reserved-account transactions — an endpoint with nothing in it under BMONI. Wages would have arrived in silence, which is the one thing a blind user cannot tell apart from a crash. An unknown payout outcome now maps to pending rather than success, so the "has it arrived" claim stays false unless the provider said completed. The Monnify webhook route is marked inert under BMONI so a quiet log there is not mistaken for no money arriving. And the create-user conflict path can recover now: GET /v1/users does exist. It matches on exact email AND phone and refuses unless exactly one row matches — the sandbox list is shared with other teams, and a loose match would bind a worker's wages to a stranger's wallet.
BMONI's development host does not perform name enquiry. It returns a plausible Nigerian name for ANY ten-digit number against ANY bank code, deterministic per number so two lookups agree and it reads exactly like a real answer. Measured against embedded-dev.bmoni.com: 4534076021 against six different banks -> "Dillon Bunch" every time 0123456789 / GTBANK -> "Ekon Orji" 1111111111 / GTBANK -> "Alika Alabi" 0000000000 / GTBANK -> "Amarachi Nwosu" Aide read that name aloud as the confirmation before moving money. Against a fabricator that is worse than having no check at all: a mistyped account number comes back as a confident "You are sending 20,000 naira to Ekon Orji", and the person listening cannot see the digits to know better. The green "✓ Account found" said the same thing on screen. VerifiedAccount now carries nameVerified, false whenever the BMONI base URL is not the production host — and false when the host is unknown, because "cannot tell" must not resolve to "verified". Monnify stays true: its sandbox rejects accounts that do not exist. When the name is not verified it is never shown and never spoken. The confirmation reads the digits back one at a time instead — "to account 3 4 6 3 4 5 5 7 2 2" — because those are the thing the user supplied and can actually check. The line is built server-side so the screen and the spoken path cannot drift apart on the one sentence that matters. Saved beneficiaries captured from a fabricating endpoint are marked unverified too; sitting in our database for a week does not make a name true. save_beneficiary refuses to store one rather than launder it into something that later looks user-confirmed. Also corrects an earlier misreading of mine: bankCode 035 failing was an invalid code being rejected, not evidence that account lookup was real.
Surfaces whatever BMONI returns, so Aide behaves on the development host exactly as it will on production, rather than second-guessing the sandbox. "✓ Account found: NAME" is back. The measurement stands and is recorded where it will be found: the dev host does not perform name enquiry, and returns a plausible Nigerian name for any ten-digit number against any bank code, deterministic per number so repeated look-ups agree. Aide is therefore capable of speaking a fabricated account holder's name on this host. That costs nothing while the sandbox wallets hold no money and the personas are not real people. The guard is kept and still tested, behind BMONI_STRICT_NAME_ENQUIRY=true, so the day this points at a funded wallet it is one variable rather than a rewrite. Under it, an unverified name is never shown or spoken and the confirmation reads the account digits back instead. Kept regardless of the flag: the digits are spelled out one at a time even alongside a verified name. That is accessibility, not trust — a screen reader renders "3463455722" as "three billion, four hundred and sixty-three million…", which cannot be checked against the number someone meant to type.
The Web Speech API has no Pidgin model — `pcm` is not a language tag it
accepts — so pidgin was arriving transcribed by an English recognizer as
the nearest English words: "dey" as "day", "wetin" as "we tin", "sabi" as
"savvy", "comot" as "come out", "abeg" as "I beg". The agent then read
English that meant something else, and asked the user to repeat themselves.
Three repairs, in increasing order of how much they matter:
1. maxAlternatives is now 4, and the most Pidgin-looking candidate wins.
The recognizer ranks by an English language model, so the right words are
often present but second — it offers "we tin day happen" ahead of "wetin
dey happen". The first alternative still wins ties and wins unless
another is strictly more Pidgin; the recognizer is better at English
than this heuristic is.
2. A repair pass on what survives, applied to finals only. Interims are
redrawn several times a second and never acted on, so rewriting them
would just make the on-screen text churn under the user.
3. The agent is told it will receive Pidgin, given the specific mishearings
and the phrases that matter ("wetin dey my account", "I wan comot
money"), and told to reply in whichever language the user used — never
to ask them to repeat in English, correct them, or remark on it.
Every repair rule is anchored on context that rules out the English
reading, because this text becomes a withdrawal. "day" only becomes "dey"
after a pronoun or before a possessive, so "every day" and "three day"
survive. "one" only becomes "wan" after a pronoun AND when no number
follows, so "send one thousand naira" is untouched — that one would have
cost a worker money they cannot see to check. The tests assert the
amount-preserving cases first and by name.
The prompt carries the matching rule: read through mangled words, but never
guess through a mangled AMOUNT — ask for the figure again instead.
…onas The payments page still showed Monnify. Four separate faults, each of which alone was enough to keep it there. start-nigeria requires THREE fields, not two. The client never sent `ngnWalletIndex`, so the Nigerian rail never started, so no virtual account was ever issued, so the deposit list held nothing but BMONI's pooled house account — which the parser (correctly) refuses to hand out. Nothing in any BMONI response carries that index, so it is derived, and only for the single wallet Aide provisions; with more than one it refuses rather than guessing, because a wrong index routes a worker's wages to a wallet Aide cannot read and nothing anywhere reports an error. ensureWallet returned early on `status === "active"` alone. That is true of every wallet Monnify ever made, so a switched deployment stayed permanently half-migrated: the old NUBAN on screen, no BMONI user, and every BMONI read failing at once so it looked like an outage. Wallets now record who provisioned them, and "active" means active for the provider that is live. The withdrawal ledger had the same shape of bug with a worse symptom. Three Monnify-era payouts sat at "processing" for ever, and Aide reads that list aloud — a blind worker asking what they had sent would hear transfers to names they may not know, described as still in flight, with no screen on which to notice the dates were from another era. Rows now name their provider; rows that name none are Monnify's. Personas: the demo accounts carry the sandbox identities whose BVNs actually resolve, phone numbers derived per deployment (the documented sandbox numbers are registered to other teams and answer 409 for ever), and an email for the employer, without which BMONI will not create a user at all. Wallet creation still has no uniqueness guard at BMONI, so the pre-create check reads balances first and fails closed on anything it cannot positively read as "no wallet yet" — a wrong "no" makes a second wallet and deposits route to whichever one we did not record.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.