Address feedback

This commit is contained in:
Leonid Logvinov
2018-05-22 17:41:48 -07:00
parent d49f2c40ae
commit ebc750d5bf
5 changed files with 39 additions and 27 deletions

View File

@@ -3,7 +3,7 @@
"version": "0.4.2",
"changes": [
{
"note": "Pass ZeroExArtifactsAdapter to CoverageSubprovider",
"note": "Pass SolCompilerArtifactAdapter to CoverageSubprovider",
"pr": 589
},
{

View File

@@ -1,6 +1,6 @@
// tslint:disable:number-literal-format
export const constants = {
NEW_CONTRACT: 'NEW_CONTRACT',
NEW_CONTRACT: 'NEW_CONTRACT' as 'NEW_CONTRACT',
PUSH1: 0x60,
PUSH2: 0x61,
PUSH32: 0x7f,

View File

@@ -125,13 +125,15 @@ export class CoverageManager {
}
private static _getContractDataIfExists(contractsData: ContractData[], bytecode: string): ContractData | undefined {
if (!bytecode.startsWith('0x')) {
throw new Error('0x missing');
throw new Error(`0x hex prefix missing: ${bytecode}`);
}
const contractData = _.find(contractsData, contractDataCandidate => {
const bytecodeRegex = CoverageManager._bytecodeToBytecodeRegex(contractDataCandidate.bytecode);
const runtimeBytecodeRegex = CoverageManager._bytecodeToBytecodeRegex(
contractDataCandidate.runtimeBytecode,
);
// We use that function to find by bytecode or runtimeBytecode. Those are quasi-random strings so
// collisions are practically impossible and it allows us to reuse that code
return !_.isNull(bytecode.match(bytecodeRegex)) || !_.isNull(bytecode.match(runtimeBytecodeRegex));
});
return contractData;

View File

@@ -31,13 +31,13 @@ export class CoverageSubprovider extends Subprovider {
* Instantiates a CoverageSubprovider instance
* @param artifactAdapter Adapter for used artifacts format (0x, truffle, giveth, etc.)
* @param defaultFromAddress default from address to use when sending transactions
* @param verbose If true, we will log any unknown transactions. Otherwise we will ignore them
* @param isVerbose If true, we will log any unknown transactions. Otherwise we will ignore them
*/
constructor(artifactAdapter: AbstractArtifactAdapter, defaultFromAddress: string, verbose: boolean = true) {
constructor(artifactAdapter: AbstractArtifactAdapter, defaultFromAddress: string, isVerbose: boolean = true) {
super();
this._lock = new Lock();
this._defaultFromAddress = defaultFromAddress;
this._coverageManager = new CoverageManager(artifactAdapter, this._getContractCodeAsync.bind(this), verbose);
this._coverageManager = new CoverageManager(artifactAdapter, this._getContractCodeAsync.bind(this), isVerbose);
}
/**
* Write the test coverage results to a file in Istanbul format.
@@ -120,11 +120,12 @@ export class CoverageSubprovider extends Subprovider {
method: 'debug_traceTransaction',
params: [txHash, { disableMemory: true, disableStack: false, disableStorage: true }],
};
const jsonRPCResponsePayload = await this.emitPayloadAsync(payload);
let jsonRPCResponsePayload = await this.emitPayloadAsync(payload);
const trace: TransactionTrace = jsonRPCResponsePayload.result;
const tracesByContractAddress = getTracesByContractAddress(trace.structLogs, address);
const subcallAddresses = _.keys(tracesByContractAddress);
if (address === constants.NEW_CONTRACT) {
const tracesByContractAddress = getTracesByContractAddress(trace.structLogs, address);
for (const subcallAddress of _.keys(tracesByContractAddress)) {
for (const subcallAddress of subcallAddresses) {
let traceInfo: TraceInfoNewContract | TraceInfoExistingContract;
if (subcallAddress === 'NEW_CONTRACT') {
const traceForThatSubcall = tracesByContractAddress[subcallAddress];
@@ -132,12 +133,13 @@ export class CoverageSubprovider extends Subprovider {
traceInfo = {
coveredPcs,
txHash,
address: address as 'NEW_CONTRACT',
address: constants.NEW_CONTRACT,
bytecode: data as string,
};
} else {
payload = { method: 'eth_getCode', params: [subcallAddress, BlockParamLiteral.Latest] };
const runtimeBytecode = (await this.emitPayloadAsync(payload)).result;
jsonRPCResponsePayload = await this.emitPayloadAsync(payload);
const runtimeBytecode = jsonRPCResponsePayload.result;
const traceForThatSubcall = tracesByContractAddress[subcallAddress];
const coveredPcs = _.map(traceForThatSubcall, log => log.pc);
traceInfo = {
@@ -150,10 +152,10 @@ export class CoverageSubprovider extends Subprovider {
this._coverageManager.appendTraceInfo(traceInfo);
}
} else {
const tracesByContractAddress = getTracesByContractAddress(trace.structLogs, address);
for (const subcallAddress of _.keys(tracesByContractAddress)) {
for (const subcallAddress of subcallAddresses) {
payload = { method: 'eth_getCode', params: [subcallAddress, BlockParamLiteral.Latest] };
const runtimeBytecode = (await this.emitPayloadAsync(payload)).result;
jsonRPCResponsePayload = await this.emitPayloadAsync(payload);
const runtimeBytecode = jsonRPCResponsePayload.result;
const traceForThatSubcall = tracesByContractAddress[subcallAddress];
const coveredPcs = _.map(traceForThatSubcall, log => log.pc);
const traceInfo: TraceInfoExistingContract = {

View File

@@ -8,6 +8,10 @@ export interface TraceByContractAddress {
[contractAddress: string]: StructLog[];
}
function getAddressFromStackEntry(stackEntry: string): string {
return addressUtils.padZeros(new BigNumber(addHexPrefix(stackEntry)).toString(16));
}
export function getTracesByContractAddress(structLogs: StructLog[], startAddress: string): TraceByContractAddress {
const traceByContractAddress: TraceByContractAddress = {};
let currentTraceSegment = [];
@@ -16,26 +20,32 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
for (let i = 0; i < structLogs.length; i++) {
const structLog = structLogs[i];
if (structLog.depth !== callStack.length - 1) {
throw new Error("Malformed trace. trace depth doesn't match call stack depth");
throw new Error("Malformed trace. Trace depth doesn't match call stack depth");
}
// After that check we have a guarantee that call stack is never empty
// If it would: callStack.length - 1 === structLog.depth === -1
// That means that we can always safely pop from it
currentTraceSegment.push(structLog);
if (_.includes([OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall], structLog.op)) {
const isCallLike = _.includes(
[OpCode.CallCode, OpCode.StaticCall, OpCode.Call, OpCode.DelegateCall],
structLog.op,
);
const isEndOpcode = _.includes(
[OpCode.Return, OpCode.Stop, OpCode.Revert, OpCode.Invalid, OpCode.SelfDestruct],
structLog.op,
);
if (isCallLike) {
const currentAddress = _.last(callStack) as string;
const jumpAddressOffset = 1;
const newAddress = addressUtils.padZeros(
new BigNumber(addHexPrefix(structLog.stack[structLog.stack.length - jumpAddressOffset - 1])).toString(
16,
),
const newAddress = getAddressFromStackEntry(
structLog.stack[structLog.stack.length - jumpAddressOffset - 1],
);
if (structLog === _.last(structLogs)) {
throw new Error('CALL-like opcode can not be the last one');
throw new Error('Malformed trace. CALL-like opcode can not be the last one');
}
// Sometimes calls don't change the execution context (current address). When we do a transfer to an
// externally owned account - it does the call and immidiately returns because there is no fallback
// externally owned account - it does the call and immediately returns because there is no fallback
// function. We manually check if the call depth had changed to handle that case.
const nextStructLog = structLogs[i + 1];
if (nextStructLog.depth !== structLog.depth) {
@@ -45,9 +55,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
);
currentTraceSegment = [];
}
} else if (
_.includes([OpCode.Return, OpCode.Stop, OpCode.Revert, OpCode.Invalid, OpCode.SelfDestruct], structLog.op)
) {
} else if (isEndOpcode) {
const currentAddress = callStack.pop() as string;
traceByContractAddress[currentAddress] = (traceByContractAddress[currentAddress] || []).concat(
currentTraceSegment,
@@ -81,7 +89,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
);
currentTraceSegment = [];
} else {
throw new Error('Shit broke');
throw new Error('Malformed trace. Unexpected call depth change');
}
}
}
@@ -90,7 +98,7 @@ export function getTracesByContractAddress(structLogs: StructLog[], startAddress
throw new Error('Malformed trace. Call stack non empty at the end');
}
if (currentTraceSegment.length !== 0) {
throw new Error('Malformed trace. currentTraceSegment non empty at the end');
throw new Error('Malformed trace. Current trace segment non empty at the end');
}
return traceByContractAddress;
}