Skip to content
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

Changed default PNP timeout to a variable set by env #10359

Merged
merged 10 commits into from
Jun 15, 2023
14 changes: 13 additions & 1 deletion packages/phone-number-privacy/combiner/src/config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { BlockchainConfig, rootLogger, TestUtils, toBool } from '@celo/phone-number-privacy-common'
import {
BlockchainConfig,
FULL_NODE_TIMEOUT_IN_MS,
rootLogger,
TestUtils,
toBool,
} from '@celo/phone-number-privacy-common'
import * as functions from 'firebase-functions'
export function getCombinerVersion(): string {
return process.env.npm_package_version ?? require('../package.json').version ?? '0.0.0'
Expand Down Expand Up @@ -29,6 +35,7 @@ export interface OdisConfig {
currentVersion: number
versions: string // parse as KeyVersionInfo[]
}
fullNodeTimeoutMs: number
soloseng marked this conversation as resolved.
Show resolved Hide resolved
}

export interface CombinerConfig {
Expand Down Expand Up @@ -94,6 +101,7 @@ if (DEV_MODE) {
},
]),
},
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
soloseng marked this conversation as resolved.
Show resolved Hide resolved
},
domains: {
serviceName: defaultServiceName,
Expand Down Expand Up @@ -126,6 +134,7 @@ if (DEV_MODE) {
},
]),
},
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
},
}
} else {
Expand All @@ -135,6 +144,7 @@ if (DEV_MODE) {
blockchain: {
provider: functionConfig.blockchain.provider,
apiKey: functionConfig.blockchain.api_key,
timeout_ms: Number(functionConfig.blockchain.timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
},
phoneNumberPrivacy: {
serviceName: functionConfig.pnp.service_name ?? defaultServiceName,
Expand All @@ -150,6 +160,7 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.pnp_keys.current_version),
versions: functionConfig.pnp_keys.versions,
},
fullNodeTimeoutMs: Number(functionConfig.blockchain.timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
soloseng marked this conversation as resolved.
Show resolved Hide resolved
},
domains: {
serviceName: functionConfig.domains.service_name ?? defaultServiceName,
Expand All @@ -165,6 +176,7 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.domains_keys.current_version),
versions: functionConfig.domains_keys.versions,
},
fullNodeTimeoutMs: Number(functionConfig.blockchain.timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,14 @@ export class PnpQuotaIO extends IO<PnpQuotaRequest> {
}

async authenticate(request: Request<{}, {}, PnpQuotaRequest>, logger: Logger): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.config.shouldFailOpen)
return authenticateUser(
request,
this.kit,
logger,
this.config.shouldFailOpen,
[],
this.config.fullNodeTimeoutMs
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,14 @@ export class LegacyPnpSignIO extends IO<LegacySignMessageRequest> {
request: Request<{}, {}, LegacySignMessageRequest>,
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.config.shouldFailOpen)
return authenticateUser(
request,
this.kit,
logger,
this.config.shouldFailOpen,
[],
this.config.fullNodeTimeoutMs
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,14 @@ export class PnpSignIO extends IO<SignMessageRequest> {
request: Request<{}, {}, SignMessageRequest>,
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.config.shouldFailOpen)
return authenticateUser(
request,
this.kit,
logger,
this.config.shouldFailOpen,
[],
this.config.fullNodeTimeoutMs
)
}

sendSuccess(
Expand Down
10 changes: 6 additions & 4 deletions packages/phone-number-privacy/common/src/utils/authentication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
contractKit: ContractKit,
logger: Logger,
shouldFailOpen: boolean = false,
warnings: ErrorType[] = []
warnings: ErrorType[] = [],
timeoutMs: number = FULL_NODE_TIMEOUT_IN_MS
): Promise<boolean> {
logger.debug('Authenticating user')

Expand All @@ -42,7 +43,7 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
if (authMethod && authMethod === AuthenticationMethod.ENCRYPTION_KEY) {
let registeredEncryptionKey
try {
registeredEncryptionKey = await getDataEncryptionKey(signer, contractKit, logger)
registeredEncryptionKey = await getDataEncryptionKey(signer, contractKit, logger, timeoutMs)
} 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
Expand Down Expand Up @@ -127,7 +128,8 @@ export function verifyDEKSignature(
export async function getDataEncryptionKey(
address: string,
contractKit: ContractKit,
logger: Logger
logger: Logger,
timeoutMs: number
): Promise<string> {
try {
const res = await retryAsyncWithBackOffAndTimeout(
Expand All @@ -139,7 +141,7 @@ export async function getDataEncryptionKey(
[],
RETRY_DELAY_IN_MS,
1.5,
FULL_NODE_TIMEOUT_IN_MS
timeoutMs
)
return res
} catch (error) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ContractKit, newKit, newKitWithApiKey } from '@celo/contractkit'
export interface BlockchainConfig {
provider: string
apiKey?: string
timeout_ms?: number
soloseng marked this conversation as resolved.
Show resolved Hide resolved
}

export function getContractKit(config: BlockchainConfig): ContractKit {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { hexToBuffer } from '@celo/base'
import { ContractKit } from '@celo/contractkit'
import Logger from 'bunyan'
import { Request } from 'express'
import { ErrorMessage, ErrorType } from '../../lib'
import { ErrorMessage, ErrorType, FULL_NODE_TIMEOUT_IN_MS } from '../../lib'
import { AuthenticationMethod } from '../../src/interfaces/requests'
import * as auth from '../../src/utils/authentication'

Expand All @@ -29,7 +29,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
soloseng marked this conversation as resolved.
Show resolved Hide resolved
)

expect(success).toBe(false)
Expand All @@ -50,7 +51,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand All @@ -74,7 +76,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(true)
Expand All @@ -98,7 +101,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
false,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down Expand Up @@ -132,7 +136,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down Expand Up @@ -203,7 +208,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(true)
Expand Down Expand Up @@ -254,7 +260,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down Expand Up @@ -300,7 +307,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down Expand Up @@ -347,7 +355,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down Expand Up @@ -391,7 +400,8 @@ describe('Authentication test suite', () => {
mockContractKit,
logger,
true,
warnings
warnings,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
Expand Down
7 changes: 6 additions & 1 deletion packages/phone-number-privacy/signer/src/config.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { BlockchainConfig, toBool } from '@celo/phone-number-privacy-common'
import {
BlockchainConfig,
FULL_NODE_TIMEOUT_IN_MS,
toBool,
} from '@celo/phone-number-privacy-common'
import BigNumber from 'bignumber.js'

require('dotenv').config()
Expand Down Expand Up @@ -136,6 +140,7 @@ export const config: SignerConfig = {
blockchain: {
provider: env.BLOCKCHAIN_PROVIDER,
apiKey: env.BLOCKCHAIN_API_KEY,
timeout_ms: Number(env.TIMEOUT_MS ?? FULL_NODE_TIMEOUT_IN_MS),
soloseng marked this conversation as resolved.
Show resolved Hide resolved
},
db: {
type: env.DB_TYPE ? env.DB_TYPE.toLowerCase() : SupportedDatabase.Postgres,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ export class LegacyPnpQuotaIO extends IO<LegacyPnpQuotaRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly kit: ContractKit
readonly kit: ContractKit,
readonly timeoutMs: number
) {
super(enabled)
}
Expand Down Expand Up @@ -64,7 +65,14 @@ export class LegacyPnpQuotaIO extends IO<LegacyPnpQuotaRequest> {
warnings: ErrorType[],
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.shouldFailOpen, warnings)
return authenticateUser(
request,
this.kit,
logger,
this.shouldFailOpen,
warnings,
this.timeoutMs
)
}

sendSuccess(
Expand Down
12 changes: 10 additions & 2 deletions packages/phone-number-privacy/signer/src/pnp/endpoints/quota/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ export class PnpQuotaIO extends IO<PnpQuotaRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly kit: ContractKit
readonly kit: ContractKit,
readonly timeoutMs: number
) {
super(enabled)
}
Expand Down Expand Up @@ -62,7 +63,14 @@ export class PnpQuotaIO extends IO<PnpQuotaRequest> {
warnings: ErrorType[],
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.shouldFailOpen, warnings)
return authenticateUser(
request,
this.kit,
logger,
this.shouldFailOpen,
warnings,
this.timeoutMs
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,8 @@ export class LegacyPnpSignIO extends IO<LegacySignMessageRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly kit: ContractKit
readonly kit: ContractKit,
readonly timeoutMs: number
) {
super(enabled)
}
Expand Down Expand Up @@ -76,7 +77,14 @@ export class LegacyPnpSignIO extends IO<LegacySignMessageRequest> {
warnings: ErrorType[],
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.shouldFailOpen, warnings)
return authenticateUser(
request,
this.kit,
logger,
this.shouldFailOpen,
warnings,
this.timeoutMs
)
}

sendSuccess(
Expand Down
12 changes: 10 additions & 2 deletions packages/phone-number-privacy/signer/src/pnp/endpoints/sign/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ export class PnpSignIO extends IO<SignMessageRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly kit: ContractKit
readonly kit: ContractKit,
readonly timeoutMs: number
soloseng marked this conversation as resolved.
Show resolved Hide resolved
) {
super(enabled)
}
Expand Down Expand Up @@ -72,7 +73,14 @@ export class PnpSignIO extends IO<SignMessageRequest> {
warnings: ErrorType[],
logger: Logger
): Promise<boolean> {
return authenticateUser(request, this.kit, logger, this.shouldFailOpen, warnings)
return authenticateUser(
request,
this.kit,
logger,
this.shouldFailOpen,
warnings,
this.timeoutMs
)
}

sendSuccess(
Expand Down
Loading