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

Enable coverage when viaIR compiler flag is true #854

Merged
merged 14 commits into from
Feb 9, 2024
10 changes: 6 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,9 @@ step_install_nvm: &step_install_nvm
set +e
export NVM_DIR="/opt/circleci/.nvm"
[ -s "$NVM_DIR/nvm.sh" ] && \. "$NVM_DIR/nvm.sh"
nvm install v14.19.0
nvm alias default v14.19.0
nvm install v18
echo 'export NVM_DIR="/opt/circleci/.nvm"' >> $BASH_ENV
echo "[ -s \"$NVM_DIR/nvm.sh\" ] && . \"$NVM_DIR/nvm.sh\"" >> $BASH_ENV

jobs:
unit-test:
docker:
Expand All @@ -32,9 +30,13 @@ jobs:
command: |
yarn
- run:
name: Run tests
name: Tests ( optimizer.enabled=false )
command: |
npm run test:ci
- run:
name: Tests ( viaIR=true )
command: |
npm run test:ci:viaIR
- run:
name: Upload coverage
command: |
Expand Down
3 changes: 2 additions & 1 deletion lib/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class API {
this.istanbulFolder = config.istanbulFolder || false;
this.istanbulReporter = config.istanbulReporter || ['html', 'lcov', 'text', 'json'];

this.viaIR = config.viaIR;
this.solcOptimizerDetails = config.solcOptimizerDetails;

this.setLoggingLevel(config.silent);
Expand Down Expand Up @@ -176,7 +177,7 @@ class API {
// Hardhat
async attachToHardhatVM(provider){
const self = this;
this.collector = new DataCollector(this.instrumenter.instrumentationData);
this.collector = new DataCollector(this.instrumenter.instrumentationData, this.viaIR);

if ('init' in provider) {
// Newer versions of Hardhat initialize the provider lazily, so we need to
Expand Down
50 changes: 46 additions & 4 deletions lib/collector.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,31 @@
* coverage map constructed by the Instrumenter.
*/
class DataCollector {
constructor(instrumentationData={}){
constructor(instrumentationData={}, viaIR){
this.instrumentationData = instrumentationData;

this.validOpcodes = {
"PUSH1": true,
"DUP1": viaIR,
"DUP2": viaIR,
"DUP3": viaIR,
"DUP4": viaIR,
"DUP5": viaIR,
"DUP6": viaIR,
"DUP7": viaIR,
"DUP8": viaIR,
"DUP9": viaIR,
"DUP10": viaIR,
"DUP11": viaIR,
"DUP12": viaIR,
"DUP13": viaIR,
"DUP14": viaIR,
"DUP15": viaIR,
"DUP16": viaIR,
}

this.lastHash = null;
this.viaIR = viaIR;
}

/**
Expand Down Expand Up @@ -46,7 +65,26 @@ class DataCollector {
hash = this._normalizeHash(hash);

if(this.instrumentationData[hash]){
this.instrumentationData[hash].hits++;
// abi.encode (used to circumvent viaIR) sometimes puts the hash on the stack twice
if (this.lastHash !== hash) {
this.lastHash = hash;
this.instrumentationData[hash].hits++
}
return;
}

// Detect and recover from viaIR mangled hashes by left-padding single `0`
if(this.viaIR && hash.length === 18) {
hash = hash.slice(2);
hash = '0' + hash;
hash = hash.slice(0,16);
hash = '0x' + hash;
if(this.instrumentationData[hash]){
if (this.lastHash !== hash) {
this.lastHash = hash;
this.instrumentationData[hash].hits++
}
}
}
}

Expand All @@ -56,10 +94,14 @@ class DataCollector {
* but prevents left-padding shorter irrelevant hashes
*
* @param {String} hash data hash from evm stack.
* @return {String} 0x prefixed hash of length 66.
* @return {String} 0x prefixed hash of length 18.
*/
_normalizeHash(hash){
if (hash.length < 18 && hash.length > 11){
// viaIR sometimes right-pads the hashes out to 32 bytes
// but it doesn't preserve leading zeroes when it does this
if (this.viaIR && hash.length >= 18) {
hash = hash.slice(0,18);
} else if (hash.length < 18 && hash.length > 11){
hash = hash.slice(2);
while(hash.length < 16) hash = '0' + hash;
hash = '0x' + hash
Expand Down
52 changes: 44 additions & 8 deletions lib/injector.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
const web3Utils = require("web3-utils");

class Injector {
constructor(){
constructor(viaIR){
this.viaIR = viaIR;
this.hashCounter = 0;
this.modifierCounter = 0;
this.modifiers = {};
Expand All @@ -23,7 +24,9 @@ class Injector {
case 'modifier':
return ` ${this._getModifierIdentifier(id)} `;
default:
return `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`;
return (this.viaIR)
? `${this._getAbiEncodeStatementHash(hash)} /* ${type} */ \n`
: `${this._getDefaultMethodIdentifier(id)}(${hash}); /* ${type} */ \n`;
}
}

Expand Down Expand Up @@ -51,6 +54,16 @@ class Injector {
return `c_mod${web3Utils.keccak256(id).slice(2,10)}`
}

// Way to get hash on the stack with viaIR (which seems to ignore abi.encode builtin)
// Tested with v0.8.17, v0.8.24
_getAbiEncodeStatementHash(hash){
return `abi.encode(${hash}); `
}

_getAbiEncodeStatementVar(hash){
return `abi.encode(c__${hash}); `
}

_getInjectionComponents(contract, injectionPoint, id, type){
const { start, end } = this._split(contract, injectionPoint);
const hash = this._getHash(id)
Expand All @@ -73,7 +86,10 @@ class Injector {
_getDefaultMethodDefinition(id){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getDefaultMethodIdentifier(id);
return `\nfunction ${method}(bytes8 c__${hash}) internal pure {}\n`;

return (this.viaIR)
? ``
: `\nfunction ${method}(bytes8 c__${hash}) internal pure {}\n`;
}

/**
Expand All @@ -85,7 +101,11 @@ class Injector {
_getFileScopedHashMethodDefinition(id, contract){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getDefaultMethodIdentifier(id);
return `\nfunction ${method}(bytes8 c__${hash}) pure {}\n`;
const abi = this._getAbiEncodeStatementVar(hash);

return (this.viaIR)
? `\nfunction ${method}(bytes8 c__${hash}) pure { ${abi} }\n`
: `\nfunction ${method}(bytes8 c__${hash}) pure {}\n`;
}

/**
Expand All @@ -97,7 +117,11 @@ class Injector {
_getTrueMethodDefinition(id){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getTrueMethodIdentifier(id);
return `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return true; }\n`;
const abi = this._getAbiEncodeStatementVar(hash);

return (this.viaIR)
? `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ ${abi} return true; }\n`
: `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return true; }\n`;
}

/**
Expand All @@ -110,7 +134,11 @@ class Injector {
_getFileScopeTrueMethodDefinition(id){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getTrueMethodIdentifier(id);
return `function ${method}(bytes8 c__${hash}) pure returns (bool){ return true; }\n`;
const abi = this._getAbiEncodeStatementVar(hash);

return (this.viaIR)
? `function ${method}(bytes8 c__${hash}) pure returns (bool){ ${abi} return true; }\n`
: `function ${method}(bytes8 c__${hash}) pure returns (bool){ return true; }\n`;
}

/**
Expand All @@ -122,7 +150,11 @@ class Injector {
_getFalseMethodDefinition(id){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getFalseMethodIdentifier(id);
return `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return false; }\n`;
const abi = this._getAbiEncodeStatementVar(hash);

return (this.viaIR)
? `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ ${abi} return false; }\n`
: `function ${method}(bytes8 c__${hash}) internal pure returns (bool){ return false; }\n`;
}

/**
Expand All @@ -135,7 +167,11 @@ class Injector {
_getFileScopedFalseMethodDefinition(id){
const hash = web3Utils.keccak256(id).slice(2,10);
const method = this._getFalseMethodIdentifier(id);
return `function ${method}(bytes8 c__${hash}) pure returns (bool){ return false; }\n`;
const abi = this._getAbiEncodeStatementVar(hash);

return (this.viaIR)
? `function ${method}(bytes8 c__${hash}) pure returns (bool){ ${abi} return false; }\n`
: `function ${method}(bytes8 c__${hash}) pure returns (bool){ return false; }\n`;
}

_getModifierDefinitions(contractId, instrumentation){
Expand Down
2 changes: 1 addition & 1 deletion lib/instrumenter.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class Instrumenter {

constructor(config={}){
this.instrumentationData = {};
this.injector = new Injector();
this.injector = new Injector(config.viaIR);
this.modifierWhitelist = config.modifierWhitelist || [];
this.enabled = {
statements: (config.measureStatementCoverage === false) ? false : true,
Expand Down
5 changes: 4 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@
"scripts": {
"test:unit": "./scripts/unit.sh",
"test:integration": "./scripts/integration.sh",
"test:ci": "./scripts/ci.sh"
"test:ci": "./scripts/ci.sh",
"test:uint:viaIR": "VIA_IR=true ./scripts/unit.sh",
"test:integration:viaIR": "VIA_IR=true ./scripts/integration.sh",
"test:ci:viaIR": "VIA_IR=true ./scripts/ci.sh"
},
"homepage": "https://github.com/sc-forks/solidity-coverage",
"repository": {
Expand Down
11 changes: 7 additions & 4 deletions plugins/hardhat.plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,14 @@ subtask(TASK_COMPILE_SOLIDITY_GET_COMPILATION_JOB_FOR_FILE).setAction(async (_,
}
// Unset useLiteralContent due to solc metadata size restriction
settings.metadata.useLiteralContent = false;
// Override optimizer settings for all compilers
settings.optimizer.enabled = false;

// This is fixes a stack too deep bug in ABIEncoderV2
// Experimental because not sure this works as expected across versions....
// Beginning with v0.8.7, we let the optimizer run if viaIR is true and
// instrument using `abi.encode(bytes8 covHash)`. Otherwise turn the optimizer off.
if (!settings.viaIR) settings.optimizer.enabled = false;

// This sometimes fixed a stack-too-deep bug in ABIEncoderV2 for coverage plugin versions up to 0.8.6
// Although issue should be fixed in 0.8.7, am leaving this option in because it may still be necessary
// to configure optimizer details in some cases.
if (configureYulOptimizer) {
if (optimizerDetails === undefined) {
settings.optimizer.details = {
Expand Down
21 changes: 21 additions & 0 deletions plugins/resources/nomiclabs.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ function normalizeConfig(config, args={}){
? sources = path.join(config.paths.sources, args.sources)
: sources = config.paths.sources;

if (config.solidity && config.solidity.compilers.length) {
config.viaIR = isUsingViaIR(config.solidity);
}

config.workingDir = config.paths.root;
config.contractsDir = sources;
config.testDir = config.paths.tests;
Expand All @@ -55,6 +59,23 @@ function normalizeConfig(config, args={}){
return config;
}

function isUsingViaIR(solidity) {

for (compiler of solidity.compilers) {
if (compiler.settings && compiler.settings.viaIR) {
return true;
}
}
if (solidity.overrides) {
for (key of Object.keys(solidity.overrides)){
if (solidity.overrides[key].settings && solidity.overrides[key].settings.viaIR) {
return true;
}
}
}
return false;
}

async function setupHardhatNetwork(env, api, ui){
const hardhatPackage = require('hardhat/package.json');
const { createProvider } = require("hardhat/internal/core/providers/construction");
Expand Down
4 changes: 3 additions & 1 deletion plugins/resources/plugin.utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,9 @@ function loadSolcoverJS(config={}){
coverageConfig = {};
}

// Truffle writes to coverage config
// viaIR is eval'd in `nomiclab.utils.normalizeConfig`
coverageConfig.viaIR = config.viaIR;

coverageConfig.log = log;
coverageConfig.cwd = config.workingDir;
coverageConfig.originalContractsDir = config.contractsDir;
Expand Down
8 changes: 7 additions & 1 deletion scripts/ci.sh
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
#!/usr/bin/env bash

SILENT=true node --max-old-space-size=4096 \
# Toggles optimizer on/off
VIAR_IR=$VIA_IR

# Minimize integration test output
SILENT=true

node --max-old-space-size=4096 \
./node_modules/.bin/nyc \
--reporter=lcov \
--exclude '**/sc_temp/**' \
Expand Down
3 changes: 3 additions & 0 deletions scripts/integration.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/usr/bin/env bash

# Toggles optimizer on/off
VIA_IR=$VIA_IR

node --max-old-space-size=4096 \
./node_modules/.bin/nyc \
--exclude '**/sc_temp/**' \
Expand Down
3 changes: 3 additions & 0 deletions scripts/unit.sh
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
#!/usr/bin/env bash

# Toggles optimizer on/off
VIAR_IR=$VIA_IR

node --max-old-space-size=4096 \
./node_modules/.bin/nyc \
--exclude '**/sc_temp/**' \
Expand Down
1 change: 1 addition & 0 deletions scripts/zeppelin.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ set -o errexit
# Get rid of any caches
sudo rm -rf node_modules
echo "NVM CURRENT >>>>>" && nvm current
nvm use 18

# Use PR env variables (for forks) or fallback on local if PR not available
SED_REGEX="s/git@github.com:/https:\/\/github.com\//"
Expand Down
25 changes: 25 additions & 0 deletions test/integration/standard.js
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,31 @@ describe('Hardhat Plugin: standard use cases', function() {
verify.lineCoverage(expected);
})

it('detects viaIR when specified in config overrides only', async function(){
mock.installFullProject('overrides-viaIR');
mock.hardhatSetupEnv(this);

await this.env.run("coverage");

const expected = [
{
file: mock.pathToContract(hardhatConfig, 'ContractOverA2.sol'),
pct: 33.33
},
{
file: mock.pathToContract(hardhatConfig, 'ContractOverB2.sol'),
pct: 100,
},
{
file: mock.pathToContract(hardhatConfig, 'ContractOverC2.sol'),
pct: 100,
},

];

verify.lineCoverage(expected);
})

it('locates .coverage_contracts correctly when dir is subfolder', async function(){
mock.installFullProject('contract-subfolders');
mock.hardhatSetupEnv(this);
Expand Down
Loading