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 14 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
2 changes: 2 additions & 0 deletions packages/rest-api/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@
"postinstall": "npm run build"
},
"dependencies": {
"@ethersproject/address": "^5.7.0",
"@ethersproject/bignumber": "^5.7.0",
"@ethersproject/providers": "^5.7.2",
"@ethersproject/units": "5.7.0",
"@synapsecns/sdk-router": "^0.11.1",
"axios": "^1.7.7",
"bignumber": "^1.1.0",
"ethers": "5.7.2",
"express": "^4.18.2",
Expand Down
118 changes: 118 additions & 0 deletions packages/rest-api/src/controllers/getBridgeLimitsController.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import axios from 'axios'
import { validationResult } from 'express-validator'
import { BigNumber } from 'ethers'
import { parseUnits } from '@ethersproject/units'
import { getAddress } from '@ethersproject/address'

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

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 rfqResponse = await axios.get('https://rfq-api.omnirpc.io/quotes', {
params: {
originChainId: fromChain,
originTokenAddress: fromTokenInfo.address,
destChainId: toChain,
destTokenAddress: toTokenInfo.address,
},
})
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider adding error handling for the API call to the RFQ service.

The function makes an API call to the RFQ service to retrieve quotes for the specified token transfer. However, it does not handle potential errors that may occur during this API call, such as network errors or invalid responses.

To improve the robustness of the function, consider adding error handling for this API call, such as catching any errors and returning an appropriate error response to the client.


const rfqQuotes = rfqResponse.data

let bestRfqQuote = null

if (Array.isArray(rfqQuotes) && rfqQuotes.length > 0) {
const filteredQuotes = rfqQuotes
.slice(0, 20)
.filter(
(quote) =>
Number(quote.origin_chain_id) === Number(fromChain) &&
Number(quote.dest_chain_id) === Number(toChain) &&
getAddress(quote.origin_token_addr) ===
getAddress(fromTokenInfo.address) &&
getAddress(quote.dest_token_addr) ===
getAddress(toTokenInfo.address)
)

bestRfqQuote = filteredQuotes.reduce((maxQuote, currentQuote) => {
const currentAmount = Number(currentQuote.max_origin_amount)
const maxAmount = maxQuote ? Number(maxQuote.max_origin_amount) : 0
return currentAmount > maxAmount ? currentQuote : maxQuote
}, null)
}

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

Choose a reason for hiding this comment

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

Consider adding error handling for the API calls to the Synapse service.

The function makes two API calls to the Synapse service to retrieve bridge quotes for the upper and lower limit amounts. However, it does not handle potential errors that may occur during these API calls, such as network errors or invalid responses.

To improve the robustness of the function, consider adding error handling for these API calls, such as catching any errors and returning an appropriate error response to the client.

Also applies to: 62-69


const lowerLimitAmount = parseUnits('100', fromTokenInfo.decimals)
const lowerLimitBridgeQuotes = await Synapse.allBridgeQuotes(
Number(fromChain),
Number(toChain),
fromTokenInfo.address,
toTokenInfo.address,
lowerLimitAmount
)

const bestUpperLimitSDKQuote = upperLimitBridgeQuotes[0]

let maxOriginQuote

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

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

if (bestRfqQuote) {
const bestRfqQuoteMaxAmountBN = BigNumber.from(
bestRfqQuote.max_origin_amount
)
maxOriginQuote = bestRfqQuoteMaxAmountBN.gt(
bestUpperLimitSDKQuote.maxAmountOut
)
? { source: 'RFQ', amount: bestRfqQuoteMaxAmountBN }
: {
source: bestUpperLimitSDKQuote.bridgeModuleName,
amount: bestUpperLimitSDKQuote.maxAmountOut,
}
} else {
maxOriginQuote = {
source: bestUpperLimitSDKQuote.bridgeModuleName,
amount: bestUpperLimitSDKQuote.maxAmountOut,
}
}

return res.json({
maxOriginAmount: maxOriginQuote.amount,
minOriginAmount: minBridgeFeeQuote.feeAmount,
})
} 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/getBridgeLimitsRoute.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()

router.get(
'/',
[
check('fromChain')
.isNumeric()
.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 @@ -5,6 +5,7 @@ import swapRoute from './swapRoute'
import swapTxInfoRoute from './swapTxInfoRoute'
import bridgeRoute from './bridgeRoute'
import bridgeTxInfoRoute from './bridgeTxInfoRoute'
import getBridgeLimitsRoute from './getBridgeLimitsRoute'
import getSynapseTxIdRoute from './getSynapseTxIdRoute'
import getBridgeTxStatusRoute from './getBridgeTxStatusRoute'
import getDestinationTxRoute from './getDestinationTxRoute'
Expand All @@ -17,6 +18,7 @@ router.use('/swap', swapRoute)
router.use('/swapTxInfo', swapTxInfoRoute)
router.use('/bridge', bridgeRoute)
router.use('/bridgeTxInfo', bridgeTxInfoRoute)
router.use('/getBridgeLimits', getBridgeLimitsRoute)
router.use('/getSynapseTxId', getSynapseTxIdRoute)
router.use('/getBridgeTxStatus', getBridgeTxStatusRoute)
router.use('/getDestinationTx', getDestinationTxRoute)
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/getBridgeLimitsRoute'

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)
})
17 changes: 17 additions & 0 deletions packages/rest-api/src/utils/tokenAddressToToken.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import { BRIDGE_MAP } from '../constants/bridgeMap'

export const tokenAddressToToken = (chain: string, tokenAddress: string) => {
const chainData = BRIDGE_MAP[chain]
if (!chainData) {
return null
}
const tokenInfo = chainData[tokenAddress]
if (!tokenInfo) {
return null
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Normalize tokenAddress to ensure consistent token lookups

Token addresses may differ in case, potentially leading to lookup failures since object keys in JavaScript are case sensitive. Consider normalizing tokenAddress, such as converting it to lowercase, and ensure that the keys in BRIDGE_MAP are in the same format.

Apply this diff to normalize tokenAddress:

 const tokenInfo = chainData[tokenAddress]
+const normalizedTokenAddress = tokenAddress.toLowerCase()
+const tokenInfo = chainData[normalizedTokenAddress]

 if (!tokenInfo) {
   return null
 }

Also, ensure that the keys in BRIDGE_MAP are normalized for consistency.

Committable suggestion was skipped due to low confidence.

return {
address: tokenAddress,
symbol: tokenInfo.symbol,
decimals: tokenInfo.decimals,
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Specify the return type of tokenAddressToToken for improved type safety

To enhance type safety and readability, consider specifying the return type of the tokenAddressToToken function. Defining an interface for the returned token information would clarify the expected output.

For example:

interface TokenInfo {
  address: string;
  symbol: string;
  decimals: number;
}

export const tokenAddressToToken = (chain: string, tokenAddress: string): TokenInfo | null => {
  // function body...
}

Loading
Loading