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

REST API Improvements [SLT-179] #3133

Merged
merged 7 commits into from
Sep 18, 2024
Merged

REST API Improvements [SLT-179] #3133

merged 7 commits into from
Sep 18, 2024

Conversation

Defi-Moses
Copy link
Collaborator

@Defi-Moses Defi-Moses commented Sep 16, 2024

Small fix to edit the arguments for the swaptxinfo to include the address so that approval estimation is correct

Summary by CodeRabbit

  • New Features
    • Enhanced validation for token addresses in bridge and swap transactions, ensuring only valid addresses are processed.
    • Improved error handling for unsupported tokens, providing clearer feedback to users.
    • Introduced utilities to validate token addresses and check token support on specific blockchains.
  • Bug Fixes
    • Updated tests to use specific Ethereum token addresses for better accuracy in validation.
    • Enhanced error messaging in tests for invalid token addresses and parameters.

Copy link
Contributor

coderabbitai bot commented Sep 16, 2024

Walkthrough

The changes enhance the handling and validation of token addresses in the REST API. The bridgeTxInfoController, swapTxInfoController, and bridgeController functions have been updated to directly extract and validate fromToken and toToken query parameters using the tokenAddressToToken utility. Additionally, new validation functions ensure that only valid and supported token addresses are processed. The test cases across various routes have also been updated to reflect these changes, improving input validation and error handling.

Changes

File Change Summary
packages/rest-api/src/controllers/bridgeTxInfoController.ts Modified to enhance handling of token addresses; introduced validation for fromToken and toToken.
packages/rest-api/src/controllers/swapTxInfoController.ts Streamlined handling of token addresses; updated to use tokenAddressToToken for validation.
packages/rest-api/src/controllers/bridgeController.ts Enhanced token address processing; replaced reliance on res.locals.tokenInfo with direct queries.
packages/rest-api/src/routes/bridgeRoute.ts Improved validation logic for fromToken and toToken using new utility functions.
packages/rest-api/src/routes/swapRoute.ts Updated validation for fromToken and toToken to include existence and format checks.
packages/rest-api/src/tests/bridgeRoute.test.ts Updated test cases to use specific token addresses; improved validation checks.
packages/rest-api/src/tests/swapRoute.test.ts Modified tests to utilize actual token addresses; enhanced error handling in test cases.
packages/rest-api/src/utils/isTokenAddress.ts Added utility function to validate token addresses against a predefined list.
packages/rest-api/src/utils/isTokenSupportedOnChain.ts Introduced utility function to check if a token is supported on a specific blockchain.
packages/rest-api/jest.config.js Updated Jest configuration for module resolution to improve clarity.
packages/rest-api/package.json Modified test:coverage script to run Jest with coverage collection.

Possibly related PRs

  • API method updates #3122: The changes in the bridgeTxInfoController and swapTxInfoController functions enhance the handling of token addresses, which aligns with new API endpoints introduced for retrieving transaction IDs and statuses.
  • [SLT-157] feat(rest-api): Refactor for RESTful architectural standardization #3126: This PR refactors the REST API structure, including the bridgeTxInfoController and swapTxInfoController, indicating a close relationship in terms of code organization and functionality.
  • fixing restapi gh actions flake #3139: Modifications in the package.json file to include supertest suggest enhancements in testing capabilities for the REST API, relevant to the changes in the main PR involving API controller functions.

Suggested labels

javascript, M-deps, M-docs

Suggested reviewers

🐇 In the meadow where tokens align,
Validation dances, oh how they shine!
From fromToken to toToken, all checked with glee,
A leap into clarity, as clear as can be!
With robust routes and tests that delight,
Our rabbit hops forward, ready to write! 🐰✨


Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f0a059b and e37ee32.

