Allow Compilation without via-ir

[ankurdubey521]

Coverage with via-ir enabled

Foundry

Hardhat

Coverage with via-ir disabled

Foundry

Hardhat

Significant Improvements in the coverage reported by Foundry can be observed, but interestingly, the results from hardhat are unchanged. I don't know why this is the case, perhaps the hardhat handles it differently.

[github-actions]

Changes to gas cost

Generated at commit: 0ad6973363a973f4d1b4357cbc1fb595f694e892, compared to commit: b8085a3d688afb9149c129a34b4bb9cefb93ae38

🧾 Summary (5% most significant diffs)

Contract	Method	Avg (+/-)	%
Nexus	uninstallModule validateUserOp	-27 ✅ -35 ✅	-0.27% -0.40%

---

Full diff report 👇

Contract	Deployment Cost (+/-)	Method	Min (+/-)	%	Avg (+/-)	%	Median (+/-)	%	Max (+/-)	%	# Calls (+/-)
Nexus	5,603,866 (-33,805)	execute installModule uninstallModule validateUserOp	6,125 (-27) 32,542 (-78) 5,605 (-27) 7,107 (-35)	-0.44% -0.24% -0.48% -0.49%	48,937 (-46) 38,414 (-56) 10,148 (-27) 8,678 (-35)	-0.09% -0.15% -0.27% -0.40%	37,825 (-27) 40,120 (-27) 11,771 (-27) 7,107 (-35)	-0.07% -0.07% -0.23% -0.49%	142,903 (-27) 43,907 (-27) 12,047 (-27) 35,898 (-35)	-0.02% -0.06% -0.22% -0.10%	76 (0) 23 (0) 6 (0) 351 (0)
BiconomyMetaFactory	722,709 (+5,455)
Bootstrap	2,536,039 (-16,234)

[codecov]

Codecov Report

Attention: Patch coverage is 88.34951% with 12 lines in your changes missing coverage. Please review.

Project coverage is 91.34%. Comparing base (1d419d1) to head (3b4c299).
Report is 12 commits behind head on dev.

Files with missing lines	Patch %	Lines
contracts/factory/RegistryFactory.sol	75.00%	4 Missing ⚠️
contracts/Nexus.sol	91.17%	3 Missing ⚠️
contracts/base/ModuleManager.sol	84.61%	2 Missing ⚠️
contracts/modules/validators/K1Validator.sol	83.33%	2 Missing ⚠️
contracts/factory/NexusAccountFactory.sol	75.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##              dev     #156       +/-   ##
===========================================
+ Coverage   75.07%   91.34%   +16.26%     
===========================================
  Files          13       13               
  Lines         682      716       +34     
  Branches      157      136       -21     
===========================================
+ Hits          512      654      +142     
+ Misses        170       62      -108     

Files with missing lines	Coverage Δ
contracts/base/BaseAccount.sol	100.00% <100.00%> (+40.54%)	⬆️
contracts/common/Stakeable.sol	100.00% <100.00%> (ø)
contracts/factory/BiconomyMetaFactory.sol	100.00% <100.00%> (+6.66%)	⬆️
contracts/factory/K1ValidatorFactory.sol	100.00% <100.00%> (ø)
contracts/factory/NexusAccountFactory.sol	93.75% <75.00%> (-6.25%)	⬇️
contracts/base/ModuleManager.sol	89.61% <84.61%> (+6.83%)	⬆️
contracts/modules/validators/K1Validator.sol	88.13% <83.33%> (+4.80%)	⬆️
contracts/Nexus.sol	93.14% <91.17%> (+31.46%)	⬆️
contracts/factory/RegistryFactory.sol	80.70% <75.00%> (-3.30%)	⬇️

... and 2 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

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

[VGabriel45]

Nice job.
We should exclude scripts and test folder from foundry coverage. I think hardhat coverage is so low because hardhat tests need improvement to cover more code, we have more foundry tests than hardhat tests. For example, registry is not tested at all with hardhat.

[ankurdubey521]

fwiw, I believe scripts should also be tested and hence be included in the coverage. see: https://book.getfoundry.sh/tutorials/best-practices?highlight=testing%20scripts#scripts

[ankurdubey521]

what benefit do we get by repeating the same tests in hardhat? I believe only the test which can be written in hardhat (testing against client-side or server-side code) should be written in hardhat, for rest we should just default to foundry

[ankurdubey521]

Also, there's a section in the coverage report generation script that removes test, mocks and other deps:

# Exclude test, mock, and node_modules folders
EXCLUDE="*test* *mocks* *node_modules* *scripts* *lib*"
lcov --rc lcov_branch_coverage=1 --ignore-errors unused --ignore-errors inconsistent --remove lcov.info $EXCLUDE --output-file coverage/foundry/forge-pruned-lcov.info```

[Aboudjem]

Nice job. We should exclude scripts and test folder from foundry coverage. I think hardhat coverage is so low because hardhat tests need improvement to cover more code, we have more foundry tests than hardhat tests. For example, registry is not tested at all with hardhat.

it's removed and give you the actual real coverage when you do yarn run coverage:report

[openzeppelin-code]

Allow Compilation without via-ir

Generated at commit: 3b4c2996aaf62e91b5fde14bb97437269da89392

🚨 Report Summary

	Severity Level	Results
Contracts	Critical High Medium Low Note Total	0 1 0 7 25 33

---

For more details view the full report in OpenZeppelin Code Inspector

[livingrockrises]

LGTM. just tellme how many sloc changes with a commit hash would be helpful to scope this in

[ankurdubey521]

LGTM. just tellme how many sloc changes with a commit hash would be helpful to scope this in

@Aboudjem is there a tool/GitHub integration that can report this number?

[livingrockrises]

I am opening one PR with 0.8.27
can maybe close this one then.

also, linting diffs would confuse auditors for PR diff view and scoping.

[livingrockrises]

#159