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

Add selectIssuers Transaction #1327

Merged
merged 40 commits into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
7c9859d
Add revealIssuers tx
nambrot Oct 12, 2019
301e776
Indention
nambrot Oct 14, 2019
932bd67
Record request/completion time separately
nambrot Oct 14, 2019
236fd8a
Contractkit changes
nambrot Oct 14, 2019
9982d56
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 14, 2019
f99bc97
Change reference to ActionalbeAttestations
nambrot Oct 14, 2019
bb87717
Lint
nambrot Oct 14, 2019
4b05a4b
Change to blocks instead of Unix seconds
nambrot Oct 15, 2019
9d1b99d
Add SafeCast library
nambrot Oct 15, 2019
2573113
PR comments
nambrot Oct 15, 2019
c42d317
Move events below storage variables
nambrot Oct 15, 2019
66729fd
Adjust CK
nambrot Oct 15, 2019
723f45c
lint
nambrot Oct 15, 2019
110c0b2
PR comments
nambrot Oct 16, 2019
6c8b274
PR comments
nambrot Oct 19, 2019
f61dabc
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 19, 2019
c4e9dea
Fix wrapSend
nambrot Oct 21, 2019
6663908
Rename
nambrot Oct 21, 2019
c7de3b1
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 22, 2019
a51becd
PR comments
nambrot Oct 22, 2019
ffe159c
Store attestationRequestFeeToken on each Attestation
nambrot Oct 22, 2019
54c9d10
Properly delete the unselected request
nambrot Oct 23, 2019
bf26fe5
Fix lint
nambrot Oct 23, 2019
568cad3
Delete after event emission
nambrot Oct 23, 2019
ff0efe4
PR comments
nambrot Oct 25, 2019
629c6f1
Leave walletkit untouched
nambrot Oct 25, 2019
4108dbe
Fix getActionableAttestations
nambrot Oct 25, 2019
c7a7f29
Remove kit from parameters
nambrot Oct 25, 2019
26ecd30
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 26, 2019
317610e
Undo changes in walletkit/verification pool
nambrot Oct 26, 2019
f29e3e3
Remove change from mobile test
nambrot Oct 28, 2019
121ce93
Change event formatting
nambrot Oct 28, 2019
eaccc25
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 28, 2019
c9c5ef3
[Add-on PR] Enforce wait for selectIssuers (#1503)
nambrot Oct 31, 2019
2d99e4c
Fix election random access
nambrot Oct 31, 2019
07ac718
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 31, 2019
ab3d0f8
Adjust e2e test and celotool cmd
nambrot Oct 31, 2019
3ac9474
Adjust CK
nambrot Oct 31, 2019
fa03266
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 31, 2019
7a3fbcb
Merge branch 'master' into nambrot/attestation-commit
nambrot Oct 31, 2019
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
30 changes: 23 additions & 7 deletions packages/celotool/src/cmds/account/verify.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import { AccountArgv } from '@celo/celotool/src/cmds/account'
import { portForwardAnd } from '@celo/celotool/src/lib/port_forward'
import { CeloContract, newKit } from '@celo/contractkit'
import { AttestationsWrapper } from '@celo/contractkit/lib/wrappers/Attestations'
import { ActionableAttestation, decodeAttestationCode } from '@celo/walletkit'
import { CeloContract, ContractKit, newKit } from '@celo/contractkit'
import {
ActionableAttestation,
AttestationsWrapper,
} from '@celo/contractkit/lib/wrappers/Attestations'
import { decodeAttestationCode } from '@celo/walletkit'
asaj marked this conversation as resolved.
Show resolved Hide resolved
import prompts from 'prompts'
import { switchToClusterFromEnv } from 'src/lib/cluster'
import * as yargs from 'yargs'
Expand Down Expand Up @@ -51,7 +54,12 @@ async function verifyCmd(argv: VerifyArgv) {
const attestations = await kit.contracts.getAttestations()
await printCurrentCompletedAttestations(attestations, argv.phone, account)

let attestationsToComplete = await attestations.getActionableAttestations(argv.phone, account)
let currentBlockNumber = await kit.web3.eth.getBlockNumber()
let attestationsToComplete = await attestations.getActionableAttestations(
argv.phone,
account,
currentBlockNumber
asaj marked this conversation as resolved.
Show resolved Hide resolved
)

// Request more attestations
if (argv.num > attestationsToComplete.length) {
Expand All @@ -72,12 +80,17 @@ async function verifyCmd(argv: VerifyArgv) {
await result.waitReceipt()
}

attestationsToComplete = await attestations.getActionableAttestations(argv.phone, account)
currentBlockNumber = await kit.web3.eth.getBlockNumber()
attestationsToComplete = await attestations.getActionableAttestations(
argv.phone,
account,
currentBlockNumber
)
// Find attestations we can reveal/verify
console.info(`Revealing ${attestationsToComplete.length} attestations`)
await revealAttestations(attestationsToComplete, attestations, argv.phone)

await promptForCodeAndVerify(attestations, argv.phone, account)
await promptForCodeAndVerify(kit, attestations, argv.phone, account)
}

export async function printCurrentCompletedAttestations(
Expand Down Expand Up @@ -157,14 +170,17 @@ async function verifyCode(
}

async function promptForCodeAndVerify(
kit: ContractKit,
attestations: AttestationsWrapper,
phoneNumber: string,
account: string
) {
while (true) {
const currentBlockNumber = await kit.web3.eth.getBlockNumber()
const attestationsToComplete = await attestations.getActionableAttestations(
phoneNumber,
account
account,
currentBlockNumber
)

if (attestationsToComplete.length === 0) {
Expand Down
32 changes: 21 additions & 11 deletions packages/contractkit/src/wrappers/Attestations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export interface AttestationsToken {
}

export interface AttestationsConfig {
attestationExpirySeconds: number
attestationExpiryBlocks: number
attestationRequestFees: AttestationsToken[]
}

Expand All @@ -42,13 +42,13 @@ export enum AttestationState {
export interface ActionableAttestation {
issuer: Address
attestationState: AttestationState
time: number
blockNumber: number
publicKey: string
}

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

function attestationMessageToSign(phoneHash: string, account: Address) {
Expand All @@ -63,8 +63,8 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
/**
* Returns the time an attestation can be completable before it is considered expired
*/
attestationExpirySeconds = proxyCall(
this.contract.methods.attestationExpirySeconds,
attestationExpiryBlocks = proxyCall(
this.contract.methods.attestationExpiryBlocks,
undefined,
toNumber
)
Expand Down Expand Up @@ -156,11 +156,11 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
*/
async getActionableAttestations(
phoneNumber: string,
account: Address
account: Address,
currentBlockNumber: number
): Promise<ActionableAttestation[]> {
const phoneHash = PhoneNumberUtils.getPhoneHash(phoneNumber)
const expirySeconds = await this.attestationExpirySeconds()
const nowInUnixSeconds = Math.floor(new Date().getTime() / 1000)
const expirySeconds = await this.attestationExpiryBlocks()
asaj marked this conversation as resolved.
Show resolved Hide resolved

const issuers = await this.contract.methods.getAttestationIssuers(phoneHash, account).call()
const issuerState = Promise.all(
Expand All @@ -178,14 +178,14 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
)

const isIncomplete = (status: AttestationState) => status === AttestationState.Incomplete
const hasNotExpired = (time: number) => nowInUnixSeconds < time + expirySeconds
const hasNotExpired = (time: number) => currentBlockNumber < time + expirySeconds
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks incorrect, you're comparing a block number to a time plus a block number. Would it be better to use isAttestationExpired?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The variable is called time, but it is actually a block number, so renamed it to clarify. I wanted to avoid using isAttestationExpired to reduce the number of view calls necessary. I talked with @jmrossy and we'll probably have a more optimized view call for this anyways

const isValidKey = (key: string) => key !== null && key !== '0x0'

return zip3(issuers, await issuerState, await publicKeys)
.filter(
([_issuer, attestation, publicKey]) =>
isIncomplete(attestation.attestationState) &&
hasNotExpired(attestation.time) &&
hasNotExpired(attestation.blockNumber) &&
isValidKey(publicKey)
)
.map(([issuer, attestation, publicKey]) => ({
Expand Down Expand Up @@ -248,7 +248,7 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
})
)
return {
attestationExpirySeconds: await this.attestationExpirySeconds(),
attestationExpiryBlocks: await this.attestationExpiryBlocks(),
attestationRequestFees: fees,
}
}
Expand Down Expand Up @@ -311,6 +311,16 @@ export class AttestationsWrapper extends BaseWrapper<Attestations> {
)
}

/**
* Selecets the issuers for previously requested attestations for a phone number
asaj marked this conversation as resolved.
Show resolved Hide resolved
* @param phoneNumber The phone number for which to request attestations for
* @param token The token with which to pay for the attestation fee
*/
async selectIssuers(phoneNumber: string) {
const phoneHash = PhoneNumberUtils.getPhoneHash(phoneNumber)
return wrapSend(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
Expand Down
30 changes: 30 additions & 0 deletions packages/protocol/contracts/common/MockSafeCast.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@

pragma solidity ^0.5.0;
// TODO: Remove this and use upstream when
// https://github.com/OpenZeppelin/openzeppelin-contracts/pull/1926/files gets merged

import "./SafeCast.sol";

contract MockSafeCast {
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't call this a mock. SafeCastTest would be consistent with the rest of our naming

using SafeCast for uint;

function toUint128(uint a) public pure returns (uint128) {
return a.toUint128();
}

function toUint64(uint a) public pure returns (uint64) {
return a.toUint64();
}

function toUint32(uint a) public pure returns (uint32) {
return a.toUint32();
}

function toUint16(uint a) public pure returns (uint16) {
return a.toUint16();
}

function toUint8(uint a) public pure returns (uint8) {
return a.toUint8();
}
}
88 changes: 88 additions & 0 deletions packages/protocol/contracts/common/SafeCast.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
pragma solidity ^0.5.0;
// TODO: Remove this and use upstream when
// https://github.com/OpenZeppelin/openzeppelin-contracts/pull/1926/files gets merged

/**
* @dev Wrappers over Solidity's uintXX casting operators with added overflow
* checks.
*
* Downcasting from uint256 in Solidity does not revert by default on overflow.
* This can easily result in undesired exploitation or bugs, since developers
* usually assume that overflows raise errors. `SafeCast` restores this intuition
* by reverting the transaction when such an operation overflows.
*
* Using this library instead of the unchecked operations eliminates an entire
* class of bugs, so it's recommended to use it always.
*/
library SafeCast {

/**
* @dev Returns the downcasted uint128 from uint256, reverting on
* overflow (when the input is greater than largest uint128).
*
* Counterpart to Solidity's `uint128` operator.
*
* Requirements:
* - input must fit into 128 bits
*/
function toUint128(uint256 value) internal pure returns (uint128) {
require(value < 2**128, "SafeCast: value doesn\'t fit in 128 bits");
return uint128(value);
}

/**
* @dev Returns the downcasted uint64 from uint256, reverting on
* overflow (when the input is greater than largest uint64).
*
* Counterpart to Solidity's `uint64` operator.
*
* Requirements:
* - input must fit into 64 bits
*/
function toUint64(uint256 value) internal pure returns (uint64) {
require(value < 2**64, "SafeCast: value doesn\'t fit in 64 bits");
return uint64(value);
}

/**
* @dev Returns the downcasted uint32 from uint256, reverting on
* overflow (when the input is greater than largest uint32).
*
* Counterpart to Solidity's `uint32` operator.
*
* Requirements:
* - input must fit into 32 bits
*/
function toUint32(uint256 value) internal pure returns (uint32) {
require(value < 2**32, "SafeCast: value doesn\'t fit in 32 bits");
return uint32(value);
}

/**
* @dev Returns the downcasted uint16 from uint256, reverting on
* overflow (when the input is greater than largest uint16).
*
* Counterpart to Solidity's `uint16` operator.
*
* Requirements:
* - input must fit into 16 bits
*/
function toUint16(uint256 value) internal pure returns (uint16) {
require(value < 2**16, "SafeCast: value doesn\'t fit in 16 bits");
return uint16(value);
}

/**
* @dev Returns the downcasted uint8 from uint256, reverting on
* overflow (when the input is greater than largest uint8).
*
* Counterpart to Solidity's `uint8` operator.
*
* Requirements:
* - input must fit into 8 bits.
*/
function toUint8(uint256 value) internal pure returns (uint8) {
require(value < 2**8, "SafeCast: value doesn\'t fit in 8 bits");
return uint8(value);
}
}
Loading