crytic / crytic-compile

Abstraction layer for smart contract build systems
GNU Affero General Public License v3.0
154 stars 83 forks source link

[Bug-Candidate]: Slither Vyper feature fails if custom interfaces are imported #507

Open pcaversaccio opened 1 year ago

pcaversaccio commented 1 year ago

Describe the issue:

Hey @0xalpharush great job with the Vyper feature! I'm currently testing the functionalities and I encountered parsing errors if the Vyper contract imports custom interfaces, as I do a lot in 🐍 snekmate. For example, take a look at AccessControl.vy, where I import the custom interface IAccessControl:

import interfaces.IAccessControl as IAccessControl
implements: IAccessControl

Running slither AccessControl.vy will lead to the following error:

Traceback (most recent call last):
...
    raise InvalidCompilation(vyper_standard_output["errors"])
crytic_compile.platform.exceptions.InvalidCompilation: [{'component': 'parser', 'message': "Cannot locate interface 'interfaces/IAccessControl{.vy,.json}'", 'severity': 'error', 'sourceLocation': {'file': 'AccessControl.vy'}, 'type': 'FileNotFoundError'}]

Code example to reproduce the issue:

See above. Or any other contract in 🐍 snekmate that uses custom interface imports.

Version:

0.10.0

Relevant log output:

No response

0xalpharush commented 1 year ago

Right now, crytic-compile isn't taking into account imports when it creates that standard json input, but this is definitely something we want to improve (for solidity as well). Ideally, we'd be able to delegate to something like Ape... In the meantime, I believe it should work if you do slither auth/.

pcaversaccio commented 1 year ago

In the meantime, I believe it should work if you do slither auth/.

Well, it doesn't because AccessControl can't be compiled due the interface import. All contracts in snekmate that use interface imports don't work for the same reason.

0xalpharush commented 1 year ago

My mistake. The ability to standalone compile with a glob target the recurses into subfolders isn't yet implemented. It caused issues with projects that mix Solidity and Vyper as it requires us to be able to compile an arbitrary code base (resolving imports, dependencies, version conflicts, etc). Some projects also mixed vyper versions and we only added support for 0.3.7 https://github.com/crytic/crytic-compile/blob/3a4b0de72ad418b60b9ef8c38d7de31ed39e3898/crytic_compile/crytic_compile.py#L741-L744

0xalpharush commented 1 year ago

For reference here is the issue on getting ape to support what other compilation frameworks provide https://github.com/ApeWorX/ape/issues/1590

pcaversaccio commented 1 year ago

Just as a heads-up, @charles-cooper plans to remove standard json input in a future release. The issue is that the standard json input basically defines an embedded filesystem which is confusing to interpret, especially in the presence of imports. But most probably there will another way to specify compiler settings using json.

charles-cooper commented 1 year ago

Just as a heads-up, @charles-cooper plans to remove standard json input in a future release.

Thinking about it, have not committed to a decision here yet.

0xalpharush commented 1 year ago

I experimented with automatically resolving the imports but crytic-compile/ slither both expect absolute paths while Vyper seems to expect the standard json to have something like interfaces/IAccessControl in the sources object for the import. Solidity gets around this with remappings but I'm not exactly thrilled with that outcome