Files selected for processing (1)
  • packages/rest-api/package.json (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • packages/rest-api/package.json

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
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 generate interesting stats about this repository and render them as a table.
    -- @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.
Early access features: enabled

We are currently testing the following features in early access:

  • OpenAI o1 for code reviews: OpenAI's new o1 model is being tested for code reviews. This model has advanced reasoning capabilities and can provide more detailed feedback on your code.

Note:

  • You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 7ffafac and 5ea9ba2.

Files selected for processing (1)
  • packages/rest-api/src/controllers/swapTxInfoController.ts (2 hunks)
Additional comments not posted (1)
packages/rest-api/src/controllers/swapTxInfoController.ts (1)

28-28: LGTM!

The usage of the address parameter in the Synapse.swap function call aligns with the addition of the parameter in the function signature. This change enhances the flexibility of the swapTxInfoController function by allowing the caller to specify the source address directly, eliminating the reliance on fromTokenInfo.address.

@@ -10,7 +10,7 @@ export const swapTxInfoController = async (req, res) => {
}

try {
const { chain, amount } = req.query
const { chain, amount, address } = req.query
Copy link
Contributor

Choose a reason for hiding this comment

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

Tip

Codebase Verification

Add validation for the address parameter in the route setup

The address parameter has been correctly implemented in the swapTxInfoController function, which enhances the flexibility of the swap operation as intended. However, there's a potential issue in the route setup:

  • The route validation includes checks for chain, fromToken, and toToken, but there's no validation for the newly added address parameter.

To ensure data integrity and security, please add appropriate validation for the address parameter in the route setup. This should include:

  • Checking if the address is provided (required field)
  • Validating the format of the address (e.g., using a regex pattern for Ethereum addresses)

Example:

check('address')
  .exists().withMessage('address is required')
  .matches(/^0x[a-fA-F0-9]{40}$/).withMessage('Invalid Ethereum address format')

Adding this validation will prevent potential issues caused by missing or invalid address inputs.

Analysis chain

Verify the function usage across the codebase.

The addition of the address parameter enhances the flexibility of the swapTxInfoController function by allowing the caller to specify the address from which tokens are being swapped. This is a positive change.

However, it's crucial to ensure that all the callers of this function are updated to pass the address parameter to avoid runtime exceptions.

Run the following script to verify the function usage:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify all function calls to `swapTxInfoController` pass the `address` parameter.

# Test: Search for the function usage. Expect: Occurrences of the function call with the `address` parameter.
rg --type typescript -A 5 $'swapTxInfoController'

Length of output: 83


Script:

#!/bin/bash
# Description: Find the definition and usage of swapTxInfoController function

# Find the function definition
echo "Function Definition:"
fd -e ts -e tsx | xargs rg -i "function\s+swapTxInfoController" -A 10

echo "\nFunction Usage:"
# Search for function usage
fd -e ts -e tsx | xargs rg -i "swapTxInfoController" -A 5

Length of output: 1424


Script:

#!/bin/bash
# Description: Inspect the swapTxInfoController function definition and route setup

# View the full swapTxInfoController function definition
echo "Full swapTxInfoController Definition:"
rg -A 20 "export const swapTxInfoController" packages/rest-api/src/controllers/swapTxInfoController.ts

echo "\nRoute Setup and Validations:"
# Check for query parameter validations in the route file
rg -A 10 "router.get" packages/rest-api/src/routes/swapTxInfoRoute.ts

Length of output: 1289

Copy link

codecov bot commented Sep 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 37.94554%. Comparing base (0357100) to head (e37ee32).
Report is 2 commits behind head on master.

Additional details and impacted files
@@                 Coverage Diff                 @@
##              master       #3133         +/-   ##
===================================================
- Coverage   38.22166%   37.94554%   -0.27612%     
===================================================
  Files            423         418          -5     
  Lines          24371       24240        -131     
  Branches         119          82         -37     
===================================================
- Hits            9315        9198        -117     
+ Misses         14317       14303         -14     
  Partials         739         739                 
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.

Copy link

cloudflare-workers-and-pages bot commented Sep 16, 2024

Deploying sanguine-fe with  Cloudflare Pages  Cloudflare Pages

Latest commit: e37ee32
Status: ✅  Deploy successful!
Preview URL: https://2adea33d.sanguine-fe.pages.dev
Branch Preview URL: https://feat-api-swaptxinfo-fix.sanguine-fe.pages.dev

View logs

@abtestingalpha
Copy link
Collaborator

address is a new required field, so in that case need to

  • validate it in routes/swapTxInfoRoute.ts (currently only checking for other fields, need to validate new field)
  • update test to tests/swapTxInfoRoute.test.ts
  • run yarn test locally to make sure passing

Tests are currently failing bc no validation / missing address parameter

@github-actions github-actions bot added size/m and removed size/xs labels Sep 16, 2024
@Defi-Moses Defi-Moses changed the title fix swaptxinfo function fix swaptxinfo function & migrating to using tokenAddresses Sep 16, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range and nitpick comments (1)
packages/rest-api/src/utils/tokenAddressToToken.ts (1)

3-17: Consider adding type annotations and input validation to improve the function.

The tokenAddressToToken function is well-structured and follows a clear logic flow. However, consider the following suggestions to enhance its robustness and maintainability:

  1. Add a return type annotation for the function to explicitly define the expected return value type. This improves code readability and catches potential type-related issues at compile time.

  2. Add input validation for the chain and tokenAddress parameters to ensure they are of the expected type and format. This helps prevent unexpected behavior and provides better error handling.

Here's an example of how you can apply these suggestions:

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

export const tokenAddressToToken = (chain: string, tokenAddress: string): TokenInfo | null => {
  if (typeof chain !== 'string' || chain.trim().length === 0) {
    throw new Error('Invalid chain parameter');
  }
  if (typeof tokenAddress !== 'string' || tokenAddress.trim().length === 0) {
    throw new Error('Invalid tokenAddress parameter');
  }

  // Rest of the function code remains the same
  // ...
};

By adding these improvements, the function becomes more robust, maintainable, and self-documenting.

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e2bab25 and d35d50a.

Files selected for processing (15)
  • packages/rest-api/src/controllers/bridgeController.ts (1 hunks)
  • packages/rest-api/src/controllers/bridgeTxInfoController.ts (3 hunks)
  • packages/rest-api/src/controllers/swapController.ts (1 hunks)
  • packages/rest-api/src/controllers/swapTxInfoController.ts (2 hunks)
  • packages/rest-api/src/routes/bridgeRoute.ts (2 hunks)
  • packages/rest-api/src/routes/bridgeTxInfoRoute.ts (2 hunks)
  • packages/rest-api/src/routes/swapRoute.ts (2 hunks)
  • packages/rest-api/src/routes/swapTxInfoRoute.ts (2 hunks)
  • packages/rest-api/src/tests/bridgeRoute.test.ts (3 hunks)
  • packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts (3 hunks)
  • packages/rest-api/src/tests/swapRoute.test.ts (2 hunks)
  • packages/rest-api/src/tests/swapTxInfoRoute.test.ts (1 hunks)
  • packages/rest-api/src/utils/tokenAddressToToken.ts (1 hunks)
  • packages/rest-api/src/utils/tokenSymbolToToken.ts (1 hunks)
  • packages/rest-api/src/validations/validateTokens.ts (2 hunks)
Files skipped from review due to trivial changes (1)
  • packages/rest-api/src/utils/tokenSymbolToToken.ts
Additional context used
Learnings (1)
packages/rest-api/src/tests/swapRoute.test.ts (1)
Learnt from: abtestingalpha
PR: synapsecns/sanguine#3126
File: packages/rest-api/src/tests/indexRoute.test.ts:21-31
Timestamp: 2024-09-16T17:19:34.740Z
Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.
Gitleaks
packages/rest-api/src/tests/swapRoute.test.ts

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeRoute.test.ts

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/swapTxInfoRoute.test.ts

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


35-35: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


78-78: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Additional comments not posted (57)
packages/rest-api/src/validations/validateTokens.ts (2)

12-12: LGTM!

The change is consistent with the import statement change at line 3. The rest of the validation logic remains unchanged.


3-3: Verify the impact of the utility function change.

The findTokenInfo function has been replaced with the tokenSymbolToToken function. While the validation process remains intact, it's important to ensure that the new utility function is thoroughly tested and provides the expected behavior.

Run the following script to verify the usage of the tokenSymbolToToken function:

Verification successful

Verification of tokenSymbolToToken function implementation and usage

The tokenSymbolToToken function has been successfully located and verified. It is implemented in packages/rest-api/src/utils/tokenSymbolToToken.ts and its usage in validateTokens.ts is appropriate. Here are the key findings:

  1. The function is correctly exported and takes the expected parameters (chain and tokenSymbol).
  2. It uses the BRIDGE_MAP constant to look up chain and token information.
  3. The function returns null if the chain is not found, which allows for proper error handling in validateTokens.ts.
  4. When a matching token is found, it returns an object with the token's address and decimals.
  5. The implementation is consistent with its usage in the validation process.

Based on this analysis, the change from findTokenInfo to tokenSymbolToToken appears to be a valid refactoring that maintains the necessary functionality for token validation.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the usage of the `tokenSymbolToToken` function.

# Test 1: Search for the function definition. Expect: The function is defined.
ast-grep --lang typescript --pattern $'function tokenSymbolToToken($_,$_) {
  $$$
}'

# Test 2: Search for the function usage. Expect: The function is used correctly.
rg --type typescript -A 5 $'tokenSymbolToToken'

Length of output: 167


Script:

#!/bin/bash
# Description: Verify the existence and usage of the `tokenSymbolToToken` function.

# Test 1: Check for TypeScript files in the repository
echo "TypeScript files in the repository:"
fd -e ts -e tsx

# Test 2: Search for the tokenSymbolToToken function definition
echo -e "\nSearching for tokenSymbolToToken function definition:"
rg "function tokenSymbolToToken" -A 5

# Test 3: Search for imports of tokenSymbolToToken
echo -e "\nSearching for imports of tokenSymbolToToken:"
rg "import.*tokenSymbolToToken"

# Test 4: Search for usage of tokenSymbolToToken
echo -e "\nSearching for usage of tokenSymbolToToken:"
rg "tokenSymbolToToken\(" -A 2

Length of output: 47556


Script:

#!/bin/bash
# Description: Verify the implementation of the tokenSymbolToToken function

# Search for the tokenSymbolToToken function definition
echo "Searching for tokenSymbolToToken function definition:"
rg "export (const|function) tokenSymbolToToken" -A 10 "packages/rest-api/src/utils/tokenSymbolToToken.ts"

# Search for any imports used in the tokenSymbolToToken function
echo -e "\nSearching for imports in tokenSymbolToToken.ts:"
rg "^import" "packages/rest-api/src/utils/tokenSymbolToToken.ts"

# Search for any type definitions related to the function
echo -e "\nSearching for type definitions:"
rg "type|interface" "packages/rest-api/src/utils/tokenSymbolToToken.ts"

Length of output: 961

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

20-24: LGTM!

The validation logic for the fromToken parameter is correct. It checks for the existence of the parameter and validates it as a valid Ethereum address using the isAddress function from the ethers library. The error messages are appropriate for missing or invalid fromToken values.


25-29: LGTM!

The validation logic for the toToken parameter is correct. It checks for the existence of the parameter and validates it as a valid Ethereum address using the isAddress function from the ethers library. The error messages are appropriate for missing or invalid toToken values.

packages/rest-api/src/routes/swapTxInfoRoute.ts (5)

3-3: LGTM!

Importing the isAddress function from the ethers library is a good practice for validating Ethereum addresses.


20-24: LGTM!

The validation checks for the fromToken parameter ensure that it exists and is a valid Ethereum address, which improves the robustness of the input validation.


25-29: LGTM!

The validation checks for the toToken parameter ensure that it exists and is a valid Ethereum address, which improves the robustness of the input validation.


30-30: LGTM!

The validation check for the amount parameter ensures that it exists and is a numeric value, which improves the robustness of the input validation.


31-35: LGTM!

The validation checks for the new address parameter ensure that it exists and is a valid Ethereum address, which addresses the concern raised by abtestingalpha and improves the robustness of the input validation.

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

3-3: LGTM!

The import of the isAddress utility from the ethers library is necessary for the validation logic changes made in this file.


26-30: LGTM!

The validation for the fromToken field has been improved by checking for its existence and validating it as an Ethereum address using the isAddress utility. This change enhances the robustness of the validation logic.


31-35: LGTM!

The validation for the toToken field has been improved by checking for its existence and validating it as an Ethereum address using the isAddress utility. This change enhances the robustness of the validation logic.


36-36: LGTM!

The validation for the amount field has been improved by checking for its existence and validating it as a numeric value. This change enhances the robustness of the validation logic.

packages/rest-api/src/controllers/swapController.ts (4)

3-3: LGTM!

Importing the isAddress function from the ethers library is a good practice for validating Ethereum addresses.


6-6: LGTM!

Importing the tokenAddressToToken function from the ../utils/tokenAddressToToken module is a good practice for retrieving token information based on token addresses.


14-27: LGTM!

The code segment performs the following validations and error handling:

  1. Checks if both fromToken and toToken are valid Ethereum addresses using the isAddress function.
  2. Returns a 400 status with an appropriate error message if either address is invalid.
  3. Retrieves token information for fromToken and toToken using the tokenAddressToToken function.
  4. Returns a 400 status with an appropriate error message if either token is not supported on the specified chain.

These validations and error handling practices ensure that only valid and supported tokens are processed for swaps.


32-33: Verify the expected input format for the Synapse.swapQuote function.

The code passes the original fromToken and toToken addresses directly to the Synapse.swapQuote function instead of using fromTokenInfo.address and toTokenInfo.address.

Please ensure that the Synapse.swapQuote function expects the original token addresses and not the addresses from the token information objects.

packages/rest-api/src/routes/bridgeTxInfoRoute.ts (4)

3-3: LGTM!

Importing the isAddress function from the ethers library is a good choice for validating Ethereum addresses in the route.


26-30: LGTM!

The validation checks for the fromToken field are implemented correctly. The code ensures that the field exists and is a valid Ethereum address, providing appropriate error messages.


31-35: LGTM!

The validation checks for the toToken field are implemented correctly. The code ensures that the field exists and is a valid Ethereum address, providing appropriate error messages.


36-41: LGTM!

The validation checks for the amount and destAddress fields are implemented correctly. The code ensures that the amount field exists and is numeric, and the destAddress field exists and is a valid Ethereum address, providing appropriate error messages.

packages/rest-api/src/controllers/swapTxInfoController.ts (5)

3-3: LGTM!

Importing the isAddress utility from the ethers library is a good practice for validating Ethereum addresses.


6-6: LGTM!

Importing the tokenAddressToToken utility can help retrieve token information based on the address, which is useful for the swapTxInfoController function.


15-28: Great job with the validation and error handling!

The changes in this code segment enhance the robustness of the swapTxInfoController function:

  • Destructuring fromToken and toToken from req.query is a clean way to access the query parameters.
  • Validating fromToken and toToken using the isAddress utility ensures that only valid addresses are processed.
  • Returning a 400 error response for invalid addresses is a good practice for error handling.
  • Using the tokenAddressToToken utility to retrieve token information is a modular approach.
  • Checking if the tokens are supported on the specified chain adds an extra layer of validation.
  • Returning a 400 error response for unsupported tokens is a good practice for error handling.

These changes improve the functionality and error handling of the controller.


34-35: LGTM!

Using fromToken and toToken directly in the Synapse.swapQuote function call is a cleaner approach. It eliminates the need to access the token addresses from fromTokenInfo and toTokenInfo, and aligns with the previous validation and error handling for these parameters.


41-42: LGTM!

Using address and toToken directly in the Synapse.swap function call is consistent with the changes made to the Synapse.swapQuote function call. It indicates a shift in how the swap operation is executed, and aligns with the previous validation and error handling for toToken.

packages/rest-api/src/controllers/bridgeController.ts (5)

3-3: LGTM!

Importing the isAddress function from the ethers library is a good practice for validating Ethereum addresses. This will help ensure data integrity and prevent potential errors related to invalid addresses.


7-7: LGTM!

Importing the tokenAddressToToken function is a good practice for validating and retrieving token information based on the token address and chain. This will help ensure that only supported tokens are processed, preventing potential errors related to unsupported tokens.


15-16: LGTM!

Extracting fromToken and toToken directly from the request query is a good change that allows the controller to access the token addresses without relying on res.locals.tokenInfo. This aligns with the goal of validating token addresses and retrieving token information based on the provided addresses.


17-28: LGTM!

The added validation for token addresses using the isAddress function and the retrieval of token information using the tokenAddressToToken function are excellent improvements to the controller. These changes ensure that only valid and supported token addresses are processed, preventing potential errors related to invalid or unsupported tokens.

Furthermore, responding with appropriate error messages and status codes enhances the error handling and provides meaningful feedback to the client, making the API more user-friendly and easier to debug.


35-36: LGTM!

Passing fromToken and toToken directly to the Synapse.allBridgeQuotes function is consistent with the changes made to extract these values from the request query. This ensures that the validated and supported token addresses are used for retrieving bridge quotes, maintaining the integrity of the data flow throughout the controller.

packages/rest-api/src/controllers/bridgeTxInfoController.ts (5)

3-3: LGTM!

Importing the isAddress function from the ethers library is a good practice for validating Ethereum addresses.


6-6: LGTM!

Importing the tokenAddressToToken function from the ../utils/tokenAddressToToken module is a good practice for mapping token addresses to their corresponding token information.


15-28: Great job on adding token address validation and error handling!

The code segment:

  • Extracts fromToken and toToken from the request query parameters.
  • Validates the token addresses using the isAddress function.
  • Returns appropriate error responses for invalid token addresses and unsupported tokens.
  • Maps the token addresses to their corresponding token information using the tokenAddressToToken function.

These changes enhance the robustness and security of the bridgeTxInfoController function by ensuring that only valid and supported token addresses are processed.


35-36: LGTM!

Updating the parameters passed to the Synapse.allBridgeQuotes method to use the raw token addresses is consistent with the rest of the code changes and aligns with the updated logic.


47-47: LGTM!

Updating the parameter passed to the Synapse.bridge method to use the raw fromToken address is consistent with the rest of the code changes and aligns with the updated logic.

packages/rest-api/src/tests/swapRoute.test.ts (4)

13-14: LGTM!

The test case has been updated to use token addresses instead of symbolic names, which is a good practice to ensure specificity and accuracy in tests. The core functionality remains unaffected by this change.

The static analysis hints about potential API key exposure can be safely ignored as the values are token addresses, not API keys.

Tools
Gitleaks

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-28: LGTM!

The test case has been updated to use a token address instead of a symbolic name for the fromToken parameter, which is consistent with the changes in the first test case. The core functionality remains unaffected by this change.

The static analysis hints about potential API key exposure can be safely ignored as the values are token addresses, not API keys.

Tools
Gitleaks

27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


36-48: LGTM!

The test case has been updated to use a token address instead of a symbolic name for the fromToken parameter, which is consistent with the changes in the previous test cases. Additionally, the test case now checks for an invalid toToken address, which is a good addition to ensure the validation of token addresses. The error message expectation is updated accordingly to match the new validation.

The static analysis hints about potential API key exposure can be safely ignored as the values are token addresses, not API keys.

Tools
Gitleaks

39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


54-55: LGTM!

The test case has been updated to use token addresses instead of symbolic names for the fromToken and toToken parameters, which is consistent with the changes in the previous test cases. The core functionality remains unaffected by this change.

The static analysis hints about potential API key exposure can be safely ignored as the values are token addresses, not API keys.

Tools
Gitleaks

54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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

14-15: LGTM!

Using specific token addresses instead of generic token names enhances the accuracy and relevance of the test case. The token addresses used are well-known, publicly available token contract addresses, not API keys.

Tools
Gitleaks

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


29-30: LGTM!

The changes are consistent with the updates made in the previous test case. The token addresses used are well-known, publicly available token contract addresses, not API keys.

Tools
Gitleaks

29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


44-45: LGTM!

The changes are consistent with the updates made in the previous test cases. The token addresses used are well-known, publicly available token contract addresses, not API keys.

Tools
Gitleaks

44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


52-52: LGTM!

Updating the test case description to check for an invalid fromToken address aligns with the modifications made to the test case itself and is a relevant and valuable test scenario.


56-57: LGTM!

Using an invalid address for the fromToken parameter is appropriate for testing the error handling of the route. The toToken address used is a well-known, publicly available token contract address, not an API key. The changes are consistent with the updates made in the previous test cases.

Tools
Gitleaks

57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


61-64: LGTM!

Updating the expected error message to reflect the scenario being tested (invalid fromToken address) aligns with the modifications made to the test case itself and improves the clarity of the test.


71-71: LGTM!

The change is consistent with the updates made in the previous test cases. The token address used is a well-known, publicly available token contract address, not an API key.

Tools
Gitleaks

71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


72-72: LGTM!

The change is consistent with the updates made in the previous test cases. The token address used is a well-known, publicly available token contract address, not an API key.

Tools
Gitleaks

72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


Line range hint 1-78: Test cases are not related to the swaptxinfo function or the swapTxInfoRoute.ts file.

The test cases in this file (packages/rest-api/src/tests/bridgeRoute.test.ts) are focused on the bridge route and do not cover the changes mentioned in the PR objectives and comments summary, which are related to the swaptxinfo function and the swapTxInfoRoute.ts file. This is expected, as the test cases for the swaptxinfo function would typically be located in a separate test file specific to the swapTxInfoRoute.ts file.

The changes made to the test cases in this file are relevant to the bridge route and improve the specificity and accuracy of the tests. No additional test cases are required in this file based on the provided PR objectives and comments summary.

Tools
Gitleaks

44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/swapTxInfoRoute.test.ts (5)

13-14: LGTM!

The changes to use specific Ethereum token addresses for USDC and DAI improve clarity and precision in testing. The addition of the address parameter is consistent with the PR objective.

Also applies to: 16-16

Tools
Gitleaks

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


24-34: Great addition!

This new test case covers an important edge case of handling invalid Ethereum addresses. It is well-structured and follows the same pattern as other test cases. The expected error message is clear and specific.

Tools
Gitleaks

27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


39-40: LGTM!

The addition of the address parameter maintains consistency with the updated swaptxinfo function signature. Using specific token addresses improves clarity and precision in testing.

Also applies to: 42-42

Tools
Gitleaks

39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


40-40: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


48-48: LGTM!

The renamed test case better reflects its focus on validating the toToken address. Using a specific token address for fromToken improves clarity and precision in testing. The addition of the address parameter maintains consistency with the updated swaptxinfo function signature.

Also applies to: 51-51, 54-54, 57-60


66-68: LGTM!

Using specific token addresses improves clarity and precision in testing. The addition of the address parameter maintains consistency with the updated swaptxinfo function signature.

Tools
Gitleaks

66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts (3)

14-15: LGTM!

Using specific token contract addresses instead of symbolic names improves clarity and specificity in the tests.

Tools
Gitleaks

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


46-59: Great work on improving error handling and messaging!

The new test case for handling invalid fromToken address is a valuable addition. Checking for a specific error message in the response body enhances the feedback provided during testing.

Tools
Gitleaks

51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


74-87: Excellent addition of the new test case for invalid destAddress!

The test case improves error handling and messaging by checking for a specific error message in the response body. This enhances the feedback provided during testing.

Tools
Gitleaks

78-78: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


79-79: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

const { chain, amount } = req.query
const fromTokenInfo = res.locals.tokenInfo.fromToken
const toTokenInfo = res.locals.tokenInfo.toToken
const { chain, amount, fromToken, toToken } = req.query
Copy link
Contributor

Choose a reason for hiding this comment

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

Add validation for the chain variable.

The code assumes that the chain variable is present in the req.query object and uses it directly without any validation.

It's important to validate the chain variable to ensure it is a valid and supported chain identifier. Consider adding validation checks for the chain variable to handle cases where an invalid or unsupported chain identifier is provided.

Here's an example of how you can validate the chain variable:

const { chain, amount, fromToken, toToken } = req.query

// Validate the chain variable
if (!isValidChain(chain)) {
  return res.status(400).json({ error: 'Invalid chain identifier' })
}

// Rest of the code...

// Helper function to validate the chain identifier
function isValidChain(chain: string): boolean {
  // Add your chain validation logic here
  // For example, check if the chain is in a list of supported chains
  const supportedChains = ['ethereum', 'polygon', 'binance']
  return supportedChains.includes(chain)
}

@Defi-Moses Defi-Moses changed the title fix swaptxinfo function & migrating to using tokenAddresses [SLT-179] REST API Improvements Sep 17, 2024
@Defi-Moses Defi-Moses changed the title [SLT-179] REST API Improvements REST API Improvements [SLT-179] Sep 17, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Outside diff range and nitpick comments (2)
packages/rest-api/src/controllers/swapTxInfoController.ts (2)

17-19: Improve error message specificity

The current error message 'Invalid token address' does not specify which token address is invalid (fromToken or toToken). Providing specific error messages can help developers and users diagnose issues more efficiently.

Apply this diff to enhance the error message:

 if (!isAddress(fromToken) || !isAddress(toToken)) {
-  return res.status(400).json({ error: 'Invalid token address' })
+  const invalidTokens = []
+  if (!isAddress(fromToken)) invalidTokens.push('fromToken')
+  if (!isAddress(toToken)) invalidTokens.push('toToken')
+  return res.status(400).json({ error: `Invalid token address: ${invalidTokens.join(', ')}` })
 }

Line range hint 50-55: Avoid exposing internal error details

Returning err.message in the response can expose sensitive internal error information to the client. It's safer to log the error internally and return a generic error message to the client.

Apply this diff to prevent exposing internal error details:

 } catch (err) {
+  console.error('Error in swapTxInfoController:', err)
   res.status(500).json({
     error:
       'An unexpected error occurred in /swapTxInfo. Please try again later.',
-    details: err.message,
   })
 }
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b7ba450 and 42890d9.

Files selected for processing (1)
  • packages/rest-api/src/controllers/swapTxInfoController.ts (2 hunks)
Additional comments not posted (1)
packages/rest-api/src/controllers/swapTxInfoController.ts (1)

15-16: Verify 'address' parameter validation in route setup

It's important to ensure that the address parameter is properly validated in the route definitions to prevent invalid data from reaching the controller.

Run the following script to check if the address parameter is validated in the route setup:

Comment on lines +15 to +16
const { chain, amount, address, fromToken, toToken } = req.query

Copy link
Contributor

Choose a reason for hiding this comment

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

Missing validation for 'address', 'amount', and 'chain' parameters

Currently, the code validates fromToken and toToken but lacks validation for other critical parameters: address, amount, and chain. This could lead to unexpected errors or security vulnerabilities if invalid or malicious inputs are processed.

Apply the following diff to add validation for these parameters:

 try {
   const { chain, amount, address, fromToken, toToken } = req.query

+  if (!address || !isAddress(address)) {
+    return res.status(400).json({ error: 'Invalid address parameter' })
+  }
+
+  if (!chain || isNaN(Number(chain))) {
+    return res.status(400).json({ error: 'Invalid chain parameter' })
+  }
+
+  if (!amount || isNaN(Number(amount)) || Number(amount) <= 0) {
+    return res.status(400).json({ error: 'Invalid amount parameter' })
+  }

   if (!isAddress(fromToken) || !isAddress(toToken)) {
     return res.status(400).json({ error: 'Invalid token address' })
   }
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.

Suggested change
const { chain, amount, address, fromToken, toToken } = req.query
const { chain, amount, address, fromToken, toToken } = req.query
if (!address || !isAddress(address)) {
return res.status(400).json({ error: 'Invalid address parameter' })
}
if (!chain || isNaN(Number(chain))) {
return res.status(400).json({ error: 'Invalid chain parameter' })
}
if (!amount || isNaN(Number(amount)) || Number(amount) <= 0) {
return res.status(400).json({ error: 'Invalid amount parameter' })
}
if (!isAddress(fromToken) || !isAddress(toToken)) {
return res.status(400).json({ error: 'Invalid token address' })
}

Copy link

codecov bot commented Sep 17, 2024

Bundle Report

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

Detailed changes
Bundle name Size Change
sdk-router-@synapsecns/sdk-router-cjs 527.05kB 409.81kB ⬆️
synapse-interface-server-cjs 1.47MB 147.21kB ⬇️

@github-actions github-actions bot added the javascript Pull requests that update Javascript code label Sep 17, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Outside diff range and nitpick comments (1)
packages/rest-api/src/routes/bridgeRoute.ts (1)

4-8: Organize imports for consistency and readability.

The newly added imports for isTokenAddress and isTokenSupportedOnChain may disrupt the existing import order. Maintaining a consistent import structure improves code readability.

Consider reorganizing the imports:

 import express from 'express'
 import { check } from 'express-validator'

+ import { CHAINS_ARRAY } from '../constants/chains'
+ import { showFirstValidationError } from '../middleware/showFirstValidationError'
+ import { bridgeController } from '../controllers/bridgeController'
+ import { isTokenAddress } from '../utils/isTokenAddress'
+ import { isTokenSupportedOnChain } from '../utils/isTokenSupportedOnChain'
Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 42890d9 and f0a059b.

Files selected for processing (16)
  • packages/rest-api/jest.config.js (1 hunks)
  • packages/rest-api/package.json (2 hunks)
  • packages/rest-api/src/controllers/bridgeController.ts (1 hunks)
  • packages/rest-api/src/controllers/bridgeTxInfoController.ts (3 hunks)
  • packages/rest-api/src/controllers/swapController.ts (1 hunks)
  • packages/rest-api/src/controllers/swapTxInfoController.ts (2 hunks)
  • packages/rest-api/src/routes/bridgeRoute.ts (2 hunks)
  • packages/rest-api/src/routes/bridgeTxInfoRoute.ts (2 hunks)
  • packages/rest-api/src/routes/swapRoute.ts (2 hunks)
  • packages/rest-api/src/routes/swapTxInfoRoute.ts (2 hunks)
  • packages/rest-api/src/tests/bridgeRoute.test.ts (3 hunks)
  • packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts (3 hunks)
  • packages/rest-api/src/tests/swapRoute.test.ts (2 hunks)
  • packages/rest-api/src/tests/swapTxInfoRoute.test.ts (1 hunks)
  • packages/rest-api/src/utils/isTokenAddress.ts (1 hunks)
  • packages/rest-api/src/utils/isTokenSupportedOnChain.ts (1 hunks)
Files skipped from review as they are similar to previous changes (6)
  • packages/rest-api/package.json
  • packages/rest-api/src/controllers/bridgeController.ts
  • packages/rest-api/src/controllers/bridgeTxInfoController.ts
  • packages/rest-api/src/controllers/swapController.ts
  • packages/rest-api/src/routes/bridgeTxInfoRoute.ts
  • packages/rest-api/src/routes/swapTxInfoRoute.ts
Additional context used
Learnings (1)
packages/rest-api/src/tests/swapRoute.test.ts (1)
Learnt from: abtestingalpha
PR: synapsecns/sanguine#3126
File: packages/rest-api/src/tests/indexRoute.test.ts:21-31
Timestamp: 2024-09-16T17:19:34.740Z
Learning: In this project, tests intentionally use hardcoded values for counts to capture changes in the number of available chains and tokens.
Gitleaks
packages/rest-api/src/tests/swapRoute.test.ts

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeRoute.test.ts

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


86-86: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


87-87: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/swapTxInfoRoute.test.ts

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: 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)


43-43: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


85-85: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


35-35: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


95-95: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

Additional comments not posted (28)
packages/rest-api/jest.config.js (1)

9-9: Verify the impact of changing moduleDirectories.

Changing moduleDirectories from ['node_modules', 'src'] to ['node_modules', '<rootDir>'] alters Jest's module resolution. Ensure that this change does not affect the ability to import modules using absolute paths, especially those starting from the src directory.

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

4-12: Function isTokenAddress correctly validates token addresses.

The implementation accurately checks if the provided address matches any of the bridgeable token addresses, ensuring case-insensitive comparison.

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

4-16: Function isTokenSupportedOnChain properly checks token support per chain.

The function effectively verifies whether a token address is supported on a specified chain by normalizing addresses and comparing them against the bridgeable tokens' addresses.

packages/rest-api/src/controllers/swapTxInfoController.ts (1)

29-30: Ensure address is valid before using in Synapse.swap.

The address parameter is used in the Synapse.swap function without prior validation. This could lead to errors if an invalid address is passed.

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

27-45: Ensure token validation logic is robust and accurate.

The validation checks for fromToken and toToken accurately ensure that the tokens are valid addresses and supported on the specified chains. This strengthens the API's reliability.

packages/rest-api/src/tests/swapRoute.test.ts (5)

13-14: Improved specificity by using actual token contract addresses

Replacing symbolic token names with the actual Ethereum contract addresses for fromToken and toToken enhances the accuracy and reliability of the tests. Considering the project's practice of intentionally using hardcoded values in tests, this approach is appropriate.

Tools
Gitleaks

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


27-28: Consistent use of token contract addresses in unsupported chain test

Maintaining the use of actual token addresses ensures uniformity across test cases, improving test clarity and consistency.

Tools
Gitleaks

27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


36-49: Addition of test case for invalid toToken address

The new test case effectively validates the API's response when an invalid toToken address is provided, enhancing input validation testing and ensuring robust error handling.

Tools
Gitleaks

39-39: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


51-63: New test case for unsupported token on specified chain

Including this test strengthens the test suite by verifying that the API correctly handles tokens not supported on the specified chain, thus improving the reliability of the system under various scenarios.

Tools
Gitleaks

54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


55-55: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


69-70: Consistent token addresses in missing amount test case

Using actual token addresses in the test for a missing amount parameter maintains consistency and improves the accuracy of the test cases.

Tools
Gitleaks

69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

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

10-10: Updated test to bridge USDC from Ethereum to Optimism using real contract addresses

The change enhances the test's relevance by using actual token contract addresses and adjusting the toChain to Optimism (chain ID 10), which reflects a realistic bridge scenario and improves test specificity.

Also applies to: 13-15


29-30: Consistent use of token contract addresses in unsupported fromChain test

Updating the token addresses ensures that the test accurately reflects the input parameters, maintaining consistency across tests and improving clarity.

Tools
Gitleaks

29-29: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


30-30: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


44-45: Consistent token addresses in unsupported toChain test

Ensuring the use of actual token contract addresses in the test contributes to the reliability and clarity of the test case, reinforcing consistent testing practices.

Tools
Gitleaks

44-44: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


45-45: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


52-64: Added test case for invalid fromToken address

The new test case effectively verifies that the API responds appropriately when an invalid fromToken address is provided, enhancing validation coverage and error handling capabilities.

Tools
Gitleaks

57-57: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-80: New test case for unsupported token on specified chain

Adding this test enhances the robustness of the test suite by confirming that the API handles unsupported tokens correctly, thereby improving system stability under various conditions.

Tools
Gitleaks

71-71: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


72-72: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


82-88: Added test case for missing amount parameter

This test ensures that the API properly handles requests when the amount parameter is missing, returning the expected error and improving input validation.

Tools
Gitleaks

86-86: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


87-87: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/swapTxInfoRoute.test.ts (6)

13-14: Updated test with address parameter and actual token addresses

Including the address parameter and using actual token contract addresses improves the test's accuracy, ensuring it reflects the updated API requirements and validating the new mandatory field effectively.

