-
Notifications
You must be signed in to change notification settings - Fork 30
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
[DNM] Bridge endpoint consolidation [SLT-453] #3371
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,7 +12,7 @@ export const swapController = async (req, res) => { | |
return res.status(400).json({ errors: errors.array() }) | ||
} | ||
try { | ||
const { chain, amount, fromToken, toToken } = req.query | ||
const { chain, amount, fromToken, toToken, address } = req.query | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add address parameter validation The new Add this validation to your route configuration: check('address')
.optional()
.isString()
.custom((value) => isAddress(value))
.withMessage('Invalid Ethereum address format'), |
||
|
||
const fromTokenInfo = tokenAddressToToken(chain.toString(), fromToken) | ||
const toTokenInfo = tokenAddressToToken(chain.toString(), toToken) | ||
|
@@ -30,9 +30,20 @@ export const swapController = async (req, res) => { | |
toTokenInfo.decimals | ||
) | ||
|
||
const callData = address | ||
? await Synapse.swap( | ||
Number(chain), | ||
address, | ||
fromToken, | ||
amountInWei, | ||
quote.query | ||
) | ||
: null | ||
Comment on lines
+33
to
+41
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Enhance error handling for swap operation The conditional swap operation needs specific error handling to distinguish between different failure modes (e.g., invalid address, insufficient funds, network issues). Consider wrapping the swap call in a try-catch block with specific error types: let callData = null;
if (address) {
try {
callData = await Synapse.swap(
Number(chain),
address,
fromToken,
amountInWei,
quote.query
);
} catch (error) {
if (error instanceof InvalidAddressError) {
throw new Error('Invalid address provided');
}
// Handle other specific error cases
throw error;
}
} |
||
|
||
const payload = { | ||
...quote, | ||
maxAmountOut: formattedMaxAmountOut, | ||
callData, | ||
} | ||
|
||
logger.info(`Successful swapController response`, { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,6 +197,15 @@ | |
"type": "string" | ||
}, | ||
"description": "The address of the user on the origin chain" | ||
}, | ||
{ | ||
"in": "query", | ||
"name": "destAddress", | ||
"required": true, | ||
"schema": { | ||
"type": "string" | ||
}, | ||
"description": "The destination address of the user on the destination chain" | ||
Comment on lines
+200
to
+208
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Add address validation patterns for security. Consider adding regex patterns to validate Ethereum addresses, preventing invalid inputs and potential security issues. {
"in": "query",
"name": "destAddress",
"required": true,
"schema": {
- "type": "string"
+ "type": "string",
+ "pattern": "^0x[a-fA-F0-9]{40}$",
+ "minLength": 42,
+ "maxLength": 42
},
"description": "The destination address of the user on the destination chain"
} Also applies to: 1218-1226 |
||
} | ||
], | ||
"responses": { | ||
|
@@ -1206,6 +1215,15 @@ | |
"type": "number" | ||
}, | ||
"description": "The amount of tokens to swap" | ||
}, | ||
{ | ||
"in": "query", | ||
"name": "address", | ||
"required": true, | ||
"schema": { | ||
"type": "string" | ||
}, | ||
"description": "The address of the user" | ||
} | ||
], | ||
"responses": { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add error handling for Synapse.bridge call
The
Synapse.bridge
call could fail independently, but there's no specific error handling for this operation. This could lead to unclear error messages or unhandled promise rejections.Consider wrapping the bridge call in a try-catch:
📝 Committable suggestion