compilers icon indicating copy to clipboard operation
compilers copied to clipboard

feat(`forge flatten`): add dead code elimination

Open sakulstra opened this issue 3 years ago • 7 comments

Component

Forge

Describe the feature you would like

When using some libraries, the --flatten output will be gigantic as flatten will inline all imports. It would be neat if flatten would detect & remove unused imports.

When using re-export index files like here https://github.com/bgd-labs/aave-address-book/blob/main/src/AaveAddressBook.sol that makes it essentially impossible to use in foundry as the generated code will be unusable long.

Additional context

In addition to what i mentioned above i guess it might make sense to even eliminate unused code in the actually imported files. Not sure sure about this though.

sakulstra avatar Jul 11 '22 14:07 sakulstra

Good catch.

gakonst avatar Jul 11 '22 18:07 gakonst

@gakonst just realized the same is true for non flattened files.

When you have a file like:

// index.sol
import {A} from "a.sol";
import {B} from "b.sol";
import {C} from "c.sol";
// contract.sol
import {A, B} from "index.sol";

contract Contract {
  function test() public {
    A.noop();
  }
}

When I verify this contract it will include A, B and C although only A is used.

sakulstra avatar Oct 05 '22 08:10 sakulstra

We'd need to do deeper parsing of the dependency graph for this kind of flattening I think?

gakonst avatar Oct 05 '22 19:10 gakonst

It's probably worth an additional flag as the automatic removal of unused imports may confuse some developers.

I believe this would be a valuable feature as we'll see more utility in index files, as shown by protocols in addition to aave.

The dependency graph only grows as protocols build across many chains, requiring index files.

pegahcarter avatar May 18 '24 20:05 pegahcarter

@sakulstra is this still the case in newer versions? Thanks

grandizzy avatar Mar 20 '25 16:03 grandizzy

@grandizzy just tried on latest nightly and it's still an issue yes.

As an easy repro you can create two files:

contract B {
  function b() external {}
}

and

import {B} from "./B.sol";

contract A {
  function a() external {}
}

When you now flatten A, you will receive:

// src/B.sol
contract B {
  function b() external {}
}

// src/Test.sol
contract A {
  function a() external {}
}

although B is not used in A. This is true, for both flatten and commands generating standard json (like forge verify-contract --show-standard-json-input).

sakulstra avatar Mar 21 '25 08:03 sakulstra

Just adding two sentences of context to the above:

This is quite suboptimal, as what happens sadly not so rarely is that somewhere in the import tree one might have a leftover console2 or similar. Now when you try to verify, chances are you exceed max codesize on most explorers. If you now deployed with metadata hash, you are in a deadlock. If you remove the dead code, the hash will change, if you don't the code is to big.

Changing the behavior, i guess will be quite breaking for ppl using metadata hash, but overall would probably be good for the ecosystem as there's dead imports on almost every contract i ever reviewed 😅

sakulstra avatar Mar 21 '25 08:03 sakulstra