feat(core,react): add the custody client - #202
Open
TaprootFreak wants to merge 4 commits into
Open
Conversation
Contributor
Author
|
Took 4 review passes to reach a clean run — the most of the five, and the ones that found the most. What the passes turned up, in order:
|
TaprootFreak
marked this pull request as ready for review
August 2, 2026 16:31
6 tasks
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.
Step 4 of #198: move the customer-facing custody contract into the packages.
Changes
@dfx.swiss/coreCustodyUrlcovering signup, balance, history, order, order confirm, PDF report, the account listand per-account balance / history / order / PDF, plus the access grants
CustodyAccount,CustodyBalance,CustodyAssetBalance,CustodyFiatValue,CustodyHistory,CustodyOrder,CustodyOrderPaymentInfo,CustodyOrderHistory,CreateCustodyOrder,CustodyAccountAccessand friends, plus the enums (CustodyOrderType,CustodyOrderStatus,CustodyOrderHistoryStatus,CustodyAccessLevel,CustodyAddressType,CustodyValueCurrency)CustodyApi, wired intoDfxApiClientasclient.custody@dfx.swiss/reactuseCustody()with the same surfaceTests cover signup, account-scoped vs. own-account reads, the legacy account, order creation and
confirmation, the PDF query and the access grants.
What the API actually says
Verifying against the API turned up four differences from a locally modelled version of this
contract:
isOwneris part of the account. The API states outright whether the caller owns an accountor was granted access to it. Deriving that from the owner id is guesswork that the response
already answers.
interestandinterestValuefor interest-bearing positions. Both arealready folded into
balance/value— a breakdown, never an amount to add on top. Typing themaway hides the breakdown; adding them up double-counts.
Date, not a string.createdis always present on an order history entry, so it is typed as required.The legacy account has a URL of its own. The API accepts
legacyas an account id(
custody/account/legacy/balance), so the legacy account does not have to be routed to the plainendpoints. Both forms are available here:
LegacyCustodyAccountIdfor the account-scoped path, andgetBalance/getHistory/getOrdersfor the caller's own account.Scope
from signup, not the session token. Both methods take it as a parameter so the caller can hold
both tokens side by side.
deposit/withdraw asset pairing, PDF download to disk. That is UI orchestration, not the wire
contract.
custody/admin/*(order list, approve, balance correction) is not partof this client.
Additive only — no existing export changes shape.
Version fields, changelogs and lockfile pins are untouched, per CONTRIBUTING.
Corrections from review
createOrderandconfirmOrdernow require the custody token. It used to be optional, and theHTTP client falls back to the session token when no token is passed — so the one call that must not
use the session token would have used it, and failed against the API instead of failing to compile.
CreateCustodyOrderis a union over the order type. The API requires a destination address andblockchain for a send, an IBAN for a withdrawal, and exactly one of the two amounts. All of that was
modelled as independently optional fields, which typechecks a request the API rejects.
PUT custody/account/:id,GET :id/access,PUT :id/access/:accessIdandDELETE :id/access/:accessIddo not accept thelegacy marker; only the reading endpoints and
POST :id/accessdo.PersistedCustodyAccountIdkeeps the two apart.
toCustodyAccountId(account)closes the null-id gap. A legacy account hasid: null; passingthat straight through would request
custody/account/null/….moderatoris dropped fromCustodySignup. The API validates it against an enum of internalvalues; a customer-facing SDK should neither invent that set nor freeze it.
Not changed, with reasons:
error?: TransactionErrormirrors what the package already does for the same API field elsewhere(
Buy.error), even though the API's own enum has drifted apart from it. Fixing that is apackage-wide change, not a custody one.
Beneficiary.nameis required here while the API has it optional — that is the existing sharedtype, untouched by this change.
Later passes
CustodyOrderService.createOrderhas cases forDEPOSIT,WITHDRAWAL,SWAP,SENDandRECEIVEand no default; the four equity and savingtypes fall through, leave
paymentInfonull and produce a 500 on the next line. Those types doappear in the history, so the enum keeps all nine —
CreatableCustodyOrderTypenames the five thatcan be asked for.
CustodyOrderPaymentInfo.idis optional. A swap on a configured equity pair is priced without atransaction request of its own and comes back with
id: undefined.grantAccessis the one access call that also takes the legacy marker — the controller resolves itthere but not when reading, changing or revoking a grant. That asymmetry is now stated at both call
sites instead of being left to be rediscovered.