-
Notifications
You must be signed in to change notification settings - Fork 269
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
Setting viaIR
to true in solc results in zero coverage
#715
Comments
Could you try running Also if you could share your config or a reproducible example that would be helpful. |
The output didn't change :(
Ill setup a reproducible example and report back! Thanks for the quick response :) |
@cgewecke here you go! Thanks again for your help! |
Thanks for the repro. If I comment out
[EDIT - For clarification, some optimizer settings remove the instrumentation soldiity-coverage injects into the contracts to track the execution trace. I've edited your issue title to reflect what I think the underlying cause is so other people can easily find it.] TODO: set |
viaIR
to true in compiler settings results in zero coverage
ah, I see! thanks for the suggestion, works like a charm now :) |
Hi @cgewecke, do y'all have plans to support coverage with viaIR == true? The default of turning off viaIR for our code means coverage cannot run because viaIR eliminates stack depth too deep errors. Example error from our code:
|
hello,do you need to write tests manually? |
Any updates on this - would be great to get coverage back in action for |
Hi, any progress on this one? I'm using this on module.exports = {
...
configureYulOptimizer: true,
solcOptimizerDetails: {
peephole: false,
inliner: false,
jumpdestRemover: false,
orderLiterals: true, // <-- TRUE! Stack too deep when false
deduplicate: false,
cse: false,
constantOptimizer: false,
yul: false
},
} But still getting (without ViaIR):
And with ViaIR:
Using Here you can see a repo to reproduce the error, it compiles fine, but coverage does not run. Also tried
|
I've found a decent workaround for the issue: As @cgewecke mentioned some optimiser settings remove the instrumentation soldiity-coverage injects, so we just need to disable those settings. Default yul-optimizer steps can be found in the solidity repo and the rules that remove instrumentation are With this in mind I've set my
And now the coverage works for builds with yul optimizer. |
It didn't worked for me @ZumZoom, it took literally hours to compile, and when it ran, it throws:
|
I have the same issue and the workarounds above doesn't work for me either. |
Can we remove the |
We've tried this approach on a bunch of more repos, and we discovered two more things:
So now the module.exports = {
configureYulOptimizer: true,
solcOptimizerDetails: {
yul: true,
yulDetails: {
optimizerSteps:
"dhfoDgvlfnTUtnIf" + // None of these can make stack problems worse
"[" +
"xa[r]EsLM" + // Turn into SSA and simplify
"CTUtTOntnfDIl" + // Perform structural simplification
"Ll" + // Simplify again
"Vl [j]" + // Reverse SSA
// should have good "compilability" property here.
"Tpel" + // Run functional expression inliner
"xa[rl]" + // Prune a bit more in SSA
"xa[r]L" + // Turn into SSA again and simplify
"gvf" + // Run full inliner
"CTUa[r]LSsTFOtfDna[r]Il" + // SSA plus simplify
"]" +
"jml[jl] VTOl jml : fDnTO",
},
},
} Long compile time is sort of viaIR feature, but it seems that removing inlining and pruning makes the process even slower than with default config. @jmendiola222 Can you please try this config in your project? Edit: I've tried my workaround on your example, and it works and coverage produces valid results. You can check here: https://github.com/ZumZoom/solidity-coverage-via-ir |
tried but didn't work for me |
Hey, it did work 🥳 ! It took and hour to compile, and a couple more to run, 4 tests failed (no clear reason why) but overall all the rest worked. Thanks @ZumZoom |
Update, even if the workaround technically works, it's quite inconvenient, for example our CI failed with:
On dev's 8core/16Gb PCs also takes ~8hs to complete, and almost making it unusable to any other task. As reference, running the tests (also with VIAIR enable) takes ~1hs. |
A potential workaround to identify the problematic contracts here: #760 (comment) |
For me, the below code in
Solidity: 0.8.19 However, some tests use higher gas compared to just running tests. So, might need to change some tests accordingly. |
Please remove the |
viaIR
to true in compiler settings results in zero coverageviaIR
to true in solc results in zero coverage
A patch for this problem has been published at the
Should also be live in the next release (0.8.7). If you're using either of those versions, you can remove any special solc config or solc related options (like Gas consumption has changed slightly when using viaIR compatible instrumentation - if you're looking for specific values in your tests you might need to update them. #854 was E2E tested on 1inch's solidity-utils and everything looked ok. Coverage is the same etc. Apologies for how long it took to fix this, I didn't think there was a way to do it, but in the end there was. Also thank you for sharing your solc configs. |
This should be fixed in v0.8.7 - viaIR is natively supported and no special config is required. If you're still running into issues with this please report them to #861 |
Fixed on v0.8.7 for us indeed, and the time drooped to ~2x normal test run (from ~8x with the "hack"). |
Following the docs here, and running
npx hardhat coverage
gives this result:which is weird. My
.solcover.js
content is:Not sure what I'm missing.
The text was updated successfully, but these errors were encountered: