Add support for async calls under coverage

4 files changed, 52 insertions, 25 deletions

diff --git a/packages/contracts/test/token_registry.ts b/packages/contracts/test/token_registry.ts
index 5313ec1b1..9dcc77b82 100644
--- a/packages/contracts/test/token_registry.ts
+++ b/packages/contracts/test/token_registry.ts
@@ -142,8 +142,10 @@ describe('TokenRegistry', () => {
                 await tokenReg.setTokenName.sendTransactionAsync(token1.address, token2.name, {
                     from: owner,
                 });
-                const newData = await tokenRegWrapper.getTokenByNameAsync(token2.name);
-                const oldData = await tokenRegWrapper.getTokenByNameAsync(token1.name);
+                const [newData, oldData] = await Promise.all([
+                    tokenRegWrapper.getTokenByNameAsync(token2.name),
+                    tokenRegWrapper.getTokenByNameAsync(token1.name),
+                ]);
 
                 const expectedNewData = _.assign({}, token1, { name: token2.name });
                 const expectedOldData = nullToken;
@@ -177,8 +179,10 @@ describe('TokenRegistry', () => {
 
             it('should change the token symbol when called by owner', async () => {
                 await tokenReg.setTokenSymbol.sendTransactionAsync(token1.address, token2.symbol, { from: owner });
-                const newData = await tokenRegWrapper.getTokenBySymbolAsync(token2.symbol);
-                const oldData = await tokenRegWrapper.getTokenBySymbolAsync(token1.symbol);
+                const [newData, oldData] = await Promise.all([
+                    tokenRegWrapper.getTokenBySymbolAsync(token2.symbol),
+                    tokenRegWrapper.getTokenBySymbolAsync(token1.symbol),
+                ]);
 
                 const expectedNewData = _.assign({}, token1, { symbol: token2.symbol });
                 const expectedOldData = nullToken;
diff --git a/packages/sol-cov/package.json b/packages/sol-cov/package.json
index f80e186ca..6a0b61ce9 100644
--- a/packages/sol-cov/package.json
+++ b/packages/sol-cov/package.json
@@ -22,10 +22,11 @@
     "dependencies": {
         "@0xproject/subproviders": "^0.7.0",
         "@0xproject/utils": "^0.3.4",
-        "glob": "^7.1.2",
         "ethereumjs-util": "^5.1.1",
+        "glob": "^7.1.2",
         "istanbul": "^0.4.5",
         "lodash": "^4.17.4",
+        "semaphore-async-await": "^1.5.1",
         "solidity-coverage": "^0.4.10",
         "solidity-parser-sc": "^0.4.4",
         "web3": "^0.20.0"
diff --git a/packages/sol-cov/src/coverage_subprovider.ts b/packages/sol-cov/src/coverage_subprovider.ts
index f91b95c79..c84211d3a 100644
--- a/packages/sol-cov/src/coverage_subprovider.ts
+++ b/packages/sol-cov/src/coverage_subprovider.ts
@@ -1,21 +1,32 @@
 import { Callback, NextCallback, Subprovider } from '@0xproject/subproviders';
 import { promisify } from '@0xproject/utils';
 import * as _ from 'lodash';
+import { Lock } from 'semaphore-async-await';
 import * as Web3 from 'web3';
 
 import { constants } from './constants';
 import { CoverageManager } from './coverage_manager';
 import { TraceInfoExistingContract, TraceInfoNewContract } from './types';
 
+interface MaybeFakeTxData extends Web3.TxData {
+    isFakeTransaction?: boolean;
+}
+
 /*
  * This class implements the web3-provider-engine subprovider interface and collects traces of all transactions that were sent and all calls that were executed.
+ * Because there is no notion of call trace in the rpc - we collect them in rather non-obvious/hacky way.
+ * On each call - we create a snapshot, execute the call as a transaction, get the trace, revert the snapshot.
+ * That allows us to not influence the test behaviour.
  * Source: https://github.com/MetaMask/provider-engine/blob/master/subproviders/subprovider.js
  */
 export class CoverageSubprovider extends Subprovider {
+    // Lock is used to not accept normal transactions while doing call/snapshot magic because they'll be reverted later otherwise
+    private _lock: Lock;
     private _coverageManager: CoverageManager;
     private _defaultFromAddress: string;
     constructor(artifactsPath: string, sourcesPath: string, networkId: number, defaultFromAddress: string) {
         super();
+        this._lock = new Lock();
         this._defaultFromAddress = defaultFromAddress;
         this._coverageManager = new CoverageManager(
             artifactsPath,
@@ -50,11 +61,16 @@ export class CoverageSubprovider extends Subprovider {
         await this._coverageManager.writeCoverageAsync();
     }
     private async _onTransactionSentAsync(
-        txData: Web3.TxData,
+        txData: MaybeFakeTxData,
         err: Error | null,
-        txHash?: string,
-        cb?: Callback,
+        txHash: string | undefined,
+        cb: Callback,
     ): Promise<void> {
+        if (!txData.isFakeTransaction) {
+            // This transaction is a usual ttransaction. Not a call executed as one.
+            // And we don't want it to be executed within a snapshotting period
+            await this._lock.acquire();
+        }
         if (_.isNull(err)) {
             await this._recordTxTraceAsync(txData.to || constants.NEW_CONTRACT, txData.data, txHash as string);
         } else {
@@ -72,9 +88,12 @@ export class CoverageSubprovider extends Subprovider {
                 );
             }
         }
-        if (!_.isUndefined(cb)) {
-            cb();
+        if (!txData.isFakeTransaction) {
+            // This transaction is a usual ttransaction. Not a call executed as one.
+            // And we don't want it to be executed within a snapshotting period
+            await this._lock.release();
         }
+        cb();
     }
     private async _onCallExecutedAsync(
         callData: Partial<Web3.CallData>,
@@ -115,22 +134,25 @@ export class CoverageSubprovider extends Subprovider {
         }
     }
     private async _recordCallTraceAsync(callData: Partial<Web3.CallData>, blockNumber: Web3.BlockParam): Promise<void> {
+        // We don't want other transactions to be exeucted during snashotting period, that's why we lock the
+        // transaction execution for all transactions except our fake ones.
+        await this._lock.acquire();
         const snapshotId = Number((await this.emitPayloadAsync({ method: 'evm_snapshot' })).result);
-        const txData = callData;
-        if (_.isUndefined(txData.from)) {
-            txData.from = this._defaultFromAddress;
-        }
-        const txDataWithFromAddress = txData as Web3.TxData;
+        const fakeTxData: MaybeFakeTxData = {
+            from: this._defaultFromAddress,
+            isFakeTransaction: true, // This transaction (and only it) is allowed to come through when the lock is locked
+            ...callData,
+        };
         try {
-            const txHash = (await this.emitPayloadAsync({
+            await this.emitPayloadAsync({
                 method: 'eth_sendTransaction',
-                params: [txDataWithFromAddress],
-            })).result;
-            await this._onTransactionSentAsync(txDataWithFromAddress, null, txHash);
+                params: [fakeTxData],
+            });
         } catch (err) {
-            await this._onTransactionSentAsync(txDataWithFromAddress, err, undefined);
+            // Even if this transaction failed - we've already recorded it's trace.
         }
         const jsonRPCResponse = await this.emitPayloadAsync({ method: 'evm_revert', params: [snapshotId] });
+        await this._lock.release();
         const didRevert = jsonRPCResponse.result;
         if (!didRevert) {
             throw new Error('Failed to revert the snapshot');
diff --git a/yarn.lock b/yarn.lock
index e3dcde420..bcb436056 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -711,7 +711,7 @@ async-eventemitter@^0.2.2:
   dependencies:
     async "^2.4.0"
 
-"async-eventemitter@github:ahultgren/async-eventemitter#fa06e39e56786ba541c180061dbf2c0a5bbf951c":
+async-eventemitter@ahultgren/async-eventemitter#fa06e39e56786ba541c180061dbf2c0a5bbf951c:
   version "0.2.3"
   resolved "https://codeload.github.com/ahultgren/async-eventemitter/tar.gz/fa06e39e56786ba541c180061dbf2c0a5bbf951c"
   dependencies:
@@ -1493,13 +1493,13 @@ big.js@^3.1.3:
   version "3.2.0"
   resolved "https://registry.yarnpkg.com/big.js/-/big.js-3.2.0.tgz#a5fc298b81b9e0dca2e458824784b65c52ba588e"
 
-"bignumber.js@git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2":
+"bignumber.js@git+https://github.com/debris/bignumber.js#master":
   version "2.0.7"
-  resolved "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2"
+  resolved "git+https://github.com/debris/bignumber.js#c7a38de919ed75e6fb6ba38051986e294b328df9"
 
-"bignumber.js@git+https://github.com/debris/bignumber.js.git#master":
+"bignumber.js@git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2":
   version "2.0.7"
-  resolved "git+https://github.com/debris/bignumber.js.git#c7a38de919ed75e6fb6ba38051986e294b328df9"
+  resolved "git+https://github.com/debris/bignumber.js.git#94d7146671b9719e00a09c29b01a691bc85048c2"
 
 "bignumber.js@git+https://github.com/frozeman/bignumber.js-nolookahead.git":
   version "2.0.7"