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

fix(forge): set script verification retries at 5 with a 5 second delay #2739

Merged
merged 2 commits into from Aug 12, 2022

Conversation

joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Aug 12, 2022

Motivation

Getting quite a few user reports of the verification process failing with:

Error: 
Etherscan could not detect the deployment.

This usually happens when the indexer is too slow on picking up the contract (looking at you rinkeby...).

ref #2435

Solution

While Retry args are available, most users are not aware. And given that we have a lot of flags, it's understandable. Other commands have their own specific defaults for Retry, so I added more reasonable ones to forge script.

@joshieDo joshieDo added T-bug Type: bug Cmd-forge-verify Command: forge verify-contract Cmd-forge-script Command: forge script labels Aug 12, 2022
@joshieDo joshieDo changed the title chore(forge): set script verification retries at 5 with a 5 second delay fix(forge): set script verification retries at 5 with a 5 second delay Aug 12, 2022
@@ -549,6 +549,12 @@ impl VerifyBundle {
config_path: if config_path.exists() { Some(config_path) } else { None },
};

// Default from `RetryArgs` struct.
if retry.delay.is_none() && retry.retries == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

I think we can just add these defaults directly on RetryArgs if they are more sane defaults for the create-related commands

Copy link
Member

Choose a reason for hiding this comment

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

+1 on that, let's just bump the Retry defaults instead

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.

lgtm

@mattsse mattsse merged commit e530c73 into foundry-rs:master Aug 12, 2022
iFrostizz pushed a commit to iFrostizz/foundry that referenced this pull request Nov 9, 2022
foundry-rs#2739)

* 5 retries with 5s delay for forge script verification

* set default on RetryArgs instead
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cmd-forge-script Command: forge script Cmd-forge-verify Command: forge verify-contract T-bug Type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants