From 83c37c6a7a320326975c8afd9d49a42c9afcefd4 Mon Sep 17 00:00:00 2001 From: Leonid Logvinov Date: Tue, 22 May 2018 11:05:24 -0700 Subject: Address feedback --- packages/sol-cov/src/coverage_manager.ts | 4 +-- packages/sol-cov/src/trace.ts | 27 ++++++------------- .../sol-cov/test/collect_contracts_data_test.ts | 30 ---------------------- .../test/sol_compiler_artifact_adapter_test.ts | 30 ++++++++++++++++++++++ packages/sol-resolver/CHANGELOG.json | 4 +-- .../subproviders/src/utils/subprovider_utils.ts | 2 ++ packages/utils/package.json | 1 + packages/utils/src/abi_decoder.ts | 13 ++-------- packages/utils/src/address_utils.ts | 5 ++++ 9 files changed, 52 insertions(+), 64 deletions(-) delete mode 100644 packages/sol-cov/test/collect_contracts_data_test.ts create mode 100644 packages/sol-cov/test/sol_compiler_artifact_adapter_test.ts diff --git a/packages/sol-cov/src/coverage_manager.ts b/packages/sol-cov/src/coverage_manager.ts index 3aa62acff..ef893527a 100644 --- a/packages/sol-cov/src/coverage_manager.ts +++ b/packages/sol-cov/src/coverage_manager.ts @@ -139,12 +139,12 @@ export class CoverageManager { constructor( artifactAdapter: AbstractArtifactAdapter, getContractCodeAsync: (address: string) => Promise, - verbose: boolean, + isVerbose: boolean, ) { this._getContractCodeAsync = getContractCodeAsync; this._artifactAdapter = artifactAdapter; this._logger = getLogger('sol-cov'); - this._logger.setLevel(verbose ? levels.TRACE : levels.ERROR); + this._logger.setLevel(isVerbose ? levels.TRACE : levels.ERROR); } public appendTraceInfo(traceInfo: TraceInfo): void { this._traceInfos.push(traceInfo); diff --git a/packages/sol-cov/src/trace.ts b/packages/sol-cov/src/trace.ts index 30508898b..81c8bb0ff 100644 --- a/packages/sol-cov/src/trace.ts +++ b/packages/sol-cov/src/trace.ts @@ -1,5 +1,5 @@ import { OpCode, StructLog, TransactionTrace } from '@0xproject/types'; -import { BigNumber, logUtils } from '@0xproject/utils'; +import { addressUtils, BigNumber, logUtils } from '@0xproject/utils'; import { addHexPrefix, stripHexPrefix } from 'ethereumjs-util'; import * as fs from 'fs'; import * as _ from 'lodash'; @@ -7,17 +7,13 @@ import * as _ from 'lodash'; export interface TraceByContractAddress { [contractAddress: string]: StructLog[]; } -function padZeros(address: string): string { - return addHexPrefix(_.padStart(stripHexPrefix(address), 40, '0')); -} export function getTracesByContractAddress(structLogs: StructLog[], startAddress: string): TraceByContractAddress { const traceByContractAddress: TraceByContractAddress = {}; let currentTraceSegment = []; const callStack = [startAddress]; - // tslint:disable-next-line: prefer-for-of - for (let i = 0; i < structLogs.length; ++i) { - const structLog = structLogs[i]; + // tslint:disable-next-line:prefer-for-of + for (const structLog of structLogs) { if (structLog.depth !== callStack.length - 1) { throw new Error("Malformed trace. trace depth doesn't match call stack depth"); } @@ -26,26 +22,19 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress // That means that we can always safely pop from it currentTraceSegment.push(structLog); - if ( - structLog.op === OpCode.CallCode || - structLog.op === OpCode.StaticCall || - structLog.op === OpCode.Call || - structLog.op === OpCode.DelegateCall - ) { + if (_.includes([OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall], structLog.op)) { const currentAddress = _.last(callStack) as string; const jumpAddressOffset = structLog.op === OpCode.DelegateCall ? 4 : 2; - const newAddress = padZeros(new BigNumber(addHexPrefix(structLog.stack[jumpAddressOffset])).toString(16)); + const newAddress = addressUtils.padZeros( + new BigNumber(addHexPrefix(structLog.stack[jumpAddressOffset])).toString(16), + ); callStack.push(newAddress); traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( currentTraceSegment, ); currentTraceSegment = []; } else if ( - structLog.op === OpCode.Return || - structLog.op === OpCode.Stop || - structLog.op === OpCode.Revert || - structLog.op === OpCode.Invalid || - structLog.op === OpCode.SelfDestruct + _.includes([OpCode.Return, OpCode.Stop, OpCode.Revert, OpCode.Invalid, OpCode.SelfDestruct], structLog.op) ) { const currentAddress = callStack.pop() as string; traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat( diff --git a/packages/sol-cov/test/collect_contracts_data_test.ts b/packages/sol-cov/test/collect_contracts_data_test.ts deleted file mode 100644 index d423bc603..000000000 --- a/packages/sol-cov/test/collect_contracts_data_test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import * as chai from 'chai'; -import * as _ from 'lodash'; -import 'make-promises-safe'; -import 'mocha'; -import * as path from 'path'; - -import { SolCompilerArtifactAdapter } from '../src/artifact_adapters/sol_compiler_artifact_adapter'; - -const expect = chai.expect; - -describe('Collect contracts data', () => { - describe('#collectContractsData', () => { - it('correctly collects contracts data', async () => { - const artifactsPath = path.resolve(__dirname, 'fixtures/artifacts'); - const sourcesPath = path.resolve(__dirname, 'fixtures/contracts'); - const zeroExArtifactsAdapter = new SolCompilerArtifactAdapter(artifactsPath, sourcesPath); - const contractsData = await zeroExArtifactsAdapter.collectContractsDataAsync(); - _.forEach(contractsData, contractData => { - expect(contractData).to.have.keys([ - 'sourceCodes', - 'sources', - 'sourceMap', - 'sourceMapRuntime', - 'bytecode', - 'runtimeBytecode', - ]); - }); - }); - }); -}); diff --git a/packages/sol-cov/test/sol_compiler_artifact_adapter_test.ts b/packages/sol-cov/test/sol_compiler_artifact_adapter_test.ts new file mode 100644 index 000000000..0ebad669b --- /dev/null +++ b/packages/sol-cov/test/sol_compiler_artifact_adapter_test.ts @@ -0,0 +1,30 @@ +import * as chai from 'chai'; +import * as _ from 'lodash'; +import 'make-promises-safe'; +import 'mocha'; +import * as path from 'path'; + +import { SolCompilerArtifactAdapter } from '../src/artifact_adapters/sol_compiler_artifact_adapter'; + +const expect = chai.expect; + +describe('SolCompilerArtifactAdapter', () => { + describe('#collectContractsData', () => { + it('correctly collects contracts data', async () => { + const artifactsPath = path.resolve(__dirname, 'fixtures/artifacts'); + const sourcesPath = path.resolve(__dirname, 'fixtures/contracts'); + const zeroExArtifactsAdapter = new SolCompilerArtifactAdapter(artifactsPath, sourcesPath); + const contractsData = await zeroExArtifactsAdapter.collectContractsDataAsync(); + _.forEach(contractsData, contractData => { + expect(contractData).to.have.keys([ + 'sourceCodes', + 'sources', + 'sourceMap', + 'sourceMapRuntime', + 'bytecode', + 'runtimeBytecode', + ]); + }); + }); + }); +}); diff --git a/packages/sol-resolver/CHANGELOG.json b/packages/sol-resolver/CHANGELOG.json index 6d5917212..895bd2104 100644 --- a/packages/sol-resolver/CHANGELOG.json +++ b/packages/sol-resolver/CHANGELOG.json @@ -3,11 +3,11 @@ "version": "0.0.5", "changes": [ { - "note": "Fix a bug in FsResolver trying to read directories as files", + "note": "Fix a bug in FsResolver where it tries to read directories as files", "pr": 589 }, { - "note": "Fix a bug in NameResolver not ignoring .sol files", + "note": "Fix a bug in NameResolver where it is not ignoring .sol files", "pr": 589 } ] diff --git a/packages/subproviders/src/utils/subprovider_utils.ts b/packages/subproviders/src/utils/subprovider_utils.ts index 380f98606..24ebedd06 100644 --- a/packages/subproviders/src/utils/subprovider_utils.ts +++ b/packages/subproviders/src/utils/subprovider_utils.ts @@ -9,5 +9,7 @@ import { Subprovider } from '../subproviders/subprovider'; */ export function prependSubprovider(provider: ProviderEngine, subprovider: Subprovider): void { subprovider.setEngine(provider); + // HACK: We use implementation details of provider engine here + // https://github.com/MetaMask/provider-engine/blob/master/index.js#L68 (provider as any)._providers = [subprovider, ...(provider as any)._providers]; } diff --git a/packages/utils/package.json b/packages/utils/package.json index 99d3d9256..24551dd93 100644 --- a/packages/utils/package.json +++ b/packages/utils/package.json @@ -38,6 +38,7 @@ "@0xproject/types": "^0.7.0", "@0xproject/typescript-typings": "^0.3.2", "@types/node": "^8.0.53", + "ethereumjs-util": "^5.1.1", "bignumber.js": "~4.1.0", "ethers": "^3.0.15", "js-sha3": "^0.7.0", diff --git a/packages/utils/src/abi_decoder.ts b/packages/utils/src/abi_decoder.ts index c78bfa343..d2d8364ca 100644 --- a/packages/utils/src/abi_decoder.ts +++ b/packages/utils/src/abi_decoder.ts @@ -12,21 +12,12 @@ import { import * as ethers from 'ethers'; import * as _ from 'lodash'; +import { addressUtils } from './address_utils'; import { BigNumber } from './configured_bignumber'; export class AbiDecoder { private _savedABIs: AbiDefinition[] = []; private _methodIds: { [signatureHash: string]: EventAbi } = {}; - private static _padZeros(address: string): string { - let formatted = address; - if (_.startsWith(formatted, '0x')) { - formatted = formatted.slice(2); - } - - const addressLength = 40; - formatted = _.padStart(formatted, addressLength, '0'); - return `0x${formatted}`; - } constructor(abiArrays: AbiDefinition[][]) { _.forEach(abiArrays, this.addABI.bind(this)); } @@ -56,7 +47,7 @@ export class AbiDecoder { } if (param.type === SolidityTypes.Address) { const baseHex = 16; - value = AbiDecoder._padZeros(new BigNumber(value).toString(baseHex)); + value = addressUtils.padZeros(new BigNumber(value).toString(baseHex)); } else if (param.type === SolidityTypes.Uint256 || param.type === SolidityTypes.Uint) { value = new BigNumber(value); } else if (param.type === SolidityTypes.Uint8) { diff --git a/packages/utils/src/address_utils.ts b/packages/utils/src/address_utils.ts index e25bde249..cc43bd477 100644 --- a/packages/utils/src/address_utils.ts +++ b/packages/utils/src/address_utils.ts @@ -1,4 +1,6 @@ +import { addHexPrefix, stripHexPrefix } from 'ethereumjs-util'; import * as jsSHA3 from 'js-sha3'; +import * as _ from 'lodash'; const BASIC_ADDRESS_REGEX = /^(0x)?[0-9a-f]{40}$/i; const SAME_CASE_ADDRESS_REGEX = /^(0x)?([0-9a-f]{40}|[0-9A-F]{40})$/; @@ -38,4 +40,7 @@ export const addressUtils = { return isValidChecksummedAddress; } }, + padZeros(address: string): string { + return addHexPrefix(_.padStart(stripHexPrefix(address), 40, '0')); + }, }; -- cgit v1.2.3