Skip to content
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.

Sign up for GitHub

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

REST API Improvements [SLT-179] #3133

Merged
merged 7 commits into from
Sep 18, 2024
Merged

REST API Improvements [SLT-179] #3133

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5ea9ba2
fix swaptxinfo function
Defi-Moses Sep 16, 2024
e2bab25
Updates test coverage command
abtestingalpha Sep 16, 2024
d35d50a
migrating to using token addresses instead of symbols
Defi-Moses Sep 16, 2024
b7ba450
fix linting errors
Defi-Moses Sep 17, 2024
42890d9
fixing swaptxinfocontroller
Defi-Moses Sep 17, 2024
f0a059b
new tests and new functionality
Defi-Moses Sep 17, 2024
e37ee32
update to master
Defi-Moses Sep 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/rest-api/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ module.exports = {
'^.+\\.(ts|tsx)$': 'babel-jest',
},
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx', 'json', 'node'],
moduleDirectories: ['node_modules', 'src'],
moduleDirectories: ['node_modules', '<rootDir>'],
}
3 changes: 2 additions & 1 deletion packages/rest-api/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"lint:check": "eslint . --max-warnings=0",
"ci:lint": "npm run lint:check",
"test": "jest",
"test:coverage": "echo 'No tests defined.'"
"test:coverage": "jest --collect-coverage"
},
"dependencies": {
"@ethersproject/bignumber": "^5.7.0",
Expand All @@ -26,6 +26,7 @@
"ethers": "5.7.2",
"express": "^4.18.2",
"express-validator": "^7.2.0",
"jest": "^29.7.0",
"lodash": "^4.17.21",
"supertest": "^6.3.3",
"typescript": "^4.8.3"
Expand Down
12 changes: 7 additions & 5 deletions packages/rest-api/src/controllers/bridgeController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,24 +3,26 @@ import { parseUnits } from '@ethersproject/units'

import { formatBNToString } from '../utils/formatBNToString'
import { Synapse } from '../services/synapseService'
import { tokenAddressToToken } from '../utils/tokenAddressToToken'

export const bridgeController = async (req, res) => {
const errors = validationResult(req)
if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() })
}
try {
const { fromChain, toChain, amount } = req.query
const fromTokenInfo = res.locals.tokenInfo.fromToken
const toTokenInfo = res.locals.tokenInfo.toToken
const { fromChain, toChain, amount, fromToken, toToken } = req.query

const fromTokenInfo = tokenAddressToToken(fromChain.toString(), fromToken)
const toTokenInfo = tokenAddressToToken(toChain.toString(), toToken)

const amountInWei = parseUnits(amount.toString(), fromTokenInfo.decimals)

const resp = await Synapse.allBridgeQuotes(
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
toTokenInfo.address,
fromToken,
toToken,
amountInWei
)
const payload = resp.map((quote) => ({
Expand Down
14 changes: 8 additions & 6 deletions packages/rest-api/src/controllers/bridgeTxInfoController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { validationResult } from 'express-validator'
import { parseUnits } from '@ethersproject/units'

import { Synapse } from '../services/synapseService'
import { tokenAddressToToken } from '../utils/tokenAddressToToken'

export const bridgeTxInfoController = async (req, res) => {
const errors = validationResult(req)
Expand All @@ -10,17 +11,18 @@ export const bridgeTxInfoController = async (req, res) => {
}

try {
const { fromChain, toChain, amount, destAddress } = req.query
const fromTokenInfo = res.locals.tokenInfo.fromToken
const toTokenInfo = res.locals.tokenInfo.toToken
const { fromChain, toChain, amount, destAddress, fromToken, toToken } =
req.query

const fromTokenInfo = tokenAddressToToken(fromChain.toString(), fromToken)

const amountInWei = parseUnits(amount.toString(), fromTokenInfo.decimals)

const quotes = await Synapse.allBridgeQuotes(
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
toTokenInfo.address,
fromToken,
toToken,
amountInWei
)

Expand All @@ -31,7 +33,7 @@ export const bridgeTxInfoController = async (req, res) => {
quote.routerAddress,
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
fromToken,
amountInWei,
quote.originQuery,
quote.destQuery
Expand Down
21 changes: 15 additions & 6 deletions packages/rest-api/src/controllers/swapController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,37 @@
import { validationResult } from 'express-validator'
import { formatUnits, parseUnits } from '@ethersproject/units'
import { BigNumber } from '@ethersproject/bignumber'

import { Synapse } from '../services/synapseService'
import { tokenAddressToToken } from '../utils/tokenAddressToToken'

export const swapController = async (req, res) => {
const errors = validationResult(req)
if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() })
}
try {
const { chain, amount } = req.query
const fromTokenInfo = res.locals.tokenInfo.fromToken
const toTokenInfo = res.locals.tokenInfo.toToken
const { chain, amount, fromToken, toToken } = req.query
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add validation for the chain variable.

The code assumes that the chain variable is present in the req.query object and uses it directly without any validation.

It's important to validate the chain variable to ensure it is a valid and supported chain identifier. Consider adding validation checks for the chain variable to handle cases where an invalid or unsupported chain identifier is provided.

Here's an example of how you can validate the chain variable:

const { chain, amount, fromToken, toToken } = req.query

// Validate the chain variable
if (!isValidChain(chain)) {
  return res.status(400).json({ error: 'Invalid chain identifier' })
}

// Rest of the code...

// Helper function to validate the chain identifier
function isValidChain(chain: string): boolean {
  // Add your chain validation logic here
  // For example, check if the chain is in a list of supported chains
  const supportedChains = ['ethereum', 'polygon', 'binance']
  return supportedChains.includes(chain)
}


const fromTokenInfo = tokenAddressToToken(chain.toString(), fromToken)
const toTokenInfo = tokenAddressToToken(chain.toString(), toToken)

const amountInWei = parseUnits(amount.toString(), fromTokenInfo.decimals)
const quote = await Synapse.swapQuote(
Number(chain),
fromTokenInfo.address,
toTokenInfo.address,
fromToken,
toToken,
amountInWei
)

const formattedMaxAmountOut = formatUnits(
BigNumber.from(quote.maxAmountOut),
toTokenInfo.decimals
)

res.json({
maxAmountOut: formatUnits(quote.maxAmountOut, toTokenInfo.decimals),
...quote,
maxAmountOut: formattedMaxAmountOut,
})
} catch (err) {
res.status(500).json({
Expand Down
15 changes: 8 additions & 7 deletions packages/rest-api/src/controllers/swapTxInfoController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { validationResult } from 'express-validator'
import { parseUnits } from '@ethersproject/units'

import { Synapse } from '../services/synapseService'
import { tokenAddressToToken } from '../utils/tokenAddressToToken'

export const swapTxInfoController = async (req, res) => {
const errors = validationResult(req)
Expand All @@ -10,23 +11,23 @@ export const swapTxInfoController = async (req, res) => {
}

try {
const { chain, amount } = req.query
const fromTokenInfo = res.locals.tokenInfo.fromToken
const toTokenInfo = res.locals.tokenInfo.toToken
const { chain, amount, address, fromToken, toToken } = req.query
Defi-Moses marked this conversation as resolved.
Show resolved Hide resolved

Comment on lines +14 to +15
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing validation for 'address', 'amount', and 'chain' parameters

Currently, the code validates fromToken and toToken but lacks validation for other critical parameters: address, amount, and chain. This could lead to unexpected errors or security vulnerabilities if invalid or malicious inputs are processed.

Apply the following diff to add validation for these parameters:

 try {
   const { chain, amount, address, fromToken, toToken } = req.query

+  if (!address || !isAddress(address)) {
+    return res.status(400).json({ error: 'Invalid address parameter' })
+  }
+
+  if (!chain || isNaN(Number(chain))) {
+    return res.status(400).json({ error: 'Invalid chain parameter' })
+  }
+
+  if (!amount || isNaN(Number(amount)) || Number(amount) <= 0) {
+    return res.status(400).json({ error: 'Invalid amount parameter' })
+  }

   if (!isAddress(fromToken) || !isAddress(toToken)) {
     return res.status(400).json({ error: 'Invalid token address' })
   }
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.

Suggested change
const { chain, amount, address, fromToken, toToken } = req.query
const { chain, amount, address, fromToken, toToken } = req.query
if (!address || !isAddress(address)) {
return res.status(400).json({ error: 'Invalid address parameter' })
}
if (!chain || isNaN(Number(chain))) {
return res.status(400).json({ error: 'Invalid chain parameter' })
}
if (!amount || isNaN(Number(amount)) || Number(amount) <= 0) {
return res.status(400).json({ error: 'Invalid amount parameter' })
}
if (!isAddress(fromToken) || !isAddress(toToken)) {
return res.status(400).json({ error: 'Invalid token address' })
}

const fromTokenInfo = tokenAddressToToken(chain.toString(), fromToken)
Defi-Moses marked this conversation as resolved.
Show resolved Hide resolved

const amountInWei = parseUnits(amount.toString(), fromTokenInfo.decimals)

const quote = await Synapse.swapQuote(
Number(chain),
fromTokenInfo.address,
toTokenInfo.address,
fromToken,
toToken,
amountInWei
)

const txInfo = await Synapse.swap(
Number(chain),
fromTokenInfo.address,
toTokenInfo.address,
address,
fromToken,
amountInWei,
quote.query
)
Expand Down
25 changes: 21 additions & 4 deletions packages/rest-api/src/routes/bridgeRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import express from 'express'
import { check } from 'express-validator'

import { isTokenAddress } from '../utils/isTokenAddress'
import { CHAINS_ARRAY } from '../constants/chains'
import { validateTokens } from '../validations/validateTokens'
import { showFirstValidationError } from '../middleware/showFirstValidationError'
import { bridgeController } from '../controllers/bridgeController'
import { isTokenSupportedOnChain } from '../utils/isTokenSupportedOnChain'

const router = express.Router()

Expand All @@ -23,9 +24,25 @@ router.get(
.withMessage('Unsupported toChain')
.exists()
.withMessage('toChain is required'),
validateTokens('fromChain', 'fromToken', 'fromToken'),
validateTokens('toChain', 'toToken', 'toToken'),
check('amount').isNumeric(),
check('fromToken')
.exists()
.withMessage('fromToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid fromToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.fromChain as string)
)
.withMessage('Token not supported on specified chain'),
check('toToken')
.exists()
.withMessage('toToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid toToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.toChain as string)
)
.withMessage('Token not supported on specified chain'),
check('amount').isNumeric().exists().withMessage('amount is required'),
],
showFirstValidationError,
bridgeController
Expand Down
32 changes: 27 additions & 5 deletions packages/rest-api/src/routes/bridgeTxInfoRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import express from 'express'
import { check } from 'express-validator'
import { isAddress } from 'ethers/lib/utils'

import { CHAINS_ARRAY } from '../constants/chains'
import { validateTokens } from '../validations/validateTokens'
import { showFirstValidationError } from '../middleware/showFirstValidationError'
import { bridgeTxInfoController } from '../controllers/bridgeTxInfoController'
import { isTokenAddress } from '../utils/isTokenAddress'
import { isTokenSupportedOnChain } from '../utils/isTokenSupportedOnChain'

const router = express.Router()

Expand All @@ -23,10 +25,30 @@ router.get(
.withMessage('Unsupported toChain')
.exists()
.withMessage('toChain is required'),
validateTokens('fromChain', 'fromToken', 'fromToken'),
validateTokens('toChain', 'toToken', 'toToken'),
check('amount').isNumeric(),
check('destAddress').isString(),
check('fromToken')
.exists()
.withMessage('fromToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid fromToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.fromChain as string)
)
.withMessage('Token not supported on specified chain'),
check('toToken')
.exists()
.withMessage('toToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid toToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.toChain as string)
)
.withMessage('Token not supported on specified chain'),
check('amount').isNumeric().exists().withMessage('amount is required'),
check('destAddress')
.exists()
.withMessage('destAddress is required')
.custom((value) => isAddress(value))
.withMessage('Invalid destination address'),
],
showFirstValidationError,
bridgeTxInfoController
Expand Down
23 changes: 20 additions & 3 deletions packages/rest-api/src/routes/swapRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import express from 'express'
import { check } from 'express-validator'

import { validateTokens } from '../validations/validateTokens'
import { showFirstValidationError } from '../middleware/showFirstValidationError'
import { swapController } from '../controllers/swapController'
import { CHAINS_ARRAY } from '../constants/chains'
import { isTokenAddress } from '../utils/isTokenAddress'
import { isTokenSupportedOnChain } from '../utils/isTokenSupportedOnChain'

const router = express.Router()

Expand All @@ -17,8 +18,24 @@ router.get(
.withMessage('Unsupported chain')
.exists()
.withMessage('chain is required'),
validateTokens('chain', 'fromToken', 'fromToken'),
validateTokens('chain', 'toToken', 'toToken'),
check('fromToken')
.exists()
.withMessage('fromToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid fromToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.chain as string)
)
.withMessage('Token not supported on specified chain'),
check('toToken')
.exists()
.withMessage('toToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid toToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.chain as string)
)
.withMessage('Token not supported on specified chain'),
check('amount').isNumeric().exists().withMessage('amount is required'),
],
showFirstValidationError,
Expand Down
31 changes: 27 additions & 4 deletions packages/rest-api/src/routes/swapTxInfoRoute.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
import express from 'express'
import { check } from 'express-validator'
import { isAddress } from 'ethers/lib/utils'

import { CHAINS_ARRAY } from '../constants/chains'
import { validateTokens } from '../validations/validateTokens'
import { showFirstValidationError } from '../middleware/showFirstValidationError'
import { swapTxInfoController } from '../controllers/swapTxInfoController'
import { isTokenAddress } from '../utils/isTokenAddress'
import { isTokenSupportedOnChain } from '../utils/isTokenSupportedOnChain'

const router = express.Router()

Expand All @@ -17,9 +19,30 @@ router.get(
.withMessage('Unsupported chain')
.exists()
.withMessage('chain is required'),
validateTokens('chain', 'fromToken', 'fromToken'),
validateTokens('chain', 'toToken', 'toToken'),
check('amount').isNumeric(),
check('fromToken')
.exists()
.withMessage('fromToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid fromToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.chain as string)
)
.withMessage('Token not supported on specified chain'),
check('toToken')
.exists()
.withMessage('toToken is required')
.custom((value) => isTokenAddress(value))
.withMessage('Invalid toToken address')
.custom((value, { req }) =>
isTokenSupportedOnChain(value, req.query.chain as string)
)
.withMessage('Token not supported on specified chain'),
check('amount').isNumeric().exists().withMessage('amount is required'),
check('address')
.exists()
.withMessage('address is required')
.custom((value) => isAddress(value))
.withMessage('Invalid Ethereum address'),
],
showFirstValidationError,
swapTxInfoController
Expand Down
Loading
Loading