Skip to content

Commit

Permalink
[Wallet] Add support for 8 digits verification codes (#5925)
Browse files Browse the repository at this point in the history
### Description

This PR implements short verification codes. See the demo below.
To enable this feature we need all the validators to support `https`, which is not the case atm, so this feature are disabled by feature flag.

### Short code
Essentially, short code, which user receives consist of two parts: prefix and security code (e.g. `12345678`: `1` - prefix, `2345678` - security code).

We use prefix to identify which issuer has sent the code. Because we can not store persistent mapping of prefix<>issuer on the client side (will be gone after reinstall), we are doing the following:
* Set prefix to issuer's address mod 10 ([0,9]) and pass it with reveal request.
* When we receive a short code with a prefix back, we create a list of validators which map to the same prefix.
* We send security code to obtain attestation code to all issuers from the previous step
* One of them should return a full attestation code, all others should fail.

This approach allows us to scale for more than 10 actionable attestations at the same time and to avoid changing `Attestation.sol` contract to store mapping prefix<>issuer in it.

#### Caveat
We do occasionally redundant requests with security code to wrong validators and those exposing the code itself and increasing waiting time for the user. For 3 SMSs the probability of this happening is 30%.

For more information see this discussion https://discord.com/channels/600834479145353243/732977310101405736/778336853505081344
### Demo
![screen-20201119-212100~2](https://user-images.githubusercontent.com/6160124/99726972-426faf80-2af2-11eb-97b4-0909b8ded3ac.gif)

### Tested

With short codes enabled/disabled.

### Related issues

- Fixes #5384
- Fixes #5394

### Backwards compatibility

Yes
  • Loading branch information
i1skn authored Nov 25, 2020
1 parent a2cf49d commit 486a02e
Show file tree
Hide file tree
Showing 27 changed files with 419 additions and 292 deletions.
41 changes: 28 additions & 13 deletions packages/celotool/src/lib/attestation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,25 @@ import { Address, CeloTransactionParams, ContractKit, OdisUtils } from '@celo/co
import { AuthSigner } from '@celo/contractkit/lib/identity/odis/query'
import {
ActionableAttestation,
AttestationsWrapper
AttestationsWrapper,
} from '@celo/contractkit/lib/wrappers/Attestations'
import { AttestationUtils, PhoneNumberUtils } from '@celo/utils'
import { concurrentMap } from '@celo/utils/lib/async'
import { AttestationRequest } from '@celo/utils/lib/io'
import { sample } from 'lodash'
import moment from 'moment'
import { Twilio } from 'twilio'

const DUMMY_SMS_URL = 'https://enzyutth0wxme.x.pipedream.net/'

// Use the supplied salt, or if none supplied, go to ODIS and retrieve a pepper
export async function getIdentifierAndPepper(kit : ContractKit, context : string, account : string, phoneNumber : string, salt : string | null) {
export async function getIdentifierAndPepper(
kit: ContractKit,
context: string,
account: string,
phoneNumber: string,
salt: string | null
) {
if (salt) {
return {
pepper: salt,
Expand Down Expand Up @@ -47,15 +54,18 @@ export async function requestMoreAttestations(
txParams: CeloTransactionParams = {}
) {
const unselectedRequest = await attestations.getUnselectedRequest(phoneNumber, account)
if (unselectedRequest.blockNumber === 0 || (await attestations.isAttestationExpired(unselectedRequest.blockNumber))) {
if (
unselectedRequest.blockNumber === 0 ||
(await attestations.isAttestationExpired(unselectedRequest.blockNumber))
) {
await attestations
.approveAttestationFee(attestationsRequested)
.then((txo) => txo.sendAndWaitForReceipt(txParams))
await attestations
.request(phoneNumber, attestationsRequested)
.then((txo) => txo.sendAndWaitForReceipt(txParams))
}

const selectIssuers = await attestations.selectIssuersAfterWait(phoneNumber, account)
await selectIssuers.sendAndWaitForReceipt(txParams)
}
Expand All @@ -70,16 +80,22 @@ export async function requestAttestationsFromIssuers(
attestations: AttestationsWrapper,
phoneNumber: string,
account: string,
pepper: string,
pepper: string
): Promise<RequestAttestationError[]> {
return concurrentMap(5, attestationsToReveal, async (attestation) => {
try {
const response = await attestations.revealPhoneNumberToIssuer(
phoneNumber,
const revealRequest: AttestationRequest = {
account,
attestation.issuer,
issuer: account,
phoneNumber,
salt: pepper,
smsRetrieverAppSig: undefined,
securityCodePrefix: undefined,
language: undefined,
}
const response = await attestations.revealPhoneNumberToIssuer(
attestation.attestationServiceURL,
pepper
revealRequest
)
if (!response.ok) {
return {
Expand Down Expand Up @@ -192,7 +208,7 @@ export async function chooseFromAvailablePhoneNumbers(
attestations: AttestationsWrapper,
twilioClient: Twilio,
maximumNumberOfAttestations: number,
salt: string,
salt: string
) {
const availableNumbers = (await twilioClient.incomingPhoneNumbers.list()).filter(
(number) => number.smsUrl === DUMMY_SMS_URL
Expand All @@ -214,7 +230,7 @@ async function findSuitableNumber(
attestations: AttestationsWrapper,
numbers: string[],
maximumNumberOfAttestations: number,
salt: string,
salt: string
) {
const attestedAccountsLookup = await attestations.lookupIdentifiers(
numbers.map((n) => PhoneNumberUtils.getPhoneHash(n, salt))
Expand Down Expand Up @@ -249,8 +265,7 @@ export async function createPhoneNumber(
let numbers
try {
numbers = await context.mobile.list({ limit: 100 })
}
catch {
} catch {
// Some geos inc US appear to have no 'mobile' subcategory.
numbers = await context.local.list({ limit: 100 })
}
Expand Down
10 changes: 9 additions & 1 deletion packages/contractkit/src/wrappers/Attestations.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { testWithGanache } from '@celo/dev-utils/lib/ganache-test'
import { PhoneNumberUtils } from '@celo/utils'
import { newKitFromWeb3 } from '../kit'
import { AttestationsWrapper } from './Attestations'
import { AttestationsWrapper, getSecurityCodePrefix } from './Attestations'

testWithGanache('Attestations Wrapper', (web3) => {
const PHONE_NUMBER = '+15555555555'
Expand Down Expand Up @@ -54,3 +54,11 @@ testWithGanache('Attestations Wrapper', (web3) => {
})
})
})

describe(getSecurityCodePrefix, () => {
it('should compute correct hash', () => {
expect(getSecurityCodePrefix('0x000000000000000000000008')).toEqual('8')
// 0xf7f551752A78Ce650385B58364225e5ec18D96cB -> 1415591498931780605110544902041322891412830525131
expect(getSecurityCodePrefix('0xf7f551752A78Ce650385B58364225e5ec18D96cB')).toEqual('1')
})
})
66 changes: 35 additions & 31 deletions packages/contractkit/src/wrappers/Attestations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { notEmpty, zip3 } from '@celo/base/lib/collections'
import { parseSolidityStringArray } from '@celo/base/lib/parsing'
import { appendPath } from '@celo/base/lib/string'
import { AttestationUtils, SignatureUtils } from '@celo/utils/lib'
import { AttestationRequest, GetAttestationRequest } from '@celo/utils/lib/io'
import BigNumber from 'bignumber.js'
import fetch from 'cross-fetch'
import { Address, CeloContract, NULL_ADDRESS } from '../base'
Expand All @@ -20,6 +21,14 @@ import {
} from './BaseWrapper'
import { Validator } from './Validators'

function hashAddressToSingleDigit(address: Address): number {
return new BigNumber(address.toLowerCase()).modulo(10).toNumber()
}

export function getSecurityCodePrefix(issuerAddress: Address) {
return `${hashAddressToSingleDigit(issuerAddress)}`
}

export interface AttestationStat {
completed: number
total: number
Expand Down Expand Up @@ -59,16 +68,6 @@ type AttestationServiceRunningCheckResult =
| { isValid: true; result: ActionableAttestation }
| { isValid: false; issuer: Address }

export interface AttesationServiceRevealRequest {
account: Address
phoneNumber: string
issuer: string
// TODO rename to pepper here and in Attesation Service
salt?: string
smsRetrieverAppSig?: string
language?: string
}

export interface UnselectedRequest {
blockNumber: number
attestationsRequested: number
Expand Down Expand Up @@ -525,34 +524,16 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {

/**
* Reveal phone number to issuer
* @param phoneNumber: attestation's phone number
* @param account: attestation's account
* @param issuer: validator's address
* @param serviceURL: validator's attestation service URL
* @param pepper: phone number privacy pepper
* @param smsRetrieverAppSig?: Android app's hash
* @param body
*/
revealPhoneNumberToIssuer(
phoneNumber: string,
account: Address,
issuer: Address,
serviceURL: string,
pepper?: string,
smsRetrieverAppSig?: string
) {
const body: AttesationServiceRevealRequest = {
account,
phoneNumber,
issuer,
salt: pepper,
smsRetrieverAppSig,
}
revealPhoneNumberToIssuer(serviceURL: string, requestBody: AttestationRequest) {
return fetch(appendPath(serviceURL, 'attestations'), {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify(body),
body: JSON.stringify(requestBody),
})
}

Expand Down Expand Up @@ -583,6 +564,29 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
})
}

/**
* Returns attestation code for provided security code from validator's attestation service
* @param serviceURL: validator's attestation service URL
* @param body
*/
getAttestationForSecurityCode(serviceURL: string, requestBody: GetAttestationRequest) {
const urlParams = new URLSearchParams({
phoneNumber: requestBody.phoneNumber,
account: requestBody.account,
issuer: requestBody.issuer,
})
if (requestBody.salt) {
urlParams.set('salt', requestBody.salt)
}
if (requestBody.securityCode) {
urlParams.set('securityCode', requestBody.securityCode)
}
return fetch(appendPath(serviceURL, 'get_attestations') + '?' + urlParams, {
method: 'GET',
headers: { 'Content-Type': 'application/json' },
})
}

/**
* Validates a given code by the issuer on-chain
* @param identifier Attestation identifier (e.g. phone hash)
Expand Down
2 changes: 0 additions & 2 deletions packages/docs/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,6 @@
- [AttestationState](developer-resources/contractkit/reference/enums/_wrappers_attestations_.attestationstate.md)
- [AttestationsWrapper](developer-resources/contractkit/reference/classes/_wrappers_attestations_.attestationswrapper.md)
- [ActionableAttestation](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.actionableattestation.md)
- [AttesationServiceRevealRequest](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attesationservicerevealrequest.md)
- [AttestationServiceStatusResponse](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationservicestatusresponse.md)
- [AttestationStat](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationstat.md)
- [AttestationStateForIssuer](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationstateforissuer.md)
Expand Down Expand Up @@ -485,7 +484,6 @@
- [UnlockableWallet](developer-resources/contractkit/reference/interfaces/_wallets_wallet_.unlockablewallet.md)
- [Wallet](developer-resources/contractkit/reference/interfaces/_wallets_wallet_.wallet.md)
- [ActionableAttestation](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.actionableattestation.md)
- [AttesationServiceRevealRequest](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attesationservicerevealrequest.md)
- [AttestationServiceStatusResponse](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationservicestatusresponse.md)
- [AttestationStat](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationstat.md)
- [AttestationStateForIssuer](developer-resources/contractkit/reference/interfaces/_wrappers_attestations_.attestationstateforissuer.md)
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit 486a02e

Please sign in to comment.