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(forge): verify broadcasted contracts on forge script #1778

Merged
merged 7 commits into from Jun 1, 2022

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented May 30, 2022

Motivation

Verify CREATE and CREATE2 contracts broadcasted on forge script.

Solution

For CREATE, we just match receipts to transactions, extract the constructor arguments and pass that to VerifyArgs.
For CREATE2, we have to collect and store the created addresses from the on-chain simulation. After that, it's pretty much the same, apart from offsetting the transaction data.

Behaviour:

--broadcast --verify: compiles, runs, broadcasts and verifies after submitting all transactions successfully.
--resume --verify: compiles, broadcasts and verifies after submitting all transactions successfully.
--verify: compiles and verifies existing contracts from receipts.

Working but...

  • If it's a contract inside a standalone script and outside of a project, then VerifyArgs doesn't like it, and says it cannot find it.
  • Etherscan needs a Retry for rate limiting as well, so we can just send everything at once.

@gakonst
Copy link
Member

gakonst commented May 30, 2022

If it's a contract inside a standalone script and outside of a project, then VerifyArgs doesn't like it, and says it cannot find it.

Meh this seems fine, would expect most ppl doing single file scripts don't have that expectation.

Cc @meetmangukiya for adding Retry/rate limit in the etherscan provider

@joshieDo joshieDo marked this pull request as ready for review May 31, 2022
@joshieDo
Copy link
Collaborator Author

joshieDo commented May 31, 2022

In that case, I'm opening it for review, and we could add the etherscan client in another PR.

I'm waiting on #1790 to include something small here, but that's it.

Copy link
Member

@gakonst gakonst left a comment

Should we also add support to --resume so that you can re-run it somehow to verify the already existing contracts?

LGTM otherwise, modulo the merge conflicts

mattsse
mattsse approved these changes Jun 1, 2022
/// Data struct to help `ScriptSequence` verify contracts on `etherscan`.
pub struct VerifyBundle {
pub num_of_optimizations: Option<usize>,
pub known_contracts: BTreeMap<ArtifactId, (Abi, Vec<u8>)>,
pub etherscan_key: Option<String>,
pub project_paths: ProjectPathsArgs,
}
Copy link
Member

@mattsse mattsse Jun 1, 2022

Choose a reason for hiding this comment

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

I'll leave this for a followup to move all script/verify related code from utils to their own module(s)

name: artifact.name.clone(),
};

let verify = verify::VerifyArgs {
Copy link
Member

@mattsse mattsse Jun 1, 2022

Choose a reason for hiding this comment

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

optional followup: impl Default for VerifyArgs to make this a tiny bit nicer,

@mattsse
Copy link
Member

mattsse commented Jun 1, 2022

@joshieDo good to merge?

@mattsse mattsse merged commit 375c39e into foundry-rs:master Jun 1, 2022
9 checks passed
Copy link

@lokera666 lokera666 left a comment

Yup

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.

None yet

4 participants