Fixing REST API docs to include route changes #3401
Conversation
WalkthroughThe changes involve significant updates to the Synapse REST API documentation, particularly enhancing clarity regarding the API's read-only nature and its integration capabilities with the Synapse liquidity network. Key updates include the deprecation of the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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: |
e5d43fb
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://e39fba68.sanguine-fe.pages.dev |
| Branch Preview URL: | https://docs-api-improvements.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
docs/bridge/docs/02-Bridge/02-REST-API.md (2)
7-15: Add deprecation notice in the introduction sectionConsider adding a prominent notice about the deprecation of
/bridgeTxInfoand/swapTxInfoendpoints early in the introduction. This would help users understand the migration path upfront.Add this paragraph after line 15:
The API is available at [`https://api.synapseprotocol.com/`](https://api.synapseprotocol.com/). + > **Important:** The `/bridgeTxInfo` and `/swapTxInfo` endpoints have been deprecated. Please use the `/bridge` and `/swap` endpoints respectively, which now return the transaction data directly. See the [Previous versions](#previous-versions) section for more details.
Documentation needs comprehensive update to reflect API consolidation
The review needs to be expanded as there are multiple documentation inconsistencies:
- The commented-out API documentation is outdated and conflicts with the November 19, 2024 changelog that shows
/bridgeTxInfowas consolidated into/bridge- The blog post from October 10, 2024 still references the deprecated endpoints
- The
APIFlowcomponent in docs/bridge/src/components/APIFlow.tsx still references/bridgeTxInfoAction items:
- Remove the deprecated
/bridgeTxInfoand/swapTxInfoendpoint documentation- Update the
/bridgeand/swapendpoint documentation to include the new transaction data response fields- Update the blog post to reflect the latest API changes
- Update the APIFlow component to use the consolidated endpoints
🔗 Analysis chain
Line range hint
31-326: Update the commented API documentationThe commented-out API documentation section needs to be updated to:
- Remove the deprecated
/bridgeTxInfoand/swapTxInfoendpoint documentation- Update the
/bridgeand/swapendpoint documentation to include the transaction data in their response structures- Uncomment the documentation once updated
Let's verify if there are any remaining references to the deprecated endpoints in other documentation files:
Let me gather more information about the updated API endpoints to ensure we provide accurate guidance.
Let me check the blog post and other API documentation to understand the full scope of the API changes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to deprecated endpoints in documentation rg -i "bridgeTxInfo|swapTxInfo" docs/Length of output: 1552
Script:
#!/bin/bash # Search for updated endpoint documentation and changelog entries rg -A 5 -B 5 "consolidated into the /bridge endpoint" docs/ # Check for any other API documentation files fd -e md -e mdx "api|endpoint" docs/Length of output: 1966
Script:
#!/bin/bash # Check the blog post about API changes cat docs/bridge/blog-posts/2024-10-10-rest-api-post.md # Check the current /bridge endpoint documentation rg -A 10 "/bridge" docs/bridge/docs/02-Bridge/02-REST-API.mdLength of output: 7771
packages/rest-api/swagger.json (1)
399-400: Consider enhancing migration documentationWhile the deprecation notices are clear, it would be helpful to add more detailed migration guidance for users. Consider:
- Adding examples showing how to migrate from the old endpoints to the new ones
- Documenting any differences in request/response formats between old and new endpoints
- Providing a migration timeline/deadline
Example addition to the description:
-"description": "[Deprecated] Originally used to get Bridge transaction information", +"description": "[Deprecated] Originally used to get Bridge transaction information. To migrate, use the /bridge endpoint which returns the same transaction data in its response. Migration example: GET /bridgeTxInfo?fromChain=1&... -> GET /bridge?fromChain=1&... The /bridge response includes the transaction data in the returned quote object. Migration deadline: December 2024",Also applies to: 1348-1349
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
docs/bridge/docs/02-Bridge/02-REST-API.md(2 hunks)packages/rest-api/src/routes/bridgeTxInfoRoute.ts(1 hunks)packages/rest-api/src/routes/swapTxInfoRoute.ts(1 hunks)packages/rest-api/swagger.json(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/rest-api/src/routes/bridgeTxInfoRoute.ts
🔇 Additional comments (4)
packages/rest-api/src/routes/swapTxInfoRoute.ts (2)
21-22: LGTM! Documentation changes align with PR objectives.
The OpenAPI documentation now correctly indicates that this endpoint is deprecated in favor of /swap.
21-22: Verify the replacement endpoint implementation.
Since this endpoint is being deprecated in favor of /swap, we should verify:
- The existence and documentation of the
/swapendpoint - Feature parity between the endpoints
- Non-breaking changes for existing clients
✅ Verification successful
Let me gather more information about the /swap endpoint implementation.
Deprecation notice is accurate and well-implemented
The /swap endpoint fully replaces /swapTxInfo with enhanced functionality:
- It provides all the original transaction information
- Adds call data when an address is provided
- Has comprehensive OpenAPI documentation
- Maintains backward compatibility with additional features
The migration path is clear:
- Original endpoint is properly marked as deprecated
- Documentation explicitly points users to the
/swapendpoint - New endpoint has identical base functionality plus enhancements
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the replacement endpoint implementation
# Check if the /swap endpoint exists and is documented
echo "Checking for /swap endpoint implementation..."
fd -t f "swapRoute.ts" "packages/rest-api/src/routes"
# Check OpenAPI documentation for /swap endpoint
echo "Checking OpenAPI documentation for /swap endpoint..."
rg -A 10 "/swap:" "packages/rest-api/src/**/*.ts"
# Look for any existing usages of /swapTxInfo that need migration
echo "Checking for existing usages that need migration..."
rg -g "!*.{md,txt}" "/swapTxInfo"
Length of output: 2231
Script:
#!/bin/bash
# Check the implementation and documentation of /swap endpoint
echo "Checking /swap endpoint implementation..."
cat packages/rest-api/src/routes/swapRoute.ts
echo "\nChecking swagger documentation..."
cat packages/rest-api/swagger.json | jq '.paths."/swap"'
echo "\nChecking for call data in /swap response..."
rg -A 5 "callData" packages/rest-api/src/controllers/swapController.ts
Length of output: 13562
packages/rest-api/swagger.json (2)
399-400: LGTM! Clear deprecation notice for /bridgeTxInfo
The deprecation notice clearly indicates that users should migrate to the /bridge endpoint which now includes the transaction data in its response.
1348-1349: LGTM! Clear deprecation notice for /swapTxInfo
The deprecation notice clearly indicates that users should migrate to the /swap endpoint which now includes the transaction data in its response.
| * summary: "[Deprecated] in favor of using the /swap endpoint, which now returns call data | ||
| * description: "[Deprecated] Originally used to get Swap transaction information |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance deprecation handling with proper HTTP headers and migration guidance.
While the documentation indicates deprecation, the implementation should follow API deprecation best practices:
- Add a deprecation warning header to responses
- Include a sunset date
- Provide specific migration steps to the new
/swapendpoint
Consider implementing these changes:
router.get(
'/',
normalizeNativeTokenAddress(['fromToken', 'toToken']),
checksumAddresses(['fromToken', 'toToken']),
+ (req, res, next) => {
+ res.set({
+ 'Deprecation': 'true',
+ 'Sunset': '2025-01-01',
+ 'Link': '</swap>; rel="successor-version"',
+ 'Warning': '299 - "This endpoint is deprecated. Please migrate to /swap endpoint which provides the same functionality plus call data."'
+ });
+ next();
+ },
[
check('chain')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * summary: "[Deprecated] in favor of using the /swap endpoint, which now returns call data | |
| * description: "[Deprecated] Originally used to get Swap transaction information | |
| * summary: "[Deprecated] in favor of using the /swap endpoint, which now returns call data | |
| * description: "[Deprecated] Originally used to get Swap transaction information | |
| router.get( | |
| '/', | |
| normalizeNativeTokenAddress(['fromToken', 'toToken']), | |
| checksumAddresses(['fromToken', 'toToken']), | |
| (req, res, next) => { | |
| res.set({ | |
| 'Deprecation': 'true', | |
| 'Sunset': '2025-01-01', | |
| 'Link': '</swap>; rel="successor-version"', | |
| 'Warning': '299 - "This endpoint is deprecated. Please migrate to /swap endpoint which provides the same functionality plus call data."' | |
| }); | |
| next(); | |
| }, | |
| [ | |
| check('chain') |
| @@ -17,6 +26,7 @@ Get read-only data from on-chain Synapse contracts, and generate Bridge and Swap | |||
| | Date | Description | | |||
| | ---------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------ | | |||
| | 2024‑10‑01 | [https://synapse-rest-api-v2.herokuapp.com/](https://synapse-rest-api-v2.herokuapp.com/) is no longer maintained and has been fully deprecated as of October 2024. | | |||
| | 2024‑11‑19 | [https://api.synapseprotocol.com/](https://api.synapseprotocol.com/) the /bridgeTxInfo endpoint has been consolidated into the /bridge endpoint, which now returns call data | | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complete the version history entry
The version history entry should mention both endpoint consolidations and provide clear migration guidance.
Update the entry to include both changes:
-| 2024‑11‑19 | [https://api.synapseprotocol.com/](https://api.synapseprotocol.com/) the /bridgeTxInfo endpoint has been consolidated into the /bridge endpoint, which now returns call data |
+| 2024‑11‑19 | [https://api.synapseprotocol.com/](https://api.synapseprotocol.com/) - The `/bridgeTxInfo` and `/swapTxInfo` endpoints have been consolidated into the `/bridge` and `/swap` endpoints respectively. The consolidated endpoints now return transaction data directly. Users should update their integrations accordingly. |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | 2024‑11‑19 | [https://api.synapseprotocol.com/](https://api.synapseprotocol.com/) the /bridgeTxInfo endpoint has been consolidated into the /bridge endpoint, which now returns call data | | |
| | 2024‑11‑19 | [https://api.synapseprotocol.com/](https://api.synapseprotocol.com/) - The `/bridgeTxInfo` and `/swapTxInfo` endpoints have been consolidated into the `/bridge` and `/swap` endpoints respectively. The consolidated endpoints now return transaction data directly. Users should update their integrations accordingly. | |
Bundle ReportChanges will decrease total bundle size by 3.51MB (-9.86%) ⬇️. This is within the configured threshold ✅ Detailed changes
ℹ️ *Bundle size includes cached data from a previous commit |
Merging the /txInfo endpoints with their relevant /swap or /bridge endpoints. Updating the docs to reflect this and then also adding more clarity on how the docs work.
Docs likely need more iteration.
Summary by CodeRabbit
/bridgeendpoint, including new required parameters and detailed response structures.4b78d54: docs preview link