foundry-rs / foundry

Foundry is a blazing fast, portable and modular toolkit for Ethereum application development written in Rust.
https://getfoundry.sh
Apache License 2.0
8.35k stars 1.77k forks source link

forge coverage ignores empty receive() function #9444

Closed krakovia-evm closed 18 hours ago

krakovia-evm commented 1 day ago

Component

Forge

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

What version of Foundry are you on?

forge 0.2.0 (7a23a5c 2024-12-01T00:29:03.740254700Z)

What command(s) is the bug in?

forge coverage

Operating System

Windows

Describe the bug

"forge coverage" seems to ignore receive() functions if their body is empty in tests. Simple poc

  1. forge init —no-git
  2. add receive() external payable {} to Counter.sol
  3. add this test to Counter.t.sol
    function test_receive() public {
        (bool success,) = address(counter).call{value: 1 ether}("");
        assertTrue(success);
    }
  4. run forge coverage / forge coverage —report lcov

we should get 100% coverage, instead the receive function is red (but we touch it in the tests)

[14675] CounterTest::test_receive()
    ├─ [55] Counter::receive{value: 1000000000000000000}()
    │   └─ ← [Stop]
    ├─ [0] VM::assertTrue(true) [staticcall]
    │   └─ ← [Return]
    └─ ← [Stop]

If i add an emit inside the receive() function it gets correctly (green) flagged.

DaniPopes commented 1 day ago

cc @grandizzy the easiest fix would be to skip adding fallback/receive if the body is empty as well but I don't know if this is right

grandizzy commented 1 day ago

cc @grandizzy the easiest fix would be to skip adding fallback/receive if the body is empty as well but I don't know if this is right

That makes sense, I overlooked empty body when enabled support for receive in https://github.com/foundry-rs/foundry/pull/9288

DaniPopes commented 1 day ago

I am asking because we can register hits for empty functions but not receive/fallback, I would like to fix the underlying issue

grandizzy commented 1 day ago

ah, I see, I had the impression we already register hits for empty functions so my suggestion was to ignore empty receive/fallback only