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

[wip] feat(api): bridge limit script #3131

Closed
wants to merge 38 commits into from
Closed
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
38 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
64f6816
use generated bridge limits json to provide min/max values
bigboydiamonds Sep 24, 2024
6990b0f
return generated min/max values
bigboydiamonds Sep 24, 2024
fdefa7e
bridge limit map
bigboydiamonds Sep 24, 2024
eb7aa76
generate limits map
bigboydiamonds Sep 24, 2024
68c6c48
use map logic in bridge limits controller, remove sdk fetches at time…
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
1 change: 1 addition & 0 deletions packages/rest-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
"test:coverage": "jest --collect-coverage"
},
"dependencies": {
"@ethersproject/address": "^5.7.0",
"@ethersproject/bignumber": "^5.7.0",
"@ethersproject/providers": "^5.7.2",
"@ethersproject/units": "5.7.0",
Expand Down
110 changes: 110 additions & 0 deletions packages/rest-api/src/controllers/bridgeLimitsController.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
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'
import { BRIDGE_LIMIT_MAPPING } from '../utils/bridgeLimitMapping'

export const getBridgeLimitsController = async (req, res) => {
const errors = validationResult(req)
if (!errors.isEmpty()) {
return res.status(400).json({ errors: errors.array() })
}
Comment on lines +6 to +9
Copy link
Contributor

Choose a reason for hiding this comment

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

Ensure validation middleware is correctly set up for request parameters

While validationResult(req) checks for validation errors, there is no indication that validation rules for fromChain, fromToken, toChain, and toToken have been established. To prevent invalid or malicious inputs, please ensure that appropriate validation middleware is applied to these parameters. This should include checks such as isNumeric for chain IDs and isTokenAddress for token addresses to enhance error messaging and debugging.

try {
const { fromChain, fromToken, toChain, toToken } = req.query
Comment on lines +6 to +11
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

Add validation rules for query parameters

While the code correctly checks for validation errors, there are no visible validation rules for the query parameters (fromChain, fromToken, toChain, toToken). Consider adding specific validation rules, such as:

[
  query('fromChain').isNumeric(),
  query('toChain').isNumeric(),
  query('fromToken').isEthereumAddress(),
  query('toToken').isEthereumAddress(),
]

This will ensure that the input parameters are of the correct type and format before processing.


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

Copy link
Contributor

Choose a reason for hiding this comment

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

Add error handling for invalid token information

Currently, if tokenAddressToToken returns undefined or null for fromTokenInfo or toTokenInfo, accessing properties like .decimals will cause a runtime error. To prevent this, please add error handling to check if fromTokenInfo and toTokenInfo are valid before proceeding.

Apply this diff to implement error handling:

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

+ if (!fromTokenInfo) {
+   return res.status(400).json({ error: 'Invalid fromToken address' })
+ }
+ if (!toTokenInfo) {
+   return res.status(400).json({ error: 'Invalid toToken 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 fromTokenInfo = tokenAddressToToken(fromChain, fromToken)
const toTokenInfo = tokenAddressToToken(toChain, toToken)
const fromTokenInfo = tokenAddressToToken(fromChain, fromToken)
const toTokenInfo = tokenAddressToToken(toChain, toToken)
if (!fromTokenInfo) {
return res.status(400).json({ error: 'Invalid fromToken address' })
}
if (!toTokenInfo) {
return res.status(400).json({ error: 'Invalid toToken address' })
}

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
}
}

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
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Handle empty upperLimitBridgeQuotes to prevent runtime errors

The reduce method on upperLimitBridgeQuotes assumes that the array has at least one element. If upperLimitBridgeQuotes is empty or null, this will cause a runtime error. Please add a check to ensure upperLimitBridgeQuotes is not empty before calling reduce.

Apply this diff to add the necessary check:

+ let maxBridgeAmountQuote = null
+ if (upperLimitBridgeQuotes && upperLimitBridgeQuotes.length > 0) {
    maxBridgeAmountQuote = upperLimitBridgeQuotes.reduce(
      (maxQuote, currentQuote) => {
        const currentMaxAmount = currentQuote.maxAmountOut
        const maxAmount = maxQuote ? maxQuote.maxAmountOut : BigNumber.from(0)

        return currentMaxAmount.gt(maxAmount) ? currentQuote : maxQuote
      },
      null
    )
+ }

Similarly, ensure that upperLimitBridgeQuotes is not null or empty before proceeding.

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 maxBridgeAmountQuote = upperLimitBridgeQuotes.reduce(
(maxQuote, currentQuote) => {
const currentMaxAmount = currentQuote.maxAmountOut
const maxAmount = maxQuote ? maxQuote.maxAmountOut : BigNumber.from(0)
return currentMaxAmount.gt(maxAmount) ? currentQuote : maxQuote
},
null
)
let maxBridgeAmountQuote = null
if (upperLimitBridgeQuotes && upperLimitBridgeQuotes.length > 0) {
maxBridgeAmountQuote = upperLimitBridgeQuotes.reduce(
(maxQuote, currentQuote) => {
const currentMaxAmount = currentQuote.maxAmountOut
const maxAmount = maxQuote ? maxQuote.maxAmountOut : BigNumber.from(0)
return currentMaxAmount.gt(maxAmount) ? currentQuote : maxQuote
},
null
)
}

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

return !minFeeAmount || currentFeeAmount.lt(minFeeAmount)
? currentQuote
: minQuote
},
null
)

Copy link
Contributor

Choose a reason for hiding this comment

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

Handle empty lowerLimitBridgeQuotes to prevent runtime errors

Just like with upperLimitBridgeQuotes, calling reduce on lowerLimitBridgeQuotes without ensuring it's not empty can lead to a runtime error. Please add a check to ensure lowerLimitBridgeQuotes has elements before using reduce.

Apply this diff to add the necessary check:

+ let minBridgeAmountQuote = null
+ if (lowerLimitBridgeQuotes && lowerLimitBridgeQuotes.length > 0) {
    minBridgeAmountQuote = lowerLimitBridgeQuotes.reduce(
      (minQuote, currentQuote) => {
        const currentFeeAmount = currentQuote.feeAmount
        const minFeeAmount = minQuote ? minQuote.feeAmount : null

        return !minFeeAmount || currentFeeAmount.lt(minFeeAmount)
          ? currentQuote
          : minQuote
      },
      null
    )
+ }

Ensure that you handle cases where lowerLimitBridgeQuotes may be null or empty.

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 minBridgeAmountQuote = lowerLimitBridgeQuotes.reduce(
(minQuote, currentQuote) => {
const currentFeeAmount = currentQuote.feeAmount
const minFeeAmount = minQuote ? minQuote.feeAmount : null
return !minFeeAmount || currentFeeAmount.lt(minFeeAmount)
? currentQuote
: minQuote
},
null
)
let minBridgeAmountQuote = null
if (lowerLimitBridgeQuotes && lowerLimitBridgeQuotes.length > 0) {
minBridgeAmountQuote = lowerLimitBridgeQuotes.reduce(
(minQuote, currentQuote) => {
const currentFeeAmount = currentQuote.feeAmount
const minFeeAmount = minQuote ? minQuote.feeAmount : null
return !minFeeAmount || currentFeeAmount.lt(minFeeAmount)
? currentQuote
: minQuote
},
null
)
}

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
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Verify correct usage of token decimals in amount formatting

When formatting maxOriginAmount and minOriginAmount, the decimals used are from maxAmountOriginQueryTokenOutInfo.decimals and minAmountOriginQueryTokenOutInfo.decimals, which are derived from destQuery.tokenOut and originQuery.tokenOut. Please verify that these are the appropriate token decimals to use.

For maxOriginAmount, if maxBridgeAmountQuote.maxAmountOut corresponds to the destination token, you should use toTokenInfo.decimals to ensure correct formatting. Similarly, confirm the correctness for minOriginAmount.

Consider updating the formatting to use the correct token decimals:

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

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

Committable suggestion was skipped due to low confidence.


return res.json({
BRIDGE_LIMIT_MAPPING,
maxOriginAmount,
minOriginAmount,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Evaluate the necessity of including BRIDGE_LIMIT_MAPPING in the response

Including BRIDGE_LIMIT_MAPPING in the API response may expose internal configuration details that are not necessary for the client. Unless there is a specific need for the client to receive this data, consider removing it from the response to keep the API output clean and focused.

Apply this diff to remove BRIDGE_LIMIT_MAPPING from the response:

return res.json({
- BRIDGE_LIMIT_MAPPING,
  maxOriginAmount,
  minOriginAmount,
})
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
return res.json({
BRIDGE_LIMIT_MAPPING,
maxOriginAmount,
minOriginAmount,
})
return res.json({
maxOriginAmount,
minOriginAmount,
})

} catch (err) {
res.status(500).json({
error:
'An unexpected error occurred in /getBridgeLimits. Please try again later.',
details: err.message,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid exposing sensitive error details in the response

Returning err.message in the API response can inadvertently expose sensitive server information or implementation details. For security best practices, consider logging the error internally and returning a generic error message to the client.

Apply this diff to modify the error handling:

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

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

import { CHAINS_ARRAY } from '../constants/chains'
import { showFirstValidationError } from '../middleware/showFirstValidationError'
import { getBridgeLimitsController } from '../controllers/bridgeLimitsController'

const router = express.Router()

Copy link
Collaborator

Choose a reason for hiding this comment

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

add swagger docs (see other routes for examples)

router.get(
'/',
[
check('fromChain')
.isNumeric()
Copy link
Collaborator

@abtestingalpha abtestingalpha Sep 23, 2024

Choose a reason for hiding this comment

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

existence checks should come first:

does it exist ->
if it exists, is it numeric ->
if it exists and is numeric, is it supported

.custom((value) => CHAINS_ARRAY.some((c) => c.id === Number(value)))
.withMessage('Unsupported fromChain')
.exists()
.withMessage('originChainId is required'),
check('toChain')
.isNumeric()
.custom((value) => CHAINS_ARRAY.some((c) => c.id === Number(value)))
.withMessage('Unsupported toChain')
.exists()
.withMessage('toChain is required'),
check('fromToken')
.exists()
.withMessage('fromToken is required')
.custom((value) => isAddress(value))
.withMessage('Invalid fromToken address'),
check('toToken')
.exists()
.withMessage('toToken is required')
.custom((value) => isAddress(value))
.withMessage('Invalid toToken address'),
],
showFirstValidationError,
getBridgeLimitsController
)

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'

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'

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

describe('Get Bridge Limits Route', () => {
it('should return min/max origin amounts for valid input', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: 1,
fromToken: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
toChain: 10,
toToken: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85',
})

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: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359',
})
expect(response.status).toBe(400)
expect(response.body.error).toHaveProperty(
'message',
'Unsupported fromChain'
)
}, 10_000)
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM: Unsupported fromChain test case is correct, but there's a duplication.

The test case for unsupported fromChain is well-structured, checking both the status code and the error message. However, there's a duplicate test case (lines 37-49) with identical content.

Remove the duplicate test case (lines 37-49) to maintain a clean and efficient test suite.

Also applies to: 37-49

Tools
Gitleaks

27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


it('should return 400 for unsupported ', async () => {
const response = await request(app).get('/bridgeLimits').query({
fromChain: '999',
toChain: '137',
fromToken: '0x176211869cA2b568f2A7D4EE941E073a821EE1ff',
toToken: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359',
})
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: '0x3c499c542cEF5E3811e1192ce70d8cC03d5c3359',
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: '0x176211869cA2b568f2A7D4EE941E073a821EE1ff',
})
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: '0xa0b86991c6218b36c1d19d4a2e9eb0ce3606eb48',
})
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)

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.

Inconsistent property name 'swappableType' vs 'swapableType'

The property name is inconsistently spelled between constructJSON and transformPair. In constructJSON (line 28), it's referenced as swappableType, while in transformPair (line 99), it's defined as swapableType. This inconsistency could lead to undefined values at runtime.

Apply this diff to fix the inconsistency:

Option 1: Rename swapableType to swappableType in transformPair:

-     swapableType: token.swapableType,
+     swappableType: token.swapableType,

Option 2: Rename swappableType to swapableType in constructJSON:

-               swappableType: transformedOriginToken.swapableType, // Fetch swappableType
+               swapableType: transformedOriginToken.swapableType, // Fetch swapableType

Ensure that the property name is consistent throughout the codebase.

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)
) {
Comment on lines +59 to +63
Copy link
Contributor

Choose a reason for hiding this comment

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

Redundant condition in bridge compatibility check

The condition originToken.origin.includes(bridgeSymbol) inside the loop is redundant since bridgeSymbol is derived from originToken.origin and will always be included. This redundancy doesn't affect functionality but can be simplified for clarity.

Simplify the condition by removing the redundant check:

for (const bridgeSymbol of originToken.origin) {
-   if (
-     originToken.origin.includes(bridgeSymbol) &&
-     destinationToken.destination.includes(bridgeSymbol)
-   ) {
+   if (destinationToken.destination.includes(bridgeSymbol)) {
      // Initialize destination token
   }
}
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
for (const bridgeSymbol of originToken.origin) {
if (
originToken.origin.includes(bridgeSymbol) &&
destinationToken.destination.includes(bridgeSymbol)
) {
for (const bridgeSymbol of originToken.origin) {
if (destinationToken.destination.includes(bridgeSymbol)) {

// 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
}
Comment on lines +6 to +90
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider optimizing nested loops for performance

The constructJSON function contains multiple nested loops over swappableMap, which might impact performance with large datasets. Evaluating the necessity of iterating over all possible combinations of origin and destination tokens could lead to performance improvements.

Explore ways to optimize the loops, such as:

  • Limiting iterations to relevant origin-destination pairs.
  • Caching computed values if possible.
  • Parallelizing the processing if the environment supports it.

This could enhance the efficiency of the function when dealing with extensive token maps.


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