feat(rest-api): Adds validateRouteExists validation [SLT-260]

[abtestingalpha]

Description

• removes unused validateTokens
• Adds validateRouteExists to check for route existence before passing request params to controller

Summary by CodeRabbit

• New Features

  • Introduced a validation step for route existence in bridge-related API endpoints, ensuring only valid requests are processed.
  • Added a new function to validate routes based on specified parameters.

• Bug Fixes

  • Enhanced error handling to return informative messages when unsupported routes are requested.

• Tests

  • Added new test cases to verify the behavior of the API when unsupported routes are accessed.

• Chores

  • Removed deprecated token validation functionality to streamline the validation process.

[coderabbitai]

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

Commits

Files that changed from the base of the PR and between 07a08f4 and fe54496.

Files selected for processing (2)

• packages/rest-api/src/routes/bridgeTxInfoRoute.ts (2 hunks)
• packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts (7 hunks)

___________________________________________________________________________
< Your code and I are going to be best friends. Very critical best friends. >
---------------------------------------------------------------------------
 \
  \   \
       \ /\
       ( )
     .( o ).

Walkthrough

The pull request introduces validation enhancements across several files in the REST API. It adds a new function validateRouteExists to check the validity of routes based on specified parameters. This function is integrated into the bridgeLimitsRoute.ts and bridgeRoute.ts files to ensure that only requests with valid routes are processed. Additionally, new test cases are created to verify the behavior of these validations, while a previous validation utility file is removed.

Changes

Files	Change Summary
packages/rest-api/src/routes/bridgeLimitsRoute.ts, packages/rest-api/src/routes/bridgeRoute.ts	Added validateRouteExists function to validate routes based on query parameters in GET request handlers. Returns error messages for invalid routes.
packages/rest-api/src/tests/bridgeLimitsRoute.test.ts, packages/rest-api/src/tests/bridgeRoute.test.ts	Introduced new test cases to check for 400 status responses when unsupported routes are requested. Enhances error handling coverage for bridge routes.
packages/rest-api/src/utils/bridgeRouteMapping.ts	Modified constructJSON function's return type and structure. Introduced BRIDGE_ROUTE_MAPPING_SYMBOLS export to manage bridge route mappings.
packages/rest-api/src/validations/validateRouteExists.ts	Added validateRouteExists function to validate routes based on token and chain parameters. Returns true or false based on route availability.
packages/rest-api/src/validations/validateTokens.ts	Removed the validateTokens function and its associated file, eliminating previous token validation logic.

Possibly related PRs

• REST API Migration #3049: Involves validation logic for the bridgeRoute.ts file, utilizing the validateRouteExists function.
• [SLT-157] feat(rest-api): Refactor for RESTful architectural standardization #3126: Focuses on refactoring for RESTful architectural standardization, possibly improving validation logic.
• REST API Improvements [SLT-179] #3133: Modifies validation logic in various controllers, including checks for fromToken and toToken.
• Adds /destinationTokens route [SLT-204] #3151: Introduces the /destinationTokens route with validation for fromToken, relevant to the main PR.
• fix(synapse-interface): Reorders validation to check existence first #3156: Focuses on reordering validation checks, similar to the validation enhancements in the main PR.
• feat(api): bridge limits [SLT-165]  #3179: Introduces the /bridgeLimits endpoint, directly relating to the changes made in the main PR involving validation.

Suggested labels

size/m, needs-yarn-install

Suggested reviewers

• traian0x
• Defi-Moses
• bigboydiamonds

Poem

In the garden where routes intertwine,
A rabbit hops, with a heart so fine.
Validations bloom, like flowers in spring,
Ensuring each path is a safe, joyful thing.
With tokens and chains, we dance and play,
Hopping along the bridge, come what may! 🐇✨

Tip

CodeRabbit can suggest fixes for GitHub Check annotations.

Configure reviews.tools.github-checks in your project's settings in CodeRabbit to adjust the time to wait for GitHub Checks to complete.

