API method updates #3122
API method updates #3122
Conversation
WalkthroughThe changes introduce three new API endpoints to the Express application. The Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Deploying sanguine-fe with
|
| Latest commit: |
eb72eef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://bedcc419.sanguine-fe.pages.dev |
| Branch Preview URL: | https://api-method-updates.sanguine-fe.pages.dev |
Bundle ReportChanges will increase total bundle size by 409.81kB (1.14%) ⬆️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3122 +/- ##
===================================================
+ Coverage 36.37896% 37.95310% +1.57413%
===================================================
Files 438 418 -20
Lines 25512 24222 -1290
Branches 82 82
===================================================
- Hits 9281 9193 -88
+ Misses 15486 14287 -1199
+ Partials 745 742 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
packages/rest-api/src/app.ts (1)
475-507: Refactor error handling and improve parameter validation.The
/getSynapseTxIdendpoint implementation looks good overall. It properly validates the input parameters, handles errors, and returns the expected response. However, consider the following improvements:
- Refactor the duplicated error handling code to a separate function for better maintainability.
- Make the error messages more descriptive to help users debug issues. For example, include the specific parameter that is missing or invalid.
- For consistency, validate and convert all parameters to the expected types. The
originChainIdparameter is converted to a number, but thebridgeModuleandtxHashparameters are kept as strings.Apply this diff to implement the suggested improvements:
+const sendErrorResponse = (res, message, err) => { + res.status(400).send({ + message, + error: err.message, + }); +}; + app.get('/getSynapseTxId', (req, res) => { try { const query = req.query; - const originChainId = Number(query.originChainId); - const bridgeModule = String(query.bridgeModule); - const txHash = String(query.txHash); + const { originChainId, bridgeModule, txHash } = query; + + if (!originChainId || isNaN(Number(originChainId))) { + sendErrorResponse(res, 'Invalid originChainId parameter. Expected a number.', new Error('Invalid originChainId')); + return; + } + + if (!bridgeModule) { + sendErrorResponse(res, 'Missing bridgeModule parameter.', new Error('Missing bridgeModule')); + return; + } + + if (!txHash) { + sendErrorResponse(res, 'Missing txHash parameter.', new Error('Missing txHash')); + return; + } - if (!originChainId || !bridgeModule || !txHash) { - res.status(400).send({ - message: 'Invalid request: Missing required parameters', - }); - return; - } - Synapse.getSynapseTxId(originChainId, bridgeModule, txHash) + Synapse.getSynapseTxId(Number(originChainId), bridgeModule, txHash) .then((synapseTxId) => { res.json({ synapseTxId }); }) .catch((err) => { - res.status(400).send({ - message: - 'Ensure that your request matches the following format: /getSynapseTxId?originChainId=8453&bridgeModule=SynapseRFQ&txHash=0x4acd82091b54cf584d50adcad9f57c61055beaca130016ecc3798d3d61f5487a', - error: err.message, - }); + sendErrorResponse( + res, + 'Failed to retrieve Synapse transaction ID. Ensure that your request matches the following format: /getSynapseTxId?originChainId=8453&bridgeModule=SynapseRFQ&txHash=0x4acd82091b54cf584d50adcad9f57c61055beaca130016ecc3798d3d61f5487a', + err + ); }); } catch (err) { - res.status(400).send({ - message: - 'Ensure that your request matches the following format: /getSynapseTxId?originChainId=8453&bridgeModule=SynapseRFQ&txHash=0x4acd82091b54cf584d50adcad9f57c61055beaca130016ecc3798d3d61f5487a', - error: err.message, - }); + sendErrorResponse( + res, + 'Invalid request. Ensure that your request matches the following format: /getSynapseTxId?originChainId=8453&bridgeModule=SynapseRFQ&txHash=0x4acd82091b54cf584d50adcad9f57c61055beaca130016ecc3798d3d61f5487a', + err + ); } });
| }) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
If we wanted to prevent unnecessary SDK calls in the scenario where Client passes in an unsupported destChainId, we can do a check if a valid chainId is outputted from, and return an error when destination ID isn't supported
Could change line 513 to
const destChainId = CHAINS_BY_ID[Number(query.destChainId)]
(using https://github.com/synapsecns/sanguine/blob/api/method-updates/packages/rest-api/src/constants/chains.ts/#L495)
but if we wanted to provide a more semantic error, we could do something like
try {
const query = req.query
const destChainId = Number(query.destChainId)
const bridgeModule = String(query.bridgeModule)
const synapseTxId = String(query.synapseTxId)
if (!destChainId || !bridgeModule || !synapseTxId) {
res.status(400).send({
message: 'Invalid request: Missing required parameters',
})
return
}
const supportedDestChainId = CHAINS_BY_ID[Number(query.destChainId)]
if (!supportedDestChainId) {
res.status(400).send({
message: 'Invalid destChainId: Unsupported chainId',
})
return
}
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- packages/rest-api/src/app.ts (1 hunks)
Additional comments not posted (1)
packages/rest-api/src/app.ts (1)
475-507: LGTM!The
/getSynapseTxIdendpoint is implemented correctly. It validates the required parameters, invokes the appropriate SDK method, and returns the transaction ID or a detailed error message based on the result.
This adds the sdk functions getBridgeTxStatus and getSynapseTxId to the rest API, also adds a sub-method for finding the destination transaction information if a transaction is completed.
docs are already updated. Examples could be significantly improved.
Summary by CodeRabbit
Summary by CodeRabbit
/getSynapseTxIdfor retrieving a Synapse transaction ID./getBridgeTxStatusfor checking the status of a bridge transaction./getDestinationTxfor retrieving the destination transaction hash.