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
13 changes: 12 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 @@ -150,6 +159,7 @@ if (DEV_MODE) {
currentVersion: Number(functionConfig.pnp_keys.current_version),
versions: functionConfig.pnp_keys.versions,
},
fullNodeTimeoutMs: Number(functionConfig.pnp.full_node_timeout_ms ?? FULL_NODE_TIMEOUT_IN_MS),
},
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,
},
fullNodeTimeoutMs: Number(functionConfig.pnp.full_node_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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
domainRestrictedSignatureRequestEIP712,
DomainRestrictedSignatureResponse,
ErrorMessage,
FULL_NODE_TIMEOUT_IN_MS,
genSessionID,
getContractKit,
KEY_VERSION_HEADER,
Expand Down Expand Up @@ -138,6 +139,7 @@ const signerConfig: SignerConfig = {
},
timeout: 5000,
test_quota_bypass_percentage: 0,
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
}

describe('domainService', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
AuthenticationMethod,
CombinerEndpoint,
ErrorMessage,
FULL_NODE_TIMEOUT_IN_MS,
genSessionID,
KEY_VERSION_HEADER,
LegacySignMessageRequest,
Expand Down Expand Up @@ -142,6 +143,7 @@ const signerConfig: SignerConfig = {
},
timeout: 5000,
test_quota_bypass_percentage: 0,
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
}

const testBlockNumber = 1000000
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
AuthenticationMethod,
CombinerEndpoint,
ErrorMessage,
FULL_NODE_TIMEOUT_IN_MS,
genSessionID,
KEY_VERSION_HEADER,
PnpQuotaRequest,
Expand Down Expand Up @@ -146,6 +147,7 @@ const signerConfig: SignerConfig = {
},
timeout: 5000,
test_quota_bypass_percentage: 0,
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
}

const testBlockNumber = 1000000
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
8 changes: 7 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 @@ -94,6 +98,7 @@ export interface SignerConfig {
}
timeout: number
test_quota_bypass_percentage: number
fullNodeTimeoutMs: number
}

const env = process.env as any
Expand Down Expand Up @@ -175,4 +180,5 @@ export const config: SignerConfig = {
},
timeout: Number(env.ODIS_SIGNER_TIMEOUT ?? 5000),
test_quota_bypass_percentage: Number(env.TEST_QUOTA_BYPASS_PERCENTAGE ?? 0),
fullNodeTimeoutMs: Number(env.TIMEOUT_MS ?? FULL_NODE_TIMEOUT_IN_MS),
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export class LegacyPnpQuotaIO extends IO<LegacyPnpQuotaRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly timeoutMs: number,
readonly kit: ContractKit
) {
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
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export class PnpQuotaIO extends IO<PnpQuotaRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly timeoutMs: number,
readonly kit: ContractKit
) {
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,6 +32,7 @@ export class LegacyPnpSignIO extends IO<LegacySignMessageRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly timeoutMs: number,
readonly kit: ContractKit
) {
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
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export class PnpSignIO extends IO<SignMessageRequest> {
constructor(
readonly enabled: boolean,
readonly shouldFailOpen: boolean,
readonly timeoutMs: number,
readonly kit: ContractKit
) {
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
4 changes: 4 additions & 0 deletions packages/phone-number-privacy/signer/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ export function startSigner(
new PnpQuotaIO(
config.api.phoneNumberPrivacy.enabled,
config.api.phoneNumberPrivacy.shouldFailOpen, // TODO (https://github.com/celo-org/celo-monorepo/issues/9862) consider refactoring config to make the code cleaner
config.fullNodeTimeoutMs,
kit
)
)
Expand All @@ -111,6 +112,7 @@ export function startSigner(
new PnpSignIO(
config.api.phoneNumberPrivacy.enabled,
config.api.phoneNumberPrivacy.shouldFailOpen,
config.fullNodeTimeoutMs,
kit
)
)
Expand All @@ -124,6 +126,7 @@ export function startSigner(
new LegacyPnpSignIO(
config.api.legacyPhoneNumberPrivacy.enabled,
config.api.legacyPhoneNumberPrivacy.shouldFailOpen,
config.fullNodeTimeoutMs,
kit
)
)
Expand All @@ -135,6 +138,7 @@ export function startSigner(
new LegacyPnpQuotaIO(
config.api.legacyPhoneNumberPrivacy.enabled,
config.api.legacyPhoneNumberPrivacy.shouldFailOpen,
config.fullNodeTimeoutMs,
kit
)
)
Expand Down