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 27 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/getBridgeLimitsController.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) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to bridgeLimitsController

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

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
)

const 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
)

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

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/getBridgeLimits.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import request from 'supertest'
import express from 'express'

Copy link
Collaborator

Choose a reason for hiding this comment

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

rename this test file to bridgeLimitsRoute.test.ts to match the rest of the tests. we might write controller/middleware tests later, so this naming convention keeps them unique.

import getBridgeLimitsRoute from '../routes/bridgeLimitsRoute'

const app = express()
app.use('/getBridgeLimits', getBridgeLimitsRoute)

describe('Get Bridge Limits Route', () => {
it('should return min/max origin amounts for valid input', async () => {
const response = await request(app).get('/getBridgeLimits').query({
fromChain: 1,
fromToken: '0xA0b86991c6218b36c1d19D4a2e9Eb0cE3606eB48',
bigboydiamonds marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Collaborator

Choose a reason for hiding this comment

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

import USDC from bridgeable vs. hardcoding address where we already have it in app

toChain: 10,
toToken: '0x0b2C639c533813f4Aa9D7837CAf62653d097Ff85',
})

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add tests not just for stables but also ETH.

it('should return 400 for unsupported fromChain', async () => {
const response = await request(app).get('/getBridgeLimits').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 ', async () => {
const response = await request(app).get('/getBridgeLimits').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.

Incomplete test description and potential duplicate test case.

The test case starting at line 37 has an incomplete description: 'should return 400 for unsupported '. Additionally, it appears to be a duplicate of the previous test case for an unsupported 'fromChain' value.

Please update the test description to accurately reflect the scenario being tested. If this test is intended to check a different unsupported parameter (e.g., fromToken, toToken), adjust both the test description and the query parameters accordingly. If it's a duplicate, consider removing it to avoid redundancy.

Tools
Gitleaks

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

(generic-api-key)


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

(generic-api-key)


it('should return 400 for unsupported toChain', async () => {
const response = await request(app).get('/getBridgeLimits').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('/getBridgeLimits').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('/getBridgeLimits').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