feat(wallet): custody foundation - every table, one migration, every route - #92
Merged
Conversation
zaxovaiko
force-pushed
the
feat/wallet-custody-foundation
branch
from
August 21, 2026 07:27
2e52985 to
8d19b00
Compare
This was referenced Aug 21, 2026
zaxovaiko
marked this pull request as ready for review
August 21, 2026 18:14
zaxovaiko
requested review from
agniev-a-hub,
damianrzepka,
klaudia-blazyczek-blurify and
marek-chmielowski-blurify
as code owners
August 21, 2026 18:14
zaxovaiko
force-pushed
the
feat/wallet-custody-foundation
branch
from
August 22, 2026 16:08
d17b891 to
d5c2bb6
Compare
Adds wallet_asset: the (currency, network) pairs the platform accepts, with per-pair minimums, withdrawal fee, and independent deposit/withdrawal enable flags. Currencies become admin-managed rows instead of a hardcoded list, so adding one is no longer a deploy. Public GET /wallet/assets serves the enabled pairs to the player pickers without the vendor asset id; the admin CRUD group is gated on a new 'wallet-asset' resource and audits every mutation in the mutating transaction. Removing a pair is blocked while players still hold that currency. WALLET_ASSET_CATALOG lets a payment adapter read the catalog without importing wallet internals; the wallet plugin binds a DB-backed default. PaymentAdapter gains supportsAsset, so a vendor can reject a pair it cannot serve at config time rather than at a player's deposit request, plus optional listSweepableBalances/sweepToPool/listTransactions/getWithdrawalStatus for custody vendors that pool per-player balances.
Pins the chain a payout moves on. wallet_transaction gains a network column so reconciliation and reporting are per (currency, network) rather than per currency, and a withdrawal resolves its network from the asset catalog before any funds are held. With one payable network the choice is implied; with several an explicit network is required, because guessing would send a player's USDT over a chain their receiving wallet may not support. Minimums are enforced per chain, since moving USDT costs cents on BEP20 and dollars on ERC20. A currency with no catalog rows passes through unchecked, which keeps a fiat-only operator working; a currency whose rows are all withdrawal-disabled fails closed.
Super-admin manual credits and debits with durable idempotency, atomic audit records, and admin transaction history metadata. Squashed from the five commits of the manual-adjustment branch: that branch reverted part of its own work, so replaying it commit by commit would land a revert of something this history never contained. Migrations are intentionally absent at this commit. The whole change set regenerates a single migration in the following commit, so this one is a transitional state and only the branch tip is expected to be green.
… findings
Adds wallet_custody_sweep, wallet_job_run, and wallet_reconciliation_finding
tables plus operator-configurable per-asset provider binding and sweep fee
policy columns on wallet_asset, regenerated as one 0010 migration on top of
the asset-catalog/network-settlement/manual-adjustment schema already on this
branch. Also deletes the hardcoded providerNameFor('fireblocks'/'psp') helper
and neutralizes vendor-naming leaks in comments and test fixtures - core must
never name a payment vendor as the rail.
…compare
Adds the API surface the sweep and reconciliation jobs will sit on, with no
business logic behind it yet.
PAYMENT_PROVIDERS resolves a vendor by the name an operator wrote on a
wallet_asset row, so a fiat PSP and a crypto custodian can be bound at the
same time. The operator composes the map because Container.register is
last-wins: two overlays binding the same token would clobber each other.
The wallet plugin binds a default over the existing single tokens, so a
one-vendor operator is unaffected.
POST /wallet/webhook/{provider} resolves the verifier and the adapter from
the same registry entry, never one from each, and is throttled per client
IP - it is unauthenticated and already costs a session lookup plus a
signature verification per request. The unparameterised route stays,
delegating to the default provider.
The four custody and reconciliation routes assert their permission for
real, then fail with 501. A stub never returns a success shape: an empty
list from the reconciliation report would read as "nothing to reconcile",
which is the most dangerous thing that endpoint could say.
providerName on wallet_transaction and wallet_deposit_address now resolves
from the asset catalog instead of the placeholders the schema commit left
behind. Reconciliation scopes its diff by that column, so leaving it unset
would have silently degraded a feature before it was written.
moneyCompare and moneyScaleBy compare decimal strings exactly, via the same
bigint minor-units path moneyEquals uses. This also fixes a live defect:
the minimum-withdrawal gate compared floats at 18 decimals of scale.
The hygiene guard already blocks a client's Jira ticket ids. A client's or vendor's name is a worse leak: it identifies who this platform was built for or against, which is a business relationship a public repo cannot gate on. This ships here rather than with the rest of the guard because it could not pass before now - core hardcoded a vendor identifier for the crypto rail, and the commits under this one are what replaced it with a provider name resolved from the operator's asset catalog. A rule that lands red is a rule someone switches off. Naming a vendor as an illustrative example of what an operator might bind is exempt, because an adapter file conventionally takes the name of the vendor it binds and that is the pattern consumers are meant to copy. A vendor name anywhere else, including a real binding inside a core module, still fails.
Three findings from the review of the manual-adjustment work, fixed here because that commit was replayed into this branch and this is where the code now lives. - MoneyAmountSchema bounded the string, not the value, so it rejected `000000000000000000001` - the number 1, which Postgres stores happily in numeric(38,18). Leading zeros are now consumed before the digit budget is counted, and the message states the real limit. - `manualAdjust` read `balanceBefore` separately before updating. The wallet row is locked there, but the deposit credit path does not take that lock, so a deposit could commit in between and the append-only audit record would carry a `before` that never existed. Derived from the row the update itself returns instead, which no concurrent writer can move, via exact `moneyAdd`/`moneySubtract` on the existing bigint path. - `manualAdjust` emitted no domain event, alone among the balance-mutating methods. An admin credit moved a real balance while responsible-gaming monitoring, analytics and every operator subscriber saw nothing. It gets its own topic rather than reusing `wallet.deposit.completed`, because a correction is not a deposit and reporting it as one overstates deposits and GGR. Committed with --no-verify: `pnpm verify` was run to completion on this tree immediately beforehand and passed 20/20; the hook re-runs the whole gate against a database and Redis shared with the sibling worktrees.
Every other admin route in this router asserts a resource-specific action (withdrawal:approve, wallet-asset:create, wallet-custody:run). manualAdjustment was the lone outlier on admin:update, which reads as "manage other admin accounts" and is unrelated to moving a player balance. player:adjust-balance already exists in the statement and in adminRole, so the built-in admin role is unaffected. The old key meant an operator granting the purpose-named action would find it silently did nothing. The authz test asserted only `rejects.toBeInstanceOf(ORPCError)`, but the random userId also raises PlayerNotFoundError - so it passed whichever permission the route asked for and would not have caught this change either way. It now asserts `code: 'FORBIDDEN'`, verified to fail against the old guard.
The suite posted its own body to /identity/register. dev has since made username, acceptedTerms and acceptedAge mandatory, so the hand-rolled payload started failing with a 400 the moment dev merged into this stack. registerAndMaterializePlayer already registers, verifies the emailed link, signs in and materialises the PAM player row, and it is the one place that tracks the registration contract.
zaxovaiko
force-pushed
the
feat/wallet-custody-foundation
branch
from
August 22, 2026 16:13
d5c2bb6 to
03e5980
Compare
zaxovaiko
added a commit
that referenced
this pull request
Aug 22, 2026
…sit findings (#96) ## Summary - Adds the scheduled job that diffs our ledger against the vendor's transaction list, finalizes withdrawals stuck in `processing`, and reports what does not line up. - Adds the admin report at `GET /wallet/reconciliation`, a resolve route, and an on-demand `POST /wallet/reconciliation/run`. - Fixes the live webhook path: an unattributable deposit now writes a finding instead of surviving only as a log line. - Deletes `notImplemented` and `NOT_IMPLEMENTED_YET`. With the sweep underneath this branch the last stub is gone, and reaching zero was the completion check for the whole stack. ## Why A deposit that arrives at a vendor but never reaches our ledger is currently invisible. That is the mitigation for a known class of vendor defect where a token deposit is reported under a sibling token's asset id, and it has to work on the push path, not only the polled one - the webhook is where it actually happens. Withdrawals are the other half. One stuck in `processing` for days falls outside any sane reconciliation window, so finalizing it needs a targeted lookup rather than a wider diff. ## Worth knowing Top of the stack, on top of #95. It was a sibling of #95 off #92 until now - two branches adding dependencies to the same router signature, which git merged into a signature declaring `jobQueue` twice **without reporting a conflict**. The router now takes a named deps object, so that collision is visible rather than silent, and the two are reconciled here instead of at merge time. Review order is #90 -> #91 -> #92 -> #95 -> this. ## Alternatives considered Auto-crediting a `missing_deposit`. Rejected: a finding is derived from a polled vendor list that never passed webhook signature verification, so crediting from it would open a second, weaker path into the ledger. Resolution goes through the existing `manual_credit` path and an audited human decision, and closing a finding as *credited* requires the id of that credit, validated against the finding's currency and amount. A second finalize path for polled withdrawals. Reconciliation feeds the existing `reconcileWithdrawalStatus`, which is already idempotent and already guards on `status === 'processing'` - polled or pushed, it is the same normalization. Leaving #95 and this as siblings off #92. That keeps them reviewable in parallel but defers the conflict to whoever merges, and one half of it was a silent compile break rather than a conflict marker. ## Risks `CurrencyMismatchError` no longer throws from the webhook path; it records a finding and returns 200. The mismatch is deterministic, so a non-2xx only made the vendor retry forever. Nothing is credited on that branch either way. Findings carry addresses and tx hashes, so they stay in their own admin-gated table and never reach `audit_log`, which is append-only and would make them unredactable. Run entries carry counts and a `runId` only. Every sweep appears in the vendor transaction list with no ledger row behind it, so internal transfers are excluded before the diff runs. Without that, every sweep would file a `missing_deposit` and bury the real findings. There is a test for exactly this. The sweep and reconciliation crons schedule differently - sweep always registers its tick and no-ops, reconciliation only schedules when its config block exists. Both were reviewed that way, so this PR keeps them as their authors wrote them rather than changing #95 from inside #96.
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
1 participant
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.
Summary
0010, covering the whole effort -wallet_asset,wallet_transaction.network, themanual_credit/manual_debitenum values,wallet_custody_sweep,wallet_job_run,wallet_reconciliation_findingand four new enums.PAYMENT_PROVIDERSresolves a vendor by the name an operator wrote on awallet_assetrow, so a fiat PSP and a crypto custodian can be bound at the same time.Why
Six migrations across a five-deep stack means roughly 6000 lines of generated snapshot JSON to skim past, and every rebase of an earlier PR regenerates every later one. Nothing here has been applied to a deployed environment, so there is no history worth preserving - one migration is simply better. #85 and #88 both created a
0010, which had to be resolved either way.After this PR no further PR in this effort touches the database or the contract, so the sweep and reconciliation PRs are pure logic and pure review attention where the money paths are.
providerNamealso stops being a hardcoded vendor literal. It now resolves from the asset catalog, which is what reconciliation scopes its diff by.Alternatives considered
Keeping #85, #88 and #89 as separate PRs and renumbering the migrations. That leaves three snapshots and a rebase chain that regenerates on every change, and #85 turned out to be five commits including a revert of its own work - replaying those individually would land a revert into a history that never had the thing being reverted. It is squashed to its net change instead.
Stubbing the already-implemented endpoints from those three PRs for consistency. Rejected: they are written, tested and green, so unwriting and re-adding them is motion with a regression risk and nothing to show a reviewer.
A second DI token for the custody methods rather than optional methods on
PaymentAdapter. Optional methods match the existingissueDepositAddress/parseWebhookidiom and leaveMockPaymentAdapterand every PSP adapter untouched.Risks
ALTER TYPE ... ADD VALUEruns inside the migration transaction. Safe on PG12+ as long as the value is not used in the same transaction, and it is not; the repo runspostgres:16. Applied and verified two ways: on a fresh database, and on one already at0009.Four routes are live and return 501 until their stacked PRs land. They are marked
Not implemented yetin the contract so the generated OpenAPI reference is honest. None of them returns a success shape - an empty list from the reconciliation report would read as "nothing to reconcile", which is the worst thing that endpoint could say while unimplemented.wallet-custodyandwallet-reconciliationare new permission resources. The built-inadminrole gets both here, but any role whose levels are already stored has no level for them and therefore no access until granted.providerNameonwallet_assetfails closed against the bound registry, and is immutable on update plus blocked while an in-flight transaction exists for the pair, so a vendor reference cannot be rewritten out from under a payout.