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

hh flatten doesn't support circular dependencies #1486

Open
fvictorio opened this issue May 26, 2021 · 13 comments
Open

hh flatten doesn't support circular dependencies #1486

fvictorio opened this issue May 26, 2021 · 13 comments
Labels
status:blocked Blocked by other issues or external reasons type:feature Feature request

Comments

@fvictorio
Copy link
Member

fvictorio commented May 26, 2021

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 fvictorio added type:bug Something isn't working package:hardhat-core labels May 26, 2021
@Endlessline
Copy link

How can one recreate this bug?

@fvictorio
Copy link
Member Author

fvictorio commented May 26, 2021

// 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 fvictorio added status:ready This issue is ready to be worked on status:blocked Blocked by other issues or external reasons and removed package:hardhat-core status:ready This issue is ready to be worked on labels Dec 22, 2022
@BitMaster001
Copy link

+1

4 similar comments
@nekooei
Copy link

nekooei commented Jun 5, 2023

+1

@slvrfn
Copy link

slvrfn commented Jun 6, 2023

+1

@vinkent420
Copy link

+1

@nxqbao
Copy link

nxqbao commented Aug 8, 2023

+1

@alcuadrado alcuadrado added type:feature Feature request and removed type:bug Something isn't working labels Aug 8, 2023
@alcuadrado
Copy link
Member

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.

@liu-zhipeng
Copy link

+1

4 similar comments
@gnarvaja
Copy link

+1

@danilaplee
Copy link

+1

@aaitor
Copy link

aaitor commented Nov 21, 2024

+1

@jadrvel
Copy link

jadrvel commented Feb 10, 2025

+1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:blocked Blocked by other issues or external reasons type:feature Feature request
Projects
Status: No status
Development

No branches or pull requests