[Tracker] EVM Engineering: solidity code coverage

[tynes]

It would be ideal to have some form of code coverage for the solidity code that is reliable. Foundry does have code coverage, but we hit stack too deep issues when compiling for it (because it turns optimizations off). This means that we do not have any form of reliable code coverage right now for the solidity contracts. This results in a lot of extra work for devs doing code review - they must manually check that code paths have been covered.

Once we fix the ability to use foundry code coverage, we could set something up like what uniswap v4 uses. https://github.com/Uniswap/v4-core/blob/main/.github/workflows/coverage.yml. It is a simple github action that automatically posts the outputs of forge coverage into a github comment on a pull request.

[iainnash]

Forge forge coverage has a --minimal-ir option that allows some optimizations at the cost of potentially breaking source maps in some places.

[mds1]

We also get stack too deep when compiling with via-ir, so the --ir-minimum will not work for us. (More info in https://github.com/ethereum/solidity/issues/14358#issuecomment-2086251107, I believe even when marking blocks assembly-safe we still get the issue)

Since coverage maps are still not ideal with via-ir, the best solution here is to just fix all stack too deep sources so we can compile with no optimizer. Below is a snippet that will list all contracts that fail to compile due to stack too deep:

while IFS= read -r -d '' file; do
  solc $(forge re) "$file" --bin 2>&1 | grep -q "Stack too deep"
  if [ $? -eq 0 ]; then
    echo "Stack too deep error found in $file"
  fi
done < <(find . -type f -name "*.sol" -print0)

Most contracts that get listed are because a contract they inherit from (like Deploy and LibKeccak IIRC) has stack too deep itself, so in reality its probably <5 contracts to fix. What I mean is, given:

contract A { /*compiles ok */ }
contract B is A { /* stack too deep */ }
contract C is B { /* stack too deep */ }

the above snippet will output

Stack too deep error found in B.sol
Stack too deep error found in C.sol

But in reality, we most likely only need to fix B.sol, as it's the cause of the stack too deep in C as well

[AmadiMichael]

I'll take on this cc @smartcontracts can this get assigned to me?