From ab83828b1b242daef5511953a035248fdc8b5092 Mon Sep 17 00:00:00 2001 From: jerryfan01234 <44346807+jerryfan01234@users.noreply.github.com> Date: Tue, 24 Sep 2024 17:47:28 -0400 Subject: [PATCH] [OTE-784] Limit addresses for compliance check to dydx wallets with deposit --- .../src/clients/elliptic-provider.ts | 4 +- indexer/packages/compliance/src/index.ts | 1 + .../stores/compliance-data-table.test.ts | 25 ++++++++ .../postgres/src/stores/compliance-table.ts | 11 ++++ .../postgres/src/types/query-types.ts | 2 + .../api/v4/compliance-controller.test.ts | 34 ++++++++++- .../api/v4/compliance-controller.ts | 17 +++++- .../tasks/update-compliance-data.test.ts | 60 +++++++++++++++++++ .../src/tasks/update-compliance-data.ts | 28 ++++++++- 9 files changed, 178 insertions(+), 4 deletions(-) diff --git a/indexer/packages/compliance/src/clients/elliptic-provider.ts b/indexer/packages/compliance/src/clients/elliptic-provider.ts index 74df5c538d..9004470047 100644 --- a/indexer/packages/compliance/src/clients/elliptic-provider.ts +++ b/indexer/packages/compliance/src/clients/elliptic-provider.ts @@ -26,6 +26,8 @@ export const API_PATH: string = '/v2/wallet/synchronous'; export const API_URI: string = `https://aml-api.elliptic.co${API_PATH}`; export const RISK_SCORE_KEY: string = 'risk_score'; export const NO_RULES_TRIGGERED_RISK_SCORE: number = -1; +// We use different negative values of risk score to represent different elliptic response states +export const NOT_IN_BLOCKCHAIN_RISK_SCORE: number = -2; export class EllipticProviderClient extends ComplianceClient { private apiKey: string; @@ -98,7 +100,7 @@ export class EllipticProviderClient extends ComplianceClient { `${config.SERVICE_NAME}.get_elliptic_risk_score.status_code`, { status: '404' }, ); - return NO_RULES_TRIGGERED_RISK_SCORE; + return NOT_IN_BLOCKCHAIN_RISK_SCORE; } if (error?.response?.status === 429) { diff --git a/indexer/packages/compliance/src/index.ts b/indexer/packages/compliance/src/index.ts index b3a587079a..0fc7422a05 100644 --- a/indexer/packages/compliance/src/index.ts +++ b/indexer/packages/compliance/src/index.ts @@ -5,3 +5,4 @@ export * from './geoblocking/util'; export * from './types'; export * from './config'; export * from './constants'; +export * from './clients/elliptic-provider'; diff --git a/indexer/packages/postgres/__tests__/stores/compliance-data-table.test.ts b/indexer/packages/postgres/__tests__/stores/compliance-data-table.test.ts index 2802726c83..f620ca50a1 100644 --- a/indexer/packages/postgres/__tests__/stores/compliance-data-table.test.ts +++ b/indexer/packages/postgres/__tests__/stores/compliance-data-table.test.ts @@ -1,5 +1,6 @@ import { ComplianceDataFromDatabase, ComplianceProvider } from '../../src/types'; import * as ComplianceDataTable from '../../src/stores/compliance-table'; +import * as WalletTable from '../../src/stores/wallet-table'; import { clearData, migrate, @@ -9,6 +10,7 @@ import { blockedComplianceData, blockedAddress, nonBlockedComplianceData, + defaultWallet, } from '../helpers/constants'; import { DateTime } from 'luxon'; @@ -139,6 +141,29 @@ describe('Compliance data store', () => { expect(complianceData).toEqual(blockedComplianceData); }); + it('Successfully filters by onlyDydxAddressWithDeposit', async () => { + // Create two compliance entries, one with a corresponding wallet entry and another without + await Promise.all([ + WalletTable.create(defaultWallet), + ComplianceDataTable.create(nonBlockedComplianceData), + ComplianceDataTable.create({ + ...nonBlockedComplianceData, + address: 'not_dydx_address', + }), + ]); + + const complianceData: ComplianceDataFromDatabase[] = await ComplianceDataTable.findAll( + { + addressInWalletsTable: true, + }, + [], + { readReplica: true }, + ); + + expect(complianceData.length).toEqual(1); + expect(complianceData[0]).toEqual(nonBlockedComplianceData); + }); + it('Unable finds compliance data', async () => { const complianceData: ComplianceDataFromDatabase | undefined = await ComplianceDataTable.findByAddressAndProvider( diff --git a/indexer/packages/postgres/src/stores/compliance-table.ts b/indexer/packages/postgres/src/stores/compliance-table.ts index 162edff16d..d3d53c9bb3 100644 --- a/indexer/packages/postgres/src/stores/compliance-table.ts +++ b/indexer/packages/postgres/src/stores/compliance-table.ts @@ -13,6 +13,7 @@ import { } from '../helpers/stores-helpers'; import Transaction from '../helpers/transaction'; import ComplianceDataModel from '../models/compliance-data-model'; +import WalletModel from '../models/wallet-model'; import { ComplianceDataFromDatabase, ComplianceDataQueryConfig, @@ -34,6 +35,7 @@ export async function findAll( provider, blocked, limit, + addressInWalletsTable, }: ComplianceDataQueryConfig, requiredFields: QueryableField[], options: Options = DEFAULT_POSTGRES_OPTIONS, @@ -45,6 +47,7 @@ export async function findAll( provider, blocked, limit, + addressInWalletsTable, } as QueryConfig, requiredFields, ); @@ -70,6 +73,14 @@ export async function findAll( baseQuery = baseQuery.where(ComplianceDataColumns.blocked, blocked); } + if (addressInWalletsTable === true) { + baseQuery = baseQuery.innerJoin( + WalletModel.tableName, + `${ComplianceDataModel.tableName}.${ComplianceDataColumns.address}`, + '=', + `${WalletModel.tableName}.${WalletModel.idColumn}`); + } + if (options.orderBy !== undefined) { for (const [column, order] of options.orderBy) { baseQuery = baseQuery.orderBy( diff --git a/indexer/packages/postgres/src/types/query-types.ts b/indexer/packages/postgres/src/types/query-types.ts index c5c18cab71..3b5cd025e2 100644 --- a/indexer/packages/postgres/src/types/query-types.ts +++ b/indexer/packages/postgres/src/types/query-types.ts @@ -92,6 +92,7 @@ export enum QueryableField { REFEREE_ADDRESS = 'refereeAddress', KEY = 'key', TOKEN = 'token', + ADDRESS_IN_WALLETS_TABLE = 'addressInWalletsTable', } export interface QueryConfig { @@ -291,6 +292,7 @@ export interface ComplianceDataQueryConfig extends QueryConfig { [QueryableField.UPDATED_BEFORE_OR_AT]?: string, [QueryableField.PROVIDER]?: string, [QueryableField.BLOCKED]?: boolean, + [QueryableField.ADDRESS_IN_WALLETS_TABLE]?: boolean, } export interface ComplianceStatusQueryConfig extends QueryConfig { diff --git a/indexer/services/comlink/__tests__/controllers/api/v4/compliance-controller.test.ts b/indexer/services/comlink/__tests__/controllers/api/v4/compliance-controller.test.ts index 34e12bddaa..571abd389f 100644 --- a/indexer/services/comlink/__tests__/controllers/api/v4/compliance-controller.test.ts +++ b/indexer/services/comlink/__tests__/controllers/api/v4/compliance-controller.test.ts @@ -9,7 +9,11 @@ import { } from '@dydxprotocol-indexer/postgres'; import { stats } from '@dydxprotocol-indexer/base'; import { complianceProvider } from '../../../../src/helpers/compliance/compliance-clients'; -import { ComplianceClientResponse, INDEXER_COMPLIANCE_BLOCKED_PAYLOAD } from '@dydxprotocol-indexer/compliance'; +import { + ComplianceClientResponse, + INDEXER_COMPLIANCE_BLOCKED_PAYLOAD, + NOT_IN_BLOCKCHAIN_RISK_SCORE, +} from '@dydxprotocol-indexer/compliance'; import { ratelimitRedis } from '../../../../src/caches/rate-limiters'; import { redis } from '@dydxprotocol-indexer/redis'; import { DateTime } from 'luxon'; @@ -257,5 +261,33 @@ describe('compliance-controller#V4', () => { { provider: complianceProvider.provider }, ); }); + + it('GET /screen for invalid address does not upsert compliance data', async () => { + const invalidAddress: string = 'invalidAddress'; + const notInBlockchainRiskScore: string = NOT_IN_BLOCKCHAIN_RISK_SCORE.toString(); + + jest.spyOn(complianceProvider.client, 'getComplianceResponse').mockImplementation( + (address: string): Promise => { + return Promise.resolve({ + address, + blocked, + riskScore: notInBlockchainRiskScore, + }); + }, + ); + + const response: any = await sendRequest({ + type: RequestMethod.GET, + path: `/v4/screen?address=${invalidAddress}`, + }); + + expect(response.body).toEqual({ + restricted: false, + reason: undefined, + }); + + const data = await ComplianceTable.findAll({}, [], {}); + expect(data).toHaveLength(0); + }); }); }); diff --git a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts index 09b5989c67..f8f5bd23e1 100644 --- a/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts +++ b/indexer/services/comlink/src/controllers/api/v4/compliance-controller.ts @@ -1,5 +1,9 @@ import { logger, stats, TooManyRequestsError } from '@dydxprotocol-indexer/base'; -import { ComplianceClientResponse, INDEXER_COMPLIANCE_BLOCKED_PAYLOAD } from '@dydxprotocol-indexer/compliance'; +import { + ComplianceClientResponse, + INDEXER_COMPLIANCE_BLOCKED_PAYLOAD, + NOT_IN_BLOCKCHAIN_RISK_SCORE, +} from '@dydxprotocol-indexer/compliance'; import { ComplianceDataCreateObject, ComplianceDataFromDatabase, ComplianceTable } from '@dydxprotocol-indexer/postgres'; import express from 'express'; import { checkSchema, matchedData } from 'express-validator'; @@ -85,6 +89,17 @@ export class ComplianceControllerHelper extends Controller { ComplianceClientResponse = await complianceProvider.client.getComplianceResponse( address, ); + // Don't upsert invalid addresses (address causing ellitic error) to compliance table. + // When the elliptic request fails with 404, getComplianceResponse returns + // riskScore=NOT_IN_BLOCKCHAIN_RISK_SCORE + if (response.riskScore === undefined || + Number(response.riskScore) === NOT_IN_BLOCKCHAIN_RISK_SCORE) { + return { + restricted: false, + reason: undefined, + }; + } + complianceData = await ComplianceTable.upsert({ ..._.omitBy(response, _.isUndefined) as ComplianceDataCreateObject, provider: complianceProvider.provider, diff --git a/indexer/services/roundtable/__tests__/tasks/update-compliance-data.test.ts b/indexer/services/roundtable/__tests__/tasks/update-compliance-data.test.ts index 9b5c2f4a62..e0493b2ec7 100644 --- a/indexer/services/roundtable/__tests__/tasks/update-compliance-data.test.ts +++ b/indexer/services/roundtable/__tests__/tasks/update-compliance-data.test.ts @@ -678,6 +678,66 @@ describe('update-compliance-data', () => { config.MAX_COMPLIANCE_DATA_QUERY_PER_LOOP = defaultMaxQueries; }); + + it('Only updates old addresses that are in wallets table', async () => { + const rogueWallet: string = 'address_not_in_wallets'; + // Seed database with old compliance data, and set up subaccounts to not be active + // Create a compliance dataentry that is not in the wallets table + await Promise.all([ + setupComplianceData(config.MAX_COMPLIANCE_DATA_AGE_SECONDS * 2), + setupInitialSubaccounts(config.MAX_ACTIVE_COMPLIANCE_DATA_AGE_SECONDS * 2), + ]); + await ComplianceTable.create({ + ...testConstants.nonBlockedComplianceData, + address: rogueWallet, + }); + + const riskScore: string = '75.00'; + setupMockProvider( + mockProvider, + { [testConstants.defaultAddress]: { blocked: true, riskScore } }, + ); + + await updateComplianceDataTask(mockProvider); + + const updatedCompliancnceData: ComplianceDataFromDatabase[] = await ComplianceTable.findAll({ + address: [testConstants.defaultAddress], + }, [], {}); + const unchangedComplianceData: ComplianceDataFromDatabase[] = await ComplianceTable.findAll({ + address: [rogueWallet], + }, [], {}); + + expectUpdatedCompliance( + updatedCompliancnceData[0], + { + address: testConstants.defaultAddress, + blocked: true, + riskScore, + }, + mockProvider.provider, + ); + expectUpdatedCompliance( + unchangedComplianceData[0], + { + address: rogueWallet, + blocked: testConstants.nonBlockedComplianceData.blocked, + riskScore: testConstants.nonBlockedComplianceData.riskScore, + }, + mockProvider.provider, + ); + expectGaugeStats({ + activeAddresses: 0, + newAddresses: 0, + oldAddresses: 1, + addressesScreened: 1, + upserted: 1, + statusUpserted: 1, + activeAddressesWithStaleCompliance: 0, + inactiveAddressesWithStaleCompliance: 1, + }, + mockProvider.provider, + ); + }); }); async function setupComplianceData( diff --git a/indexer/services/roundtable/src/tasks/update-compliance-data.ts b/indexer/services/roundtable/src/tasks/update-compliance-data.ts index ba6be2e32b..04db96294c 100644 --- a/indexer/services/roundtable/src/tasks/update-compliance-data.ts +++ b/indexer/services/roundtable/src/tasks/update-compliance-data.ts @@ -1,7 +1,7 @@ import { STATS_NO_SAMPLING, delay, logger, stats, } from '@dydxprotocol-indexer/base'; -import { ComplianceClientResponse } from '@dydxprotocol-indexer/compliance'; +import { ComplianceClientResponse, NOT_IN_BLOCKCHAIN_RISK_SCORE } from '@dydxprotocol-indexer/compliance'; import { ComplianceDataColumns, ComplianceDataCreateObject, @@ -151,6 +151,7 @@ export default async function runTask( blocked: false, provider: complianceProvider.provider, updatedBeforeOrAt: ageThreshold, + addressInWalletsTable: true, }, [], { readReplica: true }, @@ -318,10 +319,19 @@ async function getComplianceData( return result.value; }, )); + const addressNotFoundResponses: + PromiseFulfilledResult[] = successResponses.filter( + (result: PromiseSettledResult): + result is PromiseFulfilledResult => { + // riskScore = NOT_IN_BLOCKCHAIN_RISK_SCORE denotes elliptic 404 responses + return result.status === 'fulfilled' && result.value.riskScore === NOT_IN_BLOCKCHAIN_RISK_SCORE.toString(); + }, + ); if (failedResponses.length > 0) { const addressesWithoutResponses: string[] = _.without( addresses, + // complianceResponses includes 404 responses ..._.map(complianceResponses, 'address'), ); stats.increment( @@ -337,6 +347,22 @@ async function getComplianceData( errors: failedResponses, }); } + + if (addressNotFoundResponses.length > 0) { + const notFoundAddresses = addressNotFoundResponses.map((result) => result.value.address); + + stats.increment( + `${config.SERVICE_NAME}.${taskName}.get_compliance_data_404`, + 1, + undefined, + { provider: complianceProvider.provider }, + ); + logger.error({ + at: 'updated-compliance-data#getComplianceData', + message: 'Failed to retrieve compliance data for the addresses due to elliptic 404', + addresses: notFoundAddresses, + }); + } stats.timing( `${config.SERVICE_NAME}.${taskName}.get_batch_compliance_data`, Date.now() - startBatch,