Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(api): bridge limits [SLT-165] #3179
Changes from 33 commits
f131926
acd653c
6ada55a
8530122
4428e8d
69f36c3
b48a36e
d4fac70
5e5e2b7
7eab9a7
f56517a
ddb245d
139e27c
95a93ca
96ec975
be863d5
c6cd501
bb9aa82
9fb48d8
d006117
8ce1097
f844897
8a8c923
07980e5
46e73c2
6696b4d
0cce954
2039353
b342984
f6246d4
a1d4650
33f5584
60138d4
12de2ee
53e3c87
dc5bb55
60dcf57
03dc45b
c6b5d30
30fd0c5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
🛠️ Refactor suggestion
Consider including additional relevant information in the response
Returning only
maxOriginAmount
andminOriginAmount
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.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.
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:
Committable suggestion
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.
🛠️ 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:
And add types for other variables as needed.
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.
See other routes that have
normalizeNativeTokenAddress
andchecksumAddreses
middleware.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.
Thanks, updated to use those middleware functions in 30fd0c5
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.
🛠️ 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:
This refactoring would make the route definition more concise and easier to read.
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.
💡 Codebase verification
The
/bridgeLimits
route is not defined inbridgeLimitsRoute.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:
Length of output: 130
Script:
Length of output: 246
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.
Fix inconsistent property name 'swappableType' vs. 'swapableType'
There is a spelling inconsistency in the property name
swappableType
(line 28) andswapableType
(line 99). This inconsistency can lead to undefined values or errors. Please standardize the property name toswappableType
throughout the code.Apply this diff to fix the inconsistency:
Also applies to: 99-99