-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(rest-api): API gateway for rfq-indexer API and rfq API + swagger merge [SLT-339] #3323
Changes from all commits
cf1ae18
b9d62d8
67ae193
37a2662
9ad94cb
411555e
e060d11
6bcfd07
5bdb610
c9f1663
92975a3
da4d67e
81877a7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -49,15 +49,15 @@ | |
"copyfiles": "^2.3.0", | ||
"depcheck": "^1.4.3", | ||
"doctoc": "^2.2.0", | ||
"eslint": "^8.16.0", | ||
"eslint": "^8.53.0", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification The version updates are mostly compatible, but there's one important issue:
Please add the missing peer dependency: "eslint-plugin-n": "^15.0.0" 🔗 Analysis chainVerify compatibility with major version updates. The updates include major version bumps that might introduce breaking changes:
Also applies to: 54-54, 60-60 Let me gather more information about the ESLint configuration and potential peer dependency issues. Let me check one more thing to verify the peer dependencies of the updated packages. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check for peer dependency warnings in package.json files
echo "Checking for peer dependency requirements..."
rg -A 5 '"peerDependencies"'
# Check if there are any existing ESLint configuration files
echo "Checking for ESLint config files..."
fd -e .eslintrc.js -e .eslintrc.json -e .eslintrc.yml
Length of output: 685 Script: #!/bin/bash
# Check for ESLint configuration
echo "Checking ESLint configuration..."
fd '.eslintrc' --type f --exec cat {}
# Check package.json for ESLint related dependencies
echo -e "\nChecking ESLint related dependencies..."
rg '"eslint.*":' package.json
# Check for any existing ESLint errors with current config
echo -e "\nChecking for potential ESLint configuration issues..."
fd '.js' --type f --exec grep -l "eslint-disable" {} \;
Length of output: 876 Script: #!/bin/bash
# Check npm registry for peer dependencies of the updated packages
echo "Checking peer dependencies for [email protected]..."
curl -s https://registry.npmjs.org/eslint-config-standard/17.0.0 | jq '.peerDependencies'
echo -e "\nChecking peer dependencies for [email protected]..."
curl -s https://registry.npmjs.org/eslint-plugin-promise/6.0.0 | jq '.peerDependencies'
Length of output: 619 |
||
"eslint-config-prettier": "^8.3.0", | ||
"eslint-config-standard": "^16.0.3", | ||
"eslint-config-standard": "^17.0.0", | ||
"eslint-plugin-import": "^2.26.0", | ||
"eslint-plugin-jsdoc": "^35.1.2", | ||
"eslint-plugin-node": "^11.1.0", | ||
"eslint-plugin-prefer-arrow": "^1.2.3", | ||
"eslint-plugin-prettier": "^4.0.0", | ||
"eslint-plugin-promise": "^5.1.0", | ||
"eslint-plugin-promise": "^6.0.0", | ||
"eslint-plugin-react": "^7.24.0", | ||
"eslint-plugin-unicorn": "^42.0.0", | ||
"lerna": "^4.0.0", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,28 +1,11 @@ | ||
import swaggerJsdoc from 'swagger-jsdoc' | ||
import fs from 'fs' | ||
import path from 'path' | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-var-requires | ||
const packageJson = require('../package.json') | ||
// Define the path to your static swagger.json file | ||
const swaggerFilePath = path.join(__dirname, '../swagger.json') | ||
|
||
const isDevelopment = process.env.NODE_ENV === 'development' | ||
const serverUrl = isDevelopment | ||
? 'http://localhost:3000' | ||
: 'https://api.synapseprotocol.com' | ||
// Read the static swagger.json file | ||
const specs = JSON.parse(fs.readFileSync(swaggerFilePath, 'utf8')) | ||
|
||
Comment on lines
+4
to
+8
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling and consider asynchronous file reading. The current implementation has several potential issues:
Consider implementing this more robust approach: -// Define the path to your static swagger.json file
-const swaggerFilePath = path.join(__dirname, '../swagger.json')
-
-// Read the static swagger.json file
-const specs = JSON.parse(fs.readFileSync(swaggerFilePath, 'utf8'))
+import { OpenAPIV3 } from 'openapi-types'
+
+const swaggerFilePath = path.join(__dirname, '../swagger.json')
+
+function loadSwaggerSpec(): OpenAPIV3.Document {
+ try {
+ const fileContent = fs.readFileSync(swaggerFilePath, 'utf8')
+ const parsedSpec = JSON.parse(fileContent)
+
+ // Basic validation of the OpenAPI spec
+ if (!parsedSpec.openapi || !parsedSpec.info || !parsedSpec.paths) {
+ throw new Error('Invalid OpenAPI specification format')
+ }
+
+ return parsedSpec
+ } catch (error) {
+ if (error instanceof Error) {
+ throw new Error(`Failed to load Swagger specification: ${error.message}`)
+ }
+ throw error
+ }
+}
+
+const specs = loadSwaggerSpec()
|
||
const options: swaggerJsdoc.Options = { | ||
definition: { | ||
openapi: '3.0.0', | ||
info: { | ||
title: 'Synapse Protocol REST API', | ||
version: packageJson.version, | ||
description: 'API documentation for the Synapse Protocol REST API', | ||
}, | ||
servers: [ | ||
{ | ||
url: serverUrl, | ||
}, | ||
], | ||
}, | ||
apis: ['./src/routes/*.ts', './src/*.ts'], | ||
} | ||
|
||
export const specs = swaggerJsdoc(options) | ||
// Export the specs for use in your application | ||
export { specs } |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,34 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { createProxyMiddleware } from 'http-proxy-middleware' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const isRFQIndexerRequest = (route: string): boolean => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/conflicting-proofs') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/disputes') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/invalid-relaus') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/pending-transactions') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/refunded-and-relayed') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/transaction-id') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+12
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Security and reliability improvements needed in route checking. The current implementation has several issues:
Consider this safer implementation: export const isRFQIndexerRequest = (route: string): boolean => {
+ if (!route) return false;
+ const validPaths = [
+ '/conflicting-proofs',
+ '/disputes',
+ '/invalid-relays',
+ '/pending-transactions',
+ '/refunded-and-relayed',
+ '/transaction-id'
+ ];
+ return validPaths.some(path =>
+ route === path || route.startsWith(`${path}/`)
+ );
- return (
- route.includes('/conflicting-proofs') ||
- route.includes('/disputes') ||
- route.includes('/invalid-relaus') ||
- route.includes('/pending-transactions') ||
- route.includes('/refunded-and-relayed') ||
- route.includes('/transaction-id')
- )
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const isRFQAPIRequest = (route: string): boolean => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
return ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/ack') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/bulk_quotes') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/contracts') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/open_quote_requests') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/quotes') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/rfq') || | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
route.includes('/rfq_stream') | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+14
to
+24
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apply the same security improvements to RFQ API route checking. The function has the same security vulnerabilities as Apply similar improvements: export const isRFQAPIRequest = (route: string): boolean => {
+ if (!route) return false;
+ const validPaths = [
+ '/ack',
+ '/bulk_quotes',
+ '/contracts',
+ '/open_quote_requests',
+ '/quotes',
+ '/rfq',
+ '/rfq_stream'
+ ];
+ return validPaths.some(path =>
+ route === path || route.startsWith(`${path}/`)
+ );
- return (
- route.includes('/ack') ||
- route.includes('/bulk_quotes') ||
- route.includes('/contracts') ||
- route.includes('/open_quote_requests') ||
- route.includes('/quotes') ||
- route.includes('/rfq') ||
- route.includes('/rfq_stream')
- )
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const rfqApiProxy = createProxyMiddleware({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: 'https://rfq-api.omnirpc.io', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
changeOrigin: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const rfqIndexerProxy = createProxyMiddleware({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
target: 'https://rfq-indexer.synapseprotocol.com/api', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
changeOrigin: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+26
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Codebase verification Security enhancements needed for proxy middleware configuration The codebase analysis reveals:
Recommended changes:
export const rfqApiProxy = createProxyMiddleware({
target: process.env.RFQ_API_URL || 'https://rfq-api.omnirpc.io',
changeOrigin: true,
headers: {
'X-Forwarded-Proto': 'https',
},
onError: (err, req, res) => {
logger.error('RFQ API Proxy Error:', err);
res.status(502).json({ error: 'Gateway Error' });
},
})
export const rfqIndexerProxy = createProxyMiddleware({
target: process.env.RFQ_INDEXER_URL || 'https://rfq-indexer.synapseprotocol.com/api',
changeOrigin: true,
headers: {
'X-Forwarded-Proto': 'https',
},
onError: (err, req, res) => {
logger.error('RFQ Indexer Proxy Error:', err);
res.status(502).json({ error: 'Gateway Error' });
},
}) 🔗 Analysis chainEnhance proxy middleware security and configuration. The current proxy setup lacks several important security features and best practices:
Consider this improved implementation: +const RFQ_API_URL = process.env.RFQ_API_URL || 'https://rfq-api.omnirpc.io';
+const RFQ_INDEXER_URL = process.env.RFQ_INDEXER_URL || 'https://rfq-indexer.synapseprotocol.com/api';
export const rfqApiProxy = createProxyMiddleware({
- target: 'https://rfq-api.omnirpc.io',
+ target: RFQ_API_URL,
changeOrigin: true,
+ pathRewrite: {
+ '^/api': '', // remove /api prefix if needed
+ },
+ headers: {
+ 'X-Forwarded-Proto': 'https',
+ },
+ onError: (err, req, res) => {
+ console.error('Proxy Error:', err);
+ res.status(500).json({ error: 'Proxy Error' });
+ },
})
export const rfqIndexerProxy = createProxyMiddleware({
- target: 'https://rfq-indexer.synapseprotocol.com/api',
+ target: RFQ_INDEXER_URL,
changeOrigin: true,
+ pathRewrite: {
+ '^/api': '',
+ },
+ headers: {
+ 'X-Forwarded-Proto': 'https',
+ },
+ onError: (err, req, res) => {
+ console.error('Proxy Error:', err);
+ res.status(500).json({ error: 'Proxy Error' });
+ },
}) Additionally, consider implementing rate limiting middleware and request validation before proxying requests. Let's verify the environment configuration in the project: 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if environment variables are properly configured
rg -l "RFQ_API_URL|RFQ_INDEXER_URL" --type yaml --type env
Length of output: 88 Script: #!/bin/bash
# Check for environment variable configurations in various config files
rg -l "RFQ_API_URL|RFQ_INDEXER_URL"
# Check for any existing environment configuration patterns
rg -l "process\.env\." packages/rest-api/src/
# Look for any configuration files
fd -e yaml -e yml -e env -e config.js -e config.ts packages/rest-api/
# Check the proxy middleware implementation details
ast-grep --pattern 'createProxyMiddleware({$$$})'
Length of output: 1349 Script: #!/bin/bash
# Check how environment variables are used in the existing files
rg -A 5 "process\.env\." packages/rest-api/src/app.ts packages/rest-api/src/utils/getOmniRpcUrl.ts packages/rest-api/src/middleware/logger.ts
# Check if there's any existing error handling in proxy routes
rg -A 5 "onError|error" packages/rest-api/src/utils/isGatewayRoute.ts
# Look for any rate limiting implementations
rg -l "rate|limit" packages/rest-api/src/
Length of output: 1465 |
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.
💡 Codebase verification
API endpoint inconsistency detected in configuration
There's an inconsistency in the API endpoints across the documentation. While the RFQ API documentation has been updated to use
api.synapseprotocol.com/quotes
for mainnet, thedocusaurus.config.ts
file still contains a reference to the old endpoint:baseUrl: 'https://rfq-api.omnirpc.io/'
.docs/bridge/docusaurus.config.ts
: Update thebaseUrl
configuration to use the new API endpointrfq-api-testnet.omnirpc.io
is still valid and intended to be used🔗 Analysis chain
Verify API endpoint consistency across documentation.
The mainnet API URL has been updated to use the new base URL. Let's ensure this change is consistently reflected across all related documentation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
Length of output: 430
Script:
Length of output: 12309
🧰 Tools
🪛 Markdownlint