From 0118b1a4091eb4880980eb85605817b2875ba1be Mon Sep 17 00:00:00 2001 From: Anthony Laibe Date: Tue, 18 Dec 2018 14:51:48 +0000 Subject: [PATCH] feat(coverage): gas usage improvements Do not submit event for function and branch but detect where the statement is --- src/lib/modules/coverage/contractEnhanced.ts | 91 +++++++++++++------ src/lib/modules/coverage/eventId.ts | 23 ++--- src/lib/modules/coverage/index.ts | 2 +- src/lib/modules/coverage/injector.ts | 16 ---- src/lib/modules/coverage/instrumenter.ts | 8 +- src/lib/modules/coverage/types.ts | 2 +- .../framework/reactBuilder/index.ts | 2 +- 7 files changed, 79 insertions(+), 65 deletions(-) diff --git a/src/lib/modules/coverage/contractEnhanced.ts b/src/lib/modules/coverage/contractEnhanced.ts index cd315f905c..4d6afa78ce 100644 --- a/src/lib/modules/coverage/contractEnhanced.ts +++ b/src/lib/modules/coverage/contractEnhanced.ts @@ -1,5 +1,5 @@ import * as path from "path"; -import parser, { Location } from "solidity-parser-antlr"; +import parser, { LineColumn, Location } from "solidity-parser-antlr"; import { EventLog } from "web3/types"; import { decrypt } from "./eventId"; @@ -12,11 +12,8 @@ import { BranchType, Coverage } from "./types"; const fs = require("../../core/fs"); -enum EventEnum { - statement = "__StatementCoverage", - branch = "__BranchCoverage", - function = "__FunctionCoverage", -} +const STATEMENT_EVENT = "__StatementCoverage"; +const POINT_FACTOR = 1000000000; let id = 0; function nextId() { @@ -31,6 +28,7 @@ export class ContractEnhanced { public source: string; private ast: parser.ASTNode; private coverageFilepath: string; + private functionsBodyLocation: {[id: number]: Location} = {}; constructor(public filepath: string) { this.id = nextId(); @@ -71,29 +69,24 @@ export class ContractEnhanced { public updateCoverage(events: EventLog[]) { events.filter(this.filterCoverageEvent).forEach((event) => { const value = parseInt(event.returnValues[0], 10); - const {contractId, injectionPointId, locationIdx} = decrypt(value); + const {contractId, injectionPointId} = decrypt(value); if (contractId !== this.id) { return; } - switch (event.event) { - case "__StatementCoverage": { - this.coverage.s[injectionPointId] += 1; - const statement = this.coverage.statementMap[injectionPointId]; - this.coverage.l[statement.start.line] += 1; - break; - } - case "__FunctionCoverage": { - this.coverage.f[injectionPointId] += 1; - const fn = this.coverage.fnMap[injectionPointId]; - this.coverage.l[fn.line] += 1; - break; - } - case "__BranchCoverage": { - this.coverage.b[injectionPointId][locationIdx] += 1; - break; - } + this.coverage.s[injectionPointId] += 1; + const location = this.coverage.statementMap[injectionPointId]; + this.coverage.l[location.start.line] += 1; + + const fnMapId = this.findFnMapId(location); + if (fnMapId) { + this.coverage.f[fnMapId] += 1; + } + + const [fnBranchId, locationId] = this.findFnBranchIdAndLocationId(location); + if (fnBranchId) { + this.coverage.b[fnBranchId][locationId] += 1; } }); } @@ -118,7 +111,7 @@ export class ContractEnhanced { return coverageId; } - public addFunction(location: Location, name: string) { + public addFunction(location: Location, name: string, bodyLocation: Location) { const coverageId = this.getNewCoverageId(this.coverage.fnMap); const line = location.start.line; this.coverage.fnMap[coverageId] = { @@ -127,7 +120,7 @@ export class ContractEnhanced { name, }; this.coverage.f[coverageId] = 0; - this.coverage.l[line] = 0; + this.functionsBodyLocation[coverageId] = bodyLocation; return coverageId; } @@ -137,7 +130,51 @@ export class ContractEnhanced { } private filterCoverageEvent(event: EventLog) { - return [EventEnum.function, EventEnum.branch, EventEnum.statement].includes(event.event as EventEnum); + return STATEMENT_EVENT === event.event; + } + + private findFnMapId(location: Location) { + const result = Object.keys(this.functionsBodyLocation).find((value: string) => { + const bodyLocation = this.functionsBodyLocation[parseInt(value, 10)]; + + return this.isWithin(location.start, bodyLocation); + }); + + if (result) { + return parseInt(result, 10); + } + + return 0; + } + + private findFnBranchIdAndLocationId(location: Location) { + let fnBranchId = 0; + let locationId = 0; + Object.keys(this.coverage.branchMap).forEach((value: string) => { + if (fnBranchId && locationId) { + return; + } + + const branch = this.coverage.branchMap[parseInt(value, 10)]; + + branch.locations.forEach((branchLocation, index) => { + if (this.isWithin(location.start, branchLocation)) { + fnBranchId = parseInt(value, 10); + locationId = index; + } + }); + }); + + return [fnBranchId, locationId]; + } + + private isWithin(point: LineColumn, location: Location) { + const toCompare = this.getComparablePoint(point); + return toCompare >= this.getComparablePoint(location.start) && toCompare <= this.getComparablePoint(location.end); + } + + private getComparablePoint(point: LineColumn) { + return point.line * POINT_FACTOR + point.column; } } diff --git a/src/lib/modules/coverage/eventId.ts b/src/lib/modules/coverage/eventId.ts index 8eb5784587..6a2321dfd8 100644 --- a/src/lib/modules/coverage/eventId.ts +++ b/src/lib/modules/coverage/eventId.ts @@ -1,24 +1,22 @@ -const CONTRACT_ID_FACTOR = 100000000; -const INJECTION_POINT_ID_FACTOR = 10000; +const CONTRACT_ID_FACTOR = 1000000; /** - * Convert the 3 params as uint32 where the first 2 digits are for contractsId, - * the next 3 digit are for injectionPoint id and the rest if for the locationIds + * Convert the 2 params as uint32 where the first 4 digits are for contractsId, + * the followings one are for injectionPoint id * * @export * @param {number} contractId * @param {number} injectionPointId - * @param {number} [locationIdx] - * @returns a number representing the 3 params as uint32 + * @returns a number representing the 2 params as uint32 */ -export function encrypt(contractId: number, injectionPointId: number, locationIdx?: number) { - return contractId * CONTRACT_ID_FACTOR + injectionPointId * INJECTION_POINT_ID_FACTOR + (locationIdx || 0); +export function encrypt(contractId: number, injectionPointId: number) { + return contractId * CONTRACT_ID_FACTOR + injectionPointId; } /** * Explore the uint32 into contractId, injectionPointId and locationIds where - * the first 2 digits are for contractsId, - * the next 3 digit are for injectionPoint id and the rest if for the locationIds + * the first 4 digits are for contractsId, + * the rest are for injectionPoint id * * @export * @param {number} value @@ -26,8 +24,7 @@ export function encrypt(contractId: number, injectionPointId: number, locationId */ export function decrypt(value: number) { const contractId = Math.floor(value / CONTRACT_ID_FACTOR); - const injectionPointId = Math.floor(value / INJECTION_POINT_ID_FACTOR) - contractId * INJECTION_POINT_ID_FACTOR; - const locationIdx = value - contractId * CONTRACT_ID_FACTOR - injectionPointId * INJECTION_POINT_ID_FACTOR; + const injectionPointId = value - contractId * CONTRACT_ID_FACTOR; - return {contractId, injectionPointId, locationIdx}; + return {contractId, injectionPointId}; } diff --git a/src/lib/modules/coverage/index.ts b/src/lib/modules/coverage/index.ts index 29a4e54334..6dd5423f16 100644 --- a/src/lib/modules/coverage/index.ts +++ b/src/lib/modules/coverage/index.ts @@ -1,6 +1,6 @@ import * as globule from "globule"; import * as path from "path"; -import { Contract as Web3Contract } from "web3/types"; +import Web3Contract from "web3/eth/contract"; import { Contract } from "../../../typings/contract"; import { Embark } from "../../../typings/embark"; diff --git a/src/lib/modules/coverage/injector.ts b/src/lib/modules/coverage/injector.ts index 45e3aa5a3a..924a2ce279 100644 --- a/src/lib/modules/coverage/injector.ts +++ b/src/lib/modules/coverage/injector.ts @@ -11,10 +11,6 @@ export class Injector { switch (injectionPoint.type) { case "statement": return this.statement(injectionPoint); - case "function": - return this.function(injectionPoint); - case "branch": - return this.branch(injectionPoint); case "contractDefinition": return this.contractDefinition(injectionPoint); } @@ -25,21 +21,9 @@ export class Injector { this.insertAt(injectionPoint.location.start.line - 1, data); } - private function(injectionPoint: InjectionPoint) { - const data = `emit __FunctionCoverage(${encrypt(this.contract.id, injectionPoint.id)});`; - this.insertAt(injectionPoint.location.start.line, data); - } - - private branch(injectionPoint: InjectionPoint) { - const data = `emit __BranchCoverage(${encrypt(this.contract.id, injectionPoint.id, injectionPoint.locationIdx)});`; - this.insertAt(injectionPoint.location.start.line, data); - } - private contractDefinition(injectionPoint: InjectionPoint) { const data = [ - "event __FunctionCoverage(uint32 value);", "event __StatementCoverage(uint32 value);", - "event __BranchCoverage(uint32 value);", ].join("\n"); this.insertAt(injectionPoint.location.start.line, data); diff --git a/src/lib/modules/coverage/instrumenter.ts b/src/lib/modules/coverage/instrumenter.ts index 4d4e1eeafe..2126a8ee28 100644 --- a/src/lib/modules/coverage/instrumenter.ts +++ b/src/lib/modules/coverage/instrumenter.ts @@ -44,8 +44,7 @@ export class Instrumenter { return; } - const id = this.contract.addFunction(node.loc, node.name); - this.addInjectionPoints("function", id, node.body.loc); + this.contract.addFunction(node.loc, node.name, node.body.loc); } public instrumentStatement(node: Statement) { @@ -69,10 +68,7 @@ export class Instrumenter { return acc; }, []); - const id = this.contract.addBranch(node.loc.start.line, "if", locations); - locations.forEach((location, index) => { - this.addInjectionPoints("branch", id, location, index); - }); + this.contract.addBranch(node.loc.start.line, "if", locations); } private addInjectionPoints(type: InjectionPointType, id: number, location: Location, locationIdx?: number) { diff --git a/src/lib/modules/coverage/types.ts b/src/lib/modules/coverage/types.ts index 097d90d047..247967cde4 100644 --- a/src/lib/modules/coverage/types.ts +++ b/src/lib/modules/coverage/types.ts @@ -1,6 +1,6 @@ import { Location } from "solidity-parser-antlr"; -export type InjectionPointType = "statement" | "branch" | "function" | "contractDefinition"; +export type InjectionPointType = "statement" | "contractDefinition"; export type BranchType = "if" | "switch"; export interface InjectionPoint { diff --git a/src/lib/modules/scaffolding/framework/reactBuilder/index.ts b/src/lib/modules/scaffolding/framework/reactBuilder/index.ts index 42b91d0194..41b5fea0b5 100644 --- a/src/lib/modules/scaffolding/framework/reactBuilder/index.ts +++ b/src/lib/modules/scaffolding/framework/reactBuilder/index.ts @@ -1,6 +1,6 @@ import Handlebars from "handlebars"; import * as path from "path"; -import { ABIDefinition } from "web3/types"; +import { ABIDefinition } from "web3/eth/abi"; import { Contract } from "../../../../../typings/contract"; import { Embark } from "../../../../../typings/embark";