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

chore: improve Retry usage and warning #9503

Merged
merged 1 commit into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

33 changes: 24 additions & 9 deletions crates/common/src/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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),
Expand All @@ -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,
);
}
}
10 changes: 4 additions & 6 deletions crates/forge/tests/cli/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}");
Expand All @@ -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);
Expand Down
78 changes: 36 additions & 42 deletions crates/verify/src/etherscan/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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 [{}] {}.",
Expand Down Expand Up @@ -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")
Expand Down
7 changes: 4 additions & 3 deletions crates/verify/src/retry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down
16 changes: 10 additions & 6 deletions crates/verify/src/sourcify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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!(
Expand All @@ -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,
);
}
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion crates/verify/src/verify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading