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: smarter verification #7937

Merged
merged 8 commits into from
May 22, 2024
Merged

feat: smarter verification #7937

merged 8 commits into from
May 22, 2024

Conversation

klkvr
Copy link
Member

@klkvr klkvr commented May 17, 2024

Motivation

There is no need in requiring contract name/path in forge verify-contract. For example, hardhat does not require that.

Solution

This PR introduces smarter way of identifying contracts by deployed code which respects immutable/link references and targets exact bytecode match (ContractsByArtifact::find_by_deployed_code_exact). I believe this could be useful for stuff unrelated to verification as current diff score method is not really great.

Also, this PR makes contract: ContractInfo parameter on VerifyArgs optional, thus allowing to do just forge verify-contract <address>. The artifact to use is determined by matching local bytecode through find_by_deployed_code_exact.

I've also changed flow to not rely on cache (except for determining version by looking at recent artifacts). We now trigger compiler directly if we need to get cached abi or metadata and it just uses cache if it's available instead of throwing a error when run on a project without cache.

In combination with --guess-constructor-args it now allows users to verify deployments of arbitrary project contracts much easier by just doing forge verify-contract <address> --guess-constructor-args

Next steps could probably be identifying libraries used when linking contract automatically as well.

crates/common/src/contracts.rs Outdated Show resolved Hide resolved
crates/common/src/contracts.rs Outdated Show resolved Hide resolved
crates/verify/src/etherscan/flatten.rs Outdated Show resolved Hide resolved
crates/verify/src/etherscan/standard_json.rs Outdated Show resolved Hide resolved
Copy link
Member

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

great, pending @DaniPopes nits

@klkvr klkvr requested a review from DaniPopes May 18, 2024 12:41
/// references and immutables.
pub fn find_by_deployed_code_exact(&self, code: &[u8]) -> Option<ArtifactWithContractRef> {
self.iter().find(|(_, contract)| {
let Some(ref deployed_bytecode) = contract.deployed_bytecode else {
Copy link
Member

Choose a reason for hiding this comment

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

pls use & in the expression instead of ref in the pattern where possible

@mattsse
Copy link
Member

mattsse commented May 22, 2024

eth_feeHistory bug should go away with #7934

@mattsse mattsse merged commit 3e9385b into master May 22, 2024
18 of 20 checks passed
@mattsse mattsse deleted the klkvr/refactor-verify branch May 22, 2024 19:28
klkvr added a commit that referenced this pull request May 24, 2024
* fix: smarter verification

* version build regex

* rm println

* fix tests

* clippy

* review fixes

* fmt

* ref -> &
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants