Merge pull request #938 from 0xProject/sol-cov-fixes

Sol cov fixes

10 files changed, 162 insertions, 22 deletions

diff --git a/packages/json-schemas/CHANGELOG.json b/packages/json-schemas/CHANGELOG.json
index 6017b2f2b..5af23dbe9 100644
--- a/packages/json-schemas/CHANGELOG.json
+++ b/packages/json-schemas/CHANGELOG.json
@@ -3,6 +3,10 @@
         "version": "1.0.1-rc.4",
         "changes": [
             {
+                "note": "Allow for additional properties in txData schema",
+                "pr": 938
+            },
+            {
                 "note": "Change hexSchema to match `0x`",
                 "pr": 937
             },
diff --git a/packages/json-schemas/schemas/tx_data_schema.ts b/packages/json-schemas/schemas/tx_data_schema.ts
index 41a5c708a..c57e18461 100644
--- a/packages/json-schemas/schemas/tx_data_schema.ts
+++ b/packages/json-schemas/schemas/tx_data_schema.ts
@@ -29,5 +29,4 @@ export const txDataSchema = {
     },
     required: ['from'],
     type: 'object',
-    additionalProperties: false,
 };
diff --git a/packages/json-schemas/test/schema_test.ts b/packages/json-schemas/test/schema_test.ts
index 39cdec8a2..4e0f66ef5 100644
--- a/packages/json-schemas/test/schema_test.ts
+++ b/packages/json-schemas/test/schema_test.ts
@@ -849,10 +849,6 @@ describe('Schema', () => {
                 {
                     gas: new BigNumber(42),
                 },
-                {
-                    from: NULL_ADDRESS,
-                    unknownProp: 'here',
-                },
                 {},
                 [],
                 new BigNumber(1),
diff --git a/packages/sol-cov/CHANGELOG.json b/packages/sol-cov/CHANGELOG.json
index 5a1fdc712..1a0fe43ca 100644
--- a/packages/sol-cov/CHANGELOG.json
+++ b/packages/sol-cov/CHANGELOG.json
@@ -1,5 +1,29 @@
 [
     {
+        "version": "2.0.0",
+        "changes": [
+            {
+                "note":
+                    "Fix a bug when eth_call coverage was not computed because of silent schema validation failures",
+                "pr": 938
+            },
+            {
+                "note": "Make `TruffleArtifactAdapter` read the `truffle.js` config for `solc` settings",
+                "pr": 938
+            },
+            {
+                "note":
+                    "Change the first param of `TruffleArtifactAdapter` to be the `projectRoot` instead of `sourcesDir`",
+                "pr": 938
+            },
+            {
+                "note":
+                    "Throw a helpful error message if truffle artifacts were generated with a different solc version than the one passed in",
+                "pr": 938
+            }
+        ]
+    },
+    {
         "version": "1.0.3",
         "changes": [
             {
diff --git a/packages/sol-cov/src/artifact_adapters/truffle_artifact_adapter.ts b/packages/sol-cov/src/artifact_adapters/truffle_artifact_adapter.ts
index 53b77aed5..2706435cc 100644
--- a/packages/sol-cov/src/artifact_adapters/truffle_artifact_adapter.ts
+++ b/packages/sol-cov/src/artifact_adapters/truffle_artifact_adapter.ts
@@ -1,25 +1,40 @@
 import { Compiler, CompilerOptions } from '@0xproject/sol-compiler';
-import * as rimraf from 'rimraf';
+import * as fs from 'fs';
+import * as glob from 'glob';
+import * as path from 'path';
 
 import { ContractData } from '../types';
 
 import { AbstractArtifactAdapter } from './abstract_artifact_adapter';
 import { SolCompilerArtifactAdapter } from './sol_compiler_artifact_adapter';
 
+const DEFAULT_TRUFFLE_ARTIFACTS_DIR = './build/contracts';
+
+interface TruffleConfig {
+    solc?: any;
+    contracts_build_directory?: string;
+}
+
 export class TruffleArtifactAdapter extends AbstractArtifactAdapter {
     private readonly _solcVersion: string;
-    private readonly _sourcesPath: string;
-    constructor(sourcesPath: string, solcVersion: string) {
+    private readonly _projectRoot: string;
+    constructor(projectRoot: string, solcVersion: string) {
         super();
         this._solcVersion = solcVersion;
-        this._sourcesPath = sourcesPath;
+        this._projectRoot = projectRoot;
     }
     public async collectContractsDataAsync(): Promise<ContractData[]> {
         const artifactsDir = '.0x-artifacts';
+        const contractsDir = path.join(this._projectRoot, 'contracts');
+        const truffleConfig = this._getTruffleConfig();
+        const solcConfig = truffleConfig.solc || {};
+        const truffleArtifactsDirectory = truffleConfig.contracts_build_directory || DEFAULT_TRUFFLE_ARTIFACTS_DIR;
+        this._assertSolidityVersionIsCorrect(truffleArtifactsDirectory);
         const compilerOptions: CompilerOptions = {
-            contractsDir: this._sourcesPath,
+            contractsDir,
             artifactsDir,
             compilerSettings: {
+                ...solcConfig,
                 outputSelection: {
                     ['*']: {
                         ['*']: ['abi', 'evm.bytecode.object', 'evm.deployedBytecode.object'],
@@ -31,9 +46,38 @@ export class TruffleArtifactAdapter extends AbstractArtifactAdapter {
         };
         const compiler = new Compiler(compilerOptions);
         await compiler.compileAsync();
-        const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(artifactsDir, this._sourcesPath);
+        const solCompilerArtifactAdapter = new SolCompilerArtifactAdapter(artifactsDir, contractsDir);
         const contractsDataFrom0xArtifacts = await solCompilerArtifactAdapter.collectContractsDataAsync();
-        rimraf.sync(artifactsDir);
         return contractsDataFrom0xArtifacts;
     }
+    private _getTruffleConfig(): TruffleConfig {
+        const truffleConfigFileShort = path.resolve(path.join(this._projectRoot, 'truffle.js'));
+        const truffleConfigFileLong = path.resolve(path.join(this._projectRoot, 'truffle-config.js'));
+        if (fs.existsSync(truffleConfigFileShort)) {
+            const truffleConfig = require(truffleConfigFileShort);
+            return truffleConfig;
+        } else if (fs.existsSync(truffleConfigFileLong)) {
+            const truffleConfig = require(truffleConfigFileLong);
+            return truffleConfig;
+        } else {
+            throw new Error(
+                `Neither ${truffleConfigFileShort} nor ${truffleConfigFileLong} exists. Make sure the project root is correct`,
+            );
+        }
+    }
+    private _assertSolidityVersionIsCorrect(truffleArtifactsDirectory: string): void {
+        const artifactsGlob = `${truffleArtifactsDirectory}/**/*.json`;
+        const artifactFileNames = glob.sync(artifactsGlob, { absolute: true });
+        for (const artifactFileName of artifactFileNames) {
+            const artifact = JSON.parse(fs.readFileSync(artifactFileName).toString());
+            const compilerVersion = artifact.compiler.version;
+            if (!compilerVersion.startsWith(this._solcVersion)) {
+                throw new Error(
+                    `${artifact.contractName} was compiled with solidity ${compilerVersion} but specified version is ${
+                        this._solcVersion
+                    } making it impossible for sol-cov to process traces`,
+                );
+            }
+        }
+    }
 }
diff --git a/packages/sol-cov/src/trace_collection_subprovider.ts b/packages/sol-cov/src/trace_collection_subprovider.ts
index b530b59db..b6486e6f7 100644
--- a/packages/sol-cov/src/trace_collection_subprovider.ts
+++ b/packages/sol-cov/src/trace_collection_subprovider.ts
@@ -1,7 +1,7 @@
 import { BlockchainLifecycle } from '@0xproject/dev-utils';
 import { Callback, ErrorCallback, NextCallback, Subprovider } from '@0xproject/subproviders';
-import { Web3Wrapper } from '@0xproject/web3-wrapper';
-import { CallData, JSONRPCRequestPayload, Provider, TxData } from 'ethereum-types';
+import { CallDataRPC, marshaller, Web3Wrapper } from '@0xproject/web3-wrapper';
+import { JSONRPCRequestPayload, Provider, TxData } from 'ethereum-types';
 import * as _ from 'lodash';
 import { Lock } from 'semaphore-async-await';
 
@@ -152,7 +152,7 @@ export abstract class TraceCollectionSubprovider extends Subprovider {
         cb();
     }
     private async _onCallOrGasEstimateExecutedAsync(
-        callData: Partial<CallData>,
+        callData: Partial<CallDataRPC>,
         _err: Error | null,
         _callResult: string,
         cb: Callback,
@@ -160,22 +160,24 @@ export abstract class TraceCollectionSubprovider extends Subprovider {
         await this._recordCallOrGasEstimateTraceAsync(callData);
         cb();
     }
-    private async _recordCallOrGasEstimateTraceAsync(callData: Partial<CallData>): Promise<void> {
+    private async _recordCallOrGasEstimateTraceAsync(callData: Partial<CallDataRPC>): 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 blockchainLifecycle = new BlockchainLifecycle(this._web3Wrapper);
         await blockchainLifecycle.startAsync();
-        const fakeTxData: MaybeFakeTxData = {
-            gas: BLOCK_GAS_LIMIT,
+        const fakeTxData = {
+            gas: BLOCK_GAS_LIMIT.toString(16), // tslint:disable-line:custom-no-magic-numbers
             isFakeTransaction: true, // This transaction (and only it) is allowed to come through when the lock is locked
             ...callData,
             from: callData.from || this._defaultFromAddress,
         };
         try {
-            const txHash = await this._web3Wrapper.sendTransactionAsync(fakeTxData);
+            const txData = marshaller.unmarshalTxData(fakeTxData);
+            const txHash = await this._web3Wrapper.sendTransactionAsync(txData);
             await this._web3Wrapper.awaitTransactionMinedAsync(txHash, 0);
         } catch (err) {
+            // TODO(logvinov) Check that transaction failed and not some other exception
             // Even if this transaction failed - we've already recorded it's trace.
             _.noop();
         }
diff --git a/packages/subproviders/src/subproviders/signer.ts b/packages/subproviders/src/subproviders/signer.ts
index 8c4e89caf..d5fd86897 100644
--- a/packages/subproviders/src/subproviders/signer.ts
+++ b/packages/subproviders/src/subproviders/signer.ts
@@ -1,5 +1,4 @@
-import { Web3Wrapper } from '@0xproject/web3-wrapper';
-import { marshaller } from '@0xproject/web3-wrapper/lib/src/marshaller';
+import { marshaller, Web3Wrapper } from '@0xproject/web3-wrapper';
 import { JSONRPCRequestPayload, Provider } from 'ethereum-types';
 
 import { Callback, ErrorCallback } from '../types';
diff --git a/packages/web3-wrapper/CHANGELOG.json b/packages/web3-wrapper/CHANGELOG.json
index 7bf28dfa8..9224d89b0 100644
--- a/packages/web3-wrapper/CHANGELOG.json
+++ b/packages/web3-wrapper/CHANGELOG.json
@@ -1,5 +1,18 @@
 [
     {
+        "version": "1.2.0",
+        "changes": [
+            {
+                "note": "Export marshaller to convert between RPC and user-space data formats",
+                "pr": 938
+            },
+            {
+                "note": "Export RPC types",
+                "pr": 938
+            }
+        ]
+    },
+    {
         "timestamp": 1532619515,
         "version": "1.1.2",
         "changes": [
diff --git a/packages/web3-wrapper/src/index.ts b/packages/web3-wrapper/src/index.ts
index 6787e0102..5d3d135e4 100644
--- a/packages/web3-wrapper/src/index.ts
+++ b/packages/web3-wrapper/src/index.ts
@@ -1,2 +1,13 @@
 export { Web3Wrapper } from './web3_wrapper';
-export { Web3WrapperErrors, NodeType } from './types';
+export { marshaller } from './marshaller';
+export {
+    Web3WrapperErrors,
+    NodeType,
+    CallDataRPC,
+    CallTxDataBaseRPC,
+    AbstractBlockRPC,
+    BlockWithoutTransactionDataRPC,
+    BlockWithTransactionDataRPC,
+    TransactionRPC,
+    TxDataRPC,
+} from './types';
diff --git a/packages/web3-wrapper/src/marshaller.ts b/packages/web3-wrapper/src/marshaller.ts
index fed197822..572a322d6 100644
--- a/packages/web3-wrapper/src/marshaller.ts
+++ b/packages/web3-wrapper/src/marshaller.ts
@@ -25,7 +25,15 @@ import {
     TxDataRPC,
 } from './types';
 
+/**
+ * Utils to convert ethereum structures from user-space format to RPC format. (marshall/unmarshall)
+ */
 export const marshaller = {
+    /**
+     * Unmarshall block without transaction data
+     * @param blockWithHexValues block to unmarshall
+     * @return unmarshalled block without transaction data
+     */
     unmarshalIntoBlockWithoutTransactionData(
         blockWithHexValues: BlockWithoutTransactionDataRPC,
     ): BlockWithoutTransactionData {
@@ -41,6 +49,11 @@ export const marshaller = {
         };
         return block;
     },
+    /**
+     * Unmarshall block with transaction data
+     * @param blockWithHexValues block to unmarshall
+     * @return unmarshalled block with transaction data
+     */
     unmarshalIntoBlockWithTransactionData(blockWithHexValues: BlockWithTransactionDataRPC): BlockWithTransactionData {
         const block = {
             ...blockWithHexValues,
@@ -59,6 +72,11 @@ export const marshaller = {
         });
         return block;
     },
+    /**
+     * Unmarshall transaction
+     * @param txRpc transaction to unmarshall
+     * @return unmarshalled transaction
+     */
     unmarshalTransaction(txRpc: TransactionRPC): Transaction {
         const tx = {
             ...txRpc,
@@ -73,6 +91,11 @@ export const marshaller = {
         };
         return tx;
     },
+    /**
+     * Unmarshall transaction data
+     * @param txDataRpc transaction data to unmarshall
+     * @return unmarshalled transaction data
+     */
     unmarshalTxData(txDataRpc: TxDataRPC): TxData {
         if (_.isUndefined(txDataRpc.from)) {
             throw new Error(`txData must include valid 'from' value.`);
@@ -86,6 +109,11 @@ export const marshaller = {
         };
         return txData;
     },
+    /**
+     * Marshall transaction data
+     * @param txData transaction data to marshall
+     * @return marshalled transaction data
+     */
     marshalTxData(txData: Partial<TxData>): Partial<TxDataRPC> {
         if (_.isUndefined(txData.from)) {
             throw new Error(`txData must include valid 'from' value.`);
@@ -107,6 +135,11 @@ export const marshaller = {
         });
         return txDataRPC;
     },
+    /**
+     * Marshall call data
+     * @param callData call data to marshall
+     * @return marshalled call data
+     */
     marshalCallData(callData: Partial<CallData>): Partial<CallDataRPC> {
         const callTxDataBase = {
             ...callData,
@@ -119,12 +152,22 @@ export const marshaller = {
         };
         return callDataRPC;
     },
+    /**
+     * Marshall address
+     * @param address address to marshall
+     * @return marshalled address
+     */
     marshalAddress(address: string): string {
         if (addressUtils.isAddress(address)) {
             return ethUtil.addHexPrefix(address);
         }
         throw new Error(`Invalid address encountered: ${address}`);
     },
+    /**
+     * Marshall block param
+     * @param blockParam block param to marshall
+     * @return marshalled block param
+     */
     marshalBlockParam(blockParam: BlockParam | string | number | undefined): string | undefined {
         if (_.isUndefined(blockParam)) {
             return BlockParamLiteral.Latest;
@@ -132,6 +175,11 @@ export const marshaller = {
         const encodedBlockParam = _.isNumber(blockParam) ? utils.numberToHex(blockParam) : blockParam;
         return encodedBlockParam;
     },
+    /**
+     * Unmarshall log
+     * @param rawLog log to unmarshall
+     * @return unmarshalled log
+     */
     unmarshalLog(rawLog: RawLogEntry): LogEntry {
         const formattedLog = {
             ...rawLog,