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

fix: dropped jupiter sol swap transactions #562

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

NickKelly1
Copy link
Contributor

@NickKelly1 NickKelly1 commented Nov 21, 2024

Summary by CodeRabbit

  • New Features

    • Enhanced retry logic for API interactions, reducing maximum wait times.
    • Introduced a new property, prioritizationFeeLamports, to optimize swap transaction fees.
    • Added a function to extract compute unit pricing from transactions.
  • Type Updates

    • Updated prioritizationFeeLamports type to allow for more flexible prioritization strategies.
  • Bug Fixes

    • Minor adjustments to ensure accurate timing in retry logic.

Copy link

coderabbitai bot commented Nov 21, 2024

Walkthrough

The changes primarily involve modifications to the Jupiter swap provider's functionality within the packages/swap/src/providers/jupiter directory. Key updates include adjustments to the backoff timing for retry logic in several functions, the addition of a new property prioritizationFeeLamports in the getJupiterSwap function, and enhancements to the JupiterSwapParams type to accommodate various prioritization strategies. Additionally, a new utility function, extractComputeUnitPriceMicroLamports, has been introduced in the Solana utility module to extract compute unit pricing from transactions.

Changes

File Path Change Summary
packages/swap/src/providers/jupiter/index.ts - Adjusted backoff timing in getJupiterTokens, getJupiterQuote, and getJupiterSwap functions, reducing maximum wait times.
- Added prioritizationFeeLamports property in swapParams of getJupiterSwap.
- Minor adjustments to the sleep function.
packages/swap/src/providers/jupiter/types.ts - Updated prioritizationFeeLamports type in JupiterSwapParams to accept number, "auto", or an object with autoMultiplier.
- Clarified comments regarding the type of prioritizationFeeLamports.
packages/swap/src/utils/solana.ts - Added extractComputeUnitPriceMicroLamports function to extract compute unit price from VersionedTransaction.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant JupiterProvider
    participant SolanaUtils

    User->>JupiterProvider: Request Swap
    JupiterProvider->>JupiterProvider: getJupiterSwap()
    JupiterProvider->>JupiterProvider: Adjust Backoff Timing
    JupiterProvider->>JupiterProvider: Include prioritizationFeeLamports
    JupiterProvider->>SolanaUtils: extractComputeUnitPriceMicroLamports()
    SolanaUtils-->>JupiterProvider: Return Compute Unit Price
    JupiterProvider-->>User: Return Swap Result
Loading

