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

Adding more content to the compilation artifacts output #705

Closed
montyly opened this issue Feb 8, 2022 · 15 comments · Fixed by #762
Closed

Adding more content to the compilation artifacts output #705

montyly opened this issue Feb 8, 2022 · 15 comments · Fixed by #762
Labels
C-forge Command: forge Cmd-forge-build Command: forge build P-normal Priority: normal T-feature Type: feature

Comments

@montyly
Copy link

montyly commented Feb 8, 2022

Component

Forge

Describe the feature you would like

Hi,

Running forge build leads to generating compilation artifacts in out with only abi/bytecode/deployed_bytecode. This limits the integration with other tools (like Slither, Echidna etc..).

Would it be possible to generate all the compilation artifacts? (or to have a flag for it)? The ones that are needed for crytic-tools are (and probably for most of the other third parties integration):

  • abi
  • ast
  • bin
  • bin-runtime
  • srcmap
  • srcmap-runtime
  • userdoc
  • devdoc
  • hashes [from 0.4.12]
  • compact-format [from 0.4.12, and until 0.8.10, after that this was dropped as the legacy ast format was removed]

I tried with --extra-output ast but it did not work. However because some of these flags depend on the compiler version, it would be great to let foundry figure out which ones are available.

Related: crytic/crytic-compile#230

Additional context

No response

@montyly montyly added the T-feature Type: feature label Feb 8, 2022
@gakonst
Copy link
Member

gakonst commented Feb 8, 2022

I think this should be possible via foundry toml? Cc @mattsse

@sambacha
Copy link
Contributor

sambacha commented Feb 9, 2022

this question was also asked in the support chat today by me and another individual.

I need to output the metadata.json so that I can verify on sourcify

@montyly
Copy link
Author

montyly commented Feb 11, 2022

@gakonst it would be great to have a flag's integration instead of the config file, as it would allow smooth third parties integration.
Otherwise, it would mean that every foundry's user that wants to use Slither need to update their config file (as opposed to a flag that Slither will directly use when running foundry)

@sambacha
Copy link
Contributor

@gakonst it would be great to have a flag's integration instead of the config file, as it would allow smooth third parties integration.

Otherwise, it would mean that every foundry's user that wants to use Slither need to update their config file (as opposed to a flag that Slither will directly use when running foundry)

using the flags as-is is flakey.

#718

It will be a very very nested toml file if you want to configure it using some of the more nested options too.

@sambacha
Copy link
Contributor

Metadata is also needed for verification

@mattsse
Copy link
Member

mattsse commented Feb 11, 2022

better metadata bindings have been added to ethers-rs gakonst/ethers-rs#894

Is there any (good) reason we should support configuring the output on a file or contract basis?
I feel only supporting { "*": { "*": [<contract level settings>] } } is reasonable

this should be either a list of

output_selection = ["ir", "storageLayout", "evm",...]

or
output_selection = ["all"] which is { "*": { "*": [ "*" ], "": [ "*" ] } }

From the docs, the only file level config is ast, right?

what if we make a
metadata = true field to

  1. set "metadata" field in output selection
  2. write a metadata.json file for every contract

@sambacha ?

@gakonst
Copy link
Member

gakonst commented Feb 11, 2022

+1 on @montyly's point on also having a flag on the CLI to allow doing that without a Config file

@sambacha
Copy link
Contributor

better metadata bindings have been added to ethers-rs gakonst/ethers-rs#894

Is there any (good) reason we should support configuring the output on a file or contract basis? I feel only supporting { "*": { "*": [<contract level settings>] } } is reasonable

this should be either a list of

output_selection = ["ir", "storageLayout", "evm",...]

or output_selection = ["all"] which is { "*": { "*": [ "*" ], "": [ "*" ] } }

From the docs, the only file level config is ast, right?

what if we make a metadata = true field to

  1. set "metadata" field in output selection
  2. write a metadata.json file for every contract

@sambacha ?

The reason we would want this is for verification, which is dependent on how etherscan consumes this. I would guess that they normally get 1 metadata.json file, as opposed to 1 for every contract. I will email their support and ask, thanks @mattsse

@gakonst
Copy link
Member

gakonst commented Feb 17, 2022

@montyly should be fixed now! Will be released on today's nightly. From @mattsse's PR:

--extra-output <metadata|ir|ir-optimized|...> will add the provided selection to the contract's json artifact.

@montyly
Copy link
Author

montyly commented Mar 14, 2022

I am not sure that @762 fixed this issue. I can't find a way to generate ast, or hashes (but I can generate all the others). Am I missing something?

For ast, it would be great if compact-format was enabled for solc from 0.4.12, and until 0.8.10.

@mattsse
Copy link
Member

mattsse commented Mar 14, 2022

wrt to hashes, do you mean evm.methodIdentifiers? https://docs.soliditylang.org/en/v0.4.12/using-the-compiler.html#input-description

The ast can only be set for the whole solc CompilerInput, it's enabled by default but we currently do not move the ast, which is stored separately in the CompilerOutput to the Artifact.
will add this shortly.

compact-format

you mean the solc --ast-compact-json argument?

@mattsse mattsse reopened this Mar 14, 2022
@montyly
Copy link
Author

montyly commented Mar 14, 2022

Great, thanks.

For hashes: yes it is evm.methodIdentifiers; this one actually already works in forge with this name (I got confused because hashes is the naming with solc's --combined-json)

For the compact format, yes it is --ast-compact-json (or compact-format through --combined-json)

@sambacha
Copy link
Contributor

sambacha commented Apr 5, 2022

just to be sure, the JSON interface returns contract metadata in case of an internal compiler error while the CLI interface does not for solc.

@onbjerg
Copy link
Member

onbjerg commented Apr 16, 2022

@montyly You can now get ast and evm.methodIdentifiers as well - good to close? 😄

@onbjerg onbjerg added this to Foundry Apr 17, 2022
@onbjerg onbjerg moved this to Todo in Foundry Apr 17, 2022
@onbjerg onbjerg moved this from Todo to May be solved in Foundry Apr 18, 2022
@gakonst
Copy link
Member

gakonst commented Apr 18, 2022

I think so - here's a slither job example https://github.com/thirdweb-dev/contracts/blob/main/.github/workflows/slither.yml#L27-L43. feel free to re-open if still an issue

@gakonst gakonst closed this as completed Apr 18, 2022
Repository owner moved this from May be solved to Done in Foundry Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-forge Command: forge Cmd-forge-build Command: forge build P-normal Priority: normal T-feature Type: feature
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants