Skip to content

Commit

Permalink
Changed default PNP timeout to a variable set by env (#10359)
Browse files Browse the repository at this point in the history
* change default timeout to a variable set by env

* updated authentication test

* PR feedback

* fix optinal timout

* PR feedback

* refactor configs

* updated tests
  • Loading branch information
soloseng authored Jun 15, 2023
1 parent 4a72b74 commit 74afbe4
Show file tree
Hide file tree
Showing 14 changed files with 95 additions and 13 deletions.
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
}

export interface CombinerConfig {
Expand Down Expand Up @@ -94,6 +101,7 @@ if (DEV_MODE) {
},
]),
},
fullNodeTimeoutMs: FULL_NODE_TIMEOUT_IN_MS,
},
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

0 comments on commit 74afbe4

Please sign in to comment.