Circular dependency not detected by forge

[doerfli]

Component

Forge

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

• [X] Foundry
• [X] Foundryup

What version of Foundry are you on?

forge 0.2.0 (3e3b30c 2024-08-15T00:20:11.258291162Z)

What command(s) is the bug in?

forge build

Operating System

Linux

Describe the bug

Hello!

I have a situation on our repository where the forge compiler (forge build) does not detect a circular dependency in the smart contracts.

Repository URL: https://github.com/etherisc/gif-next Branch: bugfix/circular-dependency-riskset

The contract RiskSet (https://github.com/etherisc/gif-next/blob/bugfix/circular-dependency-riskset/contracts/instance/RiskSet.sol) inherits from the contract ObjectSet (https://github.com/etherisc/gif-next/blob/bugfix/circular-dependency-riskset/contracts/instance/base/ObjectSet.sol). ObjectSet has a internal member _instance which is of type IInstance (https://github.com/etherisc/gif-next/blob/bugfix/circular-dependency-riskset/contracts/instance/IInstance.sol) which has a dependency on RiskSet.

When running forge compile everything compiles and is deployable without further issues. But when I deployed on Polygon Amoy and started a code verification, i got the error Definition of base has to precede definition of derived when verifying contract TargetManagerLib (https://github.com/etherisc/gif-next/blob/bugfix/circular-dependency-riskset/contracts/staking/TargetManagerLib.sol) which through some other contract depends on RiskSet. If found this very peculiar and it took me a long time to figure out what was wrong, but in the end when opening the contract RiskSet in VSCode (use devcontainer of the project to reproduce easily) i saw the error message locally too (Definition of base has to precede definition of derived). But only as a red underlined error in the VSCode ui, not as a compiler error from forge. After some back and forth i detected above mentioned circular dependency and then i was able to resolve it.

Now interestingly the hardhat compiler also does not detect the issue so i assume its somehow related to the configuration and probably the compiler is able to straighten this out locally. But since the error finally caught up with us during code verification and then locally in the IDE (but only the IDE error display, not the compiler) it was really tricky to figure out. I am unsure if this is a bug in the compiler or if this is just some state that would warrant a warning and polygonscan complains about this and shouldn't. It really confused me and that why i decided to open up an issue and hope you can give me some insight on this? Is there maybe a configuration that would allow me to detect such errors locally using the compiler too?

Please let me know if you need any more information or if there is any way i can further support with analysing the issue.

Thanks for your insight and help.

PS yes, i know the code base is huge and therefor its really hard to strip it down to a small example that is easily read, but i hope by mentioning the 4 contracts that product the loop you can still understand it.

[klkvr]

did this error occur during forge verify-contract? wondering what verification process looked like

[doerfli]

did this error occur during forge verify-contract? wondering what verification process looked like

No, i've used @nomicfoundation/hardhat-verify to verify the contracts and send them to the polygonscan api. This is the code to trigger verification (https://github.com/etherisc/gif-next/blob/bugfix/circular-dependency-riskset/scripts/libs/verification.ts#L51).

But actually the focus my request is less on the verification failure itself (thats due to the code having a cyclic dependency) and more on why the forge compiler did not report this dependency.

I already thought that maybe the compiler clever enough to resolve this issue on its own, which is very convenient for me as a compiler user since i get less errors. But it was really annoying to figure out what was wrong, because no error was reported at all (not even a warning) by the compiler. Also the error report from the polygon api was very unspecific (which file, etc). It would have been very helpful to be able to detect this locally before verification. Maybe there could be a flag to bring the compilter into the same mode that polygonscan uses (i have no idea what the difference between local compiler and polygonscan api compiler).

[klkvr]

Solidity compiler allows circular dependencies. The issue you are getting (Definition of base has to precede definition of derived) tells that contracts are ordered incorrectly in a single file. You can reproduce it without any circular dependencies by trying to compile file like this:

contract A is B {}
contract B {}

I've personally seen such issues occur in situations when trying to flatten a source file with circular dependencies, various tooling might have issues with it because of impossibility to perform topological sort of such dependency graph. So my guess would be that either hardhat or polygonscan is manipulating/reordering/flattening sources somehow changing them vs what compiler sees when you just run forge build, and thus resulting in this error.

Given that there's no issue with foundry here and I've verified that files are getting compiled correctly, I will close this issue. It would be awesome if you could try out forge verify-contract for verifying the contract you've had issues with, and if you'll still get the same error please reopen this issue

[doerfli]

thanks for looking into this and giving some insight on what might be the issue. ill try investigating further on the hardhat side.