Soloseng/set-PNP-default-as-optional-env-var (#10436)

* Updates DB timeout to use .env variable. Use constant as fallback * update ODIS to use env var & constants as fallback * increase test timeout * cleanup * lint fix * format * PR comments * forgotten pr feedback * happier linter --------- Co-authored-by: Javier Cortejoso <[email protected]>
celo-org · Jul 26, 2023 · f578337 · f578337
1 parent 504732d
commit f578337
Show file tree

Hide file tree

Showing 19 changed files with 121 additions and 29 deletions.
diff --git a/packages/phone-number-privacy/combiner/src/config.ts b/packages/phone-number-privacy/combiner/src/config.ts
@@ -1,6 +1,8 @@
 import {
   BlockchainConfig,
   FULL_NODE_TIMEOUT_IN_MS,
+  RETRY_COUNT,
+  RETRY_DELAY_IN_MS,
   rootLogger,
   TestUtils,
   toBool,
@@ -36,6 +38,8 @@ export interface OdisConfig {
     versions: string // parse as KeyVersionInfo[]
   }
   fullNodeTimeoutMs: number
+  fullNodeRetryCount: number
+  fullNodeRetryDelayMs: number
 }
 
 export interface CombinerConfig {
@@ -102,6 +106,8 @@ if (DEV_MODE) {
         ]),
       },
       fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
+      fullNodeRetryCount: RETRY_COUNT,
+      fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
     },
     domains: {
       serviceName: defaultServiceName,
@@ -135,6 +141,8 @@ if (DEV_MODE) {
         ]),
       },
       fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
+      fullNodeRetryCount: RETRY_COUNT,
+      fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
     },
   }
 } else {
@@ -160,6 +168,10 @@ if (DEV_MODE) {
         versions: functionConfig.pnp_keys.versions,
       },
       fullNodeTimeoutMs: Number(functionConfig.pnp.full_node_timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
+      fullNodeRetryCount: Number(functionConfig.pnp.full_node_retry_count ?? RETRY_COUNT),
+      fullNodeRetryDelayMs: Number(
+        functionConfig.pnp.full_node_retry_delay_ms ?? RETRY_DELAY_IN_MS
+      ),
     },
     domains: {
       serviceName: functionConfig.domains.service_name ?? defaultServiceName,
@@ -176,6 +188,10 @@ if (DEV_MODE) {
         versions: functionConfig.domains_keys.versions,
       },
       fullNodeTimeoutMs: Number(functionConfig.pnp.full_node_timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
+      fullNodeRetryCount: Number(functionConfig.pnp.full_node_retry_count ?? RETRY_COUNT),
+      fullNodeRetryDelayMs: Number(
+        functionConfig.pnp.full_node_retry_delay_ms ?? RETRY_DELAY_IN_MS
+      ),
     },
   }
 }

diff --git a/packages/phone-number-privacy/combiner/src/pnp/endpoints/quota/io.ts b/packages/phone-number-privacy/combiner/src/pnp/endpoints/quota/io.ts
@@ -72,7 +72,9 @@ export class PnpQuotaIO extends IO<PnpQuotaRequest> {
       logger,
       this.config.shouldFailOpen,
       [],
-      this.config.fullNodeTimeoutMs
+      this.config.fullNodeTimeoutMs,
+      this.config.fullNodeRetryCount,
+      this.config.fullNodeRetryDelayMs
     )
   }
 

diff --git a/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/io.legacy.ts b/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/io.legacy.ts
@@ -91,7 +91,9 @@ export class LegacyPnpSignIO extends IO<LegacySignMessageRequest> {
       logger,
       this.config.shouldFailOpen,
       [],
-      this.config.fullNodeTimeoutMs
+      this.config.fullNodeTimeoutMs,
+      this.config.fullNodeRetryCount,
+      this.config.fullNodeRetryDelayMs
     )
   }
 

diff --git a/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/io.ts b/packages/phone-number-privacy/combiner/src/pnp/endpoints/sign/io.ts
@@ -90,7 +90,9 @@ export class PnpSignIO extends IO<SignMessageRequest> {
       logger,
       this.config.shouldFailOpen,
       [],
-      this.config.fullNodeTimeoutMs
+      this.config.fullNodeTimeoutMs,
+      this.config.fullNodeRetryCount,
+      this.config.fullNodeRetryDelayMs
     )
   }
 

diff --git a/packages/phone-number-privacy/combiner/test/integration/domain.test.ts b/packages/phone-number-privacy/combiner/test/integration/domain.test.ts
@@ -1,5 +1,6 @@
 import {
   CombinerEndpoint,
+  DB_TIMEOUT,
   DisableDomainRequest,
   disableDomainRequestEIP712,
   DisableDomainResponse,
@@ -18,6 +19,8 @@ import {
   getContractKit,
   KEY_VERSION_HEADER,
   PoprfClient,
+  RETRY_COUNT,
+  RETRY_DELAY_IN_MS,
   SequentialDelayDomain,
   SequentialDelayStage,
   TestUtils,
@@ -110,6 +113,7 @@ const signerConfig: SignerConfig = {
     port: undefined,
     ssl: true,
     poolMaxSize: 50,
+    timeout: DB_TIMEOUT,
   },
   keystore: {
     type: SupportedKeystore.MOCK_SECRET_MANAGER,
@@ -140,6 +144,8 @@ const signerConfig: SignerConfig = {
   timeout: 5000,
   test_quota_bypass_percentage: 0,
   fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
+  fullNodeRetryCount: RETRY_COUNT,
+  fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
 }
 
 describe('domainService', () => {

diff --git a/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts b/packages/phone-number-privacy/combiner/test/integration/legacypnp.test.ts
@@ -2,11 +2,14 @@ import { newKit } from '@celo/contractkit'
 import {
   AuthenticationMethod,
   CombinerEndpoint,
+  DB_TIMEOUT,
   ErrorMessage,
   FULL_NODE_TIMEOUT_IN_MS,
   genSessionID,
   KEY_VERSION_HEADER,
   LegacySignMessageRequest,
+  RETRY_COUNT,
+  RETRY_DELAY_IN_MS,
   SignMessageResponseFailure,
   SignMessageResponseSuccess,
   TestUtils,
@@ -114,6 +117,7 @@ const signerConfig: SignerConfig = {
     port: undefined,
     ssl: true,
     poolMaxSize: 50,
+    timeout: DB_TIMEOUT,
   },
   keystore: {
     type: SupportedKeystore.MOCK_SECRET_MANAGER,
@@ -144,6 +148,8 @@ const signerConfig: SignerConfig = {
   timeout: 5000,
   test_quota_bypass_percentage: 0,
   fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
+  fullNodeRetryCount: RETRY_COUNT,
+  fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
 }
 
 const testBlockNumber = 1000000

diff --git a/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts b/packages/phone-number-privacy/combiner/test/integration/pnp.test.ts
@@ -2,13 +2,16 @@ import { newKit } from '@celo/contractkit'
 import {
   AuthenticationMethod,
   CombinerEndpoint,
+  DB_TIMEOUT,
   ErrorMessage,
   FULL_NODE_TIMEOUT_IN_MS,
   genSessionID,
   KEY_VERSION_HEADER,
   PnpQuotaRequest,
   PnpQuotaResponseFailure,
   PnpQuotaResponseSuccess,
+  RETRY_COUNT,
+  RETRY_DELAY_IN_MS,
   SignerEndpoint,
   SignMessageRequest,
   SignMessageResponseFailure,
@@ -65,6 +68,8 @@ const {
   BLINDING_FACTOR,
 } = TestUtils.Values
 
+jest.setTimeout(20000)
+
 // create deep copy of config
 const combinerConfig: typeof config = JSON.parse(JSON.stringify(config))
 combinerConfig.phoneNumberPrivacy.enabled = true
@@ -118,6 +123,7 @@ const signerConfig: SignerConfig = {
     port: undefined,
     ssl: true,
     poolMaxSize: 50,
+    timeout: DB_TIMEOUT,
   },
   keystore: {
     type: SupportedKeystore.MOCK_SECRET_MANAGER,
@@ -148,6 +154,8 @@ const signerConfig: SignerConfig = {
   timeout: 5000,
   test_quota_bypass_percentage: 0,
   fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
+  fullNodeRetryCount: RETRY_COUNT,
+  fullNodeRetryDelayMs: RETRY_DELAY_IN_MS,
 }
 
 const testBlockNumber = 1000000

diff --git a/packages/phone-number-privacy/common/src/utils/authentication.ts b/packages/phone-number-privacy/common/src/utils/authentication.ts
@@ -26,7 +26,9 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
   logger: Logger,
   shouldFailOpen: boolean = false,
   warnings: ErrorType[] = [],
-  timeoutMs: number = FULL_NODE_TIMEOUT_IN_MS
+  timeoutMs: number = FULL_NODE_TIMEOUT_IN_MS,
+  retryCount: number = RETRY_COUNT,
+  retryDelay: number = RETRY_DELAY_IN_MS
 ): Promise<boolean> {
   logger.debug('Authenticating user')
 
@@ -43,7 +45,14 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
   if (authMethod && authMethod === AuthenticationMethod.ENCRYPTION_KEY) {
     let registeredEncryptionKey
     try {
-      registeredEncryptionKey = await getDataEncryptionKey(signer, contractKit, logger, timeoutMs)
+      registeredEncryptionKey = await getDataEncryptionKey(
+        signer,
+        contractKit,
+        logger,
+        timeoutMs,
+        retryCount,
+        retryDelay
+      )
     } catch (err) {
       // getDataEncryptionKey should only throw if there is a full-node connection issue.
       // That is, it does not throw if the DEK is undefined or invalid
@@ -129,17 +138,19 @@ export async function getDataEncryptionKey(
   address: string,
   contractKit: ContractKit,
   logger: Logger,
-  timeoutMs: number
+  timeoutMs: number,
+  fullNodeRetryCount: number,
+  fullNodeRetryDelayMs: number
 ): Promise<string> {
   try {
     const res = await retryAsyncWithBackOffAndTimeout(
       async () => {
         const accountWrapper: AccountsWrapper = await contractKit.contracts.getAccounts()
         return accountWrapper.getDataEncryptionKey(address)
       },
-      RETRY_COUNT,
+      fullNodeRetryCount,
       [],
-      RETRY_DELAY_IN_MS,
+      fullNodeRetryDelayMs,
       1.5,
       timeoutMs
     )

diff --git a/packages/phone-number-privacy/common/src/utils/constants.ts b/packages/phone-number-privacy/common/src/utils/constants.ts
@@ -1,5 +1,6 @@
 export const REASONABLE_BODY_CHAR_LIMIT: number = 16_000
 export const DB_TIMEOUT = 1000
+export const DB_POOL_MAX_SIZE = 50
 export const FULL_NODE_TIMEOUT_IN_MS = 1000
 export const RETRY_COUNT = 5
 export const RETRY_DELAY_IN_MS = 100

diff --git a/packages/phone-number-privacy/signer/src/common/database/wrappers/account.ts b/packages/phone-number-privacy/signer/src/common/database/wrappers/account.ts
@@ -1,6 +1,7 @@
-import { DB_TIMEOUT, ErrorMessage } from '@celo/phone-number-privacy-common'
+import { ErrorMessage } from '@celo/phone-number-privacy-common'
 import Logger from 'bunyan'
 import { Knex } from 'knex'
+import { config } from '../../../config'
 import { Histograms, meter } from '../../metrics'
 import { AccountRecord, ACCOUNTS_COLUMNS, ACCOUNTS_TABLE, toAccountRecord } from '../models/account'
 import { countAndThrowDBError, tableWithLockForTrx } from '../utils'
@@ -26,7 +27,7 @@ export async function getPerformedQueryCount(
         .select(ACCOUNTS_COLUMNS.numLookups)
         .where(ACCOUNTS_COLUMNS.address, account)
         .first()
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
       return queryCounts === undefined ? 0 : queryCounts[ACCOUNTS_COLUMNS.numLookups]
     },
     [],
@@ -48,7 +49,7 @@ async function getAccountExists(
       const accountRecord = await tableWithLockForTrx(accounts(db, accountsTable), trx)
         .where(ACCOUNTS_COLUMNS.address, account)
         .first()
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
 
       return !!accountRecord
     },
@@ -77,7 +78,7 @@ export async function incrementQueryCount(
           .transacting(trx)
           .where(ACCOUNTS_COLUMNS.address, account)
           .increment(ACCOUNTS_COLUMNS.numLookups, 1)
-          .timeout(DB_TIMEOUT)
+          .timeout(config.db.timeout)
       } else {
         const newAccountRecord = toAccountRecord(account, 1)
         await insertRecord(db, accountsTable, newAccountRecord, logger, trx)
@@ -98,7 +99,7 @@ async function insertRecord(
   trx: Knex.Transaction
 ): Promise<void> {
   try {
-    await accounts(db, accountsTable).transacting(trx).insert(data).timeout(DB_TIMEOUT)
+    await accounts(db, accountsTable).transacting(trx).insert(data).timeout(config.db.timeout)
   } catch (error) {
     countAndThrowDBError(error, logger, ErrorMessage.DATABASE_INSERT_FAILURE)
   }

diff --git a/packages/phone-number-privacy/signer/src/common/database/wrappers/domain-request.ts b/packages/phone-number-privacy/signer/src/common/database/wrappers/domain-request.ts
@@ -1,6 +1,7 @@
-import { DB_TIMEOUT, Domain, domainHash, ErrorMessage } from '@celo/phone-number-privacy-common'
+import { Domain, domainHash, ErrorMessage } from '@celo/phone-number-privacy-common'
 import Logger from 'bunyan'
 import { Knex } from 'knex'
+import { config } from '../../../config'
 import { Histograms, meter } from '../../metrics'
 import {
   DOMAIN_REQUESTS_COLUMNS,
@@ -35,7 +36,7 @@ export async function getDomainRequestRecordExists<D extends Domain>(
           [DOMAIN_REQUESTS_COLUMNS.blindedMessage]: blindedMessage,
         })
         .first()
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
       return !!existingRequest
     },
     [],
@@ -58,7 +59,7 @@ export async function storeDomainRequestRecord<D extends Domain>(
       await domainRequests(db)
         .transacting(trx)
         .insert(toDomainRequestRecord(domain, blindedMessage))
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
     },
     [],
     (err: any) => countAndThrowDBError(err, logger, ErrorMessage.DATABASE_INSERT_FAILURE),

diff --git a/packages/phone-number-privacy/signer/src/common/database/wrappers/domain-state.ts b/packages/phone-number-privacy/signer/src/common/database/wrappers/domain-state.ts
@@ -1,7 +1,8 @@
-import { DB_TIMEOUT, ErrorMessage } from '@celo/phone-number-privacy-common'
+import { ErrorMessage } from '@celo/phone-number-privacy-common'
 import { Domain, domainHash } from '@celo/phone-number-privacy-common/lib/domains'
 import Logger from 'bunyan'
 import { Knex } from 'knex'
+import { config } from '../../../config'
 import { Histograms, meter } from '../../metrics'
 import {
   DOMAIN_STATE_COLUMNS,
@@ -29,7 +30,7 @@ export async function setDomainDisabled<D extends Domain>(
         .transacting(trx)
         .where(DOMAIN_STATE_COLUMNS.domainHash, hash)
         .update(DOMAIN_STATE_COLUMNS.disabled, true)
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
     },
     [],
     (err: any) => countAndThrowDBError(err, logger, ErrorMessage.DATABASE_UPDATE_FAILURE),
@@ -71,7 +72,7 @@ export async function getDomainStateRecord<D extends Domain>(
       const result = await tableWithLockForTrx(domainStates(db), trx)
         .where(DOMAIN_STATE_COLUMNS.domainHash, hash)
         .first()
-        .timeout(DB_TIMEOUT)
+        .timeout(config.db.timeout)
 
       // bools are stored in db as ints (1 or 0), so we must cast them back
       if (result) {
@@ -111,7 +112,7 @@ export async function updateDomainStateRecord<D extends Domain>(
           .transacting(trx)
           .where(DOMAIN_STATE_COLUMNS.domainHash, hash)
           .update(domainState)
-          .timeout(DB_TIMEOUT)
+          .timeout(config.db.timeout)
       }
     },
     [],
@@ -130,7 +131,7 @@ export async function insertDomainStateRecord(
   return meter(
     async () => {
       logger.debug({ domainState }, 'Insert domain state')
-      await domainStates(db).transacting(trx).insert(domainState).timeout(DB_TIMEOUT)
+      await domainStates(db).transacting(trx).insert(domainState).timeout(config.db.timeout)
       return domainState
     },
     [],