-
Notifications
You must be signed in to change notification settings - Fork 18
fix(swap): correct misleading includeTx default value #2792
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Conversation
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
The controller always passes an explicit boolean via `includeTx === 'true'`, so the service default was never used. Change from `true` to `false` to accurately reflect the actual behavior (depositTx is only included when explicitly requested via ?includeTx=true).
Make custody order swap calls explicit about not including depositTx, matching the pattern used for sell orders. This ensures the default value change in swap.service.ts has no functional impact. Affected calls: - CustodyOrderType.SWAP - CustodyOrderType.SEND - CustodyOrderType.RECEIVE
TaprootFreak
added a commit
that referenced
this pull request
Jan 6, 2026
* feat(realunit): adjust KYC level requirements based on amount (#2771) - KYC Level 20 is now sufficient for amounts up to 1000 CHF - KYC Level 50 is still required for amounts above 1000 CHF - EUR amounts are converted to CHF for the limit check - Uses existing Config.tradingLimits.monthlyDefaultWoKyc threshold * feat(realunit): adjust KYC requirements and set Level 20 on registration (#2772) - KYC Level 20 is now sufficient for amounts up to 1000 CHF - KYC Level 50 is still required for amounts above 1000 CHF - EUR amounts are converted to CHF for the limit check - RealUnit registration now sets KYC Level 20 when completed - Added @ispositive validator to prevent zero/negative amounts * Fix mail login redirect to use /account instead of /kyc (#2773) After signing in via email link, users are now redirected to /account instead of /kyc, which provides a better default landing page for authenticated users without specific KYC context. * Add bank holidays for 2026 (#2780) * Add vIBAN search to compliance endpoint (#2775) * Add vIBAN search to compliance endpoint - Add VIBAN to ComplianceSearchType enum - Extend support service to search in VirtualIban table - Add search in BankTx.virtualIban column - Include unassigned BankTx for vIBAN results * [NOTASK] Refactoring --------- Co-authored-by: Yannick1712 <52333989+Yannick1712@users.noreply.github.com> * Fix email UX: display button before fallback link (#2774) Currently, emails display the URL link first, followed by the button. This creates a poor user experience as most users can see buttons, making the link redundant for the majority. Changes: - Reorder email content to show button first, then link - Update all translations (de/en/pt) to reflect new order - Add "or" prefix to link text to indicate it's a fallback option Affected emails: - Login email (auth.service.ts) - KYC reminder, failed, and missing data emails (kyc-notification.service.ts) - Account merge request email (account-merge.service.ts) This improves UX by prioritizing the primary action (button) and making the plain-text link available only for clients that don't support buttons. * Add Liechtenstein bank holidays 2026 and prevent fiat output on holidays (#2779) * Add Liechtenstein bank holidays 2026 and prevent fiat output on holidays - Add liechtenstein-bank-holiday.config.ts with 2026 holidays - Update fiat-output-job.service to check for LI/CH bank holidays - Block isReadyDate on bank holidays and day before after 16:00 * Restrict bank holiday check to LI IBANs only - Bank holiday blocking now only applies to Liechtenstein IBANs - CH and other IBANs are not affected by holiday checks - Remove unused isBankHoliday import * Restrict bank holiday check to LiqManagement type only - Bank holiday blocking now only applies to FiatOutputs with type LiqManagement - Other types (BuyFiat, BuyCryptoFail, etc.) are not affected * Improve bank holiday check implementation - Add verbose logging when FiatOutput is blocked due to bank holiday - Use early return (continue) for better readability - Only calculate isAfter16 when actually needed (performance) - Clearer variable scoping and logic flow * Address PR review comments - Remove unused getLiechtensteinBankHolidayInfoBanner function and import - Simplify holiday check: combine conditions with || operator - Remove unnecessary intermediate variables (isLiIban, isLiqManagement, etc.) - Use single logger.verbose message for bank holiday blocking * [NOTASK] Refactoring --------- Co-authored-by: Yannick1712 <52333989+Yannick1712@users.noreply.github.com> * Add Sepolia USDT token asset (#2783) * Add Sepolia USDT to asset seed file (#2784) * Fix sell deposit tx creation with missing user address and deposit relation (#2785) Two bugs were causing 'Failed to create deposit transaction: invalid address or ENS name': 1. In toPaymentInfoDto(): The transactionRequest only contained user: { id: userId } without the address field. Fixed by explicitly passing user.address. 2. In depositTx controller endpoint: getById() did not load the deposit relation, so route.deposit was undefined. Fixed by adding relations: { deposit: true }. Also added validation to throw clear errors if address or deposit is missing. * chore(ci): add npm caching and improve build tooling (#2788) - Add npm cache to GitHub Actions workflows (api-pr, api-dev, api-prd) for faster CI builds - Add new npm scripts: - type-check: Run tsc --noEmit for type validation without build - format:check: Check formatting without modifying files - Add forceConsistentCasingInFileNames to tsconfig.json - Upgrade @typescript-eslint/no-floating-promises from warn to error * feat: add EIP-7702 delegation support for gasless Sell and Swap Enable users to sell and swap tokens without holding ETH for gas. The backend relayer covers gas costs through MetaMask's delegation contract. Changes: - Add EIP7702DelegationService for gasless transaction preparation - Add EIP-7702 delegation DTOs for frontend communication - Add DELEGATION_TRANSFER PayInType enum value - Update Sell/Swap services to detect zero-balance users - Add depositTx endpoint to Swap controller (parity with Sell) - Add ?includeTx parameter to Swap for optional TX data - Improve gas estimation with fallback for EIP-7702 delegated addresses - Use StaticJsonRpcProvider for better performance Contracts: - MetaMask Delegator: 0x63c0c19a282a1b52b07dd5a65b58948a07dae32b - DelegationManager: 0xdb9B1e94B5b69Df7e401DDbedE43491141047dB3 * Skip Bitcoin pay-in processing when node is unavailable (#2789) Add isAvailable() check to PayInBitcoinService to prevent errors when Bitcoin node is not configured (e.g., local development). The BitcoinStrategy cron job and unconfirmed UTXO processing now gracefully skip when the service is unavailable. * Skip bank transaction check when Olky bank is not configured (#2790) Return early from checkTransactions() if no Olky bank is found in the database, preventing null pointer errors in local development. * Skip blockchain services when config is unavailable (#2791) - FrankencoinService: Skip processLogInfo when xchf contract not configured - DEuroService: Skip processLogInfo when graphUrl not configured - PaymentLinkFeeService: Skip updateFees in local environment Prevents errors in local development when external services are not configured. * Add warning logs for missing config (#2793) Log a warning when config is missing instead of silently skipping or throwing errors. Simple and consistent approach. Affected services: - BitcoinStrategy: warns if Bitcoin node not configured - PayInService: warns if Bitcoin service unavailable - BankTxService: warns if Olky bank not found - FrankencoinService: warns if xchf contract not configured - DEuroService: warns if graphUrl not configured * feat: different exceptions for buy endpoint (#2794) * feat: create different exceptions for buy endpoint. * chore: status codes. * chore: comments. * Log missing config warnings only once (#2795) * Log missing config warnings only once Prevent log spam by tracking if warning was already logged. Affects high-frequency cron jobs: - BitcoinStrategy (every second) - BankTxService (every 30 seconds) * Skip services gracefully when dependencies are unavailable - Add isAvailable() check to CheckoutService - Add availability check to CheckoutObserver with warning-once pattern - Add availability check to FiatPayInSyncService with warning-once pattern - Change ExchangeTxService to warn-once on sync failures - Add null check for client in TransactionHelper.getNetworkStartFee - Add null check for client in LogJobService.getAssetLog - Fix PaymentObserver to use optional chaining for outputDate * Add missing null checks for graceful degradation - Add null checks for client and targetAddress in PaymentBalanceService - Also check for coin existence before setting balance - Extend CheckoutService.isAvailable() to also check Config.checkout.entityId * Make warning-once pattern consistent across all services - Add warning-once with Set<Blockchain> to PaymentBalanceService - Add warning-once with Set<Blockchain> to LogJobService - Add warning-once with Set<Blockchain> to TransactionHelper All services now consistently log warnings only once when blockchain clients are not configured. * Add readonly to syncWarningsLogged Set for consistency * Add missing blank line after early return for consistency * fix(swap): correct misleading includeTx default value (#2792) * fix(swap): correct misleading includeTx default value The controller always passes an explicit boolean via `includeTx === 'true'`, so the service default was never used. Change from `true` to `false` to accurately reflect the actual behavior (depositTx is only included when explicitly requested via ?includeTx=true). * fix(custody): pass explicit includeTx=false to swap service Make custody order swap calls explicit about not including depositTx, matching the pattern used for sell orders. This ensures the default value change in swap.service.ts has no functional impact. Affected calls: - CustodyOrderType.SWAP - CustodyOrderType.SEND - CustodyOrderType.RECEIVE * Fix flaky timing test in lock.spec.ts (#2796) Replace timing-based test with Promise-based synchronization. The original test used setTimeout without delay causing race conditions. The new test uses explicit Promise signaling for deterministic behavior. * feat(gs): add debug endpoint for secure database queries (#2770) * feat(gs): add debug endpoint for secure database queries Add new DEBUG user role for developer database access with POST /gs/debug endpoint for executing read-only SQL queries. Security layers: - Role-based access (DEBUG/ADMIN/SUPER_ADMIN only) - SQL parsing with node-sql-parser (AST validation) - Only single SELECT statements allowed - Blocked: UNION/INTERSECT/EXCEPT, SELECT INTO, FOR XML/JSON - Blocked: OPENROWSET, OPENQUERY, OPENDATASOURCE (external connections) - Pre-execution column checking (blocks alias bypass) - Input validation with MaxLength(10000) - Post-execution PII column masking (defense in depth) - Full audit trail with user identification Blocked columns: mail, email, firstname, surname, iban, ip, apiKey, etc. * fix(gs): resolve eslint warnings in debug endpoint - Remove unused catch variable (use bare catch) - Remove unnecessary eslint-disable directive * [NOTASK] add more blockedCols * fix(gs): move restricted columns from ADMIN to DEBUG blocking - Remove organization.name, bank_tx.name, kyc_step.result from RestrictedColumns - Add 'name' and 'result' to DebugBlockedColumns - ADMIN can now see these columns on /gs/db - DEBUG role has these blocked on /gs/debug * fix(gs): implement table-specific column blocking for debug endpoint (#2782) * fix(gs): implement table-specific column blocking for debug endpoint Replace generic DebugBlockedColumns list with table-specific blocking: - TableBlockedColumns: Record<string, string[]> maps each table to its blocked columns - Pre-execution: Check columns against their specific tables - Post-execution for SELECT *: Mask columns from all query tables Examples: - SELECT name FROM asset → ALLOWED (asset has no blocked columns) - SELECT name FROM bank_tx → BLOCKED (bank_tx.name contains personal data) - SELECT * FROM bank_tx → name masked post-execution Tables with blocked columns: - user_data: mail, phone, firstname, surname, etc. - bank_tx: name, iban, addressLine1, etc. - bank_data: name, iban, label, comment - kyc_step: result (contains names, birthday) - organization: name, allBeneficialOwnersName, etc. * fix(gs): always run post-execution masking for defense in depth The previous implementation only masked post-execution for SELECT * queries. This was a security risk: if pre-execution column extraction failed (catch block), non-wildcard queries would not be masked. Now post-execution masking always runs, ensuring blocked columns are masked even if the SQL parser fails to detect them pre-execution. * fix(gs): add missing blocked columns from original list Add columns that were in the original DebugBlockedColumns but missing in the new table-specific structure: - user_data: countryId, verifiedCountryId, nationalityId - user: signature - fiat_output: accountNumber - checkout_tx: cardName (new table) - bank_account: accountNumber (new table) * fix(gs): add missing tables with sensitive columns Add tables that were missed when converting from global DebugBlockedColumns to table-specific TableBlockedColumns: - ref: ip (user IP for referral tracking) - ip_log: ip, country (user IP logging) - checkout_tx: ip (user IP during checkout, cardName already present) - buy: iban (user IBAN for buy routes) - deposit_route: iban (user IBAN for sell routes via Single Table Inheritance) These columns were blocked globally in the original implementation but were not added to all relevant tables in the table-specific version. * fix(gs): add additional sensitive columns found in codebase review Add missing blocked columns discovered during comprehensive entity scan: - buy_crypto: chargebackIban (user IBAN for refunds) - kyc_log: ipAddress (TfaLog), result (KYC data) - bank_tx_return: chargebackIban, recipientMail, chargebackRemittanceInfo - bank_tx_repeat: chargebackIban, chargebackRemittanceInfo - limit_request: recipientMail - ref_reward: recipientMail * fix(gs): add additional sensitive columns from codebase review Extend existing tables with missing blocked columns: - checkout_tx: cardBin, cardLast4, cardFingerPrint, cardIssuer, cardIssuerCountry, raw - buy_crypto: chargebackRemittanceInfo, siftResponse - buy_fiat: remittanceInfo, usedBank, info - crypto_input: senderAddresses - user_data: relatedUsers - limit_request: fundOriginText - bank_tx_return: info Add new tables with sensitive columns: - transaction_risk_assessment: reason, methods, summary, result (AML/KYC assessments) - support_issue: name, information (support tickets with user data) - support_message: message, fileUrl (message content and files) - sift_error_log: requestPayload (Sift API requests with PII) * fix(gs): add webhook.data to blocked columns * fix(gs): add notification.data to blocked columns * fix(gs): add kyc_step.data to blocked columns * feat(gs): add App Insights log query endpoint with template-based security (#2778) Add POST /gs/debug/logs endpoint for querying Azure Application Insights logs using predefined, safe KQL templates. Security features: - Template-based queries only (no free-form KQL input) - Strict parameter validation via class-validator (GUID, alphanumeric) - All KQL-relevant special characters blocked in user input - Defense-in-depth string escaping - Result limits per template (200-500 rows) - Full audit logging of queries Available templates: - traces-by-operation: Traces for specific operation ID - traces-by-message: Traces filtered by message pattern - exceptions-recent: Recent exceptions - request-failures: Failed HTTP requests - dependencies-slow: Slow external dependencies (by duration threshold) - custom-events: Custom events by name Infrastructure: - AppInsightsQueryService: OAuth2 client with token caching - Proper error handling and logging - Mock responses for LOC mode Requires UserRole.DEBUG and APPINSIGHTS_APP_ID env variable. * fix(gs): add security hardening for debug SQL endpoint Security improvements: 1. Block system tables and schemas: - Added BlockedSchemas list: sys, information_schema, master, msdb, tempdb - checkForBlockedSchemas() validates FROM clause and subqueries - Prevents access to sys.sql_logins, INFORMATION_SCHEMA.TABLES, etc. 2. Fix TOP validation to use AST instead of regex: - Previous regex /\btop\s+(\d+)/ missed TOP(n) with parentheses - Now uses stmt.top?.value from AST for accurate detection - Both TOP 100 and TOP(100) are correctly validated 3. Extend dangerous function check to all clauses: - Previous check only validated FROM clause - Now recursively checks SELECT columns and WHERE clauses - checkForDangerousFunctionsRecursive() traverses entire AST - Blocks OPENROWSET, OPENQUERY, OPENDATASOURCE, OPENXML everywhere * refactor(gs): improve code professionalism in debug endpoints - Remove commented debug code - Fix return type any[] → Record<string, unknown>[] - Remove redundant try-catch in controller (service handles errors) - Rename misleading parameter userMail → userIdentifier - Standardize comment style * fix(gs): restore table-specific column blocking for debug endpoint (#2797) PR #2778 accidentally reverted PR #2782's TableBlockedColumns changes due to a git reset that preserved stale local file contents. This commit restores: - TableBlockedColumns: Record<string, string[]> with 30+ table-specific blocked column lists (42 columns across 23 tables) - getTablesFromQuery(): Extract table names from SQL AST - getAliasToTableMap(): Map table aliases to real names - isColumnBlockedInTable(): Table-aware column blocking check - findBlockedColumnInQuery(): Pre-execution validation with table context - maskDebugBlockedColumns(): Post-execution masking with table context Security impact of the regression: - Columns like comment, label, data, message, fileUrl, remittanceInfo, txRaw, chargebackIban, siftResponse, requestPayload were unblocked - Table-specific blocking allows SELECT name FROM asset (harmless) while blocking SELECT name FROM bank_tx (PII) All new features from later commits are preserved: - BlockedSchemas, DangerousFunctions - checkForBlockedSchemas(), checkForDangerousFunctionsRecursive() - LogQueryTemplates, executeLogQuery(), AppInsightsQueryService --------- Co-authored-by: Yannick1712 <52333989+Yannick1712@users.noreply.github.com> * feat(realunit): add dev payment simulation for REALU purchases (#2798) * feat(realunit): add dev payment simulation for REALU purchases Add automatic payment simulation for REALU TransactionRequests in DEV/LOC environments. When a user creates a REALU payment info, the system now automatically simulates the bank payment and creates a BuyCrypto entry. Changes: - Add RealUnitDevService with cron job (every minute) - Query TransactionRequests for Mainnet REALU (how they're created) - Create BuyCrypto with Sepolia REALU asset (for testnet payout) - Set amlCheck: PASS to bypass AML processing - Add Sepolia REALU asset (ID 408) to seed and migration - Export BuyCryptoRepository from BuyCryptoModule - Export TransactionRequestRepository from PaymentModule - Fix toWeiAmount for tokens with 0 decimals Flow: PaymentInfo (Mainnet REALU) -> Simulation -> BuyCrypto (Sepolia) -> Cron jobs process -> Payout via SepoliaTokenStrategy * fix(realunit): allow multiple TransactionRequests per Buy route Change duplicate check from bankUsage to txInfo-based. Previously, only the first TransactionRequest for a Buy route would be simulated because subsequent requests found the existing BankTx by bankUsage. Now each TransactionRequest is tracked by its unique simulation marker in the txInfo field, allowing multiple payments to the same route. * test(evm): add unit tests for toWeiAmount with decimals=0 Add comprehensive tests for EvmUtil.toWeiAmount and fromWeiAmount: - decimals=0 (REALU case) - critical for tokens without fractional units - decimals=undefined (native coin) - defaults to 18 decimals - decimals=6 (USDT/USDC) - common stablecoin format - decimals=18 (standard ERC20) - fractional amounts with decimals=0 (rounds to nearest integer) - large amounts with decimals=0 These tests verify the fix for the decimals=0 bug where `decimals ? x : y` incorrectly treated 0 as falsy. * ci: add automatic release PR workflow (#2800) Creates a PR from develop to master automatically when: - Changes are pushed to develop - No open release PR exists Features: - Concurrency control to prevent race conditions - Explicit master branch fetch for reliable diff - Manual trigger support via workflow_dispatch - Commit count in PR description * ci: add type-check, format-check and security audit to pipelines (#2803) - Add npm run type-check step to verify TypeScript compilation - Add npm run format:check step to enforce Prettier formatting - Add npm audit --audit-level=high as non-blocking warning - Fix formatting in source files to pass format:check - Add CLAUDE.md to gitignore Applied to all workflows: api-pr.yaml, api-dev.yaml, api-prd.yaml * fix(gs): harden FOR XML/JSON check and add security tests (#2799) * fix(gs): harden FOR XML/JSON check and add security tests - Replace string-based FOR XML/JSON check with regex after normalizing comments and whitespace to prevent bypass via FOR/**/XML or FOR\tXML - Add comprehensive security test suite covering: - FOR XML/JSON bypass attempts (comment, tab, newline, multi-space) - Statement type validation (INSERT, UPDATE, DELETE, DROP) - PII column blocking (direct, aliased, subquery) - System schema blocking (sys, INFORMATION_SCHEMA, master) - Dangerous function blocking (OPENROWSET, OPENQUERY, OPENDATASOURCE) - UNION/INTERSECT/EXCEPT blocking - SELECT INTO blocking * fix(gs): use AST-based FOR XML/JSON detection instead of regex The regex approach had vulnerabilities: - Inline comment bypass: FOR -- x\nXML AUTO was not blocked - False positives: 'FOR XML' in string literals was incorrectly blocked AST-based detection using stmt.for.type is more robust: - Correctly blocks all FOR XML/JSON variants including inline comment bypass - No false positives for string literals - Consistent with the rest of the AST-based security checks Added tests: - Inline comment bypass attempt (now blocked) - String literal false positive test (now allowed) * fix(gs): check FOR XML/JSON recursively in subqueries Critical fix: The previous AST-based check only looked at stmt.for on the top-level statement. Subqueries in SELECT columns, FROM clause (derived tables), WHERE clause, and CTEs were not checked, allowing bypass via: - SELECT id, (SELECT mail FROM user_data FOR XML PATH) FROM [user] - SELECT * FROM (SELECT * FROM user_data FOR XML AUTO) as t Added recursive checkForXmlJsonRecursive() that traverses the entire AST to find FOR XML/JSON in any subquery. Tests added: - FOR XML in subquery (SELECT column) - FOR XML in derived table (FROM clause) * fix(gs): cover all FOR XML/JSON bypass vectors Extended checkForXmlJsonRecursive to cover: - HAVING clause subqueries - ORDER BY clause subqueries - JOIN ON condition subqueries - CASE expression branches (result/condition fields) Extended checkNodeForXmlJson to handle: - node.result for CASE THEN branches - node.condition for CASE WHEN conditions - node.value arrays for IN clauses Changed columns check to use checkNodeForXmlJson instead of just checking col.expr?.ast, ensuring CASE expressions and other complex column expressions are fully traversed. Added 4 new tests for the previously uncovered bypass vectors. * Add GROUP BY and WINDOW OVER FOR XML/JSON checks - Add check for subqueries in GROUP BY clause - Add check for subqueries in WINDOW OVER (ORDER BY, PARTITION BY) - Add 4 new tests covering these bypass vectors * Fix schema and dangerous function bypass vectors - checkForBlockedSchemas: Add checks for SELECT columns, HAVING, ORDER BY, GROUP BY, CTEs, JOIN ON, and WINDOW OVER clauses - checkForDangerousFunctionsRecursive: Add checks for HAVING, ORDER BY, GROUP BY, CTEs, JOIN ON, and WINDOW OVER clauses - Add CASE expression and array value checks to both functions - Add 10 new tests covering all bypass vectors * Block linked server access (4-part names) - Add check for item.server property in checkForBlockedSchemas - Block all linked server access to prevent external database access - Add 2 tests for linked server blocking * Fix formatting * perf: parallelize async operations in swap quote API (#2805) - Parallelize getSourceSpecs/getTargetSpecs calls in transaction-helper - Parallelize blockchain fee calculations in fee.service These independent operations were running sequentially, causing unnecessary latency in the swap quote endpoint. * fix(security): resolve all CodeQL security findings (#2806) * fix(security): resolve all CodeQL security findings - Path injection (ocp-sticker.service.ts): Validate lang against allowed values - Type confusion (spark.service.ts): Add nullish coalescing for Map.get() - Missing permissions (api-*.yaml): Add explicit 'contents: read' permissions - Polynomial ReDoS (gs.service.ts): Replace regex with simple string search - Incomplete sanitization (olkypay.service.ts): Use replaceAll instead of replace Resolves 9 CodeQL alerts * fix(gs): use bounded quantifier for ORDER BY detection Replace .includes('order by') with bounded regex /order\s{1,100}by/i: - Prevents ReDoS via bounded quantifier {1,100} - Maintains support for tabs and multiple spaces between ORDER and BY - Original .includes() only matched single space * feat(cron): add logging for skipped cron jobs (#2807) * Fix race condition in ProcessService initialization This reverts the regression introduced in commit 0727fed which caused all processes to be treated as "disabled" during the initial startup window before async resyncDisabledProcesses() completed. Changes: - Initialize DisabledProcesses with empty map {} instead of undefined - Simplify DisabledProcess() to only return true when explicitly disabled - Add synchronous initialization from Config.disabledProcesses() - Add logging to DfxCronService when jobs are skipped due to disabled process The previous logic returned true (disabled) when DisabledProcesses was undefined, causing a race condition where cron jobs triggered before the async initialization completed would be silently skipped. * style: fix prettier formatting * refactor: keep only logging, revert race condition fix Keep the useful logging for skipped cron jobs but revert the ProcessService changes since the race condition is self-healing and hasn't caused issues in 6 months. * [NOTASK] Refactoring Debug endpoint * fix(security): resolve remaining CodeQL security findings (#2809) * fix(security): resolve remaining CodeQL security findings - gs.service.ts: Use regex on normalized bounded string to prevent ReDoS - spark.service.ts: Add typeof check to prevent parameter tampering - ocp-sticker.service.ts: Use find() to get trusted value from list - olkypay.service.ts: Remove all bracket characters with regex - Add CodeQL config to exclude rpcauth.py (intentional credential output) * fix(bank): remove unnecessary regex escape in olkypay service ESLint no-useless-escape: '[' doesn't need escaping inside character class * style: format olkypay.service.ts * [DEV-4527] delete usedRef when merging with ref user * fix(gs): replace semicolon regex with string loop to avoid CodeQL false positive (#2810) The regex /;*$/ is O(n) and not a ReDoS risk, but CodeQL flags it. Using a while loop with endsWith() achieves the same result without triggering the static analysis warning. * Add userNonce to EIP-7702 delegation data (#2813) * Add userNonce to EIP-7702 delegation data for correct authorization signing The EIP-7702 authorization signature requires the user's current account nonce. Previously this was not provided by the API, causing transactions to fail when the user had already made EIP-7702 transactions (nonce > 0). Changes: - Make prepareDelegationData async to fetch user's account nonce from chain - Add userNonce field to Eip7702DelegationDataDto - Update sell.service.ts and swap.service.ts to await and include userNonce * Add userNonce to EIP-7702 delegation data for correct authorization signing The EIP-7702 authorization signature requires the user's current account nonce. Previously this was not provided by the API, causing transactions to fail when the user had already made EIP-7702 transactions (nonce > 0). Changes: - Make prepareDelegationData async to fetch user's account nonce from chain - Add userNonce field to Eip7702DelegationDataDto - Update sell.service.ts and swap.service.ts to await and include userNonce - Add getTransactionCount mock to all viem test mocks - Add comprehensive tests for prepareDelegationData * Disable EIP-7702 delegation for sell/swap transactions (#2816) The manual signing approach (eth_sign + eth_signTypedData_v4) doesn't work because eth_sign is disabled by default in MetaMask. This needs to be re-implemented using Pimlico's ERC-7677 paymaster service. Changes: - isDelegationSupported() now returns false unconditionally - confirmSell/confirmSwap reject eip7702 input with helpful error message * test(eip7702): skip tests while delegation is disabled (#2817) isDelegationSupported() now returns false for all chains. Tests will be re-enabled when EIP-7702 delegation is reactivated. * Add RealUnit sell endpoint with EIP-7702 support (#2818) Add new sell endpoints for RealUnit token trading: - PUT /realunit/sellPaymentInfo: Returns EIP-7702 delegation data for gasless REALU transfer plus fallback deposit info - PUT /realunit/sell/:id/confirm: Confirms sell with EIP-7702 signatures or manual transaction hash Key features: - Always returns both EIP-7702 data and fallback deposit address - RealUnit app supports eth_sign, so EIP-7702 is always available - Uses existing SellService infrastructure for route management - Creates TransactionRequest for tracking sell transactions New files: - realunit-sell.dto.ts: Request/response DTOs for sell endpoints Modified files: - realunit.controller.ts: Added sellPaymentInfo and confirm endpoints - realunit.service.ts: Added getSellPaymentInfo and confirmSell methods - realunit.module.ts: Added SellCryptoModule and Eip7702DelegationModule * [NOTASK] remove unused service import * fix(realunit): add security validation for sell confirmation (#2819) * fix(realunit): add security validation for sell confirmation - Validate delegation.delegator matches user address (defense-in-depth) - Validate transaction hash format for manual fallback (0x + 64 hex chars) * Move txHash validation to DTO - Add @matches decorator to txHash field in RealUnitSellConfirmDto - Remove redundant runtime validation from confirmSell() service method - DTO validation is the appropriate layer for input format checks * feat: Replace EIP-7702 eth_sign with EIP-5792 wallet_sendCalls (#2822) * feat: replace EIP-7702 eth_sign with EIP-5792 wallet_sendCalls Replace the EIP-7702 delegation approach (which required eth_sign) with EIP-5792 wallet_sendCalls with paymasterService capability for gasless transactions. Changes: - Add PimlicoPaymasterService for ERC-7677 paymaster URL generation - Update sell/swap services to provide EIP-5792 data instead of EIP-7702 - Add txHash field to ConfirmDto for receiving tx hash from wallet_sendCalls - Add handleTxHashInput to track sponsored transfers - Add SPONSORED_TRANSFER PayInType The new flow: 1. Backend provides paymasterUrl and calls array in response 2. Frontend uses wallet_sendCalls with paymasterService capability 3. Wallet handles EIP-7702 internally with paymaster sponsorship 4. Frontend confirms with txHash after transaction is sent This avoids the eth_sign requirement which is disabled by default in MetaMask. * chore: add pimlicoApiKey to config - Add PIMLICO_API_KEY configuration to config.ts and .env.example - Update PimlicoPaymasterService to use config instead of direct env access * test: add PimlicoPaymasterService unit tests - Test isPaymasterAvailable for supported/unsupported blockchains - Test getBundlerUrl returns correct Pimlico URLs for all chains - Test behavior when API key is not configured - Cover Ethereum, Arbitrum, Optimism, Polygon, Base, BSC, Gnosis, Sepolia * feat: differentiate trading errors by KYC level (#2823) Split TRADING_NOT_ALLOWED into specific errors: - RECOMMENDATION_REQUIRED: KycLevel >= 10, missing tradeApprovalDate - EMAIL_REQUIRED: KycLevel < 10, missing email This allows the frontend to redirect users to the appropriate KYC step instead of showing a generic error message. * test(realunit): add comprehensive unit tests for RealUnitDevService (#2801) Add 17 unit tests covering: - Environment checks (PRD skips, DEV/LOC execute) - Asset lookup (mainnet/sepolia REALU) - No waiting requests handling - Buy route not found - Duplicate prevention via txInfo field - Fiat/Bank not found cases - Bank selection (YAPEAL for CHF, OLKYPAY for EUR) - Full simulation flow (BankTx, BuyCrypto, Transaction, TransactionRequest) - Multiple request processing - Error handling (continues on failure) Coverage: 98.73% statements, 100% branches, 100% functions * style(realunit): fix prettier formatting in dev service tests * feat: auto-fill BuyFiat creditor data in FiatOutput (#2824) * feat: auto-fill BuyFiat creditor data in FiatOutput Automatically populate creditor fields (name, address, zip, city, country, currency, amount) from seller's UserData when creating FiatOutput for BuyFiat type. Changes: - fiat-output.service.ts: Extend createInternal() to populate creditor data from buyFiat.sell.user.userData for new BuyFiat FiatOutputs - buy-fiat-preparation.service.ts: Add required relations (userData, country, organizationCountry, outputAsset) to addFiatOutputs() query The bank (Yapeal) requires these fields for payment transmission. Previously admins had to manually set them via PUT /fiatOutput/:id. * feat: validate required creditor fields when creating FiatOutput Add validation to ensure currency, amount, name, address, houseNumber, zip, city, and country are provided when creating any FiatOutput. Throws BadRequestException with list of missing fields if validation fails. * feat: add iban to required creditor fields - Add iban to validation check - Auto-fill iban from sell route in createInternal for BuyFiat * feat: use payoutRoute IBAN for PaymentLink BuyFiats For PaymentLinkPayment: get IBAN from payoutRouteId in link config instead of the sell route. Falls back to sell.iban if no payoutRouteId. * feat: make creditor fields required in CreateFiatOutputDto Mark currency, amount, name, address, houseNumber, zip, city, country, iban as @isnotempty() instead of @IsOptional(). * fix: make houseNumber optional houseNumber should be provided when available but is not required. * feat: skip creditor validation for BANK_TX_RETURN Admin must provide creditor fields manually via DTO. Added TODO for future implementation. * feat: add bank-refund endpoint with required creditor fields - Add BankRefundDto with required creditor fields (name, address, zip, city, country) - Add PUT /transaction/:id/bank-refund endpoint for bank refunds - Extend BankTxRefund with creditor fields - Update refundBankTx to pass creditor data to FiatOutput - Extend createInternal with optional inputCreditorData parameter Frontend must use bank-refund endpoint and provide creditor data for bank transaction refunds to meet bank requirements. * feat: add creditor fields to UpdateBuyCryptoDto for admin flow - Add chargebackCreditorName, chargebackCreditorAddress, chargebackCreditorHouseNumber, chargebackCreditorZip, chargebackCreditorCity, chargebackCreditorCountry to DTO - Update buy-crypto.service to use DTO fields without fallback - Update bank-tx-return.service to pass creditor data to FiatOutput Admin must provide creditor data explicitly for BUY_CRYPTO_FAIL refunds. * refactor: remove bankTx fallbacks for creditor data Creditor fields must be provided explicitly via DTO. BankRefundDto enforces required fields, fallbacks were never used. * fix: add missing SellRepository to FiatOutputModule (#2825) Commit e3f815b added SellRepository injection to FiatOutputService but forgot to add it as a provider in FiatOutputModule, causing the API to crash on startup with dependency resolution error. * feat: EIP-7702 gasless transaction support via DFX relayer (#2826) * feat: add EIP-7702 gasless transaction support via DFX relayer - Add PimlicoBundlerService for EIP-7702 gasless transactions - prepareAuthorizationData(): creates typed data for frontend signing - executeGaslessTransfer(): submits tx with user's signed authorization - hasZeroNativeBalance(): checks if user needs gasless flow - Add GaslessTransferDto and Eip7702AuthorizationDto for request validation - Update SellPaymentInfoDto with gaslessAvailable and eip7702Authorization fields - Add POST /sell/paymentInfos/:id/gasless endpoint for gasless execution - Update sell.service.ts to: - Check user's native balance when creating payment info - Include gasless data in response when user has 0 ETH - Execute gasless transfers via DFX relayer Flow: User signs EIP-7702 authorization off-chain, DFX relayer submits the transaction and pays gas fees. * fix: apply Prettier formatting * feat: auto-fill and validate creditor fields for FiatOutput (#2828) * feat: auto-fill BuyFiat creditor data in FiatOutput Automatically populate creditor fields (name, address, zip, city, country, currency, amount) from seller's UserData when creating FiatOutput for BuyFiat type. Changes: - fiat-output.service.ts: Extend createInternal() to populate creditor data from buyFiat.sell.user.userData for new BuyFiat FiatOutputs - buy-fiat-preparation.service.ts: Add required relations (userData, country, organizationCountry, outputAsset) to addFiatOutputs() query The bank (Yapeal) requires these fields for payment transmission. Previously admins had to manually set them via PUT /fiatOutput/:id. * feat: validate required creditor fields when creating FiatOutput Add validation to ensure currency, amount, name, address, houseNumber, zip, city, and country are provided when creating any FiatOutput. Throws BadRequestException with list of missing fields if validation fails. * feat: add iban to required creditor fields - Add iban to validation check - Auto-fill iban from sell route in createInternal for BuyFiat * feat: use payoutRoute IBAN for PaymentLink BuyFiats For PaymentLinkPayment: get IBAN from payoutRouteId in link config instead of the sell route. Falls back to sell.iban if no payoutRouteId. * feat: make creditor fields required in CreateFiatOutputDto Mark currency, amount, name, address, houseNumber, zip, city, country, iban as @isnotempty() instead of @IsOptional(). * fix: make houseNumber optional houseNumber should be provided when available but is not required. * feat: skip creditor validation for BANK_TX_RETURN Admin must provide creditor fields manually via DTO. Added TODO for future implementation. * feat: add bank-refund endpoint with required creditor fields - Add BankRefundDto with required creditor fields (name, address, zip, city, country) - Add PUT /transaction/:id/bank-refund endpoint for bank refunds - Extend BankTxRefund with creditor fields - Update refundBankTx to pass creditor data to FiatOutput - Extend createInternal with optional inputCreditorData parameter Frontend must use bank-refund endpoint and provide creditor data for bank transaction refunds to meet bank requirements. * feat: add creditor fields to UpdateBuyCryptoDto for admin flow - Add chargebackCreditorName, chargebackCreditorAddress, chargebackCreditorHouseNumber, chargebackCreditorZip, chargebackCreditorCity, chargebackCreditorCountry to DTO - Update buy-crypto.service to use DTO fields without fallback - Update bank-tx-return.service to pass creditor data to FiatOutput Admin must provide creditor data explicitly for BUY_CRYPTO_FAIL refunds. * refactor: remove bankTx fallbacks for creditor data Creditor fields must be provided explicitly via DTO. BankRefundDto enforces required fields, fallbacks were never used. * feat: store creditor data in entity for later admin approval - Add chargebackCreditor* fields to BuyCrypto and BankTxReturn entities - Extend chargebackFillUp methods to accept and store creditor data - Update refundBankTx to save customer-provided creditor data - Use stored entity fields as fallback in admin flow Flow: Customer provides creditor data via /bank-refund endpoint, data is stored in entity. Admin later approves with chargebackAllowedDate, using stored creditor data (or can override via DTO). * refactor: consolidate creditor fields into single JSON column (#2829) - Replace 6 chargebackCreditor* columns with one chargebackCreditorData JSON column - Add CreditorData interface for type safety - Add creditorData getter for JSON parsing - Update BuyCrypto and BankTxReturn entities - Update buy-crypto.service to use creditorData getter - Add migration to add new column and drop old columns This reduces schema complexity and aligns with existing JSON patterns in the codebase (priceSteps, config, etc.). * chore: remove allowlist and bank endpoints. (#2827) * [NOTASK] remove unused import * feat: EIP-7702 gasless via Pimlico ERC-4337 (all chains) (#2834) * feat: implement EIP-7702 gasless via Pimlico ERC-4337 - Replace SimpleDelegation with MetaMask Delegator (0x63c0c19a...) - Deployed on ALL major EVM chains (not just Sepolia) - Uses onlyEntryPointOrSelf modifier for security - Implement ERC-4337 UserOperation submission via Pimlico Bundler - factory=0x7702 signals EIP-7702 UserOperation - Pimlico Paymaster sponsors gas fees - EntryPoint v0.7 (0x0000000071727De22E5E9d8BAf0edAc6f37da032) - Flow: User signs EIP-7702 authorization -> Backend creates UserOp -> Pimlico Bundler submits -> EntryPoint calls execute() on EOA -> Token transfer FROM user (gas paid by Pimlico) Supported chains: Ethereum, Arbitrum, Optimism, Polygon, Base, BSC, Gnosis, Sepolia * fix: add missing evm-chain.config.ts and fix lint warnings * [NO-TASK] Cleanup * fix: add creditor data fallback in refundBankTx methods (#2835) When batch job or admin calls refundBankTx() without creditor data in DTO, use stored creditorData from entity as fallback for FiatOutput creation. Fixed in: - BuyCryptoService.refundBankTx(): fallback to buyCrypto.creditorData - BankTxReturnService.refundBankTx(): fallback to bankTxReturn.creditorData Added unit tests for BankTxReturnService creditor data fallback. * [NO-TASK] Cleanup 2 * [NO-TASK] Cleanup 3 * fix: always show fixed IBAN and name for bank refunds (#2836) getRefundTarget now always returns bankTx.iban for bank transactions, ensuring the frontend displays IBAN and name as fixed values instead of showing editable input fields. The refund must always go back to the original sender's bank account, so allowing users to change the IBAN was incorrect behavior. * feat(eip7702): Add integration tests for gasless E2E flow (#2837) * feat(eip7702): add integration tests for gasless E2E flow Add integration tests for EIP-7702 gasless transaction flow: - gasless-e2e.integration.spec.ts: End-to-end flow test - pimlico-bundler.integration.spec.ts: Bundler API integration tests Tests verify: - Transaction request creation with delegation - Pimlico bundler UserOp submission - Sponsored transaction handling - EIP-7702 capability detection Requires PIMLICO_API_KEY environment variable * fix(eip7702): fix lint/format issues in integration tests - Add viem import at top level instead of inline require() - Remove unused ENTRY_POINT_V07 and METAMASK_DELEGATOR constants * fix: improve refund flow validation and error handling (#2838) - Enable BANK_TX_RETURN validation in FiatOutput (was skipped with TODO) - Add JSON.parse error handling for creditorData in BankTxReturn entity - Add @isiban validation to refundIban in RefundInternalDto * fix(sell,swap): only include eip5792 data when user has zero native balance (#2839) Previously, depositTx.eip5792 was always included when Pimlico paymaster was available, regardless of whether the user actually needed gasless transactions. This caused issues because: 1. Frontend checked for eip5792 presence, not gaslessAvailable flag 2. Older wallets (MetaMask <12.20) don't support EIP-5792 3. Users with ETH balance received unnecessary paymaster data Changes: - sell.service.ts: Move balance check before createDepositTx() - sell.service.ts: Pass hasZeroBalance to createDepositTx() - sell.service.ts: Only add eip5792 when includeEip5792=true - swap.service.ts: Add includeEip5792 parameter (default: false) Now eip5792 data is only included when gaslessAvailable=true, making the API response consistent and predictable. * fix: critical refund flow bugs - inverted validation and field fallback (#2840) * fix: improve refund flow validation and error handling - Enable BANK_TX_RETURN validation in FiatOutput (was skipped with TODO) - Add JSON.parse error handling for creditorData in BankTxReturn entity - Add @isiban validation to refundIban in RefundInternalDto * fix: critical refund flow bugs - inverted validation and field fallback BUG #1: Fix isRefundDataValid() inverted logic - Changed condition from `<= 0` to `> 0` - Previously: accepted expired refunds, rejected valid ones - Now: correctly validates refund expiry BUG #2: Fix bankFields fallback not working - Changed from conditional object (all-or-nothing based on name) - Now: includes all provided fields independently - Previously: if name was empty, ALL fields were discarded BUG #5: Add whitespace validation for creditor fields - Added trim() check for string fields - Previously: whitespace-only strings ' ' passed validation * revert: isRefundDataValid was correct, rollback wrong fix After deeper analysis of Util.secondsDiff(): - secondsDiff(expiryDate) = (Date.now() - expiryDate) / 1000 - If expiryDate is in FUTURE: result is NEGATIVE - If expiryDate is in PAST: result is POSITIVE Original `<= 0` is CORRECT: - Future (valid) → negative → <= 0 is TRUE → valid ✓ - Past (expired) → positive → <= 0 is FALSE → invalid ✓ The previous commit incorrectly changed this to `> 0`. * feat(swap): add gasless transaction support for Swap flow (#2841) - Add gaslessAvailable and eip7702Authorization fields to SwapPaymentInfoDto - Implement balance-check in toPaymentInfoDto() using PimlicoBundlerService - Add EIP-7702 authorization handling in confirmSwap() - Add unit tests for SellService and SwapService createDepositTx method * ci: optimize workflow step order and remove redundant type-check (#2843) - Remove redundant type-check step (build already runs tsc) - Reorder steps for fail-fast: lint → format → build → test - Clean up multiline run commands to single line This saves ~20s per run and up to 2 minutes on early failures. * feat(aml): skip phone verification for users referred by trusted referrers (#2845) * feat(aml): skip phone verification for users referred by trusted referrers Add isTrustedReferrer flag to UserData. Users who were referred by a trusted referrer (usedRef points to a user with isTrustedReferrer=true) are exempt from the phone verification check for users over 55. * test(aml): add comprehensive tests for isTrustedReferrer phone verification exemption 18 test cases covering: - Trusted referrer skips phone verification - Untrusted/no referrer requires phone verification - All edge cases (age boundaries, missing data, account types) - Truth table for refUser.userData.isTrustedReferrer values - Verification that other conditions remain unaffected * [NO-TASK] Fixed migration --------- Co-authored-by: TaprootFreak <142087526+TaprootFreak@users.noreply.github.com> Co-authored-by: Yannick1712 <52333989+Yannick1712@users.noreply.github.com> Co-authored-by: Lam Nguyen <32935491+xlamn@users.noreply.github.com> Co-authored-by: David May <david.leo.may@gmail.com>
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.
Summary
Fixes misleading default value in
createSwapPaymentInfoservice method.Problem
The service has
includeTx = trueas default:But the controller always passes an explicit boolean:
When no
?includeTxquery parameter is provided:includeTxin controller =undefinedundefined === 'true'=falseSo the service default is never used, and the actual behavior is
false.Fix
Change default from
truetofalseto accurately reflect actual behavior.This is purely a code clarity fix - no functional change.
Test Plan