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

feat(improvement): identify contracts by hash, not bytecode #9443

Open
DaniPopes opened this issue Dec 1, 2024 · 1 comment
Open

feat(improvement): identify contracts by hash, not bytecode #9443

DaniPopes opened this issue Dec 1, 2024 · 1 comment
Labels
A-internals Area: internals Cmd-forge-coverage Command: forge coverage T-feature Type: feature

Comments

@DaniPopes
Copy link
Member

DaniPopes commented Dec 1, 2024

Component

Forge

Describe the feature you would like

We already collect coverage hitmaps by bytecode hash, we can use this to avoid the hacky heuristics of finding a matching artifact using the bytecode itself.

  • In this loop, the key is the bytecode hash, which is currently ignored:
    for map in hit_maps.0.values() {
  • We try to find a match using bytecodes with ContractsByArtifact::find_by*
    if let Some((id, _)) = known_contracts.find_by_deployed_code(&map.bytecode) {
    hits.push((id, map, true));
    } else if let Some((id, _)) =
    known_contracts.find_by_creation_code(&map.bytecode)
    {
    hits.push((id, map, false));
    }

This also applies to all the other callsites of these functions, except for the getArtifactPathBy[Deployed]Code cheatcodes, where we would have to compute the hash ourselves.

By computing the hashes of the bytecodes and looking up the artifacts by code hash, we will be able to get rid of the find_by*code functions altogether.

cc @klkvr @grandizzy

Additional context

No response

@DaniPopes DaniPopes added T-feature Type: feature Cmd-forge-coverage Command: forge coverage labels Dec 1, 2024
@github-project-automation github-project-automation bot moved this to Todo in Foundry Dec 1, 2024
@DaniPopes DaniPopes changed the title coverage: identify contracts by hash, not bytecode Identify contracts by hash, not bytecode Dec 1, 2024
@zerosnacks zerosnacks added the A-internals Area: internals label Dec 2, 2024
@zerosnacks zerosnacks changed the title Identify contracts by hash, not bytecode feat(improvement): identify contracts by hash, not bytecode Dec 2, 2024
@klkvr
Copy link
Member

klkvr commented Dec 2, 2024

I don't think we can rely on pre-computed bytecode hashes for artifacts due to immutables?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-internals Area: internals Cmd-forge-coverage Command: forge coverage T-feature Type: feature
Projects
Archived in project
Development

No branches or pull requests

3 participants