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

Coverage doesn't cover internal libs when passing implicit storage vars via the using syntax #4854

Closed
2 tasks done
Craigson opened this issue Apr 29, 2023 · 5 comments · Fixed by #7510
Closed
2 tasks done
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-bug Type: bug

Comments

@Craigson
Copy link

Component

Forge

Have you ensured that all of these are up to date?

  • Foundry
  • Foundryup

What version of Foundry are you on?

forge 0.2.0 (8f246e0 2023-04-28T00:14:27.758082000Z)

What command(s) is the bug in?

forge coverage --report lcov && genhtml -o forge-coverage lcov.info

Operating System

macOS (Apple Silicon)

Describe the bug

When importing an internal lib and utilizing it with the using syntax, coverage does not detect any hits on the functions in the library.

Example is shown below (some code omitted for brevity):

import "./lib/StakeManagerLibs.sol";

contract StakeManager is IStakeManager, Initializable {
    ...
    using LibStakesManager for Stakes;

    function stakeFor(uint256 id, uint256 amount) external {
        require(id != 0 && id <= _chains.counter, "INVALID_ID");
        MATIC.safeTransferFrom(msg.sender, address(this), amount);
        _stakes.addStake(msg.sender, id, amount); // <========= Coverage registers no hits here
        ISupernetManager manager = managerOf(id);
        manager.onStake(msg.sender, amount);
        emit StakeAdded(id, msg.sender, amount);
    }

Yields the following coverage report:

Screen Shot 2023-04-29 at 7 33 30 PM

However, if I change the syntax of how the library function is called, where the storage variable isn't implicitly passed as the first argument, the coverage is correct.

    function stakeFor(uint256 id, uint256 amount) external {
        require(id != 0 && id <= _chains.counter, "INVALID_ID");
        MATIC.safeTransferFrom(msg.sender, address(this), amount);
        LibStakesManager.addStake(_stakes, msg.sender, id, amount); // <======= Use the library explicitly
        // _stakes.addStake(msg.sender, id, amount);
        ISupernetManager manager = managerOf(id);
        manager.onStake(msg.sender, amount);
        emit StakeAdded(id, msg.sender, amount);
    }

Then coverage report is accurate:

Screen Shot 2023-04-29 at 7 36 41 PM

Another important piece of information to add is that calling the Library function explicitly just once, ie the remainder of the _stakes.addStake(...) and any additional library calls to any/all functions using the implicit syntax are now covered.

@Craigson Craigson added the T-bug Type: bug label Apr 29, 2023
@gakonst gakonst added this to Foundry Apr 29, 2023
@github-project-automation github-project-automation bot moved this to Todo in Foundry Apr 29, 2023
@Craigson
Copy link
Author

@gretzke for visibility

@gretzke
Copy link

gretzke commented Apr 29, 2023

The workaround of calling the library directly once also only works once. If you have a second internal library in the same contract you can call it directly as often as you want, it won't show up in the coverage report as covered. This does not seem to be connected to the using X for Y syntax, calling all internal libraries directly shows the same behaviour.

@mds1 mds1 added C-forge Command: forge Cmd-forge-coverage Command: forge coverage labels Apr 30, 2023
@FredCoen
Copy link

having similar issue with using for directive on custom types

@cirethic
Copy link

cirethic commented Sep 5, 2023

I think I have the similar issue by using the using syntax to define a state struct define in an internal library. The code coverage report 0 coverage.

what it doesn't work is:

   using GeometricBWAP for GeometricBWAP.Observations;
   GeometricBWAP.Observations private obs;
 
   obs.configLongTermInterval(longTermIntervalConfig);

what's my work around is:

  GeometricBWAP.configLongTermInterval(obs, longTermIntervalConfig);

@aviggiano
Copy link

I am having the same issue, but interestingly I don't get 0 coverage, only partial coverage, even though all lines are being touched by my tests.

@jenpaff jenpaff moved this from Todo to Completed in Foundry Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-coverage Command: forge coverage T-bug Type: bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

6 participants