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.

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(api): bridge limits [SLT-165] #3179

Merged
merged 40 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from 33 commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
f131926
fetch best rfq quote with query params
bigboydiamonds Sep 13, 2024
acd653c
fetch best quote for sdk and rfq api
bigboydiamonds Sep 13, 2024
6ada55a
return largest quoted origin amount on 1m size with respective bridge…
bigboydiamonds Sep 13, 2024
8530122
update endpoint naming, errors
bigboydiamonds Sep 13, 2024
4428e8d
Merge branch 'master' into api/bridge-max-amounts
bigboydiamonds Sep 16, 2024
69f36c3
implement mvc pattern
bigboydiamonds Sep 16, 2024
b48a36e
return max/min origin amounts
bigboydiamonds Sep 17, 2024
d4fac70
clean
bigboydiamonds Sep 17, 2024
5e5e2b7
...
bigboydiamonds Sep 17, 2024
7eab9a7
revert package.json changes
bigboydiamonds Sep 17, 2024
f56517a
pass in from/to token addresses in query params
bigboydiamonds Sep 17, 2024
ddb245d
tests
bigboydiamonds Sep 17, 2024
139e27c
fix tests
bigboydiamonds Sep 17, 2024
95a93ca
fix test name
bigboydiamonds Sep 17, 2024
96ec975
return parsed values
bigboydiamonds Sep 17, 2024
be863d5
account for eth addresses
bigboydiamonds Sep 17, 2024
c6cd501
clean
bigboydiamonds Sep 17, 2024
bb9aa82
refactor: clean getBridgeLimitsController
bigboydiamonds Sep 18, 2024
9fb48d8
query lower limit range
bigboydiamonds Sep 18, 2024
d006117
lower limit values for min query
bigboydiamonds Sep 18, 2024
8ce1097
clean response
bigboydiamonds Sep 18, 2024
f844897
Merge branch 'master' into api/bridge-max-amounts
bigboydiamonds Sep 19, 2024
8a8c923
Construct bridge limit mapping function
bigboydiamonds Sep 19, 2024
07980e5
Add origin / dest token symbol in map
bigboydiamonds Sep 19, 2024
46e73c2
Add swapableType field to bridge limit map
bigboydiamonds Sep 19, 2024
6696b4d
improve endpoint naming to follow best practices
bigboydiamonds Sep 21, 2024
0cce954
Merge branch 'master' into api/bridge-max-amounts
bigboydiamonds Sep 21, 2024
2039353
Merge branch 'master' into api/bridge-max-amounts
bigboydiamonds Sep 23, 2024
b342984
rename
bigboydiamonds Sep 23, 2024
f6246d4
generate-limits-script (#3166)
bigboydiamonds Sep 24, 2024
a1d4650
update naming
bigboydiamonds Sep 24, 2024
33f5584
add eth test for bridge limit endpoint, use checksumd address in util
bigboydiamonds Sep 24, 2024
60138d4
middleware check order
bigboydiamonds Sep 24, 2024
12de2ee
remove scripts
bigboydiamonds Sep 24, 2024
53e3c87
adds swagger doc
bigboydiamonds Sep 24, 2024
dc5bb55
clean
bigboydiamonds Sep 24, 2024
60dcf57
clean
bigboydiamonds Sep 24, 2024
03dc45b
fix test
bigboydiamonds Sep 24, 2024
c6b5d30
update docs for null responses
bigboydiamonds Sep 24, 2024
30fd0c5
normalize addresses
bigboydiamonds Sep 24, 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
4 changes: 3 additions & 1 deletion packages/rest-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@
"lint:check": "eslint . --max-warnings=0",
"ci:lint": "npm run lint:check",
"test": "jest",
"test:coverage": "jest --collect-coverage"
"test:coverage": "jest --collect-coverage",
"generate:limits-map": "node src/scripts/generateBridgeLimitMapping.js"
},
"dependencies": {
"@ethersproject/address": "^5.7.0",
"@ethersproject/bignumber": "^5.7.0",
"@ethersproject/providers": "^5.7.2",
"@ethersproject/units": "5.7.0",
Expand Down
108 changes: 108 additions & 0 deletions packages/rest-api/src/controllers/bridgeLimitsController.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import { validationResult } from 'express-validator'
import { BigNumber } from 'ethers'
import { parseUnits } from '@ethersproject/units'

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

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

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

const upperLimitValue = parseUnits('1000000', fromTokenInfo.decimals)
const upperLimitBridgeQuotes = await Synapse.allBridgeQuotes(
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
toTokenInfo.address,
upperLimitValue
)

const lowerLimitValues = ['0.01', '10']
let lowerLimitBridgeQuotes = null

for (const limit of lowerLimitValues) {
const lowerLimitAmount = parseUnits(limit, fromTokenInfo.decimals)

lowerLimitBridgeQuotes = await Synapse.allBridgeQuotes(
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
toTokenInfo.address,
lowerLimitAmount
)

if (lowerLimitBridgeQuotes && lowerLimitBridgeQuotes.length > 0) {
break
}
}
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved

const maxBridgeAmountQuote = upperLimitBridgeQuotes.reduce(
(maxQuote, currentQuote) => {
const currentMaxAmount = currentQuote.maxAmountOut
const maxAmount = maxQuote ? maxQuote.maxAmountOut : BigNumber.from(0)

return currentMaxAmount.gt(maxAmount) ? currentQuote : maxQuote
},
null
)
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved

const minBridgeAmountQuote = lowerLimitBridgeQuotes.reduce(
(minQuote, currentQuote) => {
const currentFeeAmount = currentQuote.feeAmount
const minFeeAmount = minQuote ? minQuote.feeAmount : null

return !minFeeAmount || currentFeeAmount.lt(minFeeAmount)
? currentQuote
: minQuote
},
null
)
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved

if (!maxBridgeAmountQuote || !minBridgeAmountQuote) {
return res.json({
maxOriginAmount: null,
minOriginAmount: null,
})
}

const maxAmountOriginQueryTokenOutInfo = tokenAddressToToken(
toChain,
maxBridgeAmountQuote.destQuery.tokenOut
)

const minAmountOriginQueryTokenOutInfo = tokenAddressToToken(
fromChain,
minBridgeAmountQuote.originQuery.tokenOut
)

const maxOriginAmount = formatBNToString(
maxBridgeAmountQuote.maxAmountOut,
maxAmountOriginQueryTokenOutInfo.decimals
)

const minOriginAmount = formatBNToString(
minBridgeAmountQuote.feeAmount,
minAmountOriginQueryTokenOutInfo.decimals
)

return res.json({
maxOriginAmount,
minOriginAmount,
})
Comment on lines +97 to +100
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider including additional relevant information in the response

Returning only maxOriginAmount and minOriginAmount might not provide sufficient context to the client. Consider including additional information such as the tokens involved or any relevant metadata to enhance the API's usability.

} catch (err) {
res.status(500).json({
error:
'An unexpected error occurred in /bridgeLimits. Please try again later.',
details: err.message,
})
}
Comment on lines +101 to +107
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Avoid exposing internal error details to the client

Returning err.message in the response may expose sensitive internal error details. Consider logging the error internally and returning a generic error message to the client to prevent potential security risks.

Apply the following diff:

        res.status(500).json({
          error:
            'An unexpected error occurred in /bridgeLimits. Please try again later.',
-          details: err.message,
        })
+        // Log the error internally
+        console.error(err)
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
} catch (err) {
res.status(500).json({
error:
'An unexpected error occurred in /bridgeLimits. Please try again later.',
details: err.message,
})
}
} catch (err) {
res.status(500).json({
error:
'An unexpected error occurred in /bridgeLimits. Please try again later.',
})
// Log the error internally
console.error(err)
}

}
Comment on lines +9 to +108
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add TypeScript type annotations for better type safety

As this is a TypeScript file, consider adding type annotations to leverage TypeScript's benefits. Specify types for req, res, and other variables to improve code reliability and maintainability.

Apply the following diff:

+import { Request, Response } from 'express'

-export const bridgeLimitsController = async (req, res) => {
+export const bridgeLimitsController = async (req: Request, res: Response) => {

And add types for other variables as needed.

Committable suggestion was skipped due to low confidence.

50 changes: 50 additions & 0 deletions packages/rest-api/src/routes/bridgeLimitsRoute.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import express from 'express'
import { check } from 'express-validator'

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

const router = express.Router()

router.get(
'/',
Copy link
Collaborator

Choose a reason for hiding this comment

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

See other routes that have normalizeNativeTokenAddress and checksumAddreses middleware.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, updated to use those middleware functions in 30fd0c5

[
check('fromChain')
.exists()
.withMessage('fromChain is required')
.isNumeric()
.custom((value) => CHAINS_ARRAY.some((c) => c.id === Number(value)))
.withMessage('Unsupported fromChain'),
check('toChain')
.exists()
.withMessage('Unsupported toChain')
.isNumeric()
.custom((value) => CHAINS_ARRAY.some((c) => c.id === Number(value)))
.withMessage('toChain is required'),
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'),
],
showFirstValidationError,
bridgeLimitsController
)
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider extracting validation logic for improved maintainability.

While the current implementation is functional, consider extracting the validation logic into a separate file (e.g., bridgeLimitsValidation.ts). This separation of concerns would enhance code organization and make it easier to maintain and test the validation rules independently.

Example structure:

// bridgeLimitsValidation.ts
export const bridgeLimitsValidationRules = [
  check('fromChain')...,
  check('toChain')...,
  check('fromToken')...,
  check('toToken')...,
];

// bridgeLimitsRoute.ts
import { bridgeLimitsValidationRules } from './bridgeLimitsValidation';

router.get(
  '/',
  bridgeLimitsValidationRules,
  showFirstValidationError,
  bridgeLimitsController
);

This refactoring would make the route definition more concise and easier to read.


export default router
2 changes: 2 additions & 0 deletions packages/rest-api/src/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import bridgeTxStatusRoute from './bridgeTxStatusRoute'
import destinationTxRoute from './destinationTxRoute'
import tokenListRoute from './tokenListRoute'
import destinationTokensRoute from './destinationTokensRoute'
import bridgeLimitsRoute from './bridgeLimitsRoute'
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

The /bridgeLimits route is not defined in bridgeLimitsRoute.ts.

The import for bridgeLimitsController is present, and the router is exported, but there is no route definition for /bridgeLimits. Please add the route registration to ensure the new endpoint is accessible.

Analysis chain

LGTM! Verify the existence of 'bridgeLimitsRoute.ts'

The import statement for bridgeLimitsRoute is correctly formatted and follows the existing code style. This import is necessary for registering the new /bridgeLimits endpoint.

Please run the following script to verify the existence of the 'bridgeLimitsRoute.ts' file:


The 'bridgeLimitsRoute.ts' file exists. Next, verify that it correctly defines the /bridgeLimits route.

Please run the following script to verify the route definition:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the existence of 'bridgeLimitsRoute.ts' in the same directory

# Test: Check if the file exists
fd --type file --max-depth 1 bridgeLimitsRoute.ts packages/rest-api/src/routes

Length of output: 130


Script:

#!/bin/bash
# Description: Verify that 'bridgeLimitsRoute.ts' defines and exports the '/bridgeLimits' route

# Check for the route definition
rg '/bridgeLimits' packages/rest-api/src/routes/bridgeLimitsRoute.ts

# Check if the route handler is exported
rg 'export\s+default\s+' packages/rest-api/src/routes/bridgeLimitsRoute.ts

Length of output: 246


const router = express.Router()

Expand All @@ -18,6 +19,7 @@ router.use('/swap', swapRoute)
router.use('/swapTxInfo', swapTxInfoRoute)
router.use('/bridge', bridgeRoute)
router.use('/bridgeTxInfo', bridgeTxInfoRoute)
router.use('/bridgeLimits', bridgeLimitsRoute)
router.use('/synapseTxId', synapseTxIdRoute)
router.use('/bridgeTxStatus', bridgeTxStatusRoute)
router.use('/destinationTx', destinationTxRoute)
Expand Down
81 changes: 81 additions & 0 deletions packages/rest-api/src/tests/bridgeLimitsRoute.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import request from 'supertest'
import express from 'express'

import bridgeLimitsRoute from '../routes/bridgeLimitsRoute'
import { USDC, ETH } from '../constants/bridgeable'

const app = express()
app.use('/bridgeLimits', bridgeLimitsRoute)

describe('Get Bridge Limits Route', () => {
it('should return min/max origin amounts bridging USDC', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: 1,
fromToken: USDC.addresses[1],
toChain: 10,
toToken: USDC.addresses[10],
})

expect(response.status).toBe(200)
expect(response.body).toHaveProperty('maxOriginAmount')
expect(response.body).toHaveProperty('minOriginAmount')
}, 10_000)

it('should return min/max origin amounts bridging ETH', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: 1,
fromToken: ETH.addresses[1],
toChain: 10,
toToken: ETH.addresses[10],
})

expect(response.status).toBe(200)
expect(response.body).toHaveProperty('maxOriginAmount')
expect(response.body).toHaveProperty('minOriginAmount')
}, 10_000)

it('should return 400 for unsupported fromChain', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: '999',
toChain: '137',
fromToken: '0x176211869cA2b568f2A7D4EE941E073a821EE1ff',
toToken: USDC.addresses[137],
})
expect(response.status).toBe(400)
expect(response.body.error).toHaveProperty(
'message',
'Unsupported fromChain'
)
}, 10_000)

it('should return 400 for unsupported toChain', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: '137',
toChain: '999',
fromToken: USDC.addresses[137],
toToken: '0x176211869cA2b568f2A7D4EE941E073a821EE1ff',
})
expect(response.status).toBe(400)
expect(response.body.error).toHaveProperty('message', 'Unsupported toChain')
}, 10_000)

it('should return 400 for missing fromToken', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: '1',
toChain: '137',
toToken: USDC.addresses[137],
})
expect(response.status).toBe(400)
expect(response.body.error).toHaveProperty('field', 'fromToken')
}, 10_000)

it('should return 400 for missing toToken', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: '1',
toChain: '137',
fromToken: USDC.addresses[1],
})
expect(response.status).toBe(400)
expect(response.body.error).toHaveProperty('field', 'toToken')
}, 10_000)
})
104 changes: 104 additions & 0 deletions packages/rest-api/src/utils/bridgeLimitMapping.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { BRIDGE_MAP } from '../constants/bridgeMap'
import * as ALL_TOKENS from '../constants/bridgeable'

const constructJSON = (swappableMap, exclusionList) => {
const result = {}

// Iterate through the origin chains
for (const originChainId in swappableMap) {
for (const originTokenAddress in swappableMap[originChainId]) {
const originToken = swappableMap[originChainId][originTokenAddress]
const originKey = `${originToken.symbol}-${originChainId}`

// Use transformPair to get token object
const transformedOriginToken = transformPair(originKey)
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved

if (!transformedOriginToken || exclusionList.includes(originKey)) {
continue
}

// Initialize origin chain and origin token with symbol and swappableType if not existing
if (!result[originChainId]) {
result[originChainId] = {}
}

if (!result[originChainId][transformedOriginToken.address]) {
result[originChainId][transformedOriginToken.address] = {
symbol: transformedOriginToken.symbol,
swappableType: transformedOriginToken.swapableType, // Fetch swappableType
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Fix inconsistent property name 'swappableType' vs. 'swapableType'

There is a spelling inconsistency in the property name swappableType (line 28) and swapableType (line 99). This inconsistency can lead to undefined values or errors. Please standardize the property name to swappableType throughout the code.

Apply this diff to fix the inconsistency:

// In 'transformPair' function (line 99)
-      swapableType: token.swapableType,
+      swappableType: token.swappableType,

Also applies to: 99-99

routes: {},
}
}

// Iterate through destination chains
for (const destinationChainId in swappableMap) {
if (originChainId === destinationChainId) {
continue
}

for (const destinationTokenAddress in swappableMap[
destinationChainId
]) {
const destinationToken =
swappableMap[destinationChainId][destinationTokenAddress]
const destinationKey = `${destinationToken.symbol}-${destinationChainId}`

// Use transformPair for destination token as well
const transformedDestinationToken = transformPair(destinationKey)

if (
!transformedDestinationToken ||
exclusionList.includes(destinationKey)
) {
continue
}

// Check for bridge compatibility by comparing origin and destination symbols
for (const bridgeSymbol of originToken.origin) {
if (
originToken.origin.includes(bridgeSymbol) &&
destinationToken.destination.includes(bridgeSymbol)
) {
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved
// Initialize destination token with symbol, minValue, maxValue if not existing
if (
!result[originChainId][transformedOriginToken.address].routes[
destinationChainId
]
) {
result[originChainId][transformedOriginToken.address].routes[
destinationChainId
] = {}
}

result[originChainId][transformedOriginToken.address].routes[
destinationChainId
][transformedDestinationToken.address] = {
symbol: transformedDestinationToken.symbol,
minOriginValue: null,
maxOriginValue: null,
}
}
}
}
}
}
}

return result
}

export const transformPair = (string: string): any => {
const [symbol, chainId] = string.split('-')
const token = Object.values(ALL_TOKENS).find((t) => t.routeSymbol === symbol)
const address = token?.addresses[chainId]
if (token && address) {
return {
symbol,
chainId,
address,
swapableType: token.swapableType,
}
}
}

export const BRIDGE_LIMIT_MAPPING = constructJSON(BRIDGE_MAP, [])
Loading
Loading