Improve verification speed (#216)

* Speed up verification transactions * Pull request tx static gas in a constant
celo-org · Jul 31, 2019 · 74a6423 · 74a6423
1 parent a14f220
commit 74a6423
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 40 deletions.
diff --git a/packages/contractkit/src/contract-utils.ts b/packages/contractkit/src/contract-utils.ts
@@ -221,7 +221,8 @@ export async function sendTransactionAsync<T>(
   tx: TransactionObject<T>,
   account: string,
   gasCurrencyContract: StableToken | GoldToken,
-  logger: TxLogger = emptyTxLogger
+  logger: TxLogger = emptyTxLogger,
+  estimatedGas?: number | undefined
 ): Promise<TxPromises> {
   // @ts-ignore
   const resolvers: TxPromiseResolvers = {}
@@ -257,19 +258,21 @@ export async function sendTransactionAsync<T>(
       gasCurrency: gasCurrencyContract._address,
       gasPrice: '0',
     }
-    const estimatedGas = Math.round((await tx.estimateGas(txParams)) * gasInflateFactor)
-    logger(EstimatedGas(estimatedGas))
 
-    await tx
-      .send({
-        from: account,
-        // @ts-ignore
-        gasCurrency: gasCurrencyContract._address,
-        gas: estimatedGas,
-        // Hack to prevent web3 from adding the suggested gold gas price, allowing geth to add
-        // the suggested price in the selected gasCurrency.
-        gasPrice: '0',
-      })
+    if (estimatedGas === undefined) {
+      estimatedGas = Math.round((await tx.estimateGas(txParams)) * gasInflateFactor)
+      logger(EstimatedGas(estimatedGas))
+    }
+
+    tx.send({
+      from: account,
+      // @ts-ignore
+      gasCurrency: gasCurrencyContract._address,
+      gas: estimatedGas,
+      // Hack to prevent web3 from adding the suggested gold gas price, allowing geth to add
+      // the suggested price in the selected gasCurrency.
+      gasPrice: '0',
+    })
       .on('receipt', (r: TransactionReceipt) => {
         logger(ReceiptReceived(r))
         if (resolvers.receipt) {

diff --git a/packages/mobile/src/identity/verification.test.ts b/packages/mobile/src/identity/verification.test.ts
@@ -40,6 +40,7 @@ const MockedAnalytics = CeloAnalytics as any
 
 jest.mock('src/transactions/send', () => ({
   sendTransaction: jest.fn(),
+  sendTransactionPromises: jest.fn(() => ({ confirmation: true, transactionHash: true })),
 }))
 
 jest.mock('@celo/react-native-sms-retriever', () => ({

diff --git a/packages/mobile/src/identity/verification.ts b/packages/mobile/src/identity/verification.ts
@@ -42,7 +42,7 @@ import {
 import { attestationCodesSelector } from 'src/identity/reducer'
 import { startAutoSmsRetrieval } from 'src/identity/smsRetrieval'
 import { RootState } from 'src/redux/reducers'
-import { sendTransaction } from 'src/transactions/send'
+import { sendTransaction, sendTransactionPromises } from 'src/transactions/send'
 import Logger from 'src/utils/Logger'
 import { web3 } from 'src/web3/contracts'
 import { getConnectedAccount, getConnectedUnlockedAccount } from 'src/web3/saga'
@@ -54,7 +54,12 @@ export const NUM_ATTESTATIONS_REQUIRED = 3
 export const VERIFICATION_TIMEOUT = 5 * 60 * 1000 // 5 minutes
 export const ERROR_DURATION = 5000 // 5 seconds
 export const NULL_ADDRESS = '0x0000000000000000000000000000000000000000'
-
+// Gas estimation for concurrent pending transactions is currently not support for
+// light clients, so we have to statically specify the gas here. Furthermore, the
+// current request function does a whole validator election which is why it is very
+// expensive. When https://github.com/celo-org/celo-monorepo-old/issues/3818 gets
+// merged we should significantly reduce this number
+export const REQUEST_TX_GAS = 7000000
 export enum CodeInputType {
   AUTOMATIC = 'automatic',
   MANUAL = 'manual',
@@ -184,23 +189,19 @@ export function* doVerificationFlow() {
     )
     const autoRetrievalTask: Task = yield fork(startAutoSmsRetrieval)
 
-    // This needs to go before revealing the attesttions because that depends on the public data key being set.
-    yield call(setAccount, attestationsContract, account, dataKey)
-
-    CeloAnalytics.trackSubEvent(
-      CustomEventNames.verification,
-      CustomEventNames.verification_set_account
-    )
-
-    // Request codes for the attestations needed
-    yield call(
-      revealNeededAttestations,
-      attestationsContract,
-      account,
-      e164Number,
-      e164NumberHash,
-      attestations
-    )
+    yield all([
+      // Set acccount and data encryption key in contract
+      call(setAccount, attestationsContract, account, dataKey),
+      // Request codes for the attestations needed
+      call(
+        revealNeededAttestations,
+        attestationsContract,
+        account,
+        e164Number,
+        e164NumberHash,
+        attestations
+      ),
+    ])
 
     receiveMessageTask.cancel()
     autoRetrievalTask.cancel()
@@ -312,8 +313,6 @@ export async function requestNeededAttestations(
     numAttestationsRequestsNeeded
   )
 
-  await sendTransaction(approveTx, account, TAG, 'Approve Attestations')
-
   Logger.debug(
     `${TAG}@requestNeededAttestations`,
     `Requesting ${numAttestationsRequestsNeeded} new attestations`
@@ -325,7 +324,17 @@ export async function requestNeededAttestations(
     numAttestationsRequestsNeeded,
     stableTokenContract
   )
-  await sendTransaction(requestTx, account, TAG, 'Request Attestations')
+
+  const {
+    confirmation: approveConfirmationPromise,
+    transactionHash: approveTransactionHashPromise,
+  } = await sendTransactionPromises(approveTx, account, TAG, 'Approve Attestations')
+
+  await approveTransactionHashPromise
+  await Promise.all([
+    sendTransaction(requestTx, account, TAG, 'Request Attestations', REQUEST_TX_GAS),
+    approveConfirmationPromise,
+  ])
 }
 
 function attestationCodeReceiver(
@@ -481,7 +490,11 @@ async function setAccount(
     !compareAddresses(currentWalletDEK, dataKey)
   ) {
     const setAccountTx = makeSetAccountTx(attestationsContract, address, dataKey)
-    return sendTransaction(setAccountTx, address, TAG, `Set Wallet Address & DEK`)
+    await sendTransaction(setAccountTx, address, TAG, `Set Wallet Address & DEK`)
+    CeloAnalytics.trackSubEvent(
+      CustomEventNames.verification,
+      CustomEventNames.verification_set_account
+    )
   }
 }
 

diff --git a/packages/mobile/src/transactions/send.ts b/packages/mobile/src/transactions/send.ts
@@ -53,10 +53,11 @@ export const sendTransactionPromises = async (
   tx: TransactionObject<any>,
   account: string,
   tag: string,
-  txId: string
+  txId: string,
+  staticGas?: number | undefined
 ) => {
   const stableToken = await getStableTokenContract(web3)
-  return sendTransactionAsync(tx, account, stableToken, getLogger(tag, txId))
+  return sendTransactionAsync(tx, account, stableToken, getLogger(tag, txId), staticGas)
 }
 
 // Send a transaction and await for its confirmation
@@ -65,7 +66,8 @@ export const sendTransaction = async (
   tx: TransactionObject<any>,
   account: string,
   tag: string,
-  txId: string
+  txId: string,
+  staticGas?: number | undefined
 ) => {
-  return sendTransactionPromises(tx, account, tag, txId).then(awaitConfirmation)
+  return sendTransactionPromises(tx, account, tag, txId, staticGas).then(awaitConfirmation)
 }