---

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  -- I pushed a fix in commit <commit_id>, please review it.
  -- Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  -- @coderabbitai generate unit testing code for this file.
  -- @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  -- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  -- @coderabbitai read src/utils.ts and generate unit testing code.
  -- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  -- @coderabbitai help me debug CodeRabbit configuration file.

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 using PR comments)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[cloudflare-workers-and-pages]

Deploying sanguine-fe with    Cloudflare Pages

Latest commit:	fe54496
Status:	✅  Deploy successful!
Preview URL:	https://41c592cf.sanguine-fe.pages.dev
Branch Preview URL:	https://rest-api-route-validation.sanguine-fe.pages.dev

View logs

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.95792%. Comparing base (9418b40) to head (fe54496).
Report is 16 commits behind head on master.

Additional details and impacted files

@@                 Coverage Diff                 @@
##              master       #3180         +/-   ##
===================================================
- Coverage   41.54443%   37.95792%   -3.58651%     
===================================================
  Files            460         418         -42     
  Lines          25770       24240       -1530     
  Branches         357          82        -275     
===================================================
- Hits           10706        9201       -1505     
+ Misses         14326       14301         -25     
  Partials         738         738                 

Flag	Coverage Δ
packages	90.96267% <ø> (ø)
solidity	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[coderabbitai]

Actionable comments posted: 2

Outside diff range and nitpick comments (6)

packages/rest-api/src/utils/bridgeRouteMapping.ts (2)

99-99: Add explanatory comment for BRIDGE_ROUTE_MAPPING_SYMBOLS.

The new exported constant BRIDGE_ROUTE_MAPPING_SYMBOLS provides direct access to the stringified bridge routes. Consider adding a comment to explain its purpose and the reasoning behind passing an empty array as the second argument to constructJSON.

Add a comment above the constant declaration:

// Stores the stringified bridge routes without any exclusions
// The empty array passed to constructJSON indicates no routes should be excluded
export const BRIDGE_ROUTE_MAPPING_SYMBOLS = constructJSON(BRIDGE_MAP, [])

---

100-102: Approve the change and suggest minor readability improvement.

The modification to BRIDGE_ROUTE_MAPPING effectively separates the stringified and transformed versions of the bridge route mapping, aligning with the new return type of constructJSON. This change enhances flexibility in data usage across the codebase.

Consider using object shorthand notation for improved readability:

export const BRIDGE_ROUTE_MAPPING = transformBridgeRouteValues(BRIDGE_ROUTE_MAPPING_SYMBOLS)

packages/rest-api/src/tests/bridgeLimitsRoute.test.ts (1)

38-50: LGTM: New test case for unsupported route

The new test case correctly validates the behavior for an unsupported route, aligning with the PR objective. It properly checks for a 400 status and the expected error message.

Consider adding a comment explaining why this specific combination of tokens (NativeGasAddress to USDC) is expected to be an unsupported route. This would enhance the test's readability and maintainability.

 it('should return 400 for unsupported route', async () => {
+    // Test an unsupported route: bridging native gas token to USDC
     const response = await request(app).get('/bridgeLimits').query({
       fromChain: '1',
       toChain: '10',
       fromToken: NativeGasAddress,
       toToken: USDC.addresses[10],
     })
     expect(response.status).toBe(400)
     expect(response.body.error).toHaveProperty(
       'message',
       'No valid route exists for the chain/token combination'
     )
   }, 10_000)

packages/rest-api/src/tests/bridgeRoute.test.ts (1)

Line range hint 1-73: Consider enhancing overall test suite documentation and consistency.

The test suite covers a good range of scenarios, including the newly added test for unsupported routes. To further improve the quality and maintainability of the test suite, consider the following suggestions:

1. Add brief comments before each test case explaining its purpose and the scenario it's testing. This will make it easier for other developers to understand and maintain the tests.

2. Standardize the timeout values across test cases. Currently, some tests use 15000ms, while others use 10000ms. Choose a consistent timeout that works for all cases, or justify the differences if they're necessary.

3. Group related test cases together. For example, you could group all the error cases (400 status codes) together, and all the successful cases (200 status codes) together.

4. Consider adding more edge cases, such as testing with very large amounts or with different token decimals.

Here's an example of how you could improve the documentation for one of the existing test cases:

+  // Test case for a valid USDC bridge from Ethereum to Optimism
+  // This tests a common use case with a specific amount
   it('should return bridge quotes for valid input, 1000 USDC from Ethereum to Optimism', async () => {
     // ... (existing test code) ...
   }, 15000)

Apply similar documentation improvements to other test cases for consistency and clarity.

packages/rest-api/src/routes/bridgeLimitsRoute.ts (1)

134-140: Approved: New route validation enhances request processing.

The new custom validation check using validateRouteExists is a valuable addition that aligns with the PR objectives. It ensures that only requests with valid routes are processed further, enhancing the API's robustness.

Consider adding more specific error information to help diagnose issues:

 check()
   .custom((_value, { req }) => {
     const { fromChain, toChain, fromToken, toToken } = req.query

     return validateRouteExists(fromChain, fromToken, toChain, toToken)
   })
-  .withMessage('No valid route exists for the chain/token combination'),
+  .withMessage((_, { req }) => {
+    const { fromChain, toChain, fromToken, toToken } = req.query
+    return `No valid route exists for the chain/token combination: ${fromChain}->${toChain}, ${fromToken}->${toToken}`
+  }),

This change would provide more context in the error message, making it easier to identify the specific invalid route.

packages/rest-api/src/routes/bridgeRoute.ts (1)

226-232: LGTM with suggestions: New custom validation for route existence

The new validation check for route existence is well-implemented and aligns with the PR objectives. It effectively uses the validateRouteExists function to ensure that only valid routes are processed.

Suggestions for improvement:

1. Consider adding type checks for the query parameters before passing them to validateRouteExists to ensure type safety.
2. The error message could be more specific, potentially including details about which part of the route (fromChain, toChain, fromToken, or toToken) is invalid.

Here's a suggested improvement:

check()
  .custom((_value, { req }) => {
    const { fromChain, toChain, fromToken, toToken } = req.query;
    
    // Ensure all required parameters are strings
    if (typeof fromChain !== 'string' || typeof toChain !== 'string' ||
        typeof fromToken !== 'string' || typeof toToken !== 'string') {
      throw new Error('Invalid parameter types');
    }

    return validateRouteExists(fromChain, fromToken, toChain, toToken);
  })
  .withMessage((_, { req }) => {
    const { fromChain, toChain, fromToken, toToken } = req.query;
    return `No valid route exists for the chain/token combination: ${fromChain}-${fromToken} to ${toChain}-${toToken}`;
  }),

This suggestion adds type checking and provides a more detailed error message.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 67a04cd and 1a5f4a0.

Files selected for processing (7)

• packages/rest-api/src/routes/bridgeLimitsRoute.ts (2 hunks)
• packages/rest-api/src/routes/bridgeRoute.ts (2 hunks)
• packages/rest-api/src/tests/bridgeLimitsRoute.test.ts (2 hunks)
• packages/rest-api/src/tests/bridgeRoute.test.ts (2 hunks)
• packages/rest-api/src/utils/bridgeRouteMapping.ts (3 hunks)
• packages/rest-api/src/validations/validateRouteExists.ts (1 hunks)
• packages/rest-api/src/validations/validateTokens.ts (0 hunks)

Files not reviewed due to no reviewable changes (1)

• packages/rest-api/src/validations/validateTokens.ts

Additional comments not posted (8)

packages/rest-api/src/validations/validateRouteExists.ts (2)

1-2: LGTM: Imports are relevant and concise.

The imported utilities tokenAddressToToken and BRIDGE_ROUTE_MAPPING_SYMBOLS are appropriately used in the function, contributing to its functionality without unnecessary imports.

---

1-20: Overall assessment: Good implementation with room for improvement.

The validateRouteExists function effectively addresses the PR objective of validating routes before forwarding requests to the controller. It makes good use of imported utilities and implements a logical flow for route validation.

However, to enhance robustness and maintainability, consider:

1. Adding input validation and type checking.
2. Implementing more explicit error handling.
3. Including function documentation to explain its purpose and usage.

These improvements will make the code more resilient and easier to maintain in the long run.

packages/rest-api/src/utils/bridgeRouteMapping.ts (1)

16-16: Verify the impact of return type change and update documentation.

The return type of constructJSON has been changed from TransformedBridgeRoutes to StringifiedBridgeRoutes, and the function now returns result directly. This change aligns with the new implementation but may affect other parts of the codebase.

To ensure this change doesn't introduce bugs:

Also, please update the function's documentation to reflect this change in behavior and return type.

Also applies to: 59-59

Verification successful

Return type change verified successfully with no remaining references to TransformedBridgeRoutes. Documentation has been updated accordingly.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining usage of TransformedBridgeRoutes type with constructJSON
# Expected: No results, indicating all usages have been updated

rg --type typescript "constructJSON.*TransformedBridgeRoutes"

# Description: Verify that all calls to constructJSON are handling string arrays
# Expected: All results should show string array handling

rg --type typescript -A 3 "constructJSON\("

Length of output: 179

---

Script:

#!/bin/bash
# Description: Check for any remaining usage of TransformedBridgeRoutes type with constructJSON
# Expected: No results, indicating all usages have been updated

rg --glob "*.ts" "constructJSON.*TransformedBridgeRoutes"
rg --glob "*.tsx" "constructJSON.*TransformedBridgeRoutes"

# Description: Verify that all calls to constructJSON are handling string arrays
# Expected: All results should show string array handling

rg --glob "*.ts" -A 3 "constructJSON\("
rg --glob "*.tsx" -A 3 "constructJSON\("

Length of output: 1240

packages/rest-api/src/tests/bridgeLimitsRoute.test.ts (1)

6-6: LGTM: New import added correctly

The import for NativeGasAddress is correctly placed and is used in the new test case.

packages/rest-api/src/routes/bridgeLimitsRoute.ts (2)

11-11: LGTM: New import for route validation.

The import of validateRouteExists is correctly placed and aligns with the PR objectives to enhance route validation.

---

Line range hint 1-140: Summary: Effective enhancement of route validation.

The changes in this file successfully implement the new route validation as described in the PR objectives. The addition of validateRouteExists improves the API's robustness by ensuring that only valid routes are processed. This change is well-integrated with the existing validation checks and follows the established coding patterns in the file.

To ensure the validateRouteExists function is implemented correctly, let's verify its implementation:

This will help confirm that the validateRouteExists function is properly implemented and exported, supporting the changes made in this file.

packages/rest-api/src/routes/bridgeRoute.ts (2)

11-11: LGTM: New import statement for validateRouteExists

The import statement for validateRouteExists is correctly added and aligns with the PR objectives.

---

Line range hint 1-238: Summary: Effective implementation of route validation

The changes in this file successfully implement the new route validation feature as described in the PR objectives. The addition of validateRouteExists enhances the API's robustness by ensuring that only valid routes are processed before reaching the controller.

Key points:

1. The new import and validation check are correctly implemented.
2. The changes integrate well with the existing code structure.
3. The implementation aligns with the PR's goal of improving validation mechanisms.

While the implementation is solid, consider the suggestions for improved type safety and more detailed error messages to further enhance the user experience and code reliability.

Overall, these changes contribute positively to the API's functionality and error handling capabilities.

[codecov]

Bundle Report

Changes will increase total bundle size by 247.68kB (0.69%) ⬆️. This is within the configured threshold ✅

Detailed changes

Bundle name	Size	Change
sdk-router-@synapsecns/sdk-router-cjs	526.74kB	409.5kB (349.29%) ⬆️
synapse-interface-server-cjs	1.47MB	142.55kB (-8.82%) ⬇️
widget-cjs-esm*	273.3kB	8.62kB (-3.06%) ⬇️
widget-esm-cjs	273.3kB	10.65kB (-3.75%) ⬇️

ℹ️ *Bundle size includes cached data from a previous commit