-
Notifications
You must be signed in to change notification settings - Fork 295
fix(sdk-coin-soneium): update recoveryBlockchainExplorerQuery #6537
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
Conversation
ae2f9d8
to
f6759ba
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.
Pull Request Overview
This PR updates the Soneium blockchain explorer integration by changing the explorer URL from the Minato testnet to the mainnet endpoint and refactoring the recovery blockchain explorer query functionality into a dedicated utility module.
Key changes:
- Updated the Soneium explorer base URL from testnet (minato) to mainnet
- Refactored
recoveryBlockchainExplorerQuery
method to use centralized utility functions - Added new utility functions to handle different types of blockchain explorer queries
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
modules/sdk-core/src/bitgo/environments.ts | Updated Soneium explorer URL from testnet to mainnet |
modules/sdk-coin-soneium/src/soneiumToken.ts | Refactored to use new utility function for blockchain explorer queries |
modules/sdk-coin-soneium/src/soneium.ts | Simplified recoveryBlockchainExplorerQuery method to use centralized utilities |
modules/sdk-coin-soneium/src/lib/utils.ts | Added new utility functions for handling standard and proxy blockchain explorer queries |
modules/sdk-coin-soneium/package.json | Added dependencies for ethereumjs-util and superagent |
} | ||
return await handleStandardExplorerQuery(query, bitgoEnv); | ||
} catch (error) { | ||
throw new Error('Could not query soneium explorer, error:' + error.message); |
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.
The error message concatenation should use template literals for better readability and to handle cases where error.message might be undefined. Consider using: throw new Error(
Could not query soneium explorer, error: ${error?.message || 'Unknown error'});
throw new Error('Could not query soneium explorer, error:' + error.message); | |
throw new Error(`Could not query soneium explorer, error: ${error?.message || 'Unknown error'}`); |
Copilot uses AI. Check for mistakes.
throw new Error('could not reach soneium.network'); | ||
} | ||
|
||
if (response.body.status === '0' && response.body.message === 'NOTOK') { |
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.
The hardcoded status and message values ('0' and 'NOTOK') should be defined as constants to improve maintainability and avoid magic strings.
if (response.body.status === '0' && response.body.message === 'NOTOK') { | |
if (response.body.status === RESPONSE_STATUS_ERROR && response.body.message === RESPONSE_MESSAGE_NOTOK) { |
Copilot uses AI. Check for mistakes.
const response = await request.post(common.Environments[bitgoEnv].soneiumExplorerBaseUrl + '/api/eth-rpc').send(body); | ||
|
||
if (!response.ok) { | ||
throw new Error('could not reach soneium.network'); |
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.
The error message should be capitalized for consistency: 'Could not reach soneium.network'
throw new Error('could not reach soneium.network'); | |
throw new Error('Could not reach soneium.network'); |
Copilot uses AI. Check for mistakes.
} | ||
|
||
if (response.body.status === '0' && response.body.message === 'NOTOK') { | ||
throw new Error('soneium.network rate limit reached'); |
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.
The error message should be capitalized for consistency: 'Soneium.network rate limit reached'
throw new Error('soneium.network rate limit reached'); | |
throw new Error('Soneium.network rate limit reached'); |
Copilot uses AI. Check for mistakes.
Ticket: WIN-6372