Also applies to: 16-16

Tools
Gitleaks

13-13: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


24-37: Added test case for invalid address parameter

The new test verifies that the API correctly handles invalid address inputs, enhancing input validation for the address parameter and ensuring robust error handling.

Tools
Gitleaks

27-27: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


28-28: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


42-43: Updated unsupported chain test to include address parameter

Including the address parameter in the test ensures that all required parameters are tested, maintaining consistency with the API's requirements and improving test completeness.

Also applies to: 45-45

Tools
Gitleaks

42-42: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


43-43: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


51-64: Added test case for invalid toToken address

By testing how the API handles an invalid toToken address, the test suite's coverage of input validation scenarios is improved, ensuring the API's robustness against incorrect inputs.

Tools
Gitleaks

54-54: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


66-78: New test case for unsupported token on specified chain

This test case verifies that the API appropriately responds when a token not supported on the specified chain is used, enhancing the robustness of the test suite and ensuring reliable system behavior.

Tools
Gitleaks

69-69: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


70-70: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


84-86: Added test case for missing amount parameter

The test ensures that the API returns the correct error when the amount parameter is missing, improving error handling verification and input validation.

Tools
Gitleaks

84-84: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


85-85: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

packages/rest-api/src/tests/bridgeTxInfoRoute.test.ts (6)

14-15: Updated test with actual token contract addresses

Replacing symbolic token names with real contract addresses for fromToken and toToken enhances the test's specificity and accuracy, aligning with real-world scenarios and project practices.

Tools
Gitleaks

14-14: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


15-15: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


34-35: Consistent use of token addresses in unsupported fromChain test

Updating the token addresses ensures the test accurately reflects input parameters and maintains consistency, contributing to clearer and more reliable tests.

Tools
Gitleaks

34-34: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


35-35: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


46-59: Added test case for invalid fromToken address

This test effectively verifies the API's response to an invalid fromToken address, improving input validation coverage and ensuring robust error handling.

Tools
Gitleaks

51-51: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


62-75: New test case for unsupported token on specified chain

Adding this test enhances the robustness of the test suite by confirming that the API correctly handles unsupported tokens, thereby improving system reliability under various conditions.

Tools
Gitleaks

66-66: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


67-67: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


82-83: Added test case for missing amount parameter

This test checks that the API responds appropriately when the amount parameter is missing, ensuring proper error handling and input validation.

Tools
Gitleaks

82-82: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


83-83: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


90-103: Added test case for invalid destAddress

The new test verifies that the API correctly handles an invalid destAddress, improving validation for recipient addresses and enhancing overall input validation robustness.

Tools
Gitleaks

94-94: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)


95-95: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

@Defi-Moses Defi-Moses merged commit 7a118c9 into master Sep 18, 2024
37 checks passed
@Defi-Moses Defi-Moses deleted the feat/api-swaptxinfo-fix branch September 18, 2024 21:43
abtestingalpha added a commit that referenced this pull request Sep 26, 2024
* update bl

* remove global solidity extension settings

* use monorepo support in global workspace only

* - use Solidity extension for formatting *.sol files
- use `forge fmt` as formatter in Solidity extension

* REST API Improvements [SLT-179] (#3133)

* fix swaptxinfo function

* Updates test coverage command

* migrating to using token addresses instead of symbols

* fix linting errors

* fixing swaptxinfocontroller

* new tests and new functionality

---------

Co-authored-by: abtestingalpha <[email protected]>

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* fix harmony proxy (#3149)


Co-authored-by: Trajan0x <[email protected]>

* merging rfq indexer into monorepo [SLT-164]  [SLT-176] (#3136)

* merging rfq indexer into monorepo

* nuke .env

* fix commands

* fix package name

* test coverage script

* rough pass at docs and some linting and fixes yarn

* Upgrades wagmi & rainbowkit

* indxer

* Adds invisible but used packages

* +recent-invalid-fills [SLT-188]

* Moves wagmi to root

* new endpoints and clean up linting

---------

Co-authored-by: Trajan0x <[email protected]>
Co-authored-by: abtestingalpha <[email protected]>
Co-authored-by: parodime <[email protected]>

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* Adds /destinationTokens route [SLT-204] (#3151)

* Adds /destinationTokens route
* ZeroAddress & NativeGasAddress
* Adds test for native gas tokens
* Checksums incoming token address params

* Publish

 - @synapsecns/[email protected]

* boba pause (#3150)

* boba pause

* only boba to txns

* Publish

 - @synapsecns/[email protected]

* fix(synapse-interface): Reorders validation to check existence first (#3156)

* Reorders validation to check existence first
* Removes duplicates

* Publish

 - @synapsecns/[email protected]

* Fix boba pause (#3158)

* Publish

 - @synapsecns/[email protected]

* update bl

* feat(rest-api): Adds Swagger for api docs [SLT-205] (#3159)

* Adds Swagger for api docs

* Replace prepended verb Get routes with nouns

* Adds dev flag for swagger serverUrl

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* Pulls version from package json (#3160)

* Publish

 - @synapsecns/[email protected]

* Require vs import due to file location (#3161)

* Require vs import due to file location

* Publish

 - @synapsecns/[email protected]

* Prevent caching of api docs (#3162)

* Publish

 - @synapsecns/[email protected]

* feat(contracts-rfq): relay/prove/claim with different address [SLT-130] (#3138)

* init. solidity ^. FbV2 relay/prove/claim overloads

* +IFastBridgeV2, explicit address0 cast, func scope & inheritdoc fixes

* pragma lock, contract relabel

* feat: start scoping V2 tests

* test: override relayer role scenarios, no longer enforced by V2

* test: finish the parity test

* test: the management methods

* test: dst chain scenarios

* test: bridge

* test: prove

* test: claim

* test: dispute

* test: refund

* test: bridge reverts

* remove redundant extend. rearrange inherit list

* revert 0.8.20 in favor of user (non-ws) setting

---------

Co-authored-by: ChiTimesChi <[email protected]>

* Publish

 - [email protected]

* fix(promexporter): make spans better (#3164)

* move the errors

* [goreleaser]

* fix v to w

* changing native token address standard [SLT-210] (#3157)

* changing native token address standard

* fixing tests

* normalizeNativeTokenAddress middleware, additional tests

---------

Co-authored-by: abtestingalpha <[email protected]>

* Publish

 - @synapsecns/[email protected]

* Refactoring rfq-indexer API and adding swagger docs [SLT-228] (#3167)

* refactoring and adding swagger

* remove testing scripts

* fix typos and consistency with 404 errors

* Publish

 - @synapsecns/[email protected]

* fix read mes (#3168)

* Publish

 - @synapsecns/[email protected]
 - [email protected]
 - @synapsecns/[email protected]

* fix(opbot): use submitter get tx status [SLT-158] (#3134)

* use experimental logger to debug

* fix lint

* [goreleaser]

* use submitter instead of client

* [goreleaser]

* [goreleaser]

* fix(synapse-interface): Additional checks on screen [SLT-166] (#3152)

* Additional checks on screen

* Adds checks on chain/token changes

* Publish

 - @synapsecns/[email protected]

* feat(synapse-interface): confirm new price [SLT-150]  (#3084)

* add bridge quote history middleware

* request user confirm changes when quoted price updates

* add conditions for displaying confirm change state

* track initial quote initializing confirm change state

* specify output delta threshold

* callback functions to handle initialize/accept/reset confirm changes flow

* quote countdown timer animation to signal refresh

* implement automatic refresh intervals

* mouse move to refresh automatic intervals

* add i8n translations for button text

---------

Co-authored-by: abtestingalpha <[email protected]>

* Publish

 - @synapsecns/[email protected]

* fix: formatted bridge fee amount (#3165)

* Publish

 - @synapsecns/[email protected]

* fix(contracts-rfq): CI workflows [SLT-245] (#3178)

* fix: license, files

* fix: package name

* build: update solhint to latest

* build: remove prettier dependencies

* fix: solhint workflows

* build: update solhint in other packages as well

* chore: solhint rules, exceptions

* fix: silence linter warnings in tests

* chore: forge fmt

* add variable to test linter CI

* Revert "add variable to test linter CI"

This reverts commit 0629309.

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* feat(api): bridge limits [SLT-165]  (#3179)

* adds `/bridgeLimits` route, controller

* fetch best sdk quote for min/max origin amounts

* add tests

* implement middleware to normalize addresses

* adds swagger doc

* Publish

 - @synapsecns/[email protected]

* fix(contracts-rfq): limit the amount of solhint warnings [SLT-245] (#3182)

* ci: limit the amount of solhint warnings

* refactor: move the errors into the separate interface

* refactor: errors imports in tests

* Publish

 - @synapsecns/[email protected]

* ci: Solidity gas diff [SLT-259] (#3181)

* ci: run tests w/o coverage first for better visibility

* test: malform the test to check the adjusted workflow

* Revert "test: malform the test to check the adjusted workflow"

This reverts commit e7db6e1.

* ci: add gas-diff workflow

* try changing the contract to trigger gas diffs

* retrigger the workflow

* ci: provide the correct report path

* ci: run on pull requests only

* ci: save gas reports in monorepo root

* Revert "ci: run on pull requests only"

This reverts commit 0a01d60.

* Revert "try changing the contract to trigger gas diffs"

This reverts commit 91bc03e.

* refactor: wrap if statement

* refactor: exclude `solidity-devops` package in a more generic way

* ci: run tests w/o coverage for `solidity-devops`, add comments

* add generic comment to trigger `solidity-devops` workflows

* Revert "add generic comment to trigger `solidity-devops` workflows"

This reverts commit cc35a43.

* Publish

 - @synapsecns/[email protected]

* fix(contracts-core): set very high gas limit for intensive tests [SLT-259] (#3186)

* fix: set very high gas limit for intensive tests

* ci: speed up solidity coverage

* Publish

 - @synapsecns/[email protected]

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

* Adds validateRouteExists validation

* Remove timeouts for 400s

* Publish

 - @synapsecns/[email protected]

* add duplicate command warning (#3174)

Co-authored-by: Trajan0x <[email protected]>

* reduce solhint warnings on FbV2 (#3189)

* reduce solhint warnings on FbV2

* fix whitespace

* Publish

 - @synapsecns/[email protected]

* ci: solidity gas diff options [SLT-267] (#3193)

* ci: ignore test files in gas diff report

* add some changes to the test files

* ci: define some options for gas-diff

* try changing the contract to trigger gas diffs

* Revert "try changing the contract to trigger gas diffs"

This reverts commit 4504e3c.

* Revert "add some changes to the test files"

This reverts commit 7e7d6cb.

* prove w/ tx id [SLT-181] (#3169)

* prove w/ tx id SLT-181

* +proveOther tests, forge fmt

* fmt

* fmt

* Publish

 - @synapsecns/[email protected]

* fix(sdk-router): disable ARB airdrop tests (#3195)

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* Fixing issue for wallet integration [SLT-270] (#3194)

* slight modification to graphql call

* fixing explorer frontend as well

* Publish

 - @synapsecns/[email protected]
 - @synapsecns/[email protected]

* store relayer on relay [SLT-182] (#3170)

* store relayer on relay [SLT-182]

* +tests, zeroAddr check, fmt

* Publish

 - @synapsecns/[email protected]

* Adjust text to trigger build (#3199)

* Publish

 - @synapsecns/[email protected]

* feat(synapse-interface): refund RFQ transaction [SLT-272] (#3197)

* Txn transaction refund tracking

* Update store to support tracking

* Query FastBridge contract for `bridgeStatuses` to find refund status

* Track bridge transaction `bridgeQuote.routerAddress` in store

* Fetch FastBridge contract address when only provided router address

* add translations

---------

Co-authored-by: aureliusbtc <[email protected]>
Co-authored-by: ChiTimesChi <[email protected]>
Co-authored-by: abtestingalpha <[email protected]>
Co-authored-by: Defi-Moses <[email protected]>
Co-authored-by: trajan0x <[email protected]>
Co-authored-by: Trajan0x <[email protected]>
Co-authored-by: parodime <[email protected]>
Co-authored-by: abtestingalpha <[email protected]>
Co-authored-by: abtestingalpha <[email protected]>
Co-authored-by: parodime <[email protected]>
Co-authored-by: vro <[email protected]>
Co-authored-by: ChiTimesChi <[email protected]>
Co-authored-by: bigboydiamonds <[email protected]>
Co-authored-by: bigboydiamonds <[email protected]>
@coderabbitai coderabbitai bot mentioned this pull request Nov 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
javascript Pull requests that update Javascript code size/m Sol Typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants