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

Optimized Attestation view calls and removal of the reveal TX #1578

Merged
merged 20 commits into from
Nov 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/attestation-service/config/config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't review this

"development": {
"username": "root",
"password": null,
"database": "database_development",
"host": "db/dev.db",
"dialect": "sqlite",
"operatorsAliases": false
},
"test": {
"username": "root",
"password": null,
"database": "database_test",
"host": "127.0.0.1",
"dialect": "sqlite",
"operatorsAliases": false
},
"production": {
"use_env_variable": "DATABASE_URL"
}
}
40 changes: 26 additions & 14 deletions packages/celotool/src/cmds/account/verify.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
ActionableAttestation,
AttestationsWrapper,
} from '@celo/contractkit/lib/wrappers/Attestations'
import { concurrentMap } from '@celo/utils/lib/async'
import { base64ToHex } from '@celo/utils/lib/attestations'
import prompts from 'prompts'
import { switchToClusterFromEnv } from 'src/lib/cluster'
Expand Down Expand Up @@ -54,12 +55,13 @@ async function verifyCmd(argv: VerifyArgv) {
const attestations = await kit.contracts.getAttestations()
const accounts = await kit.contracts.getAccounts()
await printCurrentCompletedAttestations(attestations, argv.phone, account)

let attestationsToComplete = await attestations.getActionableAttestations(argv.phone, account)

// Request more attestations
if (argv.num > attestationsToComplete.length) {
console.info(`Requesting ${argv.num - attestationsToComplete.length} attestations`)
console.info(
`Requesting ${argv.num - attestationsToComplete.length} attestations from the smart contract`
)
await requestMoreAttestations(
attestations,
argv.phone,
Expand All @@ -78,9 +80,9 @@ async function verifyCmd(argv: VerifyArgv) {
}

attestationsToComplete = await attestations.getActionableAttestations(argv.phone, account)
// Find attestations we can reveal/verify
console.info(`Revealing ${attestationsToComplete.length} attestations`)
await revealAttestations(attestationsToComplete, attestations, argv.phone)
// Find attestations we can verify
console.info(`Requesting ${attestationsToComplete.length} attestations from issuers`)
await requestAttestationsFromIssuers(attestationsToComplete, attestations, argv.phone, account)

await promptForCodeAndVerify(attestations, argv.phone, account)
}
Expand Down Expand Up @@ -115,18 +117,28 @@ async function requestMoreAttestations(
await attestations.selectIssuers(phoneNumber).then((txo) => txo.sendAndWaitForReceipt())
}

async function revealAttestations(
async function requestAttestationsFromIssuers(
attestationsToReveal: ActionableAttestation[],
attestations: AttestationsWrapper,
phoneNumber: string
phoneNumber: string,
account: string
) {
return Promise.all(
attestationsToReveal.map(async (attestation) =>
attestations
.reveal(phoneNumber, attestation.issuer)
.then((txo) => txo.sendAndWaitForReceipt())
)
)
return concurrentMap(5, attestationsToReveal, async (attestation) => {
try {
const response = await attestations.revealPhoneNumberToIssuer(
phoneNumber,
account,
attestation.issuer,
attestation.attestationServiceURL
)
if (!response.ok) {
throw new Error(`Request failed with status ${response.status}: ${await response.text()}`)
}
} catch (error) {
console.error(`Error requesting attestations from issuer ${attestation.issuer}`)
console.error(error)
}
})
}

async function verifyCode(
Expand Down
5 changes: 3 additions & 2 deletions packages/celotool/src/e2e-tests/attestations_tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ describe('governance tests', () => {

const stats = await Attestations.getAttestationStat(phoneNumber, validatorAddress)
assert.equal(stats.total, 2)
const actionable = await Attestations.getActionableAttestations(phoneNumber, validatorAddress)
assert.lengthOf(actionable, 2)

const issuers = await Attestations.getAttestationIssuers(phoneNumber, validatorAddress)
assert.lengthOf(issuers, 2)
})
})
})
151 changes: 80 additions & 71 deletions packages/contractkit/src/wrappers/Attestations.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import { ECIES, PhoneNumberUtils, SignatureUtils } from '@celo/utils'
import { sleep } from '@celo/utils/lib/async'
import { zip3 } from '@celo/utils/lib/collections'
import { PhoneNumberUtils, SignatureUtils } from '@celo/utils'
import { concurrentMap, sleep } from '@celo/utils/lib/async'
import { notEmpty, zip3 } from '@celo/utils/lib/collections'
import { parseSolidityStringArray } from '@celo/utils/lib/parsing'
import BigNumber from 'bignumber.js'
import fetch from 'cross-fetch'
import * as Web3Utils from 'web3-utils'
import { Address, CeloContract, NULL_ADDRESS } from '../base'
import { Attestations } from '../generated/types/Attestations'
import { ClaimTypes, IdentityMetadataWrapper } from '../identity'
import {
BaseWrapper,
proxyCall,
Expand Down Expand Up @@ -45,16 +48,10 @@ export enum AttestationState {

export interface ActionableAttestation {
issuer: Address
attestationState: AttestationState
blockNumber: number
publicKey: string
attestationServiceURL: string
}

const parseAttestationInfo = (rawState: { 0: string; 1: string }) => ({
attestationState: parseInt(rawState[0], 10),
blockNumber: parseInt(rawState[1], 10),
})

function attestationMessageToSign(phoneHash: string, account: Address) {
const messageHash: string = Web3Utils.soliditySha3(
{ type: 'bytes32', value: phoneHash },
Expand All @@ -63,6 +60,23 @@ function attestationMessageToSign(phoneHash: string, account: Address) {
return messageHash
}

interface GetCompletableAttestationsResponse {
0: string[]
1: string[]
2: string[]
3: string[]
}
function parseGetCompletableAttestations(response: GetCompletableAttestationsResponse) {
const metadataURLs = parseSolidityStringArray(
response[2].map(toNumber),
(response[3] as unknown) as string
)

return zip3(response[0].map(toNumber), response[1], metadataURLs).map(
([blockNumber, issuer, metadataURL]) => ({ blockNumber, issuer, metadataURL })
)
}

const stringIdentity = (x: string) => x
export class AttestationsWrapper extends BaseWrapper<Attestations> {
/**
Expand Down Expand Up @@ -129,6 +143,17 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
await sleep(pollDurationSeconds * 1000)
}
}

/**
* Returns the issuers of attestations for a phoneNumber/account combo
* @param phoneNumber Phone Number
* @param account Account
*/
getAttestationIssuers = proxyCall(
this.contract.methods.getAttestationIssuers,
tupleParser(PhoneNumberUtils.getPhoneHash, (x: string) => x)
)

/**
* Returns the attestation state of a phone number/account/issuer tuple
* @param phoneNumber Phone Number
Expand Down Expand Up @@ -179,50 +204,46 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
}

/**
* Returns an array of attestations that can be completed, along with the issuers public key
* Returns an array of attestations that can be completed, along with the issuers' attestation
* service urls
* @param phoneNumber
* @param account
*/
async getActionableAttestations(
phoneNumber: string,
account: Address
): Promise<ActionableAttestation[]> {
const accounts = await this.kit.contracts.getAccounts()
const phoneHash = PhoneNumberUtils.getPhoneHash(phoneNumber)
const expiryBlocks = await this.attestationExpiryBlocks()
const currentBlockNumber = await this.kit.web3.eth.getBlockNumber()

const issuers = await this.contract.methods.getAttestationIssuers(phoneHash, account).call()
const issuerState = Promise.all(
issuers.map((issuer) =>
this.contract.methods
.getAttestationState(phoneHash, account, issuer)
.call()
.then(parseAttestationInfo)
)
)

// Typechain is not properly typing getDataEncryptionKey
const publicKeys: Promise<string[]> = Promise.all(
issuers.map((issuer) => accounts.getDataEncryptionKey(issuer) as any)
const result = await this.contract.methods.getCompletableAttestations(phoneHash, account).call()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please define a helper method that parses this into a more readable object


const withAttestationServiceURLs = await concurrentMap(
5,
parseGetCompletableAttestations(result),
async ({ blockNumber, issuer, metadataURL }) => {
try {
const metadata = await IdentityMetadataWrapper.fetchFromURL(metadataURL)
const attestationServiceURLClaim = metadata.findClaim(ClaimTypes.ATTESTATION_SERVICE_URL)

if (attestationServiceURLClaim === undefined) {
throw new Error(`No attestation service URL registered for ${issuer}`)
}

// TODO: Once we have status indicators, we should check if service is up
// https://github.com/celo-org/celo-monorepo/issues/1586
return {
blockNumber,
issuer,
attestationServiceURL: attestationServiceURLClaim.url,
}
} catch (error) {
console.error(error)
return null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider allowing the error to propagate

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking about it, but it seemed to me that a validator not or incorrectly registering an attestation service url should not result in an error for a client who just wants to complete attestations. Thus, I favored returning the attestations that are completable, by intent of the function.

}
}
)

const isIncomplete = (status: AttestationState) => status === AttestationState.Incomplete
const hasNotExpired = (blockNumber: number) => currentBlockNumber < blockNumber + expiryBlocks
const isValidKey = (key: string) => key !== null && key !== '0x0'

return zip3(issuers, await issuerState, await publicKeys)
.filter(
([_issuer, attestation, publicKey]) =>
isIncomplete(attestation.attestationState) &&
hasNotExpired(attestation.blockNumber) &&
isValidKey(publicKey)
)
.map(([issuer, attestation, publicKey]) => ({
...attestation,
issuer,
publicKey: publicKey.toString(),
}))
return withAttestationServiceURLs.filter(notEmpty)
}

/**
Expand Down Expand Up @@ -350,35 +371,23 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
return toTransactionObject(this.kit, this.contract.methods.selectIssuers(phoneHash))
}

/**
* Reveals the phone number to the issuer of the attestation on-chain
* @param phoneNumber The phone number which requested attestation
* @param issuer The address of issuer of the attestation
*/
async reveal(phoneNumber: string, issuer: Address) {
const accounts = await this.kit.contracts.getAccounts()
const publicKey: string = (await accounts.getDataEncryptionKey(issuer)) as any

if (!publicKey) {
throw new Error('Issuer data encryption key is null')
}

const encryptedPhone: any =
'0x' +
ECIES.Encrypt(
Buffer.from(publicKey.slice(2), 'hex'),
Buffer.from(phoneNumber, 'utf8')
).toString('hex')

return toTransactionObject(
this.kit,
this.contract.methods.reveal(
PhoneNumberUtils.getPhoneHash(phoneNumber),
encryptedPhone,
async revealPhoneNumberToIssuer(
phoneNumber: string,
account: Address,
issuer: Address,
serviceURL: string
) {
return fetch(serviceURL + '/attestations', {
method: 'POST',
headers: {
'Content-Type': 'application/json',
},
body: JSON.stringify({
account,
phoneNumber,
issuer,
true
)
)
}),
})
}

/**
Expand Down
30 changes: 30 additions & 0 deletions packages/protocol/contracts/common/Accounts.sol
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,36 @@ contract Accounts is IAccounts, ReentrancyGuard, Initializable, UsingRegistry {
return accounts[account].metadataURL;
}

/**
* @notice Getter for the metadata of multiple accounts.
* @param accountsToQuery The addresses of the accounts to get the metadata for.
* @return (stringLengths[] - the length of each string in bytes
* data - all strings concatenated
* )
*/
function batchGetMetadataURL(address[] calldata accountsToQuery)
asaj marked this conversation as resolved.
Show resolved Hide resolved
external
view
returns (uint256[] memory, bytes memory)
{
uint256 totalSize = 0;
uint256[] memory sizes = new uint256[](accountsToQuery.length);
for (uint256 i = 0; i < accountsToQuery.length; i = i.add(1)) {
sizes[i] = bytes(accounts[accountsToQuery[i]].metadataURL).length;
totalSize = totalSize.add(sizes[i]);
}

bytes memory data = new bytes(totalSize);
uint256 pointer = 0;
for (uint256 i = 0; i < accountsToQuery.length; i = i.add(1)) {
for (uint256 j = 0; j < sizes[i]; j = j.add(1)) {
data[pointer] = bytes(accounts[accountsToQuery[i]].metadataURL)[j];
pointer = pointer.add(1);
}
}
return (sizes, data);
}

/**
* @notice Getter for the data encryption key and version.
* @param account The address of the account to get the key for
Expand Down
4 changes: 4 additions & 0 deletions packages/protocol/contracts/common/interfaces/IAccounts.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,9 @@ interface IAccounts {
function getDataEncryptionKey(address) external view returns (bytes memory);
function getWalletAddress(address) external view returns (address);
function getMetadataURL(address) external view returns (string memory);
function batchGetMetadataURL(address[] calldata)
asaj marked this conversation as resolved.
Show resolved Hide resolved
external
view
returns (uint256[] memory, bytes memory);
function getName(address) external view returns (string memory);
}
Loading