Skip to content

Commit

Permalink
Merge branch 'alecps/odisRelease3.0.0' into soloseng/odis-cloud-funct…
Browse files Browse the repository at this point in the history
…ion-gen2
  • Loading branch information
soloseng committed Aug 29, 2023
2 parents 068bb74 + 188770a commit 8984821
Show file tree
Hide file tree
Showing 42 changed files with 425 additions and 305 deletions.
2 changes: 2 additions & 0 deletions .github/workflows/container-all-monorepo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
tag: ${{ github.sha }}
context: .
file: dockerfiles/all-monorepo/Dockerfile
trivy: true

celomonorepo-build:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -38,3 +39,4 @@ jobs:
tag: ${{ github.sha }}
context: .
file: dockerfiles/all-monorepo/Dockerfile
trivy: true
2 changes: 2 additions & 0 deletions .github/workflows/container-celotool.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
tag: ${{ github.sha }}
context: .
file: dockerfiles/celotool/Dockerfile
trivy: true

celotool-build:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -38,3 +39,4 @@ jobs:
tag: ${{ github.sha }}
context: .
file: dockerfiles/celotool/Dockerfile
trivy: true
4 changes: 4 additions & 0 deletions .github/workflows/container-circleci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jobs:
artifact-registry: us-west1-docker.pkg.dev/devopsre/dev-images/circleci-geth
tag: testing
context: dockerfiles/circleci
trivy: true

geth-build:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -61,6 +62,7 @@ jobs:
artifact-registry: us-west1-docker.pkg.dev/devopsre/celo-monorepo/circleci-geth
tag: latest
context: dockerfiles/circleci
trivy: true

node12-build-dev:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -75,6 +77,7 @@ jobs:
artifact-registry: us-west1-docker.pkg.dev/devopsre/dev-images/circleci-node12
tag: testing
context: dockerfiles/circleci
trivy: true

node12-build:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -89,4 +92,5 @@ jobs:
artifact-registry: us-west1-docker.pkg.dev/devopsre/celo-monorepo/circleci-node12
tag: latest
context: dockerfiles/circleci
trivy: true

2 changes: 2 additions & 0 deletions .github/workflows/container-cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ jobs:
tag: testing
context: .
file: dockerfiles/cli-standalone/Dockerfile
trivy: true

celocli-build:
uses: celo-org/reusable-workflows/.github/workflows/[email protected]
Expand All @@ -38,3 +39,4 @@ jobs:
tag: latest
context: .
file: dockerfiles/cli-standalone/Dockerfile
trivy: true
2 changes: 1 addition & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
"[typescript]": {
"editor.formatOnSave": true,
"editor.codeActionsOnSave": {
"source.organizeImports": false
"source.organizeImports": true
}
},
"[typescriptreact]": {
Expand Down
20 changes: 2 additions & 18 deletions packages/phone-number-privacy/TODO.md
Original file line number Diff line number Diff line change
@@ -1,25 +1,9 @@
# TODO

- (alec) fix domains tests
- (Alec) check prometheus Counter
- check prometheus Counter
- Fix types in errorResult and sendFailure so we don't have to use ANY
- Refactor domain sign handler to use db transactions properly
- refactor authorization function with the new account model
- resolve FAKE_URL for request url
- Search for TODO comments for things to fix after load test
- (nice to have) Refactor Combiner to be similar than signer (kill IO, Controller, Action)
- Make caching config parameters configurable by environment
- TODO comments

## Done

✔️ extract resultHandler() out of each handler, into the createHandler on server.ts
✔️ correct Locals Type (logger should not be an ANY)
✔️ (mariano) Implement chaching Account Service
✔️ (mariano) Check Tracing Calls
✔️ trace signature timeg
✔️ (Mariano) remove catchErrorHandler2 (move it catchErrorHandler)
✔️ Type Handler so Response has the correct Response Type
✔️ Type Handlers so that Request is the proper type, or better use the "isValid Request" function
✔️ fix primary key in requests table
✔️ drop legacy tables
- TODO comments
2 changes: 1 addition & 1 deletion packages/phone-number-privacy/combiner/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@celo/phone-number-privacy-combiner",
"version": "3.0.0-beta.5",
"version": "3.0.0-beta.6",
"description": "Orchestrates and combines threshold signatures for use in ODIS",
"author": "Celo",
"license": "Apache-2.0",
Expand Down
10 changes: 9 additions & 1 deletion packages/phone-number-privacy/combiner/src/common/combine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,14 @@ export async function thresholdCallToSigners<R extends OdisRequest>(
})

if (!signerFetchResult.ok) {
// used for log based metrics
logger.info({
message: 'Received signerFetchResult on unsuccessful signer response',
res: await signerFetchResult.json(),
status: signerFetchResult.status,
signer: signer.url,
})

errorCount++
errorCodes.set(
signerFetchResult.status,
Expand Down Expand Up @@ -181,7 +189,7 @@ function isTimeoutError(err: unknown) {
return err instanceof Error && err.name === 'TimeoutError'
}

function isAbortError(err: unknown) {
export function isAbortError(err: unknown) {
return err instanceof Error && err.name === 'AbortError'
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ export abstract class CryptoClient {
/**
* Returns true if the number of valid signatures is enough to perform a combination
*/
// TODO (mcortesi) remove
public hasSufficientSignatures(): boolean {
return this.allSignaturesLength >= this.keyVersionInfo.threshold
}
Expand Down
7 changes: 4 additions & 3 deletions packages/phone-number-privacy/combiner/src/common/io.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import * as https from 'https'
import fetch, { Response as FetchResponse } from 'node-fetch'
import { performance } from 'perf_hooks'
import { getCombinerVersion, OdisConfig } from '../config'
import { Signer } from './combine'
import { isAbortError, Signer } from './combine'

const httpAgent = new http.Agent({ keepAlive: true })
const httpsAgent = new https.Agent({ keepAlive: true })
Expand Down Expand Up @@ -95,8 +95,9 @@ export async function fetchSignerResponseWithFallback<R extends OdisRequest>(
return measureTime(signer.url + signerEndpoint + `/${request.body.sessionID}`, () =>
fetchSignerResponse(signer.url + signerEndpoint).catch((err) => {
logger.error({ url: signer.url, error: err }, `Signer failed with primary url`)
if (signer.fallbackUrl) {
logger.warn({ url: signer.fallbackUrl }, `Using fallback url to call signer`)
if (signer.fallbackUrl && !isAbortError(err)) {
// TODO should we also be checking isTimeoutError here?
logger.warn({ signer }, `Using fallback url to call signer`)
return fetchSignerResponse(signer.fallbackUrl + signerEndpoint)
} else {
throw err
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,9 @@ const signerConfig: SignerConfig = {
mockDek: '',
mockTotalQuota: 0,
shouldMockRequestService: false,
requestPrunningDays: 0,
requestPrunningAtServerStart: false,
requestPrunningJobCronPattern: '0 0 * * * *',
}

describe('domainService', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,9 @@ const signerConfig: SignerConfig = {
mockDek: '',
mockTotalQuota: 0,
shouldMockRequestService: false,
requestPrunningDays: 0,
requestPrunningAtServerStart: false,
requestPrunningJobCronPattern: '0 0 0 * * *',
}

const testBlockNumber = 1000000
Expand Down
2 changes: 1 addition & 1 deletion packages/phone-number-privacy/common/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ These instructions assume the following scenario for readability:
- i.e. search and replace `3.1.1-dev` with `3.2.0-beta.1` (note that we’ve removed the `-dev`)
4. Same idea as above -- ensure the version of the `@celo/phone-number-privacy-common` package is set to the version you are trying to release (i.e. `2.0.3-beta.1`) and that all other packages are importing this version.
5. From the monorepo root directory, run `yarn reset && yarn && yarn build` (expect this to take at least 10 mins)
6. Commit your changes with the message `3.2.0-beta.1
6. Commit your changes with the message `3.2.0-beta.1`
7. Publish the ODIS common package by navigating to the `phone-number-privacy/common` directory and running `npm publish —-tag beta`
- You will be prompted to enter your OTP
- When publishing as `latest`, omit the `--tag beta`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,33 +27,28 @@ export enum ErrorMessage {
FAILURE_TO_GET_PERFORMED_QUERY_COUNT = `CELO_ODIS_ERR_24 DB_ERR Failed to read performedQueryCount from signer db`,
FAILURE_TO_GET_TOTAL_QUOTA = `CELO_ODIS_ERR_25 NODE_ERR Failed to read on-chain state to calculate total quota`,
FAILURE_TO_GET_DEK = `CELO_ODIS_ERR_27 NODE_ERR Failed to read user's DEK from full-node`,
FAILING_OPEN = `CELO_ODIS_ERR_28 NODE_ERR Failing open on full-node error`,
FAILING_CLOSED = `CELO_ODIS_ERR_29 NODE_ERR Failing closed on full-node error`,
CAUGHT_ERROR_IN_ENDPOINT_HANDLER = `CELO_ODIS_ERR_30 Caught error in outer endpoint handler`,
ERROR_AFTER_RESPONSE_SENT = `CELO_ODIS_ERR_31 Error in endpoint thrown after response was already sent`,
SIGNATURE_AGGREGATION_FAILURE = 'CELO_ODIS_ERR_32 SIG_ERR Failed to blind aggregate signature shares',
DATABASE_REMOVE_FAILURE = 'CELO_ODIS_ERR_33 DB_ERR Failed to remove database entries',
}

export enum WarningMessage {
INVALID_INPUT = `CELO_ODIS_WARN_01 BAD_INPUT Invalid input parameters`,
UNAUTHENTICATED_USER = `CELO_ODIS_WARN_02 BAD_INPUT Missing or invalid authentication`,
EXCEEDED_QUOTA = `CELO_ODIS_WARN_03 QUOTA Requester exceeded service query quota`,
DUPLICATE_REQUEST_TO_GET_PARTIAL_SIG = `CELO_ODIS_WARN_04 BAD_INPUT Attempt to replay partial signature request`,
INCONSISTENT_SIGNER_BLOCK_NUMBERS = `CELO_ODIS_WARN_05 SIGNER Discrepancy found in signers latest block number that exceeds threshold`,
INCONSISTENT_SIGNER_QUOTA_MEASUREMENTS = `CELO_ODIS_WARN_06 SIGNER Discrepancy found in signers quota measurements`,
MISSING_SESSION_ID = `CELO_ODIS_WARN_07 BAD_INPUT Client did not provide sessionID in request`,
CANCELLED_REQUEST_TO_SIGNER = `CELO_ODIS_WARN_08 SIGNER Cancelled request to signer`,
INVALID_USER_PHONE_NUMBER_SIGNATURE = `CELO_ODIS_WARN_09 BAD_INPUT User phone number signature is invalid`,
UNKNOWN_DOMAIN = `CELO_ODIS_WARN_10 BAD_INPUT Provided domain name and version is not recognized`,
DISABLED_DOMAIN = `CELO_ODIS_WARN_11 BAD_INPUT Provided domain is disabled`,
INVALID_KEY_VERSION_REQUEST = `CELO_ODIS_WARN_12 BAD_INPUT Request key version header is invalid`,
API_UNAVAILABLE = `CELO_ODIS_WARN_13 BAD_INPUT API is unavailable`,
INCONSISTENT_SIGNER_DOMAIN_DISABLED_STATES = `CELO_ODIS_WARN_14 SIGNER Discrepency found in signer domain disabled states`,
INVALID_AUTH_SIGNATURE = `CELO_ODIS_WARN_15 BAD_INPUT Authorization signature was incorrectly generated. Request will be rejected in a future version.`,
INVALID_NONCE = `CELO_ODIS_WARN_16 BAD_INPUT SequentialDelayDomain nonce check failed on Signer request`,
SIGNER_RESPONSE_DISCREPANCIES = `CELO_ODIS_WARN_17 SIGNER Discrepancies detected in signer responses`,
INCONSISTENT_SIGNER_QUERY_MEASUREMENTS = `CELO_ODIS_WARN_18 SIGNER Discrepancy found in signers performed query count measurements`,
SIGNER_FAILED_OPEN = `CELO_ODIS_WARN_19 SIGNER Signer failed open on request`,
}

export type ErrorType = ErrorMessage | WarningMessage
47 changes: 1 addition & 46 deletions packages/phone-number-privacy/common/src/utils/authentication.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { hexToBuffer, retryAsyncWithBackOffAndTimeout } from '@celo/base'
import { ContractKit } from '@celo/contractkit'
import { AccountsWrapper } from '@celo/contractkit/lib/wrappers/Accounts'
import { AttestationsWrapper } from '@celo/contractkit/lib/wrappers/Attestations'
import { trimLeading0x } from '@celo/utils/lib/address'
import { verifySignature } from '@celo/utils/lib/signatureUtils'

Expand Down Expand Up @@ -66,13 +65,11 @@ export async function authenticateUser<R extends PhoneNumberPrivacyRequest>(
} 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
const failureStatus = ErrorMessage.FAILING_CLOSED
logger.error({
err,
warning: ErrorMessage.FAILURE_TO_GET_DEK,
failureStatus,
})
warnings.push(ErrorMessage.FAILURE_TO_GET_DEK, failureStatus)
warnings.push(ErrorMessage.FAILURE_TO_GET_DEK)
return false
}
if (!registeredEncryptionKey) {
Expand Down Expand Up @@ -171,45 +168,3 @@ export async function getDataEncryptionKey(
throw error
}
}

export async function isVerified(
account: string,
hashedPhoneNumber: string,
contractKit: ContractKit,
logger: Logger
): Promise<boolean> {
try {
const res = await retryAsyncWithBackOffAndTimeout(
async () => {
const attestationsWrapper: AttestationsWrapper =
await contractKit.contracts.getAttestations()
const {
isVerified: _isVerified,
completed,
numAttestationsRemaining,
total,
} = await attestationsWrapper.getVerifiedStatus(hashedPhoneNumber, account)

logger.debug({
account,
isVerified: _isVerified,
completedAttestations: completed,
remainingAttestations: numAttestationsRemaining,
totalAttestationsRequested: total,
})
return _isVerified
},
RETRY_COUNT,
[],
RETRY_DELAY_IN_MS,
1.5,
FULL_NODE_TIMEOUT_IN_MS
)
return res
} catch (error) {
logger.error('Failed to get verification status: ' + error)
logger.error(ErrorMessage.FULL_NODE_ERROR)
logger.warn('Assuming user is verified')
return true
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ContractKit, newKit, newKitWithApiKey, HttpProviderOptions } from '@celo/contractkit'
import { ContractKit, HttpProviderOptions, newKit, newKitWithApiKey } from '@celo/contractkit'
import http from 'http'
import https from 'https'

Expand Down
6 changes: 3 additions & 3 deletions packages/phone-number-privacy/common/src/utils/key-version.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export function getRequestKeyVersion(
return undefined
}
if (!isValidKeyVersion(keyVersion)) {
logger.error({ keyVersionHeader }, WarningMessage.INVALID_KEY_VERSION_REQUEST)
logger.error({ keyVersionHeader, keyVersion }, WarningMessage.INVALID_KEY_VERSION_REQUEST)
throw new Error(WarningMessage.INVALID_KEY_VERSION_REQUEST)
}

Expand Down Expand Up @@ -76,7 +76,7 @@ export function getResponseKeyVersion(response: FetchResponse, logger: Logger):
return undefined
}
if (!isValidKeyVersion(keyVersion)) {
logger.error({ keyVersionHeader }, ErrorMessage.INVALID_KEY_VERSION_RESPONSE)
logger.error({ keyVersionHeader, keyVersion }, ErrorMessage.INVALID_KEY_VERSION_RESPONSE)
throw new Error(ErrorMessage.INVALID_KEY_VERSION_RESPONSE)
}

Expand All @@ -93,7 +93,7 @@ function parseKeyVersionFromHeader(

const keyVersionHeaderString = keyVersionHeader.toString().trim()

if (!keyVersionHeaderString.length) {
if (!keyVersionHeaderString.length || keyVersionHeaderString === 'undefined') {
return undefined
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,47 +330,4 @@ describe('Authentication test suite', () => {
expect(warnings).toEqual([])
})
})

describe('isVerified utility', () => {
// TODO remove this
it('Should succeed when verification is ok', async () => {
const mockContractKit = {
contracts: {
getAttestations: async () => {
return {
getVerifiedStatus: async (_: string, __: string) => {
return {
isVerified: true,
}
},
}
},
},
} as ContractKit

const result = await auth.isVerified('', '', mockContractKit, logger)

expect(result).toBe(true)
})

it('Should fail when verification is not ok', async () => {
const mockContractKit = {
contracts: {
getAttestations: async () => {
return {
getVerifiedStatus: async (_: string, __: string) => {
return {
isVerified: false,
}
},
}
},
},
} as ContractKit

const result = await auth.isVerified('', '', mockContractKit, logger)

expect(result).toBe(false)
})
})
})
2 changes: 1 addition & 1 deletion packages/phone-number-privacy/signer/.env
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ ALFAJORES_ODIS_BLOCKCHAIN_PROVIDER=https://alfajores-forno.celo-testnet.org
MAINNET_ODIS_BLOCKCHAIN_PROVIDER=https://forno.celo.org
ODIS_DOMAINS_TEST_KEY_VERSION=1
ODIS_PNP_TEST_KEY_VERSION=1
DEPLOYED_SIGNER_SERVICE_VERSION=2.0.1
DEPLOYED_SIGNER_SERVICE_VERSION=3.0.0-beta.14
# PUBKEYS
STAGING_DOMAINS_PUBKEY=7FsWGsFnmVvRfMDpzz95Np76wf/1sPaK0Og9yiB+P8QbjiC8FV67NBans9hzZEkBaQMhiapzgMR6CkZIZPvgwQboAxl65JWRZecGe5V3XO4sdKeNemdAZ2TzQuWkuZoA
ALFAJORES_DOMAINS_PUBKEY=+ZrxyPvLChWUX/DyPw6TuGwQH0glDJEbSrSxUARyP5PuqYyP/U4WZTV1e0bAUioBZ6QCJMiLpDwTaFvy8VnmM5RBbLQUMrMg5p4+CBCqj6HhsMfcyUj8V0LyuNdStlCB
Expand Down
Loading

0 comments on commit 8984821

Please sign in to comment.