Possibly related PRs

  • [Fix: Rango supported network check #549] The changes in the main PR focus on enhancing the retry logic and transaction handling in the Jupiter swap provider, while the retrieved PR Fix: Rango supported network check #549 improves error handling in the Rango swap provider. Both PRs aim to improve the robustness of swap functionalities, indicating a thematic connection in enhancing API interactions, though they target different providers.

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 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.

Copy link

github-actions bot commented Nov 21, 2024

💼 Build Files
chrome: enkrypt-chrome-10dfeb06.zip
firefox: enkrypt-firefox-10dfeb06.zip

💉 Virus total analysis
chrome: 10dfeb06
firefox: 10dfeb06

Copy link

@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 (4)
packages/swap/src/providers/jupiter/types.ts (1)

90-97: LGTM! Consider enhancing the documentation.

The type enhancement for prioritizationFeeLamports looks good and aligns with the PR's objective of fixing dropped transactions. The flexibility to specify automatic fee calculation with a multiplier is a good approach.

Consider adding JSDoc comments to explain:

  • The purpose of each option
  • When to use "auto" vs explicit values
  • Recommended ranges for autoMultiplier
 /** Integer */
 prioritizationFeeLamports?:
+  /**
+   * Controls transaction priority fee:
+   * - number: Explicit fee in lamports
+   * - "auto": Automatically calculate based on network conditions
+   * - object: Customize automatic calculation with a multiplier
+   * @example
+   * // Explicit fee
+   * prioritizationFeeLamports: 100000
+   * // Automatic calculation
+   * prioritizationFeeLamports: "auto"
+   * // Custom multiplier (3x the base automatic fee)
+   * prioritizationFeeLamports: { autoMultiplier: 3 }
+   */
   | number
   | "auto"
   | {
       /** Integer */
+      /** Multiplier for automatic fee calculation (e.g., 2 = 2x the base fee) */
       autoMultiplier: number;
     };
packages/swap/src/utils/solana.ts (2)

121-123: Enhance documentation for better clarity and maintainability.

The current JSDoc only references the Solana docs. Please add:

  • Function purpose and use case
  • Parameter description
  • Return value description with possible values
  • Example usage
 /**
+ * Extracts the compute unit price (in micro lamports) from a Solana transaction.
+ * Used to verify transaction prioritization fees for debugging dropped transactions.
+ *
+ * @param tx - The versioned transaction to extract the compute unit price from
+ * @returns The compute unit price in micro lamports if found, undefined otherwise
+ * @example
+ * const tx = new VersionedTransaction(...)
+ * const price = extractComputeUnitPriceMicroLamports(tx)
+ * if (price) console.log(`Compute unit price: ${price} micro lamports`)
+ *
  * @see https://solana.com/docs/core/fees#prioritization-fees
  */

158-158: Fix incorrect comment.

The comment refers to "Compute limit" but this case handles compute unit price.

-        // Compute limit
+        // Compute unit price
packages/swap/src/providers/jupiter/index.ts (1)

939-945: LGTM! Consider monitoring the autoMultiplier value.

The addition of prioritizationFeeLamports with an autoMultiplier of 3 is a good solution to prevent transaction drops during high traffic. However, consider monitoring this value in production to ensure it's optimal for your use case.

Consider implementing metrics to track:

  • Transaction success rates
  • Average prioritization fees
  • Network congestion correlation
    This will help tune the multiplier value over time.
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between abf56be and 71749a2.

📒 Files selected for processing (3)
  • packages/swap/src/providers/jupiter/index.ts (4 hunks)
  • packages/swap/src/providers/jupiter/types.ts (1 hunks)
  • packages/swap/src/utils/solana.ts (1 hunks)
🔇 Additional comments (4)
packages/swap/src/providers/jupiter/types.ts (1)

90-97: Verify integration with fee calculation logic.

Let's verify how this type is used in the implementation.

✅ Verification successful

Let me gather more information about the fee handling implementation.


Based on the gathered information, I can now provide the final response:

The prioritization fee handling is properly integrated with fee calculation logic

The code correctly handles all three cases of prioritizationFeeLamports:

  • When set as a number, it's directly used
  • When set as "auto", it uses Jupiter's automatic fee calculation
  • When set with autoMultiplier, it multiplies Jupiter's automatic fee (currently set to 3x as seen in packages/swap/src/providers/jupiter/index.ts)

The implementation properly integrates with Solana's compute budget instructions and transaction building:

  • Fees are extracted and validated in extractComputeUnitPriceMicroLamports
  • Compute budget instructions are properly ordered at the start of transactions
  • The fee configuration is used in the Jupiter API calls
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for usage of prioritizationFeeLamports to ensure proper handling of all cases
echo "Searching for prioritizationFeeLamports usage..."
rg -A 10 "prioritizationFeeLamports" --type ts

# Search for any fee calculation related functions
echo "Searching for fee calculation functions..."
ast-grep --pattern 'function $FUNC($_) { $$$ fee $$$ }'

Length of output: 2579


Script:

#!/bin/bash
# Search for the Jupiter API integration code to understand fee handling
echo "Searching for Jupiter API integration..."
rg -A 20 "JUPITER_API_URL" --type ts

# Search for any compute budget or priority fee related code
echo "Searching for compute budget related code..."
rg -A 10 "computeBudget|ComputeBudget" --type ts

# Look for transaction building code that might handle fees
echo "Searching for transaction building code..."
ast-grep --pattern 'function $FUNC($_) { $$$ transaction $$$ }'

Length of output: 27133

packages/swap/src/utils/solana.ts (1)

124-171: LGTM! The implementation follows good practices.

The function correctly:

  • Follows the established pattern from extractComputeBudget
  • Handles instruction decoding properly
  • Uses appropriate types for Solana's compute unit prices
  • Breaks early after finding the relevant instruction

Let's verify the function is used correctly in the codebase:

✅ Verification successful

Let me gather more context about the usage of this function by searching for related compute unit price handling code.


The implementation is correctly used in the codebase

The function extractComputeUnitPriceMicroLamports is properly implemented and its usage aligns with Solana's compute unit price handling patterns. The verification shows:

  • It's used in test files to verify compute budget instructions
  • The types and interfaces in jupiter/types.ts properly define the compute unit price parameters
  • The Changelly provider correctly uses ComputeBudgetProgram.setComputeUnitPrice with microLamports
  • The implementation matches the test expectations and provider usage patterns
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check usage of extractComputeUnitPriceMicroLamports
# Expected: Function should be used in Jupiter swap related code for handling dropped transactions

# Search for function usage
rg "extractComputeUnitPriceMicroLamports" -A 5

# Search for related compute unit price handling
ast-grep --pattern 'SetComputeUnitPrice'

Length of output: 501


Script:

#!/bin/bash
# Search for files that might be using this function
rg -l "ComputeBudgetProgram|computeUnitPrice|microLamports" --type ts

# Then get content of those files to understand the usage context
rg "ComputeBudgetProgram|computeUnitPrice|microLamports" --type ts -C 5

Length of output: 11898

packages/swap/src/providers/jupiter/index.ts (2)

1207-1207: LGTM! Clean timeout variable placement.

The timeout variable declaration is now properly placed within the promise callback scope.


619-619: Verify the impact of reduced backoff times on transaction reliability.

The maximum backoff time has been reduced from 5000ms to 4000ms across multiple functions. While this improves transaction speed, it might affect reliability during network congestion.

Also applies to: 798-798, 951-951

✅ Verification successful

Based on the search results, I can now provide a final response as the code shows a comprehensive retry mechanism with proper error handling:

Backoff timing changes are well-implemented and maintain reliability

The reduced backoff times are part of a robust retry system that:

  • Implements exponential backoff starting from 0ms up to 4000ms (0, 100, 500, 1000, 2000, 4000)
  • Includes proper error handling and debugging
  • Has a 30-second timeout for each request attempt
  • Is consistently implemented across all Jupiter API interactions (tokens, quotes, and swaps)
  • Provides detailed error messages with attempt counts and failure reasons

The previous maximum of 5000ms was unnecessarily long, and the new progression provides sufficient retry coverage while improving transaction speed.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check for any error handling patterns or comments related to timeouts
rg -A 2 -B 2 "backoff|timeout|retry" packages/swap/src/providers/jupiter/

Length of output: 11956

Comment on lines +160 to +161
ComputeBudgetInstruction.decodeSetComputeUnitPrice(instruction);
computeUnitPriceMicroLamports = command.microLamports;
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add validation for negative compute unit price.

The function should validate that the compute unit price is not negative to prevent issues in fee calculations.

         const command =
           ComputeBudgetInstruction.decodeSetComputeUnitPrice(instruction);
+        // Ensure compute unit price is not negative
+        if (command.microLamports < 0) {
+          throw new Error(`Invalid negative compute unit price: ${command.microLamports}`);
+        }
         computeUnitPriceMicroLamports = command.microLamports;
📝 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
ComputeBudgetInstruction.decodeSetComputeUnitPrice(instruction);
computeUnitPriceMicroLamports = command.microLamports;
const command =
ComputeBudgetInstruction.decodeSetComputeUnitPrice(instruction);
// Ensure compute unit price is not negative
if (command.microLamports < 0) {
throw new Error(`Invalid negative compute unit price: ${command.microLamports}`);
}
computeUnitPriceMicroLamports = command.microLamports;

Copy link

@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: 0

🧹 Outside diff range and nitpick comments (1)
packages/swap/src/providers/jupiter/index.ts (1)

939-949: LGTM! Consider monitoring the effectiveness of the fee multiplier.

The automatic fee multiplier of 6x was chosen based on manual testing to address dropped transactions during high network activity. This is a reasonable approach.

Consider implementing monitoring to:

  1. Track transaction success rates
  2. Measure the effectiveness of the 6x multiplier
  3. Adjust the multiplier dynamically based on network conditions
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 71749a2 and 10dfeb0.

📒 Files selected for processing (1)
  • packages/swap/src/providers/jupiter/index.ts (4 hunks)
🔇 Additional comments (2)
packages/swap/src/providers/jupiter/index.ts (2)

1211-1211: ⚠️ Potential issue

Fix potential null reference in sleep function.

The sleep function accesses abortable.signal without checking if abortable is defined, which could cause runtime errors.

Apply this fix:

-  if (abortable.signal.aborted) return Promise.reject(abortable.signal.reason);
+  if (abortable?.signal?.aborted) return Promise.reject(abortable.signal.reason);

Likely invalid or redundant comment.


619-619: Verify the impact of reduced backoff times.

The maximum retry wait times have been reduced from 5000ms to 4000ms. While this could improve user experience by failing faster, we should verify it doesn't impact success rates.

Also applies to: 798-798, 955-955

✅ Verification successful

Let me gather more context about the retry mechanism and its usage.


Let me check one more thing to understand if there are any reported issues or metrics related to these retry operations.


Based on the gathered information, I can now provide the final response:

Backoff timing adjustment appears safe and well-implemented

The reduced backoff times (from 5000ms to 4000ms max) are consistently applied across all Jupiter operations (tokens, quote, swap). The retry mechanism:

  • Maintains the same number of retry attempts (6 attempts)
  • Preserves exponential backoff pattern
  • Includes proper error handling and debugging
  • Is used with AbortController for request cancellation

The reduction in wait times should improve user experience while still providing sufficient retry attempts for transient issues.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Search for any error handling or retry-related issues in the codebase
rg -A 3 "Failed to get Jupiter.*after.*retries" 

Length of output: 1007


Script:

#!/bin/bash
# Search for retry-related code patterns and error handling
ast-grep --pattern 'for (let backoffi = 0; backoffi < backoff.length; backoffi++) {
  $$$
}'

# Also check for any error logs or metrics related to retries
rg -B 2 -A 4 "backoff\[backoffi\]"

Length of output: 4360


Script:

#!/bin/bash
# Look for any error tracking or metrics related to Jupiter operations
rg -B 2 -A 2 "metrics\.(track|increment|timing).*jupiter" 

# Check for any comments or documentation about retry strategies
rg -B 2 -A 2 "retry.*jupiter|jupiter.*retry" -g "!*.{json,lock}"

Length of output: 126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant