hardhat icon indicating copy to clipboard operation
hardhat copied to clipboard

`hh flatten` doesn't support circular dependencies

Open fvictorio opened this issue 3 years ago • 11 comments

Hardhat flatten doesn't support circular dependencies. Some users want this functionality, yet it's not straightforward nor clear that it would work for them. Continue reading to understand why.

Hardhat flatten works by creating a topological order of the dependency graph. That is a sequence of files where every file comes after its dependencies. Then it concatenates the files in that order. This ensures that every symbol (e.g. a library, contract, function, etc) is defined before usage. Now, graphs with circular dependencies don't have a topological order, so this algorithm doesn't work.

An alternative is to analyze all the files and create a topological order of top-level definitions (e.g. contract definitions, libraries, etc). This should be possible because Solidity doesn't compile projects if you have cycles in those. The problem is that this is really complex. You need to parse and perform some semantic analyses on top of the sources (use-def analysis), and it should be really precise or it will fail. On top of that, we can't guarantee that the output generated by solidity will be equivalent to the non-flattened task, and this would make it not a good fit for contract verification.

As a general recommendation: avoid circular dependencies. They are harder to reason about, harder to create tooling for, and normally forbidden by many compilers/runtimes. There are multiple refactors that can be applied mechanically to eliminate them.

fvictorio avatar May 26 '21 15:05 fvictorio

How can one recreate this bug?

Endlessline avatar May 26 '21 21:05 Endlessline

// contracts/Foo.sol
import "./Bar.sol";

contract Foo {}
// contracts/Bar.sol
import "./Foo.sol";

contract Bar {}

hh compile works, hh flatten doesn't.

The solution is probably to get some topological order of the dependency graph and to use that to generate the flattened file, ignoring previously visited files when resolving imports.

Edit: this is wrong, check the updated issue description.

fvictorio avatar May 26 '21 23:05 fvictorio

+1

BitMaster001 avatar May 30 '23 05:05 BitMaster001

+1

nekooei avatar Jun 05 '23 05:06 nekooei

+1

slvrfn avatar Jun 06 '23 22:06 slvrfn

+1

vinkent420 avatar Jul 14 '23 10:07 vinkent420

+1

nxqbao avatar Aug 08 '23 08:08 nxqbao

I updated this issue explaining why this is more complicated than it seems, and why it may still not work for some usecases.

This is something low priority to us, but we may consider doing it once Slang is more mature.

alcuadrado avatar Aug 08 '23 10:08 alcuadrado

+1

liu-zhipeng avatar Nov 14 '23 14:11 liu-zhipeng