naddison36 / sol2uml

Solidity contract visualisation tool
MIT License
1.13k stars 268 forks source link

Topological sort misbehavior on `flatten` #102

Closed plotchy closed 1 year ago

plotchy commented 1 year ago

I ran into a bug while using sol2uml flatten and I believe it has to do with the topological sort implementation over the contract dependencies. This bug makes the contract fail to compile as the import is not defined before the usage of it.

Command to reproduce

sol2uml flatten 0xd154D208cC753bCF8006577C350c1EE732ADAdfa -n polygon

Relevant File Info

// IERC165Upgradeable sorted to top of file -> however, it only exists as a commented out import line
pragma solidity ^0.8.0;

/* import "../utils/introspection/IERC165Upgradeable.sol"; */

// ##-- snipping out ~10 contracts in between relevant info --##

// Near bottom is CrossGovDAO - whcih inherits IERC165Upgradeable. Fails to compile
pragma solidity ^0.8.0;
/* import "@openzeppelin/contracts-upgradeable/interfaces/IERC165Upgradeable.sol"; */
contract CrossGovDAO is OwnableUpgradeable, IDAOCrossGovInterface, IERC165Upgradeable {
}

// Lastly, the real IERC165Upgradeable is sorted to last within the file.
pragma solidity ^0.8.0;
interface IERC165Upgradeable {
    function supportsInterface(bytes4 interfaceId) external view returns (bool);
}

Best Guess

It seems that IERC165Upgradeable may be getting duplicated in the dependency list somehow?

There seems to be two entities of this contract:

  1. The instance that is sorted to the top - containing only the import statement that is later commented out
  2. The instance that is sorted to the bottom - containing the actual interface logic

I'm unsure of how this entity may be getting dupe'd, but if both are sorted to the top this would still be ok & compile :)

naddison36 commented 1 year ago

Thanks for raising this issue.

You are right, the problem is with the IERC165Upgradeable dependency.

file: contracts/CrossGovDAO.sol
import: @openzeppelin/contracts-upgradeable/interfaces/IERC165Upgradeable.sol
contract: CrossGovDAO

file: @openzeppelin/contracts-upgradeable/interfaces/IERC165Upgradeable.sol
import: ../utils/introspection/IERC165Upgradeable.sol

file: @openzeppelin/contracts-upgradeable/utils/introspection/IERC165Upgradeable.sol
interface: IERC165Upgradeable

sol2uml adds the file imports to the UML class in each file, but the @openzeppelin/contracts-upgradeable/interfaces/IERC165Upgradeable.sol file doesn't produce any UML classes so the dependency to the IERC165Upgradeable interface is broken.

I need to think about the best way to resolve this issue.

naddison36 commented 1 year ago

This should be fixed with the release v2.2.1