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
6 changes: 5 additions & 1 deletion packages/phone-number-privacy/combiner/src/common/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,11 @@ export abstract class IO<R extends OdisRequest> {
response: Response<OdisResponse<R>>
): Promise<Session<R> | null>

abstract authenticate(request: Request<{}, {}, R>, logger?: Logger): Promise<boolean>
abstract authenticate(
request: Request<{}, {}, R>,
logger?: Logger,
timeoutMs?: number
soloseng marked this conversation as resolved.
Show resolved Hide resolved
): Promise<boolean>

abstract sendFailure(
error: ErrorType,
Expand Down
15 changes: 13 additions & 2 deletions 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 All @@ -23,12 +29,13 @@ export interface OdisConfig {
shouldFailOpen: boolean // TODO (https://github.com/celo-org/celo-monorepo/issues/9862) consider refactoring config, this isn't relevant to domains endpoints
odisServices: {
signers: string
timeoutMilliSeconds: number
timeoutMilliSeconds: number // XXX (soloseng): what is this timeout used for? could it also be used to replace FULL_NODE_TIMEOUT_IN_MS?
soloseng marked this conversation as resolved.
Show resolved Hide resolved
}
keys: {
currentVersion: number
versions: string // parse as KeyVersionInfo[]
}
timeoutMs: 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) {
},
]),
},
timeoutMs: Number(process.env.TIMEOUT_MS ?? 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) {
},
]),
},
timeoutMs: Number(process.env.TIMEOUT_MS ?? FULL_NODE_TIMEOUT_IN_MS),
soloseng marked this conversation as resolved.
Show resolved Hide resolved
},
}
} else {
Expand All @@ -150,6 +159,7 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.pnp_keys.current_version),
versions: functionConfig.pnp_keys.versions,
},
timeoutMs: Number(process.env.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 +175,7 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.domains_keys.current_version),
versions: functionConfig.domains_keys.versions,
},
timeoutMs: Number(process.env.TIMEOUT_MS ?? FULL_NODE_TIMEOUT_IN_MS),
soloseng marked this conversation as resolved.
Show resolved Hide resolved
},
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,13 @@ 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.timeoutMs,
this.config.shouldFailOpen
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,13 @@ 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.timeoutMs,
this.config.shouldFailOpen
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,13 @@ 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.timeoutMs,
this.config.shouldFailOpen
)
}

sendSuccess(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
request: Request<{}, {}, R>,
contractKit: ContractKit,
logger: Logger,
timeoutMs: number,
alecps marked this conversation as resolved.
Show resolved Hide resolved
shouldFailOpen: boolean = false,
warnings: ErrorType[] = []
): Promise<boolean> {
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 @@ -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 @@ -28,6 +28,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
soloseng marked this conversation as resolved.
Show resolved Hide resolved
true,
warnings
)
Expand All @@ -49,6 +50,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand All @@ -73,6 +75,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand All @@ -97,6 +100,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
false,
warnings
)
Expand Down Expand Up @@ -131,6 +135,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down Expand Up @@ -161,7 +166,12 @@ describe('Authentication test suite', () => {

const warnings: ErrorType[] = []

const success = await auth.authenticateUser(sampleRequest, mockContractKit, logger)
const success = await auth.authenticateUser(
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS
)

expect(success).toBe(false)
expect(warnings).toEqual([])
Expand Down Expand Up @@ -202,6 +212,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down Expand Up @@ -253,6 +264,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down Expand Up @@ -299,6 +311,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down Expand Up @@ -346,6 +359,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down Expand Up @@ -390,6 +404,7 @@ describe('Authentication test suite', () => {
sampleRequest,
mockContractKit,
logger,
FULL_NODE_TIMEOUT_IN_MS,
true,
warnings
)
Expand Down
3 changes: 2 additions & 1 deletion packages/phone-number-privacy/signer/src/common/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export abstract class IO<R extends OdisRequest> {
abstract authenticate(
request: Request<{}, {}, R>,
warnings?: string[],
logger?: Logger
logger?: Logger,
timeoutMs?: number
soloseng marked this conversation as resolved.
Show resolved Hide resolved
): Promise<boolean>

abstract sendFailure(
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.timeoutMs,
this.shouldFailOpen,
warnings
)
}

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.timeoutMs,
this.shouldFailOpen,
warnings
)
}

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.timeoutMs,
this.shouldFailOpen,
warnings
)
}

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.timeoutMs,
this.shouldFailOpen,
warnings
)
}

sendSuccess(
Expand Down
Loading