Skip to content

refactor(kyc-controller): cleanup controller state - #10062

Merged
Akaryatrh merged 12 commits into
mainfrom
jl/kyc-controller-accepted-terms-cleanup
Sep 2, 2026
Merged

refactor(kyc-controller): cleanup controller state#10062
Akaryatrh merged 12 commits into
mainfrom
jl/kyc-controller-accepted-terms-cleanup

Conversation

@jiexi

@jiexi jiexi commented Sep 1, 2026

Copy link
Copy Markdown
Member

Explanation

  • change idosTncAccepted boolean to idosDisclaimersAccepted KycConsentRecord[]
  • change sumsubTncAccepted boolean to sumsubDisclaimersAccepted KycConsentRecord[]
  • combine termsAcceptedAt and acceptedDisclaimerIds into vendorDisclaimersAccepted
  • change sumsubDisclaimersAccepted to providerDisclaimersAccepted
  • rename sessionToken to moonpaySessionToken
  • rename accessToken to moonpayAccessToken
  • rename disclaimers to vendorDisclaimers. Rename disclaimersError to vendorError

References

See: MetaMask/metamask-mobile#35561

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

Note

High Risk
Wide breaking API and persisted-state shape changes for identity/KYC flows; integrators must migrate callers and stored state or users may be forced through terms again or fail session creation.

Overview
Breaking refactor of KycController persisted and session state so multi-vendor KYC terms and T&C2 consents are modeled explicitly and MoonPay-only tokens are named clearly.

State and API renames: sessionToken / accessToken become moonpaySessionToken / moonpayAccessToken; loaded vendor copy moves to vendorDisclaimers / vendorError. Flat termsAcceptedAt, acceptedDisclaimerIds, and termsAcceptedVendor are replaced by vendorDisclaimersAccepted (moonpay: { termsAcceptedAt } | null, iron: { disclaimerIds } | null). T&C2 booleans become providerDisclaimersAccepted (sumsub: KycConsentRecord[] | null) and idosDisclaimersAccepted (KycConsentRecord[] | null). acceptTermsAndStartSession now requires providerDisclaimersAccepted and idosDisclaimersAccepted document lists instead of sumsubTncSigned / idosTncSigned.

Behavior: A new vendorDisclaimerAcceptance helper module records, checks, and clears per-vendor T&C1 acceptance without wiping another vendor’s persisted acceptance when switching vendors (the old vendor-scoped drop on createVendorCustomer is removed). Consents-path session posting maps accepted { key, version } records onto catalog rows rather than treating whole categories as accepted booleans. MoonPay auto-resume can load disclaimers when missing before #createSession. Docs, changelog, and tests are updated throughout.

Reviewed by Cursor Bugbot for commit 4ac21d4. Bugbot is set up for automated code reviews on this repo. Configure here.

@jiexi
jiexi requested review from a team as code owners September 1, 2026 21:23
@jiexi
jiexi deployed to default-branch September 1, 2026 21:24 — with GitHub Actions Active

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 2c34b3a. Configure here.

Comment thread packages/kyc-controller/src/KycController.ts
Comment thread packages/kyc-controller/src/KycController.ts
@cursor
cursor Bot requested review from Akaryatrh and georgeweiler September 1, 2026 21:27
Extract vendor disclaimer acceptance helpers into their own module so the
branch-per-vendor logic can be covered directly, add PR links to the
breaking-change changelog entries, and resolve the eslint and prettier
violations that were failing CI.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i suppose a new helper file doesn't hurt here. The controller is a few thousand long already

@jiexi

jiexi commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

@metamaskbot publish-previews

@github-actions

github-actions Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Preview builds have been published. Learn how to use preview builds in other projects.

Expand for full list of packages and versions.
@metamask-previews/account-tree-controller@8.0.0-preview-597a80865
@metamask-previews/accounts-controller@39.1.1-preview-597a80865
@metamask-previews/address-book-controller@7.1.2-preview-597a80865
@metamask-previews/ai-controllers@1.0.0-preview-597a80865
@metamask-previews/analytics-controller@2.0.0-preview-597a80865
@metamask-previews/analytics-data-regulation-controller@0.0.0-preview-597a80865
@metamask-previews/announcement-controller@8.1.0-preview-597a80865
@metamask-previews/app-metadata-controller@2.0.1-preview-597a80865
@metamask-previews/approval-controller@9.0.2-preview-597a80865
@metamask-previews/assets-controller@14.0.3-preview-597a80865
@metamask-previews/assets-controllers@111.1.3-preview-597a80865
@metamask-previews/authenticated-user-storage@3.0.2-preview-597a80865
@metamask-previews/base-controller@9.1.0-preview-597a80865
@metamask-previews/base-data-service@1.0.0-preview-597a80865
@metamask-previews/bitcoin-regtest-up@1.0.0-preview-597a80865
@metamask-previews/bridge-controller@80.1.1-preview-597a80865
@metamask-previews/bridge-status-controller@75.4.0-preview-597a80865
@metamask-previews/build-utils@3.0.4-preview-597a80865
@metamask-previews/chain-agnostic-permission@1.7.0-preview-597a80865
@metamask-previews/chomp-api-service@4.0.1-preview-597a80865
@metamask-previews/claims-controller@0.6.1-preview-597a80865
@metamask-previews/client-controller@1.0.1-preview-597a80865
@metamask-previews/client-utils@2.1.1-preview-597a80865
@metamask-previews/compliance-controller@2.1.0-preview-597a80865
@metamask-previews/composable-controller@12.0.1-preview-597a80865
@metamask-previews/config-registry-controller@3.1.0-preview-597a80865
@metamask-previews/connectivity-controller@0.3.0-preview-597a80865
@metamask-previews/controller-utils@12.3.0-preview-597a80865
@metamask-previews/core-backend@9.0.0-preview-597a80865
@metamask-previews/delegation-controller@3.0.2-preview-597a80865
@metamask-previews/earn-controller@12.2.6-preview-597a80865
@metamask-previews/eip-5792-middleware@3.0.5-preview-597a80865
@metamask-previews/eip-7702-internal-rpc-middleware@0.1.1-preview-597a80865
@metamask-previews/eip1193-permission-middleware@2.0.1-preview-597a80865
@metamask-previews/eth-block-tracker@15.0.1-preview-597a80865
@metamask-previews/eth-json-rpc-middleware@24.0.2-preview-597a80865
@metamask-previews/eth-json-rpc-provider@6.0.1-preview-597a80865
@metamask-previews/foundryup@1.0.1-preview-597a80865
@metamask-previews/gas-fee-controller@26.3.2-preview-597a80865
@metamask-previews/gator-permissions-controller@5.0.2-preview-597a80865
@metamask-previews/geolocation-controller@1.0.0-preview-597a80865
@metamask-previews/java-tron-up@1.0.0-preview-597a80865
@metamask-previews/json-rpc-engine@10.5.0-preview-597a80865
@metamask-previews/json-rpc-middleware-stream@8.0.8-preview-597a80865
@metamask-previews/keyring-controller@27.1.1-preview-597a80865
@metamask-previews/kyc-controller@0.0.0-preview-597a80865
@metamask-previews/local-node-utils@1.0.0-preview-597a80865
@metamask-previews/logging-controller@9.0.0-preview-597a80865
@metamask-previews/message-manager@14.1.2-preview-597a80865
@metamask-previews/messenger@2.0.0-preview-597a80865
@metamask-previews/messenger-cli@0.2.0-preview-597a80865
@metamask-previews/money-account-api-data-service@0.4.1-preview-597a80865
@metamask-previews/money-account-balance-service@2.4.3-preview-597a80865
@metamask-previews/money-account-controller@1.0.0-preview-597a80865
@metamask-previews/money-account-upgrade-controller@3.0.2-preview-597a80865
@metamask-previews/money-account-utils@1.1.0-preview-597a80865
@metamask-previews/multichain-account-service@13.0.2-preview-597a80865
@metamask-previews/multichain-api-middleware@4.0.3-preview-597a80865
@metamask-previews/multichain-network-controller@3.2.4-preview-597a80865
@metamask-previews/multichain-transactions-controller@7.1.2-preview-597a80865
@metamask-previews/name-controller@9.1.2-preview-597a80865
@metamask-previews/network-connection-banner-controller@0.2.1-preview-597a80865
@metamask-previews/network-controller@36.0.0-preview-597a80865
@metamask-previews/network-enablement-controller@6.0.5-preview-597a80865
@metamask-previews/notification-services-controller@26.0.1-preview-597a80865
@metamask-previews/passkey-controller@3.1.0-preview-597a80865
@metamask-previews/permission-controller@13.1.1-preview-597a80865
@metamask-previews/permission-log-controller@5.1.0-preview-597a80865
@metamask-previews/perps-controller@15.0.0-preview-597a80865
@metamask-previews/phishing-controller@17.4.0-preview-597a80865
@metamask-previews/platform-api-docs@0.1.0-preview-597a80865
@metamask-previews/polling-controller@16.0.9-preview-597a80865
@metamask-previews/preferences-controller@23.1.0-preview-597a80865
@metamask-previews/profile-metrics-controller@4.0.3-preview-597a80865
@metamask-previews/profile-sync-controller@29.0.0-preview-597a80865
@metamask-previews/ramps-controller@20.2.0-preview-597a80865
@metamask-previews/rate-limit-controller@7.0.1-preview-597a80865
@metamask-previews/react-data-query@1.0.0-preview-597a80865
@metamask-previews/remote-feature-flag-controller@6.1.0-preview-597a80865
@metamask-previews/sample-controllers@5.0.6-preview-597a80865
@metamask-previews/seedless-onboarding-controller@10.1.1-preview-597a80865
@metamask-previews/selected-network-controller@26.1.7-preview-597a80865
@metamask-previews/sentinel-api-service@1.0.1-preview-597a80865
@metamask-previews/shield-controller@6.0.1-preview-597a80865
@metamask-previews/signature-controller@39.2.10-preview-597a80865
@metamask-previews/smart-transactions-controller@26.0.0-preview-597a80865
@metamask-previews/snap-account-service@2.1.2-preview-597a80865
@metamask-previews/social-controllers@2.8.0-preview-597a80865
@metamask-previews/solana-test-validator-up@1.0.0-preview-597a80865
@metamask-previews/stellar-quickstart-up@0.0.0-preview-597a80865
@metamask-previews/storage-service@1.0.2-preview-597a80865
@metamask-previews/subscription-controller@8.0.1-preview-597a80865
@metamask-previews/transaction-controller@69.7.0-preview-597a80865
@metamask-previews/transaction-pay-controller@27.1.1-preview-597a80865
@metamask-previews/user-operation-controller@41.2.9-preview-597a80865
@metamask-previews/wallet@12.0.2-preview-597a80865
@metamask-previews/wallet-cli@0.0.0-preview-597a80865

Akaryatrh
Akaryatrh previously approved these changes Sep 1, 2026

@Akaryatrh Akaryatrh left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Akaryatrh
Akaryatrh added this pull request to the merge queue Sep 2, 2026
Merged via the queue into main with commit 3e3ca01 Sep 2, 2026
46 checks passed
@Akaryatrh
Akaryatrh deleted the jl/kyc-controller-accepted-terms-cleanup branch September 2, 2026 20:50
jiexi added a commit to MetaMask/metamask-mobile that referenced this pull request Sep 2, 2026
<!--
Please submit this PR as a draft initially.

Do not mark it as "Ready for review" until this PR meets the canonical
Definition of Ready For Review in `docs/readme/ready-for-review.md`.

In short: the template must be materially complete (not just section
titles
present), all status checks must be currently passing, and the only
expected
follow-up commits must be reviewer-driven.
-->
<!--
mms-check directive vocabulary — read by
.github/scripts/shared/pr-template-checks.ts
at module load to build the validation plan. Directives are invisible in
rendered
markdown and must NOT be removed or edited without updating the
validator registry.

  type=text           Section must contain non-placeholder prose.
  type=changelog      Section must have a valid CHANGELOG entry: line.
type=issue-link Section must have a Fixes:/Closes:/Refs: line with a
value.
type=manual-testing Section must have real testing steps or an explicit
N/A.
type=screenshot Section must have evidence (image/URL) or an explicit
N/A.
type=checklist Section must have all checkboxes consciously checked.
required=true|false Whether a missing/invalid section runs the validator
at all.
blocking=true|false Whether a failure of this check fails the CI
workflow.
Default: false — failures are shown as warnings in the sticky
                      comment but do not block the PR.

Sections without a directive are checked for structural presence only.
-->

## **Description**

Adopts the changes in MetaMask/core#10062

and the changes in MetaMask/core#10079 


## **Changelog**

<!-- mms-check: type=changelog required=true blocking=true -->

<!--
If this PR is not End-User-Facing and should not show up in the
CHANGELOG, you can choose to either:
1. Write `CHANGELOG entry: null`
2. Label with `no-changelog`

If this PR is End-User-Facing, please write a short User-Facing
description in the past tense like:
`CHANGELOG entry: Added a new tab for users to see their NFTs`
`CHANGELOG entry: Fixed a bug that was causing some NFTs to flicker`

(This helps the Release Engineer do their job more quickly and
accurately)
-->

CHANGELOG entry:

## **Related issues**

<!-- mms-check: type=issue-link required=true -->

Fixes:

## **Manual testing steps**

<!-- mms-check: type=manual-testing required=true -->

```gherkin
Feature: my feature name

  Scenario: user [verb for user action]
    Given [describe expected initial app state]

    When user [verb for user action]
    Then [describe expected outcome]
```

## **Screenshots/Recordings**

<!-- mms-check: type=screenshot required=true -->

<!-- If applicable, add screenshots and/or recordings to visualize the
before and after of your change. -->

### **Before**

<!-- [screenshots/recordings] -->

### **After**

<!-- [screenshots/recordings] -->

## **Pre-merge author checklist**

<!-- mms-check: type=checklist required=true -->

<!--
Every checklist item must be consciously assessed before marking this PR
as
"Ready for review". A checked box means you deliberately considered that
responsibility, not that you literally performed every action listed.

Unchecked boxes are ambiguous: they are not an implicit "N/A" and they
are not
a silent "skip". See `docs/readme/ready-for-review.md` for the full
checklist
semantics.
-->

- [ ] I've followed [MetaMask Contributor
Docs](https://github.com/MetaMask/contributor-docs) and [MetaMask Mobile
Coding
Standards](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/CODING_GUIDELINES.md).
- [ ] I've completed the PR template to the best of my ability
- [ ] I've included tests if applicable
- [ ] I've documented my code using [JSDoc](https://jsdoc.app/) format
if applicable
- [ ] I've applied the right labels on the PR (see [labeling
guidelines](https://github.com/MetaMask/metamask-mobile/blob/main/.github/guidelines/LABELING_GUIDELINES.md)).
Not required for external contributors.

#### Performance checks (if applicable)

- [ ] I've tested on Android
  - Ideally on a mid-range device; emulator is acceptable
- [ ] I've tested with a power user scenario
- Use these [power-user
SRPs](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/edit-v2/401401446401?draftShareId=9d77e1e1-4bdc-4be1-9ebb-ccd916988d93)
to import wallets with many accounts and tokens
- [ ] I've instrumented key operations with Sentry traces for production
performance metrics
- See [`trace()`](/app/util/trace.ts) for usage and
[`addToken`](/app/components/Views/AddAsset/components/AddCustomToken/AddCustomToken.tsx#L274)
for an example

For performance guidelines and tooling, see the [Performance
Guide](https://consensyssoftware.atlassian.net/wiki/spaces/TL1/pages/400085549067/Performance+Guide+for+Engineers).

## **Pre-merge reviewer checklist**

<!--
Reviewer checklist items follow the same semantics as the author
checklist: an
unchecked box is ambiguous, a checked box means the reviewer consciously
assessed that responsibility. See `docs/readme/ready-for-review.md`.
-->

- [ ] I've manually tested the PR (e.g. pull and build branch, run the
app, test code being changed).
- [ ] I confirm that this PR addresses all acceptance criteria described
in the ticket it closes and includes the necessary testing evidence such
as recordings and or screenshots.


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **High Risk**
> Touches identity verification (Iron/UKYC/Sumsub), EIP-191 wallet
registration, autoramp creation against live dev proxies, and persistent
KYC state—mistakes could affect money onboarding or signing prompts.
> 
> **Overview**
> Adds **Brazil virtual bank account (VBA) demo flow** end-to-end:
navigation for mock KYC email/success and account status screens, **Iron
→ Sumsub** orchestration via new `ironKycFlow` helpers, and **Get Pix
Key** now initializes Iron KYC before advancing instead of only
navigating.
> 
> **Engine** registers **`KycService`**, **`KycController`** (with React
Native Sumsub launcher), and **`NeoBankService`**, clears KYC state on
wallet reset, and subscribes **`registerMoneyAccountOnKycCompletion`**
to `KycController:statusChanged` so completed KYC can auto **register
the Money Account wallet** and **create a BRL→mUSD/Monad autoramp**
(deduped with the manual pipeline on `MockKycSuccess`).
**`VirtualBankAccount`** refreshes autoramps and listens on a **neobank
WebSocket** while focused.
> 
> **Money tab** gains **`useNeobankSandboxDepositEvents`** (Iron
customer id resolution + dev WebSocket) to show a **deposit success
toast only**—no vault submit. **Dev tooling**: `vbaTrace` streams to the
ramps debug dashboard; neobank `fetch` is traced in `__DEV__`. Android
adds the **Sumsub Maven** repo; **idOS JWKS** URLs land in
`AppConstants`; **`addPrecreatedOrder`** passes required `chainId`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
80b869c. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants