Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.7.0: opcode trace instrumentation & plugin app architecture #372

Merged
merged 59 commits into from
Sep 25, 2019

Conversation

cgewecke
Copy link
Member

@cgewecke cgewecke commented Aug 13, 2019

NB: This work has been merged into the beta branch for pre-release staging.

(continuation of #357 )

#346 Swaps event-based instrumentation (with testrpc-sc) for variable-based instrumentation processed by an ethereumjs-vm opcode step listener.

The changes here are less extensive than they might seem at first glance. All the core instrumentation logic has been retained and should be +/- recognizable.

Summary

  • modify injector to use special statement injections we can read with an opcode tracer (see v0.7.0 Plan (Experimental)  #346)
  • add DataCollector class which listens to the VM for data-hash-detected-on-stack 'events'. (It works (hackily) with a generic ganache-core but we also publish ganache-core-sc@next (2.7.0) as a fail safe in case something goes wrong there).
  • rename instrumentSolidity --> Instrumenter & make JS class / top level controller
  • rename instrumenter --> Registrar (e.g registers injection details) & make JS class.
  • remove pure/view preprocessing
  • mark instrumentation hits in memory / transform to coverage object from memory
  • refactor test suite for this design
  • use nyc for repo coverage (not solidity-coverage). Necessary because instanbul has problems w/ async syntax in truffle.plugin.js

Truffle

Buidler

General

Gas Distortion Measurements

solidity 0.5.11 - ganache 6.6.0 - optimization = false

Instrument Gas
fn(bytes32 hash) public pure {} 114
fn(bytes32 hash) public pure returns (bool){ return true; } 134

ethereumjs-vm 554: can we modify the gas costs of our instrumentation with a VM API? Can this be done cost-effectively?

Beta Trial

npm install --save-dev solidity-coverage@beta
npx truffle run coverage

@codecov-io
Copy link

codecov-io commented Aug 13, 2019

Codecov Report

Merging #372 into beta will increase coverage by 7.39%.
The diff coverage is 99.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##             beta     #372      +/-   ##
==========================================
+ Coverage   91.66%   99.06%   +7.39%     
==========================================
  Files           6       14       +8     
  Lines         372      639     +267     
  Branches       79        0      -79     
==========================================
+ Hits          341      633     +292     
+ Misses         31        6      -25
Impacted Files Coverage Δ
lib/injector.js 100% <100%> (+10.71%) ⬆️
dist/bin.js 100% <100%> (ø)
lib/ui.js 100% <100%> (ø)
dist/plugin-assets/truffle.ui.js 100% <100%> (ø)
lib/instrumenter.js 100% <100%> (+7.46%) ⬆️
lib/validator.js 100% <100%> (ø)
lib/collector.js 100% <100%> (ø)
dist/truffle.plugin.js 100% <100%> (ø)
lib/registrar.js 100% <100%> (ø)
lib/preprocessor.js 96.15% <100%> (+6.57%) ⬆️
... and 16 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15fa08b...00f32f6. Read the comment docs.

@@ -3,5 +3,7 @@ pragma solidity ^0.5.0;
contract Test {
function a(bool test) public {//Comment immediately after function declaration
}
function b(bool test) public {uint8 x=1;}//Comment immediately after function closes
Copy link
Member Author

@cgewecke cgewecke Aug 13, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need an additional test case here - {uint8 x=1;} directly after the fn def triggers an instrumentation fault.

lib/app.js Outdated Show resolved Hide resolved
lib/app.js Outdated Show resolved Hide resolved
@yxliang01
Copy link
Contributor

@cgewecke To confirm, Truffle v4 is going to be unsupported?

@cgewecke
Copy link
Member Author

@yxliang01 V4 support is not currently planned.

However it should be possible to construct a script which uses the 0.7.0 API and some helpers written for the v5 plugin to get it working for v4.

Out of curiosity, what keeps you from upgrading to v5?

@yxliang01
Copy link
Contributor

@cgewecke At times, my team needs to conduct academic experiments on various repos which might still use Truffle v4. Generally speaking, we would not want to modify them if possible. :)

@cgewecke
Copy link
Member Author

@yxliang01 Yes that makes sense. Will start thinking more about this - it may be possible to build a recipe for Truffle V4 around the truffle exec command re-using most of the pieces here.

@cgewecke cgewecke changed the base branch from master to beta September 25, 2019 09:38
@cgewecke cgewecke marked this pull request as ready for review September 25, 2019 09:57
@cgewecke cgewecke changed the title [WIP] 0.7.0: opcode trace instrumentation & plugin app architecture 0.7.0: opcode trace instrumentation & plugin app architecture Sep 25, 2019
@cgewecke cgewecke merged commit 64a501b into beta Sep 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants