Fix: Etherscan API V1 deprecated; add V2 API support#692
Fix: Etherscan API V1 deprecated; add V2 API support#692
Conversation
|
|
👋 miro-cl, thanks for creating this pull request! To help reviewers, please consider creating future PRs as drafts first. This allows you to self-review and make any final changes before notifying the team. Once you're ready, you can mark it as "Ready for review" to request feedback. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where error messages from the Etherscan API were being swallowed when the API returned an error that was not related to an unsupported action.
Changes:
- Modified error handling in
GetContractCreationTxto properly format and return error messages from the API instead of returning a nil error variable
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If API call failed due to reasons other than action unsupported, there is no fallback. | ||
| if !strings.Contains(errMsg, "invalid Action name") { | ||
| return "", err | ||
| return "", fmt.Errorf("failed to get contract creation tx: %s", errMsg) |
There was a problem hiding this comment.
The variable err is being replaced with a newly formatted error, but the original err variable may contain valuable context (such as stack traces or wrapped errors). Consider wrapping the original error using fmt.Errorf(\"failed to get contract creation tx: %s: %w\", errMsg, err) to preserve the error chain, or verify that err is truly nil at this point in the code path.
miro-cl
changed the title
|
| // It is a short-cut provided by most Etherscan instances. | ||
| func GetContractCreationTx(ctx context.Context, endpoint string, addressStr string, apiKey string) (string, error) { | ||
| url := fmt.Sprintf("%s?module=contract&action=getcontractcreation&contractaddresses=%s&apikey=%s", endpoint, addressStr, apiKey) | ||
| func GetContractCreationTx(ctx context.Context, endpoint string, chainIDStr, addressStr string, apiKey string) (string, error) { |
There was a problem hiding this comment.
Is it possible to make this change in a backwards compatible way to avoid breaking domains that they are still using v1 and are not ready to migrate?
For example you could introduce a new functions here GetContractCreationTxV2 🤔
There was a problem hiding this comment.
Actually I talk with the team and we don't think this us used anywhere else apart from the framework so we could update it in place
There was a problem hiding this comment.
@DimitriosNaikopoulos great, let me know. AFAICT the EtherscanV1 API is fully deprecated so no need for backwards compatibility. Thanks for taking a look at this!
2 participants
Fixes issue where an unset err is returned, which swallows the error message returned from the API.
E.g.,
You are using a deprecated V1 endpoint, switch to Etherscan API V2 using https://docs.etherscan.io/v2-migration.