Skip to content
This repository has been archived by the owner on Oct 19, 2024. It is now read-only.

feat(solc): emit build info files if configured #1338

Merged
merged 3 commits into from
Jun 15, 2022

Conversation

mattsse
Copy link
Collaborator

@mattsse mattsse commented Jun 2, 2022

Motivation

Emit build-info files that include the whole compiler(input, output) and version.

Solution

  • Create bindings for BuildInfo
  • If configured in the project we create a BuildInfo instance for each solc invocation.
  • build-infos are stored as <root>/build-info/<solc>-<timestamp>.json, this differs from hardhat which uses the content hash as file name, like 0.8.14+commit.80d49f37.Darwin.appleclang-2022-06-02T12:17:16.944469+00:00.json

wdyt @tynes ?

PR Checklist

  • Added Tests
  • Added Documentation
  • Updated the changelog

@gakonst
Copy link
Owner

gakonst commented Jun 2, 2022

Do we see this as impacting performance?

@mattsse
Copy link
Collaborator Author

mattsse commented Jun 2, 2022

it's definitely an overhead because a buildinfo object is in the MB and can be 50-100MB or even higher for large projects, so perhaps we should make this opt in via config val in foundry.toml?

@gakonst
Copy link
Owner

gakonst commented Jun 2, 2022

Yeah I'd disable it by default given we only really want it for hardhat-like integrations and only enable via foundry toml / CLI. Are there other use cases beyond Hardhat that we'd want it for? I think @tynes is the main feedback source here?

@tynes
Copy link
Contributor

tynes commented Jun 2, 2022

a buildinfo object is in the MB and can be 50-100MB or even higher for large projects

If we do it based on timestamp, this could use a lot of space really quickly. The main consideration is we need to be able to go from qualified name -> build info file. If we use the timestamp methodology, the hh plugin can sort by timestamp and take the most recent buildinfo file. A way around using a lot of disk would be to have foundry delete the buildinfo file if one exists before writing the next one.

hh maps a bulidinfo + solc version to a hash and uses that as the filename, see: https://github.com/NomicFoundation/hardhat/blob/783650c5711f457ad63cc7eee9ac190d0ff264fc/packages/hardhat-core/src/internal/artifacts.ts#L244
This way the same input results in the same buildinfo file.

It looks like hh solves being able to look up qualified name -> buildinfo file by keeping a json debug file for each artifact that looks like this:

    const debugFile: DebugFile = {
      _format: DEBUG_FILE_FORMAT_VERSION,
      buildInfo: relativePathToBuildInfo,
    };

https://github.com/NomicFoundation/hardhat/blob/783650c5711f457ad63cc7eee9ac190d0ff264fc/packages/hardhat-core/src/internal/artifacts.ts#L295
The name of the debug file is simply the name of the artifact but with .dbg.json, see https://github.com/NomicFoundation/hardhat/blob/783650c5711f457ad63cc7eee9ac190d0ff264fc/packages/hardhat-core/src/internal/artifacts.ts#L580

It seems like the options are:

  • have a 1:1 mapping of buildinfo files based on what was compiled and ensure that can be dealt with in the plugin
    • the plugin currently doesn't have access to the version of solc used
  • create a buildinfo each time and prune the old one

I'm ok with either of these solutions, the first seems like the "better" solution as it would save a bunch of i/o but also more complex.

Note that hh does prune old buildinfo files: https://github.com/NomicFoundation/hardhat/blob/783650c5711f457ad63cc7eee9ac190d0ff264fc/packages/hardhat-core/src/internal/artifacts.ts#L221

Yeah I'd disable it by default given we only really want it for hardhat-like integrations and only enable via foundry toml / CLI. Are there other use cases beyond Hardhat that we'd want it for? I think @tynes is the main feedback source here?

There are other people that have asked for access to the compiler input/output directly according to the tg group, it should def be gated by a flag in the foundry.toml. Another usecase would be cat artifacts/buildinfo/file.json | jq .compilerOutput | abigen --combined-json to build golang bindings and I'm sure other existing tooling (slither?) could take advantage of it

@mattsse
Copy link
Collaborator Author

mattsse commented Jun 3, 2022

f we do it based on timestamp, this could use a lot of space really quickly. The main consideration is we need to be able to go from qualified name -> build info file. If we use the timestamp methodology, the hh plugin can sort by timestamp and take the most recent buildinfo file. A way around using a lot of disk would be to have foundry delete the buildinfo file if one exists before writing the next one.

I used the timestamp due to lack of understanding how the buildInfo Name is created by hh, I guess it's a hash of the buildInfo content?

This way the same input results in the same buildinfo file.

right, I deemed that to be too much overhead, but makes sense now.

The main consideration is we need to be able to go from qualified name -> build info file.

ah now, this makes sense :)

I'm ok with either of these solutions, the first seems like the "better" solution as it would save a bunch of i/o but also more complex.

agreed, let's do option 1.

I think what's missing is to get the build info file at least is just

  • content hashing -> filename

The additional dbg.json would also be doable but would require some followup work

@tynes
Copy link
Contributor

tynes commented Jun 6, 2022

@mattsse
Copy link
Collaborator Author

mattsse commented Jun 7, 2022

oh good to know, we already use that for the content hash I believe, trying to get this over the line today

@mattsse
Copy link
Collaborator Author

mattsse commented Jun 8, 2022

file name is now the content hash of {_format,solcVersion,solcLongVersion,input} same as hh

still needs a followup for:

  • top level dbg.json
  • foundry companion

@tynes
Copy link
Contributor

tynes commented Jun 9, 2022

An additional note: when slither is configured to work with hardhat, it uses the buildinfo files

@smartcontracts
Copy link

hardhat-deploy also relies on buildinfo files for etherscan verification

Copy link
Owner

@gakonst gakonst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mhm seems fine to use, glad it's off by default :)

@gakonst gakonst merged commit e3389f3 into gakonst:master Jun 15, 2022
@tynes
Copy link
Contributor

tynes commented Jun 16, 2022

I can handle the hh plugin implementation of this once forge is updated with the latest ethers + a way to pass through config to trigger the buildinfo being written to disk

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants