Conversation
🦋 Changeset detectedLatest commit: e401f39 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughIntroduce a consolidated ramp API (manteca + bridge), remove the legacy onramp router, extend KYC scope to include "manteca" with NOT_SUPPORTED handling, refactor persona and ramp provider utilities and types, convert manteca webhook to account-centric handling, add manteca-specific document selection logic, and add extensive unit/integration tests and a changeset. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant RampAPI as Ramp API
participant CredDB as Credential DB
participant Manteca as Manteca Provider
participant Bridge as Bridge Provider
participant Persona as Persona Utils
Client->>RampAPI: GET /ramp or POST /ramp (onboarding)
RampAPI->>CredDB: resolve credential & account
alt aggregation flow
RampAPI->>Manteca: getProvider(account, country?)
RampAPI->>Bridge: getProvider(account, country?)
Manteca-->>RampAPI: provider info
Bridge-->>RampAPI: provider info
RampAPI-->>Client: 200 { providers: { manteca, bridge } }
else onboarding flow
RampAPI->>Persona: getPendingInquiryTemplate(referenceId, scope)
alt provider = manteca
RampAPI->>Manteca: mantecaOnboarding(account, credentialId)
Manteca-->>RampAPI: result / errors
else provider = bridge
RampAPI->>Bridge: bridgeOnboarding({ credentialId, customerId, acceptedTermsId })
Bridge-->>RampAPI: result / errors
end
RampAPI-->>Client: 200 { code: "ok" } or 400 { code: ... }
end
sequenceDiagram
participant Client as Client
participant KYC as KYC Endpoint
participant Persona as Persona Utils
participant MantecaEval as Manteca evaluation
Client->>KYC: GET /kyc?scope=manteca
KYC->>Persona: getPendingInquiryTemplate(referenceId, "manteca")
Persona->>MantecaEval: evaluateAccount(referenceId)
alt supported
MantecaEval-->>Persona: template info
Persona-->>KYC: template
KYC-->>Client: 200 { status / inquiry }
else not_supported
MantecaEval-->>Persona: NOT_SUPPORTED error
Persona-->>KYC: throws NOT_SUPPORTED
KYC-->>Client: 400 { code: "not supported" }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @mainqueg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the server's capability to handle Know Your Customer (KYC) and on-ramp processes by integrating Manteca. It involves a substantial refactoring of the on-ramp module, introducing new logic for Manteca-specific user onboarding, and updating API interactions and data structures to support this new provider. The changes aim to provide a more robust and flexible system for managing user verification and fund transfers. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces support for Manteca inquiries, significantly refactoring the KYC and ramp provider integration. Key changes include renaming the 'onramp' module to 'ramp', extending the KYC scope to include 'manteca', and streamlining the logic for retrieving provider information and handling Manteca onboarding. The changes also include comprehensive new tests for the Manteca scope, ensuring the correctness of the new flows. The refactoring improves maintainability by centralizing Persona-related logic and simplifying provider interfaces.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 196-229: The onboarding handlers only return 400 for one specific
code each; update the error handling in the manteca and bridge cases (around
mantecaOnboarding and bridgeOnboarding) so that any error whose message is a
member of MantecaErrorCodes or BridgeErrorCodes returns c.json({ code:
error.message }, 400). Keep the existing captureException(error, { contexts: {
credential } }) and the instanceof Error check, replace the single-case switch
with a general check using
Object.values(MantecaErrorCodes).includes(error.message) and
Object.values(BridgeErrorCodes).includes(error.message) and return 400 for any
match instead of re-throwing.
In `@server/utils/ramps/manteca.ts`:
- Around line 250-321: In mantecaOnboarding the expression account.replace("0x",
"") is repeated multiple times; introduce a const userExternalId =
account.replace("0x", "") near the top of the function and replace all uses of
account.replace("0x", "") (calls to getUser, the externalId property passed to
initiateOnboarding, the first argument to uploadIdentityFile for FRONT/BACK, and
the argument to acceptTermsAndConditions, as well as any captureException extra
data that references the stripped account) with userExternalId to avoid repeated
string ops and improve readability.
- Around line 302-320: The current use of Promise.allSettled with only logging
can let critical operations silently fail; update the block around
Promise.allSettled (which runs uploadIdentityFile and acceptTermsAndConditions)
to treat failures as fatal: after awaiting results, inspect the results array
for any entry with status === "rejected", call captureException(result.reason, {
extra: { account } }) for each, and then throw a new Error (including a brief
context like "Manteca onboarding failed for account") or switch to Promise.all
so the function rejects immediately when any of uploadIdentityFile or
acceptTermsAndConditions fails; ensure you reference the uploadIdentityFile and
acceptTermsAndConditions calls and the results variable when making this change.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #680 +/- ##
==========================================
+ Coverage 59.98% 63.91% +3.93%
==========================================
Files 170 170
Lines 5315 5332 +17
Branches 1490 1502 +12
==========================================
+ Hits 3188 3408 +220
+ Misses 1965 1737 -228
- Partials 162 187 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@server/utils/ramps/manteca.ts`:
- Around line 705-716: The two multi-line arrow helpers getExchange and
getNationality should be converted to function declarations: replace the const
getExchange = (countryCode: string): (typeof Exchange)[number] => { ... } and
const getNationality = (countryCode: string): string => { ... } with function
getExchange(countryCode: string): (typeof Exchange)[number] { ... } and function
getNationality(countryCode: string): string { ... } respectively, preserving the
same body, type annotations, error throw logic (throw new Error(`Invalid
country: ${countryCode}`)) and return values so behavior and types remain
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 Fix all issues with AI agents
In `@server/api/kyc.ts`:
- Around line 43-44: The picklist schema for the "scope" field is duplicated in
the validators (see the scope: optional(picklist(["basic","manteca"]))
occurrences); extract a shared constant (e.g., const scopePicklistSchema =
optional(picklist(["basic","manteca"]))) and replace both inline uses with that
constant so GET/POST validators reference the same schema symbol
(scopePicklistSchema) to prevent drift.
- Around line 26-27: Replace the direct c.json(...) error responses in the "not
supported" branches with thrown HTTPException instances so the global error
handler handles them uniformly; import HTTPException from 'hono' if not already,
and in the two branches that currently return c.json for unsupported operations
(the branches that build the scopeValidationErrors and the other "not supported"
response paths) throw new HTTPException with the appropriate status code and
include the same error payload/message used in the c.json call, preserving any
fields from scopeValidationErrors so handlers downstream receive the identical
payload.
In `@server/api/ramp.ts`:
- Around line 157-168: The try/catch around getMantecaDepositDetails currently
only maps MantecaErrorCodes.NOT_SUPPORTED_CURRENCY to a 400 response and
re-throws other known MantecaErrorCodes as 500; update the catch so that any
error whose message is one of Object.values(MantecaErrorCodes) results in
returning c.json({ code: error.message }, 400) (after captureException) instead
of re-throwing, preserving the existing captureException call and only
re-throwing unknown errors; locate this logic in the catch block that references
getMantecaDepositDetails, MantecaErrorCodes, captureException and c.json to make
the change.
- Around line 47-54: ErrorCodes currently spreads MantecaErrorCodes but omits
BridgeErrorCodes; update the ErrorCodes object (the constant named ErrorCodes in
ramp.ts) to include BridgeErrorCodes as well (e.g., add ...BridgeErrorCodes
alongside ...MantecaErrorCodes), ensuring the order and names remain consistent
and that BridgeErrorCodes is imported/available in the module before use.
- Around line 186-195: The catch block in the quote endpoint erroneously only
maps BridgeErrorCodes.NOT_AVAILABLE_CRYPTO_PAYMENT_RAIL to a 400 response; other
BridgeErrorCodes bubble up as 500s. Update the catch in server/api/ramp.ts
(where captureException, BridgeErrorCodes and c.json are used) to, after
verifying error instanceof Error and
Object.values(BridgeErrorCodes).includes(error.message), return c.json({ code:
error.message }, 400) for all BridgeErrorCodes (e.g., replace the switch with a
single return), otherwise rethrow the error.
In `@server/hooks/manteca.ts`:
- Around line 184-194: The code currently constructs account via const account =
`0x${payload.data.userExternalId}` and assumes payload.data.userExternalId is
valid hex; validate that userExternalId before using it (or parse/normalize it
via the project's Address schema) inside the hook where setUser and
database.query.credentials.findFirst are called. Specifically, add a validation
step for payload.data.userExternalId (e.g., regex or Address schema helper) and
if invalid, call captureException with details and return a clear error response
instead of proceeding to setUser or querying credentials; ensure the same
validated/normalized account string is used for setUser and in the
credentials.where eq(credentials.account, account) query.
In `@server/test/api/ramp.test.ts`:
- Around line 313-340: Add parallel tests for the "bridge" provider in
server/test/api/ramp.test.ts mirroring the existing "manteca" tests: spy on
bridge.bridgeOnboarding (using vi.spyOn) and add a success test that
mocksResolvedValue and asserts a 200 response and { code: "ok" } and that
bridge.bridgeOnboarding was called with account and "ramp-test"; add an error
test that mocksRejectedValue(new Error(bridge.ErrorCodes.ALREADY_ONBOARDED)) and
asserts the endpoint returns the correct HTTP status and JSON body for the
ALREADY_ONBOARDED case; also add one or two additional bridge-specific error
tests (mockRejectedValue with other bridge.ErrorCodes) to assert expected status
and error codes.
- Around line 38-41: In the afterEach block remove the redundant
vi.clearAllMocks() call since vi.restoreAllMocks() already clears mocks; update
the afterEach cleanup to only call vi.restoreAllMocks() (refer to the afterEach
callback and the vi.clearAllMocks / vi.restoreAllMocks calls to locate the lines
to change).
- Around line 266-268: The test currently uses a `as never` cast when mocking
bridge.getCustomer which hides type mismatches; replace the `as never` with a
properly typed mock object that matches the expected return type of
`getCustomer` (or use TypeScript's `satisfies` operator) so the compiler
validates the mock shape—locate the `vi.spyOn(bridge, "getCustomer")` call in
the "returns quote and deposit info for bridge fiat" test and provide a typed
mock (or partial mock typed as the function's return type) instead of the `as
never` assertion.
In `@server/test/utils/persona.test.ts`:
- Around line 304-352: Add a new test case in the "get document for manteca"
suite that verifies Brazil (BR) uses the updated id_class priority: call
persona.getDocumentForManteca with an array of documents including multiple
id_class values ordered to trigger the BR priority map and assert the function
returns the expected document; reuse the existing document fixture shape
(id_class.value, id_number.value, id_issuing_country.value, id_document_id) and
name the test something like "returns document by id class priority for BR" to
prevent regressions in persona.getDocumentForManteca.
In `@server/utils/ramps/manteca.ts`:
- Line 21: The import line mixes runtime imports with a type; split the
type-only symbol into a separate import type statement: keep the runtime imports
getAccount, getDocument, getDocumentForManteca in the existing import and move
MantecaCountryCode (aliased as CountryCode) into its own "import type {
MantecaCountryCode as CountryCode } from \"../persona\";" declaration so the
type-only import uses TypeScript's import type syntax.
- Around line 241-248: The predicate computing hasPendingTasks on
mantecaUser.onboarding must guard against undefined OnboardingTaskInfo entries
and treat IN_PROGRESS as onboarding: update the some(...) callback used for
hasPendingTasks to use optional chaining (e.g., task?.required) and check status
values (include "PENDING" or "IN_PROGRESS" or check status !== "COMPLETED") so
undefined tasks don't throw and IN_PROGRESS keeps the user in the "ONBOARDING"
branch.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 18-19: The file currently imports "database" and "credentials"
directly; update server/api/ramp.ts to stop importing those and instead read
them from the request context (use c.var.db and c.var.credentials) inside every
route handler that interacts with the DB (replace direct references to
database.* and credentials with c.var.db and c.var.credentials). Remove the
top-level import line for database/credentials and update all handler functions
in this file (including the handlers around the previously noted block at lines
~77-80) to accept and use the context variable (c) and call c.var.db methods
rather than the imported client.
- Around line 61-107: The GET "/" route (created via new Hono().get(...)) and
the other Ramp routes (GET "/limits", GET "/quote", POST "/") are missing
OpenAPI specs; add .doc() calls chained onto each route to provide OpenAPI
metadata (summary, description, tags), request parameter schemas (reuse the
existing vValidator schemas for query/body where applicable), and response
schemas/types matching the handler outputs (e.g., provider list shape for the
GET "/" handler using ProviderInfo and ErrorCodes). Ensure you attach .doc()
immediately after the handler registration for each route, reference the same
validator schemas (e.g., the object(...) passed to vValidator) and the response
DTOs so the generated OpenAPI accurately reflects query params, request bodies,
and successful/error responses.
- Around line 115-116: Replace the bare TODO comment "TODO support multiple
providers" with a TODO that includes the project ticket reference (e.g. `#438`)
per guidelines; update the comment near the existing TODO so it reads something
like a TODO with the issue id or URL (attach the issue number you referenced) to
link this work to the tracker.
- Around line 84-88: Validate credential.account with the Address valibot schema
before passing it to getMantecaProvider: use parse(Address, credential.account)
(or equivalent validation helper) to convert/validate the plain string into the
branded Address type, assign the result to a variable (e.g., validatedAccount)
and pass that into getMantecaProvider(validatedAccount, countryCode); handle
validation errors by capturing the exception (captureException) and returning
the same fallback object used in the existing catch path so runtime typing and
safety are preserved for the getMantecaProvider call.
In `@server/hooks/manteca.ts`:
- Around line 194-200: The captureException call in manteca.ts is sending the
full webhook payload (variable payload) to Sentry and may leak PII; change the
call in the credential-not-found branch around the
credential/credentials.findFirst check so it logs only minimal identifiers
(e.g., account and any non-PII request id) instead of the entire
payload—sanitize or replace contexts payload with an object like { account } or
{ account, webhookId } and keep level:"error"; update the captureException
invocation (and any related error logging) to use the sanitized context.
In `@server/test/hooks/manteca.test.ts`:
- Around line 206-226: The test "returns credential not found when credential
does not exist" fails address validation because userExternalId is set to
"nonexistent"; change payload.data.userExternalId to a valid 40-hex character
string (an address-format hex not present in the DB) so the handler passes the
address validation and exercises the credential-not-found branch; keep
createSignature(payload) and the rest of the request unchanged.
mainqueg
force-pushed
the
manteca
branch
2 times, most recently
from
f08c05a to
6e6a96a
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/bridge.ts (1)
122-124: Stub throws unhandled error.The
onboardingfunction rejects with a generic error. If callers invoke this, they'll receive an unhandled 5xx. Consider throwing anHTTPExceptionwith a clearer status or adding documentation that Bridge onboarding is not yet implemented.♻️ Suggested improvement
export async function onboarding(_data: Onboarding): Promise<void> { - await Promise.reject(new Error("not implemented")); + throw new Error("bridge onboarding not implemented"); }
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 217-218: The code uses credential.account directly when calling
setUser and mantecaOnboarding; validate and parse credential.account first (same
as GET "/" endpoint) into an account variable and check its shape/presence
before use, then call setUser({ id: account.output }) and pass account.output
into mantecaOnboarding; update references to use the parsed account variable
instead of credential.account to keep validation consistent.
- Around line 122-123: The code passes credential.account directly into setUser
in the POST handler; make it consistent with the GET "/" handler by validating
credential.account with safeParse(Address, credential.account) (as used before
calling getMantecaProvider) and only call setUser({ id: parsed.data }) when
validation succeeds, otherwise return the same ErrorCodes.NO_CREDENTIAL (or
appropriate 400) response; update any downstream use (e.g., getMantecaProvider)
to use the validated value rather than the raw credential.account.
- Around line 154-155: Validate credential.account the same way GET "/" does
before calling setUser and before passing the address to bridge deposit detail
calls: run the existing address validation utility used in the GET "/" handler
on credential.account, handle invalid addresses by returning the same error
response, then assign the validated result to a variable (e.g., account) and
call setUser({ id: account.output }) and use account.output for subsequent
bridge deposit detail calls instead of credential.account; ensure you update
both places where bridge deposit details are called.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@server/test/api/ramp.test.ts`:
- Around line 122-132: Extract a reusable base Manteca user fixture and replace
the duplicated inline objects used in vi.spyOn(manteca,
"getUser").mockResolvedValue calls; create a single const (e.g.,
baseMantecaUser) with common fields (id, numberId, type, exchange, onboarding,
creationTime, updatedAt) and then use object spread to override only status in
each test (e.g., { ...baseMantecaUser, status: "ONBOARDING" }) where
manteca.getUser is mocked in the failing and other related tests.
- Around line 288-294: The test currently leaves the depositInfo assertion
commented out; update the test to assert the mocked deposit details are returned
by uncommenting or adding an assertion for depositInfo (e.g.,
expect(json).toMatchObject({ quote: { buyRate: "1.00", sellRate: "1.00" },
depositInfo: expect.any(Array) }) or a stricter match against the specific
mocked payload), ensuring this corresponds to the mock for getDepositDetails
used in the test so the response validation verifies depositInfo is present and
correct.
|
@coderabbitai describe the new api endpoints |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 9895
GET /ramp/Aggregates provider information from Manteca and Bridge. Query Parameters:
Response: {
"providers": {
"manteca": {
"status": "NOT_STARTED" | "ACTIVE" | "ONBOARDING" | "NOT_AVAILABLE",
"onramp": {
"currencies": string[],
"cryptoCurrencies": Array<{ cryptoCurrency, network }>
}
},
"bridge": { /* same structure */ }
}
}Error Codes: GET /ramp/limitsReturns Manteca user limits. User must be in ACTIVE status. Response: {
"manteca": {
"limits": { /* Manteca limit details */ }
}
}Error Codes:
GET /ramp/quoteRetrieves quotes and deposit details. Supports three query patterns: Query Parameters (union):
Response: {
"quote": { /* quote details, optional */ },
"depositInfo": [ /* deposit detail objects */ ]
}Error Codes:
POST /ramp/Initiates provider onboarding. Request Body (variant): // Bridge
{ "provider": "bridge", "acceptedTermsId": "string" }
// Manteca
{ "provider": "manteca" }Response: { "code": "ok" }Error Codes:
All endpoints require authentication via the |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/manteca.ts (1)
74-85: Update the return type to match the actual function behavior.
The catch block implicitly returnsundefinedwhen an error occurs (line 76-78), but the function signature doesn't reflect this. Either rethrow the error or update the return type to includeundefinedso callers can properly handle both cases.♻️ Suggested change
-export async function getQuote(coinPair: string): Promise<InferOutput<typeof shared.QuoteResponse>> { +export async function getQuote( + coinPair: string, +): Promise<InferOutput<typeof shared.QuoteResponse> | undefined> { const quote = await request(QuoteResponse, `/crypto/v2/prices/direct/${coinPair}`, {}, undefined, "GET").catch( (error: unknown) => { captureException(error, { level: "error" }); + return undefined; }, ); - if (!quote) return; + if (!quote) return undefined; return { buyRate: quote.buy, sellRate: quote.sell, }; }
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/manteca.ts (1)
556-558: TODO comments need issue references.Per coding guidelines, TODO comments should include a ticket/issue reference.
♻️ Suggested fix
- // TODO for testing, remove + // TODO(`#438`) for testing, remove US: "ARGENTINA",Also applies to line 573-574.
🤖 Fix all issues with AI agents
In `@server/hooks/manteca.ts`:
- Around line 208-211: The captureException call in the CANCELLED branch
currently sends the full payload (variable payload) to Sentry; instead sanitize
the context to only include required fields (e.g., payload.data.status and
payload.event) and avoid logging user identifiers (userExternalId, userNumberId)
or transaction details; update the call to captureException(new Error("order
cancelled"), { level: "error", contexts: { payload: { status:
payload.data.status, event: payload.event } } }) and keep
convertBalanceToUsdc(payload.data.userNumberId, payload.data.against) as-is (or
if possible also avoid passing sensitive identifiers into logs within that
function).
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@server/api/ramp.ts`:
- Around line 150-163: The quote ticker is built with an underscore but the rest
of the code expects a hyphen; update the call that constructs the ticker in the
route (currently getMantecaQuote(`USDC_${query.currency}`)) to use a hyphen
(USDC-${query.currency}) and verify any other uses in this file or nearby
helpers (getMantecaQuote, getMantecaDepositDetails) follow the same delimiter so
Manteca quote requests use the correct format.
In `@server/hooks/manteca.ts`:
- Around line 152-166: The captureEvent calls currently send the full webhook
payload (payload) to Sentry; update each handler (checks of payload.event such
as "SYSTEM_NOTICE", "COMPLIANCE_NOTICE", "PAYMENT_REFUND" and the
ORDER_STATUS_UPDATE handler) to redact sensitive fields by replacing contexts: {
payload } with a minimal context that only includes the event type (e.g.,
contexts: { event: payload.event }) or an explicit sanitized object { event:
payload.event } before calling captureEvent; ensure no personal identifiers
(legalId, userId, userNumberId, etc.) are included in the object passed to
captureEvent.
- Around line 178-181: Replace the direct use of the imported database client
with the context-injected DB client (c.var.db) in this handler: where you
currently call database.query.credentials.findFirst(...) to fetch credential,
call c.var.db.query.credentials.findFirst(...) instead (preserving the same
columns and where clause using credentials.account and account); ensure the
handler uses the provided context variable c so the query uses the injected
client.
In `@server/test/utils/manteca.test.ts`:
- Around line 67-75: Replace the hard-coded pixKey test fixture with a
non-secret placeholder to avoid secret scanning failures: update the expected
pixKey value in the test that calls manteca.getDepositDetails (reference
manteca.getDepositDetails and the pixKey field) to an obvious placeholder like
"PIX_KEY_PLACEHOLDER" and then update the corresponding source constant in the
code that supplies deposit details so the test and implementation match;
alternatively, if this value must remain literal for other reasons, add the test
value to your secret-scanner allowlist instead.
In `@server/utils/persona.ts`:
- Around line 449-457: MantecaCountryCode lists 10 countries but
allowedMantecaCountries only contains AR and BR which makes getAllowedMantecaIds
return undefined (triggering NOT_SUPPORTED) for the other entries; add a short
clarifying comment above MantecaCountryCode or above allowedMantecaCountries
stating this is a phased rollout and only AR and BR are currently supported (the
remaining country codes are reserved for future implementation) so readers
understand the intentional omission and the NOT_SUPPORTED behavior.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/bridge.ts (1)
105-114: Stub implementation returns NOT_AVAILABLE status.The
getProviderfunction currently ignores all input parameters and returns a hardcoded NOT_AVAILABLE response. This is acceptable as a placeholder while bridge integration is pending, but the unused parameters (countryCode,credentialId,customerId,redirectURL) in theGetProvidertype may cause confusion.Consider adding a brief comment indicating this is a stub implementation pending full bridge integration.
💡 Optional documentation suggestion
export async function getProvider(_data: GetProvider): Promise<InferOutput<typeof common.ProviderInfo>> { + // TODO bridge provider not yet implemented return await Promise.resolve({ status: "NOT_AVAILABLE", onramp: { currencies: [], cryptoCurrencies: [] } }); }
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/utils/ramps/bridge.ts (1)
112-124: Stub implementations for bridge provider.The
getProviderreturns a hardcodedNOT_AVAILABLEstatus, andonboardingexplicitly rejects with "not implemented". This is appropriate for a feature not yet fully implemented, but consider adding a TODO comment to track completion.♻️ Suggested documentation
+// TODO `#438` implement bridge provider availability check export async function getProvider(_data: GetProvider): Promise<InferOutput<typeof common.ProviderInfo>> { return await Promise.resolve({ status: "NOT_AVAILABLE", onramp: { currencies: [], cryptoCurrencies: [] } }); } +// TODO `#438` implement bridge onboarding flow export async function onboarding(_data: Onboarding): Promise<void> { await Promise.reject(new Error("not implemented")); }
🤖 Fix all issues with AI agents
In `@server/test/utils/manteca.test.ts`:
- Around line 220-222: The test currently only asserts
expect(result.status).toBe("ACTIVE") but doesn't validate the onramp payload
shape; update the test that produces the result object to also assert that
result.onramp exists and matches the ProviderInfo shape by checking
result.onramp.currencies and result.onramp.cryptoCurrencies are arrays (and
optionally contain expected entries), and validate any expected fields on each
provider entry (e.g., provider id/name/currencies) so regressions in the onramp
structure are caught.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/kyc.ts (1)
94-96: Guard legacy KYC shortcut to basic scope.
Withscope="manteca", returning"legacy kyc"can bypass manteca-specific evaluation and incorrectly report approval. Gate the legacy path to basic only.🐛 Proposed fix
- if (await isLegacy(credentialId, account, credential.factory)) { + if (scope === "basic" && (await isLegacy(credentialId, account, credential.factory))) { return c.json({ code: "legacy kyc", legacy: "legacy kyc" }, 200); }
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
server/api/kyc.ts (1)
94-106: Avoid legacy short‑circuit for manteca scope.The legacy branch runs before the scope-specific template evaluation. For
scope="manteca", a legacy account can be marked"legacy kyc"based on Cryptomate/Panda checks and skip manteca-required inquiry fields. Gate legacy checks to the basic scope (or move them under the basic-only branch) so manteca always evaluates its template.🐛 Suggested fix
- if (await isLegacy(credentialId, account, credential.factory)) { - return c.json({ code: "legacy kyc", legacy: "legacy kyc" }, 200); - } + if (scope === "basic" && (await isLegacy(credentialId, account, credential.factory))) { + return c.json({ code: "legacy kyc", legacy: "legacy kyc" }, 200); + }
🤖 Fix all issues with AI agents
In `@server/hooks/manteca.ts`:
- Around line 236-240: The captureEvent call inside the catch block handling
ErrorCodes.INVALID_ORDER_SIZE is currently sending the entire data object (which
contains PII like userExternalId, userNumberId, userLegalId) to Sentry; instead
build and send a sanitized object (e.g., sanitizedData) that omits or redacts
those fields and only includes non-PII safe fields (timestamp, orderId if safe,
error context, etc.), then pass sanitizedData to captureEvent in place of data;
update the catch callback where ErrorCodes.INVALID_ORDER_SIZE is checked and the
captureEvent invocation is made to ensure no PII fields are included.
| .catch((error: unknown) => { | ||
| if (error instanceof Error && error.message.includes(ErrorCodes.INVALID_ORDER_SIZE)) { | ||
| // TODO send a push notification to the user | ||
| captureEvent({ message: "MANTECA INVALID ORDER SIZE", contexts: { data } }); | ||
| captureEvent({ message: "MANTECA INVALID ORDER SIZE", level: "error", contexts: { data } }); | ||
| return; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n server/hooks/manteca.ts | head -260 | tail -40Repository: exactly/exa
Length of output: 1806
🏁 Script executed:
wc -l server/hooks/manteca.tsRepository: exactly/exa
Length of output: 82
🏁 Script executed:
cat -n server/hooks/manteca.ts | sed -n '200,280p'Repository: exactly/exa
Length of output: 2904
🏁 Script executed:
cat -n server/hooks/manteca.ts | sed -n '1,250p'Repository: exactly/exa
Length of output: 9782
Redact PII before sending to Sentry.
data contains user identifiers (userExternalId, userNumberId) and legal identifiers (userLegalId) that must not be sent to external services. Log only safe fields:
Fix
- captureEvent({ message: "MANTECA INVALID ORDER SIZE", level: "error", contexts: { data } });
+ captureEvent({
+ message: "MANTECA INVALID ORDER SIZE",
+ level: "error",
+ contexts: { details: { event: "DEPOSIT_DETECTED", depositId: data.id, asset: data.asset } },
+ });🤖 Prompt for AI Agents
In `@server/hooks/manteca.ts` around lines 236 - 240, The captureEvent call inside
the catch block handling ErrorCodes.INVALID_ORDER_SIZE is currently sending the
entire data object (which contains PII like userExternalId, userNumberId,
userLegalId) to Sentry; instead build and send a sanitized object (e.g.,
sanitizedData) that omits or redacts those fields and only includes non-PII safe
fields (timestamp, orderId if safe, error context, etc.), then pass
sanitizedData to captureEvent in place of data; update the catch callback where
ErrorCodes.INVALID_ORDER_SIZE is checked and the captureEvent invocation is made
to ensure no PII fields are included.
Issues attributed to commits in this pull requestThis pull request was merged and Sentry observed the following issues:
|
3 participants
closes #438
Summary by CodeRabbit
New Features
Bug Fixes / Reliability
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.