Skip to content

Add kyc level 2#587

Merged
gianfra-t merged 42 commits into
stagingfrom
add-kyc-level-2
May 5, 2025
Merged

Add kyc level 2#587
gianfra-t merged 42 commits into
stagingfrom
add-kyc-level-2

Conversation

@gianfra-t
Copy link
Copy Markdown
Contributor

@gianfra-t gianfra-t commented Apr 23, 2025

Changes summary

  • Companies should now be able to register with cnpj by filling the extra fields required. If registered via cnpj, the kyc flow will go directly to the level 2 screen after a successful level 1 verification.
  • cpf users are now prompted with the level 2 screen after confirming a ramp that will go above the level 1 limits.
  • Both users and companies that were rejected at level 1 can now re-attempt the registration as if it were the first time.

Code changes

  • Adds a component for KYC level 2 page. This component handles fetching the upload url's from the backend and performing the upload of the documents.
  • Adds logic to the existing useKYCProcess hook, to handle both level 1 and level 2 logic.
  • Component for KYC level 2 added to PIXKYCForm component.
  • Added company-specific fields to kyc level 1 form, to allow registering via cnpj.

Backend

  • Adds endpoint to request the upload urls for level 2.
  • Adjust kyc status endpoint to also work with level 2 kyc.

@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 23, 2025

Deploy Preview for pendulum-pay ready!

Name Link
🔨 Latest commit 60b8992
🔍 Latest deploy log https://app.netlify.com/sites/pendulum-pay/deploys/6819049faf07610008e1bc40
😎 Deploy Preview https://deploy-preview-587--pendulum-pay.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Comment thread api/package.json Outdated
Comment thread api/src/api/controllers/brla.controller.ts
Comment thread api/src/api/controllers/brla.controller.ts
}

const lastEventCached = await eventPoller.getLatestEventForUser(subaccount.id);
const lastEventCached = await eventPoller.getLatestEventForUser(subaccount.id, 'KYC');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was thinking it might be a great idea to create an enum, like SubscriptionEvents, to include values such as KYC and other related events. This way, we can avoid scattering strings throughout the codebase, making our code cleaner and easier to maintain.

Comment thread api/src/api/services/brla/mappings.ts Outdated
Comment thread api/src/api/services/kyc/kyc.service.ts Outdated
Comment thread frontend/src/components/BrlaComponents/BrlaExtendedForm.tsx
Comment thread frontend/src/constants/constants.ts Outdated
export const WALLETCONNECT_ASSETHUB_ID = 'polkadot:68d56f15f85d3136970ec16946040bc1';

export const TRANSFER_WAITING_TIME_SECONDS = 6000;
export const BRLA_MAXIMUM_LEVEL_2_AMOUNT_UNITS = 1000000;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can we link brla docs for the future reference?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This level is not known, at least to me, so it was a placeholder to test. I'll add a comment.

const { setRampKycStarted, resetRampState } = useRampActions();

const offrampKycLevel2Started = useRampKycLevel2Started();
const desiredLevel = offrampKycLevel2Started ? 2 : 1;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I was wondering if we could implement an enum for KYCLevels instead of using magic numbers?
This might help make the code more readable and maintainable.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

True, I'll add one for the numbers in this component.

if (isValidCnpj(taxId)) {
await createSubaccount({
...formData,
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please just add a condition like

if (!formData.partnerCpf) {
        throw new Error('useKYCProcess: Partner CPF must be defined at this point');
}

Comment thread frontend/src/hooks/brla/useBRLAKYCProcess/index.tsx
@gianfra-t gianfra-t marked this pull request as ready for review April 29, 2025 18:34
validSetter(false);
return;
}
if (!ALLOWED_TYPES.includes(file.type)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we should enhance our file validation, the file.type can be spoofed and we should also check for the file extension like:

const extension = `.${getFileExtension(file.name)}`;
    if (!ALLOWED_EXTENSIONS.includes(extension.toLowerCase())) {

We could also use Zod for the validation.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Let's add this to our todo's and once we are sure about the restrictions that brla imposes on the images (regarding size, etc), we can add this improvement. Wdyt?

@gianfra-t gianfra-t changed the title Add kyc level 2 [In-Progress] Add kyc level 2 Apr 30, 2025
@ebma ebma requested a review from Copilot May 2, 2025 08:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request implements a new KYC level 2 component and integrates level 2 handling into the overall KYC flow for both companies (using CNPJ) and individuals (CPF). Key changes include adding a dedicated document upload component for level 2, updating the KYC process hook and related UI components, and extending backend models, endpoints, and migrations to support KYC level 2 verification.

Reviewed Changes

Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
frontend/src/constants/constants.ts Added constant for maximum level 2 KYC amount.
frontend/src/components/Ramp/Onramp/index.tsx Removed an unused hook import.
frontend/src/components/BrlaComponents/VerificationStatus/index.tsx Updated status prop and styling to differentiate between level 1 and level 2.
frontend/src/components/BrlaComponents/KYCLevel2Form/index.tsx Introduced the new document upload UI component for KYC level 2.
frontend/src/components/BrlaComponents/KYCForm/index.tsx Updated form fields and flow based on account type and KYC level.
frontend/src/components/BrlaComponents/BrlaField/index.tsx Extended field options with company-specific fields.
api/src/models/kycLevel2.model.ts & migrations Added new model and migration for KYC level 2 data storage.
api/src/api/services/kyc/kyc.service.ts & others Integrated new endpoints and service logic to request and manage KYC level 2 verification.
api/src/api/controllers/brla.controller.ts Updated subaccount creation and introduced the startKYC2 endpoint.

Comment thread frontend/src/components/BrlaComponents/KYCLevel2Form/index.tsx Outdated
Copy link
Copy Markdown
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looking great overall, nice job 👍

I left a few comments, mostly minor stuff.

Comment thread api/src/api/controllers/brla.controller.ts
Comment thread api/src/api/controllers/brla.controller.ts
Comment thread api/src/api/controllers/brla.controller.ts Outdated
Comment thread api/src/api/routes/v1/brla.route.ts Outdated
router.route('/createSubaccount').post(validataSubaccountCreation, brlaController.createSubaccount);

router.route('/triggerPayIn').post(validateTriggerPayIn, brlaController.triggerPayIn);
router.route('/startKYC2').post(brlaController.startKYC2);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just for the sake of it, why don't we add a validate... function like we do for any other post request.

Comment thread api/src/api/services/brla/types.ts
Comment thread frontend/src/pages/ramp-form/index.tsx Outdated
Comment thread frontend/src/services/api/brla.service.ts Outdated
Comment thread frontend/src/services/initialChecks.ts Outdated
},
"companyName": {
"required": "Company name is required",
"minLength": "Company name must be at least 3 characters",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are you sure it's 3 and not 5?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

no idea, just copy the other validations. I don't think brla checks this right?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think they do actually. But we'll see.

Comment thread frontend/src/translations/pt.json
@gianfra-t gianfra-t changed the title [In-Progress] Add kyc level 2 Add kyc level 2 May 5, 2025
@gianfra-t gianfra-t requested review from Sharqiewicz, Copilot and ebma May 5, 2025 12:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extends the KYC flow to support Level 2 verification for both companies and individuals, enabling registration via CNPJ and allowing previously rejected users to reattempt verification. Key changes include the introduction of a new migration for the kyc_level_2 table, additions to API endpoints and services to support KYC Level 2, and updates to related validators and controllers.

Reviewed Changes

Copilot reviewed 48 out of 48 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
api/src/database/migrations/001-initial-schema.ts Adds the kyc_level_2 table and corresponding indexes to support KYC Level 2.
api/src/constants/constants.ts Updates BRLA_BASE_URL port configuration.
api/src/api/services/transactions/onrampTransactions.ts Formats nonce calculation for clarity.
api/src/api/services/ramp/quote.service.ts Improves formatting in funding amount logic.
api/src/api/services/phases/* Minor formatting and parameter adjustments in several phase handlers.
api/src/api/services/pendulum/apiManager.ts Standardizes object formatting in promise resolves.
api/src/api/services/moonbeam/balance.ts Adjusts import formatting and parameter definitions.
api/src/api/services/kyc/kyc.service.ts Introduces KYC Level 2 logic and integrates a new endpoint call.
api/src/api/services/brla/* Updates types, mappings, and endpoints to include KYC Level 2 and retry flows.
api/src/api/routes/v1/* Replaces the triggerPayIn route with startKYC2 and updates generics formatting.
api/src/api/middlewares/validators.ts Adds a startKYC2Validator to validate document type for KYC Level 2.
api/src/api/controllers/brla.controller.ts Integrates KYC Level 2 flows in subaccount creation and KYC status retrieval.

res.status(400).json({ error: 'Subaccount already created' });
return;
} else if (subaccount && subaccount.kyc.level === 0) {
console.log('subaccountPayload', subaccountPayload);
Copy link

Copilot AI May 5, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider removing or replacing the debug console.log statement with structured logging (e.g., using logger.info) to avoid potential performance or security concerns in production.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think it's not a bad idea to leave this for debugging using backend logs.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right, we can leave it. We could still change the line to use the logger instance that we have. We should always try to use that logger over simple console.logs in our backend because the logger will use the respective configuration (it's logging to file as well as console). I also forget about this often though.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

true, I forgot we added this after the migration.

Comment thread api/src/api/routes/v1/brla.route.ts Outdated
console.error('Validation flags were true, but file data is missing. This is a bug.');
return;
}
console.log('CNH upload URL:', response.uploadUrls);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shall we remove this?

const amountNum = Number(executionInput.quote.inputAmount);
const remainingLimitNum = Number(remainingLimitInUnits);
if (amountNum > remainingLimitNum) {
if (true) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think this is intended?

Copy link
Copy Markdown
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

Looks good to me 🆗

@gianfra-t gianfra-t merged commit e7bb0f2 into staging May 5, 2025
5 checks passed
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.

4 participants