chore: improve Retry usage and warning (#9503)

crates/common/src/retry.rs

@@ -16,23 +16,28 @@ pub enum RetryError<E = Report> {
 #[derive(Clone, Debug)]
 pub struct Retry {
     retries: u32,
-    delay: Option<Duration>,
+    delay: Duration,
 }
 
 impl Retry {
     /// Creates a new `Retry` instance.
-    pub fn new(retries: u32, delay: Option<Duration>) -> Self {
+    pub fn new(retries: u32, delay: Duration) -> Self {
         Self { retries, delay }
     }
 
+    /// Creates a new `Retry` instance with no delay between retries.
+    pub fn new_no_delay(retries: u32) -> Self {
+        Self::new(retries, Duration::ZERO)
+    }
+
     /// Runs the given closure in a loop, retrying if it fails up to the specified number of times.
     pub fn run<F: FnMut() -> Result<T>, T>(mut self, mut callback: F) -> Result<T> {
         loop {
             match callback() {
                 Err(e) if self.retries > 0 => {
                     self.handle_err(e);
-                    if let Some(delay) = self.delay {
-                        std::thread::sleep(delay);
+                    if !self.delay.is_zero() {
+                        std::thread::sleep(self.delay);
                     }
                 }
                 res => return res,
@@ -51,8 +56,8 @@ impl Retry {
             match callback().await {
                 Err(e) if self.retries > 0 => {
                     self.handle_err(e);
-                    if let Some(delay) = self.delay {
-                        tokio::time::sleep(delay).await;
+                    if !self.delay.is_zero() {
+                        tokio::time::sleep(self.delay).await;
                     }
                 }
                 res => return res,
@@ -71,8 +76,8 @@ impl Retry {
             match callback().await {
                 Err(RetryError::Retry(e)) if self.retries > 0 => {
                     self.handle_err(e);
-                    if let Some(delay) = self.delay {
-                        tokio::time::sleep(delay).await;
+                    if !self.delay.is_zero() {
+                        tokio::time::sleep(self.delay).await;
                     }
                 }
                 Err(RetryError::Retry(e) | RetryError::Break(e)) => return Err(e),
@@ -82,7 +87,17 @@ impl Retry {
     }
 
     fn handle_err(&mut self, err: Error) {
+        debug_assert!(self.retries > 0);
         self.retries -= 1;
-        let _ = sh_warn!("{} ({} tries remaining)", err.root_cause(), self.retries);
+        let _ = sh_warn!(
+            "{msg}{delay} ({retries} tries remaining)",
+            msg = crate::errors::display_chain(&err),
+            delay = if self.delay.is_zero() {
+                String::new()
+            } else {
+                format!("; waiting {} seconds before trying again", self.delay.as_secs())
+            },
+            retries = self.retries,
+        );
     }
 }

crates/forge/tests/cli/verify.rs

@@ -75,9 +75,8 @@ contract Verify is Unique {
 
 #[allow(clippy::disallowed_macros)]
 fn parse_verification_result(cmd: &mut TestCommand, retries: u32) -> eyre::Result<()> {
-    // give etherscan some time to verify the contract
-    let retry = Retry::new(retries, Some(Duration::from_secs(30)));
-    retry.run(|| -> eyre::Result<()> {
+    // Give Etherscan some time to verify the contract.
+    Retry::new(retries, Duration::from_secs(30)).run(|| -> eyre::Result<()> {
         let output = cmd.execute();
         let out = String::from_utf8_lossy(&output.stdout);
         println!("{out}");
@@ -94,9 +93,8 @@ fn parse_verification_result(cmd: &mut TestCommand, retries: u32) -> eyre::Resul
 
 fn await_verification_response(info: EnvExternalities, mut cmd: TestCommand) {
     let guid = {
-        // give etherscan some time to detect the transaction
-        let retry = Retry::new(5, Some(Duration::from_secs(60)));
-        retry
+        // Give Etherscan some time to detect the transaction.
+        Retry::new(5, Duration::from_secs(60))
             .run(|| -> eyre::Result<String> {
                 let output = cmd.execute();
                 let out = String::from_utf8_lossy(&output.stdout);

crates/verify/src/etherscan/mod.rs

@@ -15,14 +15,10 @@ use foundry_block_explorers::{
     Client,
 };
 use foundry_cli::utils::{get_provider, read_constructor_args_file, LoadConfig};
-use foundry_common::{
-    abi::encode_function_args,
-    retry::{Retry, RetryError},
-};
+use foundry_common::{abi::encode_function_args, retry::RetryError};
 use foundry_compilers::{artifacts::BytecodeObject, Artifact};
 use foundry_config::{Chain, Config};
 use foundry_evm::constants::DEFAULT_CREATE2_DEPLOYER;
-use futures::FutureExt;
 use regex::Regex;
 use semver::{BuildMetadata, Version};
 use std::{fmt::Debug, sync::LazyLock};
@@ -77,8 +73,9 @@ impl VerificationProvider for EtherscanVerificationProvider {
 
         trace!(?verify_args, "submitting verification request");
 
-        let retry: Retry = args.retry.into();
-        let resp = retry
+        let resp = args
+            .retry
+            .into_retry()
             .run_async(|| async {
                 sh_println!(
                     "\nSubmitting verification for [{}] {}.",
@@ -157,48 +154,45 @@ impl VerificationProvider for EtherscanVerificationProvider {
             args.etherscan.key().as_deref(),
             &config,
         )?;
-        let retry: Retry = args.retry.into();
-        retry
-            .run_async_until_break(|| {
-                async {
-                    let resp = etherscan
-                        .check_contract_verification_status(args.id.clone())
-                        .await
-                        .wrap_err("Failed to request verification status")
-                        .map_err(RetryError::Retry)?;
-
-                    trace!(?resp, "Received verification response");
-
-                    let _ = sh_println!(
-                        "Contract verification status:\nResponse: `{}`\nDetails: `{}`",
-                        resp.message,
-                        resp.result
-                    );
+        args.retry
+            .into_retry()
+            .run_async_until_break(|| async {
+                let resp = etherscan
+                    .check_contract_verification_status(args.id.clone())
+                    .await
+                    .wrap_err("Failed to request verification status")
+                    .map_err(RetryError::Retry)?;
 
-                    if resp.result == "Pending in queue" {
-                        return Err(RetryError::Retry(eyre!("Verification is still pending...",)))
-                    }
+                trace!(?resp, "Received verification response");
 
-                    if resp.result == "Unable to verify" {
-                        return Err(RetryError::Retry(eyre!("Unable to verify.",)))
-                    }
+                let _ = sh_println!(
+                    "Contract verification status:\nResponse: `{}`\nDetails: `{}`",
+                    resp.message,
+                    resp.result
+                );
 
-                    if resp.result == "Already Verified" {
-                        let _ = sh_println!("Contract source code already verified");
-                        return Ok(())
-                    }
+                if resp.result == "Pending in queue" {
+                    return Err(RetryError::Retry(eyre!("Verification is still pending...")))
+                }
 
-                    if resp.status == "0" {
-                        return Err(RetryError::Break(eyre!("Contract failed to verify.",)))
-                    }
+                if resp.result == "Unable to verify" {
+                    return Err(RetryError::Retry(eyre!("Unable to verify.")))
+                }
 
-                    if resp.result == "Pass - Verified" {
-                        let _ = sh_println!("Contract successfully verified");
-                    }
+                if resp.result == "Already Verified" {
+                    let _ = sh_println!("Contract source code already verified");
+                    return Ok(())
+                }
 
-                    Ok(())
+                if resp.status == "0" {
+                    return Err(RetryError::Break(eyre!("Contract failed to verify.")))
+                }
+
+                if resp.result == "Pass - Verified" {
+                    let _ = sh_println!("Contract successfully verified");
                 }
-                .boxed()
+
+                Ok(())
             })
             .await
             .wrap_err("Checking verification result failed")

crates/verify/src/retry.rs

@@ -35,9 +35,10 @@ impl Default for RetryArgs {
     }
 }
 
-impl From<RetryArgs> for Retry {
-    fn from(r: RetryArgs) -> Self {
-        Self::new(r.retries, Some(Duration::from_secs(r.delay as u64)))
+impl RetryArgs {
+    /// Converts the arguments into a `Retry` instance.
+    pub fn into_retry(self) -> Retry {
+        Retry::new(self.retries, Duration::from_secs(self.delay as u64))
     }
 }
 

crates/verify/src/sourcify.rs

@@ -5,7 +5,7 @@ use crate::{
 use alloy_primitives::map::HashMap;
 use async_trait::async_trait;
 use eyre::Result;
-use foundry_common::{fs, retry::Retry};
+use foundry_common::fs;
 use futures::FutureExt;
 use reqwest::Url;
 use serde::{Deserialize, Serialize};
@@ -36,8 +36,9 @@ impl VerificationProvider for SourcifyVerificationProvider {
 
         let client = reqwest::Client::new();
 
-        let retry: Retry = args.retry.into();
-        let resp = retry
+        let resp = args
+            .retry
+            .into_retry()
             .run_async(|| {
                 async {
                     sh_println!(
@@ -56,7 +57,9 @@ impl VerificationProvider for SourcifyVerificationProvider {
                     if !status.is_success() {
                         let error: serde_json::Value = response.json().await?;
                         eyre::bail!(
-                            "Sourcify verification request for address ({}) failed with status code {status}\nDetails: {error:#}",
+                            "Sourcify verification request for address ({}) \
+                             failed with status code {status}\n\
+                             Details: {error:#}",
                             args.address,
                         );
                     }
@@ -72,8 +75,9 @@ impl VerificationProvider for SourcifyVerificationProvider {
     }
 
     async fn check(&self, args: VerifyCheckArgs) -> Result<()> {
-        let retry: Retry = args.retry.into();
-        let resp = retry
+        let resp = args
+            .retry
+            .into_retry()
             .run_async(|| {
                 async {
                     let url = Url::from_str(

crates/verify/src/verify.rs

@@ -50,7 +50,7 @@ impl Default for VerifierArgs {
     }
 }
 
-/// CLI arguments for `forge verify`.
+/// CLI arguments for `forge verify-contract`.
 #[derive(Clone, Debug, Parser)]
 pub struct VerifyArgs {
     /// The address of the contract to verify.