REST API Migration #3049
REST API Migration #3049
Conversation
WalkthroughThe project has undergone updates that include modifications to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant API
participant Token
participant BridgeMap
participant Chain
User->>API: Request Token Info
API->>Token: Retrieve Token Data
Token->>BridgeMap: Check Bridgeable Tokens
BridgeMap->>Token: Return Token Data
API->>User: Respond with Token Info
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range, codebase verification and nitpick comments (1)
packages/rest-api/package.json (1)
26-33: RemovelodashfromdevDependenciesinrest-apipackageThe
lodashlibrary is used in the runtime code of therest-apipackage, specifically insrc/constants/chains.ts. Therefore, it should remain independencies. However, there is no evidence of its usage in a development context within this package, making its presence indevDependenciesunnecessary.
- Remove
lodashfromdevDependenciesinpackages/rest-api/package.json.Analysis chain
Verify the necessity of
lodashin bothdependenciesanddevDependencies.Ensure that
lodashis required in both sections, as it might be redundant.Run the following script to verify the usage of
lodashin the codebase:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of lodash in the codebase. # Test: Search for lodash usage. Expect: Occurrences in both runtime and development contexts. rg --type js --type ts 'lodash'Length of output: 7172
Script:
#!/bin/bash # Description: Verify the usage of lodash specifically within the rest-api package. # Test: Search for lodash usage in the rest-api package to determine the context. rg --type js --type ts 'lodash' packages/rest-apiLength of output: 114
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- packages/rest-api/package.json (2 hunks)
- packages/rest-api/src/app.ts (13 hunks)
- packages/rest-api/src/constants/bridgeMap.ts (39 hunks)
- packages/rest-api/src/constants/bridgeable.ts (17 hunks)
- packages/rest-api/src/constants/chains.ts (1 hunks)
- packages/rest-api/src/types/index.d.ts (1 hunks)
- packages/rest-api/tsconfig.json (2 hunks)
Additional context used
Biome
packages/rest-api/src/types/index.d.ts
[error] 2-2: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 3-3: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
Additional comments not posted (14)
packages/rest-api/tsconfig.json (1)
3-4: LGTM! Inclusion of./src/config/**/*and comments improve clarity.The expansion of the
includearray and the addition of comments enhance the project structure and documentation.Also applies to: 14-14
packages/rest-api/package.json (1)
9-15: LGTM! Script changes enhance the build and linting process.The reorganization and addition of scripts streamline the build and linting processes.
packages/rest-api/src/app.ts (6)
5-10: LGTM! Import restructuring.The import statements have been updated to reflect new paths, aligning with the project's reorganization.
12-12: LGTM! Simplifiedchainsvariable.The
chainsvariable is now directly assigned fromCHAINS_ARRAY, simplifying the code.
Line range hint
13-28: LGTM! Converted to arrow function.The
findTokenInfofunction has been converted to an arrow function, improving consistency with modern JavaScript practices.
32-43: LGTM! Refactored HTML generation.The logic for generating HTML for tokens has been refactored to utilize template literals and
Object.entries, enhancing readability and maintainability.
88-88: LGTM! Improved query parameter handling.Query parameters are explicitly converted to strings, improving robustness against unexpected input types.
Also applies to: 177-178, 276-276, 371-372
237-247: LGTM! Optimized asynchronous operations.The use of
Promise.alloptimizes performance by allowing concurrent execution of asynchronous tasks. Consider improving error handling for better user feedback.Also applies to: 436-452
packages/rest-api/src/constants/chains.ts (2)
5-491: LGTM! Comprehensive chain definitions.Each chain is defined with detailed properties, ensuring consistency and completeness.
493-495: LGTM! Flexible export statements.The chains are exported as
CHAINS,CHAINS_ARRAY, andCHAINS_BY_ID, providing flexibility in accessing chain data.packages/rest-api/src/constants/bridgeable.ts (1)
Line range hint
6-1391: LGTM! Detailed token definitions.Each token is defined as a
BridgeableTokenwith detailed properties, ensuring consistency and completeness.packages/rest-api/src/constants/bridgeMap.ts (3)
24-30: New entry for 'JEWEL' added.The new entry for the token 'JEWEL' with address
0x12f79f8c1A6e47a9b5F0796FDb008Bdc182fa19ehas been added correctly with appropriate properties.
218-224: New entry for 'SPEC' added.The new entry for the token 'SPEC' with address
0xAdF7C35560035944e805D98fF17d58CDe2449389has been added correctly with appropriate properties.
34-42: Inclusion of 'RFQ.USDC' in origin and destination arrays.The inclusion of 'RFQ.USDC' in the
originanddestinationarrays for multiple tokens enhances the flexibility of token interactions, aligning with the broader integration of the RFQ mechanism.Also applies to: 71-79, 101-109, 124-132, 161-169, 191-199, 256-264, 286-294, 311-312, 355-363, 368-376, 391-399, 404-412, 424-432, 444-452, 876-882, 1113-1121, 1250-1258, 1279-1287, 1298-1306, 1317-1325, 1349-1357, 1384-1392, 1397-1405, 1417-1425, 1441-1449, 1472-1480, 1485-1493, 1495-1503, 1508-1516, 1528-1536, 1796-1804, 1826-1834, 1880-1888
| addresses: {} | ||
| decimals: {} |
There was a problem hiding this comment.
Avoid using {} as a type.
Prefer explicitly defining the object shape for better type safety.
Apply this diff to define the object shape:
export interface BridgeableToken {
- addresses: {}
- decimals: {}
+ addresses: { [key: string]: string }
+ decimals: { [key: string]: number }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.
| addresses: {} | |
| decimals: {} | |
| addresses: { [key: string]: string } | |
| decimals: { [key: string]: number } |
Tools
Biome
[error] 2-2: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 3-3: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3049 +/- ##
===================================================
- Coverage 32.92244% 25.29498% -7.62747%
===================================================
Files 533 781 +248
Lines 33099 56869 +23770
Branches 82 82
===================================================
+ Hits 10897 14385 +3488
- Misses 21231 40999 +19768
- Partials 971 1485 +514
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
trajan0x
requested review from
bigboydiamonds and
abtestingalpha
as code owners
Deploying sanguine-fe with
|
| Latest commit: |
e0758ac
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fd0df7d4.sanguine-fe.pages.dev |
| Branch Preview URL: | https://api-migration.sanguine-fe.pages.dev |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
Files selected for processing (1)
- packages/rest-api/package.json (2 hunks)
Files skipped from review due to trivial changes (1)
- packages/rest-api/package.json
Migrates from heroku to the new server provider which enables auto-deploys from the target branch.
Also restructured the api:
The existing code in the repo is not connected to the live deployment so overwriting does not cause any issues.
@aureliusbtc can you setup api.synapseprotocol.com to put the finishing touches on this? Should drastically improve maintenence/cleanliness.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Documentation