-
Notifications
You must be signed in to change notification settings - Fork 166
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
base: develop
Are you sure you want to change the base?
Conversation
WalkthroughThe changes primarily involve modifications to the Jupiter swap provider's functionality within the Changes
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
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
💼 Build Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 pricepackages/swap/src/providers/jupiter/index.ts (1)
939-945
: LGTM! Consider monitoring the autoMultiplier value.The addition of
prioritizationFeeLamports
with anautoMultiplier
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
📒 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 inpackages/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
withmicroLamports
- 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
ComputeBudgetInstruction.decodeSetComputeUnitPrice(instruction); | ||
computeUnitPriceMicroLamports = command.microLamports; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add 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.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
- Track transaction success rates
- Measure the effectiveness of the 6x multiplier
- Adjust the multiplier dynamically based on network conditions
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 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
:
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
Summary by CodeRabbit
New Features
prioritizationFeeLamports
, to optimize swap transaction fees.Type Updates
prioritizationFeeLamports
type to allow for more flexible prioritization strategies.Bug Fixes