Skip to content

refactor(verify): remove redundant logging in Etherscan provider #11048

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

crStiv
Copy link

@crStiv crStiv commented Jul 19, 2025

Removed redundant warn!/error! calls to reduce log noise and improve clarity

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.

this makes sense, but there's no need to put changes as docs

@crStiv
Copy link
Author

crStiv commented Jul 26, 2025

this makes sense, but there's no need to put changes as docs

tnx for the review, does it make sense now?

@crStiv crStiv requested a review from mattsse July 26, 2025 15:14
Copy link

@kaxhnet kaxhnet left a comment

Choose a reason for hiding this comment

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

Lsh9

@@ -109,14 +108,11 @@ impl VerificationProvider for EtherscanVerificationProvider {
if resp.result.starts_with("Unable to locate ContractCode at")
|| resp.result.starts_with("The address is not a smart contract")
{
warn!("{}", resp.result);
Copy link
Member

Choose a reason for hiding this comment

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

actually we should log this because this usually provides useful context for debugging

}

warn!("Failed verify submission: {:?}", resp);
Copy link
Member

Choose a reason for hiding this comment

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

same here

@@ -186,7 +182,7 @@ impl VerificationProvider for EtherscanVerificationProvider {
}

if resp.status == "0" {
return Err(RetryError::Break(eyre!("Contract failed to verify.")));
return Err(RetryError::Break(eyre!("Contract verification failed - status: {}, result: {}", resp.status, resp.result)));
Copy link
Member

Choose a reason for hiding this comment

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

maybe adding this to the actual error is better @grandizzy

Copy link
Collaborator

Choose a reason for hiding this comment

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

makes sense, anything that helps figuring out why verification failed is good to have

@crStiv crStiv requested a review from mattsse July 27, 2025 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants