-
Notifications
You must be signed in to change notification settings - Fork 51
Sam/etherscan v2 fix #997
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
base: master
Are you sure you want to change the base?
Sam/etherscan v2 fix #997
Conversation
src/ethereum/info/abstractInfo.ts
Outdated
| type: 'evmscan', | ||
| gastrackerSupport: true, | ||
| servers: ['https://api.abscan.org/'], | ||
| servers: ['https://api.abscan.org/v2/api'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cursorbot is right. please fix the url in line 50
| type: 'evmscan' | ||
| servers: string[] | ||
| version: 1 | 2 | ||
| version: 2 | 'v2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What was wrong with using versions 1 and 2? versions 2 and 'v2' is weird
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a rebasing error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now all of the evmscan servers are set to version 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be using version 2 where it's supported
| type: 'evmscan' | ||
| servers: string[] | ||
| version: 1 | 2 | ||
| version: 2 | 'v2' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now all of the evmscan servers are set to version 2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Malformed URLs Break API Connectivity
fetchFeesFromEvmScan builds URLs by appending /v2/api or /api to the server, but server URLs in the info configs already include these paths (e.g., https://api.etherscan.io/v2/api). This creates malformed URLs like https://api.etherscan.io/v2/api/v2/api?..., causing API requests to fail. Should use the version config field like EvmScanAdapter does to determine which URL format to use.
src/ethereum/fees/feeProviders.ts#L226-L229
edge-currency-accountbased/src/ethereum/fees/feeProviders.ts
Lines 226 to 229 in 06fa880
| const chainId = networkInfo.chainParams.chainId | |
| const url = server.includes('etherscan.io') | |
| ? `${server}/v2/api?chainid=${chainId}&module=gastracker&action=gasoracle${apiKey}` | |
| : `${server}/api?module=gastracker&action=gasoracle${apiKey}` |
06fa880 to
ef0ca6f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Redundant Paths Break API Calls
The fetchFeesFromEvmScan function constructs URLs by appending /v2/api or /api path segments to server URLs, but the updated network configs now include these path segments in the server URLs themselves. This creates double path segments (e.g., /v2/api/api) in requests, resulting in 404 errors when fetching gas prices. The URL construction needs to use the base server URL without appending duplicate paths.
src/ethereum/fees/feeProviders.ts#L241-L254
edge-currency-accountbased/src/ethereum/fees/feeProviders.ts
Lines 241 to 254 in ef0ca6f
| `fetchFeesFromEvmScan unrecognized response message: ${esGasResponse.message}` | |
| ) | |
| } | |
| const { SafeGasPrice, ProposeGasPrice, FastGasPrice } = | |
| asEvmScanGasResponseResult(esGasResponse.result) | |
| const newSafeLow = parseFloat(SafeGasPrice) | |
| let newAverage = parseFloat(ProposeGasPrice) | |
| let newFast = parseFloat(FastGasPrice) | |
| // Correct inconsistencies, convert values | |
| if (newAverage <= newSafeLow) newAverage = newSafeLow + 1 | |
| if (newFast <= newAverage) newFast = newAverage + 1 | |
| serverUrl: string | ||
| ): string | string[] | undefined => { | ||
| // TODO: This is total BS. We should just have a key associated with a domain name. | ||
| // This is getting out of hand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Improve EVM Scan API Key Network Handling
The getEvmScanApiKey function checks if serverUrl.includes('etherscan.io') and requires etherscanApiKey, but this breaks BSC after removing api.bscscan.com as a fallback server. Now BSC uses only https://api.etherscan.io/v2/api, which matches the etherscan.io check and throws an error or returns the wrong API key. The function should differentiate between Etherscan being used for ETH vs other networks, allowing bscscanApiKey or the unified evmScanApiKey to be used for BSC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was always a problem. The solution is to add etherscanApiKey to bsc key list
| serverUrl: string | ||
| ): string | string[] | undefined => { | ||
| // TODO: This is total BS. We should just have a key associated with a domain name. | ||
| // This is getting out of hand. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Legacy Etherscan Check Blocks Unified API
The getEvmScanApiKey function requires etherscanApiKey when the server URL contains 'etherscan.io', throwing an error before checking for the unified evmScanApiKey. After consolidating BSC to use https://api.etherscan.io/v2/api, calls for BSC will now always match the etherscan.io check and throw if etherscanApiKey is missing, even when evmScanApiKey is configured. This breaks BSC fee queries for deployments using only the recommended evmScanApiKey pattern.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was always a problem. The solution is to add etherscanApiKey to bsc key list
78eefb8 to
4cf137b
Compare
| gastrackerSupport: true, | ||
| servers: ['https://api.abscan.org/'] | ||
| servers: ['https://api.etherscan.io/v2/api'], | ||
| version: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Abstract Chain API: Generic Endpoint Incompatibility
Abstract chain configuration is using Etherscan's v2 API with chainId 2741, but Etherscan v2 API doesn't support Abstract's chain ID. The chain-specific api.abscan.org server was replaced with a generic Etherscan endpoint that will fail queries for this network. Abstract requires its own dedicated API server configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
295dc90 to
ed8632c
Compare
- Remove arbiscan.io server due to deprecation - Remove 'https://rpc-mainnet.maticvigil.com'; This service has been shut-down - Remove 'https://rpc.polycat.finance'; This service is not longer available. - Replace abscan.org with 'etherscan.io'; Because, abscan.org has been deprecated. - Remove 'https://api-era.zksync.network/v2/api'; It has been deprecated for etherscan.io. - Remove 'https://api.polygonscan.com/v2/api'; It has been deprecated for etherscan.io. - Remove 'https://api.snowscan.xyz/v2/api'; It has been deprecated for etherscan.io. - Remove etc-network.info RPC nodes: These services are no longer available. - Replace stakely.io RPC node with hyperlend.finance; The stakely.io node doesn't work (500 error) and the hyperlend.finance does. - Remove 'https://celo-mainnet-rpc.allthatnode.com' - Remove 'https://matic-mainnet.chainstacklabs.com' - Remove 'https://api.basescan.org/v2/api' - Remove 'rpc.ankr.com'; This RPC node is complaining about needing an account to work. - Remove 'https://api.sonicscan.org/v2/api'
ed8632c to
a59d4e9
Compare
CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
noneDescription
noneNote
Introduce versioned
evmscanadapter and update network configs/fee provider to use Etherscan v2 and correct API paths; expand rate‑limit handling and add precise adapter cleaners.version: 1 | 2to config and build URLs accordingly (v2 uses?chainid=; v1 uses legacy/api). Expand rate‑limit detection strings.fetchFeesFromEvmScannow selects URL format based on adapterversion;getEvmScanApiKeylogic retained with clarifying comments.EvmScanAdapterConfig.versionto schema.ethereum,arbitrum,base,optimism,polygon,avalanche,amoy,sepolia,holesky,sonic,binancesmartchain,bobevm,botanix,fantom,rsk,pulsechain,zksync, etc.):evmscanservers to correct paths (Etherscan v2:https://api.etherscan.io/v2/apiwithversion: 2; Blockscout/Routescan/etc.: append/apiwithversion: 1).evmscanentries where needed (e.g., Avalanche Avascan, Celo explorer, zkSync explorer).version: 2onevmscanentries.api.bscscan.comserver.Written by Cursor Bugbot for commit a59d4e9. This will update automatically on new commits. Configure here.