Shaping Aderyn's dependency on end user's environment

[TilakMaddy]

This is in regards to helping build the VSCode extension - we want to make it easy not just for maintaining the official one, but also for other devs in the community to build their extension using this Aderyn as the backend

The challenge is to support as many situations (foundry, hardhat, no framework) as possible without asking for too much from the end users (apart from installing Aderyn in their CLI)

Maybe we don't have a solution yet, but I'd like for us to list out here -

a) problems (in detail) with the current system
b) approaches (in detail) we may have to mitigate them
c) pros and cons of each approach
d) How the proposed solution might break the existing guarantees (if any)

[TilakMaddy]

Something that I found - (might be valuable in making decisions)

So here, in the foundry-compilers backend we see that it shells out solc to compile the source files.

https://github.com/foundry-rs/compilers/blob/342f713da41c9738c6e775b7daf0bf6827041cde/src/compile/mod.rs#L570

The question then is, what might we gain from adding forge as a rust dependency if it relies on solc being present to compile the files?

UPDATE

My guess is that you'll be able to build json from solidity files in project where there is no framework used? (maybe)

[TilakMaddy]

For the case when no framework is used, the first challenge is the generation of the AST! And one solution is to assume solc exists in the env. Without that we have to rely on https://github.com/axic/solc-rust but that already looks outdated : (

(doesn't even support solidity 0.6)

[TilakMaddy]

For Hardhat support, I was toying with the idea of requesting users to specify 2 things explicitly to aderyn (in cases where sources is not contracts and out is not build-artifacts)-

1. Source folder (where all the contracts are)
2. Out folder (where the compiled .jsons are)

Rationale behind this:
we cannot parse hardhart.config.ts to find out the above. (It's a .js file, not a .json file). So we get the info through maybe env variables like ADERYN_SRC, ADERYN_OUT

So when these 2 environment vars are present the understanding is, compilation has already been done and aderyn just has to run the detectors.

[alexroan]

The ideal scenario of what aderyn should do:

• Recursively find all solidity files within a folder structure (and probably read remappings so that imports work properly)
  • take into account scope, exclude, other args that we do now
• Compile the AST only solc --ast-compact-json <FILENAME.sol>
  • This takes fractions of the time it takes to fully compile the files
  • Doesn't need to print to disk, can all be in memory
  • If we can find a way to use forge's compiler library (to take advantage of many compiler versions logic), hack it slightly to only use the AST json, this should work!
• Run detectors as usual

This removes any need for ANY local framework. It can be run on a folder of solidity files only.
It also removes the need to print any AST or output to disk, so no .gitignores, output folder handling, etc.
aderyn watch becomes a far more attainable target.
the VSCode extension naturally works more seamlessly from this setup.

@TilakMaddy

[alexroan]

My concern with --stop-after parsing is that many of the IDs of nodes used in sub-contracts are defined in base contracts that are imported. We would ideally like to keep that context.

if we don't include that, we lose the ability to write a lot of potential detectors (like unused imports, for example).

[TilakMaddy]

The disadvantage is that node IDs get repeated if the input files are not part of the same command

Hmmmm, this will break everything downstream.

use the 'solc' module from foundry-compilers' with utilities that will install the correct solidity compiler based on the pragma version contract source code. Then, spin our own solc` command (for that we don't have to rely on foundry-compilers backend)

If this spins our own solc command with every solidity file, does that resolve the repeated Node IDS issue too?

Hey there Alex ! Checkout "The Purge - II". What we do is club solidity files into "compilation groups" . Each group has a bunch of solidity files and is compatible with 1 solc version. So we run solc command once for every group

[TilakMaddy]

If we spin up solc once for every file, each AST will start from node ID 1. Therefore too many colliaions.

[TilakMaddy]

The disadvantage is that node IDs get repeated if the input files are not part of the same command

Hmmmm, this will break everything downstream.

use the 'solc' module from foundry-compilers' with utilities that will install the correct solidity compiler based on the pragma version contract source code. Then, spin our own solc` command (for that we don't have to rely on foundry-compilers backend)

If this spins our own solc command with every solidity file, does that resolve the repeated Node IDS issue too?

Hey there Alex ! Checkout "The Purge - II". What we do is club solidity files into "compilation groups" . Each group has a bunch of solidity files and is compatible with 1 solc version. So we run solc command once for every group

Another thing I've included in The Purge II is that we have multiple contexts generated for each project.( 1 for every compilation group ) And the detectors are made to detect on every context. Therefore node ID collisions no longer bother us

[TilakMaddy]

My concern with --stop-after parsing is that many of the IDs of nodes used in sub-contracts are defined in base contracts that are imported. We would ideally like to keep that context.

if we don't include that, we lose the ability to write a lot of potential detectors (like unused imports, for example).

I agree. Also, we require the scope field in nodes which cannot be resolved when stopped at the parsing stage.

[TilakMaddy]

Could it be that there is a plain yul file in the solidity codebase that needs to be compiled along with the other files?

[alexroan]

There may well be, but I think it will need to be wrapped in a solidity contract. We should confirm this though

[TilakMaddy]

Do we know if zksync solidity projects can be scanned with just solc instead of zksooc for aderyn.... (Because we are okay with just the first step i.e AST).... We don't necessarily have to get to the binary ??

Am i thinking correctly? 🤔

[alexroan]

AFAIK we don't need the binary, no.

ZKSolc takes the output of solc and converts the opcodes into ZK opcodes. Aderyn doesn't operate on this level 👍

[TilakMaddy]

Second thoughts regarding supporting reading scope, exclude, remappings from framework files likefoundry.toml.

@alexroan I know I said although we're not relying on forge command, we must auto-fill the above 3 parameters from files like foundry.toml. But after spending a few minutes getting into it, it's become more clear to me what you said in the beginning which is that we are becoming more framework dependent (more like framework associated I guess) , and 6 months down the line we may have to re write a lot of code. I also think by then people would have been very comfortable with it and will be hard to ask them to specify those values exclusively.

Also, hardhat is hard to integrate because all the config is in a *.ts or *.js file, not .json or .toml file that we can easily parse and read. I would much rather not associate with any framework whatsoever.

So here's a solution that I am proposing:

1. For remappings, just keep it simple and demand remappings or remappings.txt in the project root
2. Regarding scope and exclude, we will take them as input in the vscode sidebar (I'm looking into it)
3. Aderyn Watch button would also lie in the sidebar. (After filling the above inputs, users should click watch)

How do you feel about all this?
Please let me know

[TilakMaddy]

Here's the consequences to existing users:

Users running aderyn CLI would have to change their way of running the command to aderyn --scope src/ --exclude lib/
(or maybe we can just have "src" and "lib" as defaults when foundry.toml is present but make it clear that it is not deciding anything by reading from foundry.toml

On the flip side, aderyn

• Doesn't have to auto-detect profile, fallback values, etc from foundry.toml
• Doesn't have to rely on being able to forge remappings > remappings when there is a foundry.toml
• Doesn't have to rely on logic of loading exclude from lib values of all profiles and scope value from src of only the currently chosen foundry profile (if not fallback to foundry's env)(other such tricks and hacks)

[TilakMaddy]

Like this @alexroan

[TilakMaddy]

Exploring https://github.com/notify-rs/notify/blob/main/examples/monitor_raw.rs https://github.com/notify-rs/notify/blob/main/examples/debouncer_full.rs for watching file changes

[TilakMaddy]

I am starting to think we don't have to literally merge icf to dev. We can just

1. bring back the process_foundry.rs (which can create a context from reading json out files)
2. bring an extra drive_with_deprecated() function (which will run forge build --ast)

We should more or less have what we have in dev. But hey I could be wrong :P Going through the process will only reveal !

[alexroan]

Oooo this sounds like a good idea actually.

[TilakMaddy]

@alexroan We're doing that here #433 ! (We didn't even need a new drive_with_depracated() function : )

[TilakMaddy]

I think we can merge #333 to dev 🚀 .

Existing PRs to ICF can be made to dev.

[TilakMaddy]

(If we're okay with not supporting hardhat like we used to + single file) ... which i want to argue that they were never there 😅 in the first place

[TilakMaddy]

UPDATE: UX improvements needed. HAPPY PATHS all work! but errors aren't too helpful.

Example - If we don't currently have --icf we just assume it's foundry. That's wrong. We need to bring back the checks as well.
I will send a PR

[TilakMaddy]

Thanks for participating 🙏

I am closing this in favor of a new one focusing on how Aderyn can be designed to help VSCode. We have more or less solved the user environment dependency problem in 0.1.0 :)