feat: OKX sol swap#729
Conversation
commit 0928965 Author: julian.martinez <julian_martinez28@outlook.com> Date: Mon Jul 7 17:08:14 2025 -0700 end of buffer commit 2f286c9 Author: julian.martinez <julian_martinez28@outlook.com> Date: Mon Jul 7 15:59:32 2025 -0700 update okx provider logic commit 12b50c2 Merge: a687ae4 98ad4a2 Author: julian.martinez <julian_martinez28@outlook.com> Date: Mon Jul 7 15:49:53 2025 -0700 Merge branch 'main' of https://github.com/Julian-dev28/enKrypt commit a687ae4 Author: Julian Martinez <julian.martinez@okg.com> Date: Mon Jun 16 21:50:08 2025 -0700 update request/response params commit 98ad4a2 Author: julian.martinez <julian_martinez28@outlook.com> Date: Mon Jun 16 19:18:08 2025 -0700 update request/response params commit eff5837 Author: julian.martinez <julian_martinez28@outlook.com> Date: Thu Jun 12 18:39:25 2025 -0700 add okx provider commit 934e55b Author: julian.martinez <julian_martinez28@outlook.com> Date: Wed Jun 11 10:29:01 2025 -0700 add okx provider
add okx provider
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThese changes introduce support for the OKX DEX aggregator as a new provider for Solana swaps. The update includes a new provider implementation, type definitions, configuration entries, and integration into the swap workflow. Additionally, transaction handling is updated to support raw Solana transactions, including fee logic and type adjustments. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI
participant SwapModule
participant OKXProvider
participant OKXAPI
participant SolanaRPC
User->>UI: Initiate Solana swap
UI->>SwapModule: Request swap providers for Solana
SwapModule->>OKXProvider: Initialize with token list
OKXProvider->>OKXAPI: Fetch supported tokens
User->>UI: Select tokens and amount
UI->>SwapModule: Request quote
SwapModule->>OKXProvider: getQuote()
OKXProvider->>OKXAPI: Request quote
OKXAPI-->>OKXProvider: Return quote data
OKXProvider-->>SwapModule: Return structured quote
UI->>SwapModule: Confirm swap
SwapModule->>OKXProvider: getSwap()
OKXProvider->>OKXAPI: Request swap transaction
OKXAPI-->>OKXProvider: Return base64 transaction
OKXProvider-->>SwapModule: Return swap transaction
SwapModule->>SolanaRPC: Submit transaction
SolanaRPC-->>SwapModule: Return transaction status
SwapModule->>OKXProvider: getStatus()
OKXProvider->>SolanaRPC: Query transaction status
SolanaRPC-->>OKXProvider: Return status
OKXProvider-->>SwapModule: Return status
SwapModule-->>UI: Update swap status
Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
|
💼 Build Files |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/extension/src/ui/action/views/swap/views/swap-best-offer/index.vue (2)
64-64: Consider removing the type assertion.The type assertion
as GasFeeTypemay mask underlying type issues. IfgasCostValuesdoesn't naturally matchGasFeeType, investigate the root cause rather than forcing the type.-:fees="gasCostValues as GasFeeType" +:fees="gasCostValues"If this causes type errors, address the underlying type mismatch in the component definition or data handling.
313-320: Consider making debug logging conditional.The detailed trade logging is useful for debugging but might be too verbose for production. Consider making it conditional based on environment or a debug flag.
-swapData.trades.forEach((trade, index) => { - console.log(`Trade ${index + 1}:`, { - provider: trade.provider, - fromAmount: trade.fromTokenAmount.toString(), - toAmount: trade.toTokenAmount.toString(), - additionalFees: trade.additionalNativeFees.toString(), - }); -}); +if (process.env.NODE_ENV === 'development') { + swapData.trades.forEach((trade, index) => { + console.log(`Trade ${index + 1}:`, { + provider: trade.provider, + fromAmount: trade.fromTokenAmount.toString(), + toAmount: trade.toTokenAmount.toString(), + additionalFees: trade.additionalNativeFees.toString(), + }); + }); +}packages/swap/src/providers/okx/index.ts (2)
95-108: Consider using optional chaining for cleaner code.The static analysis tool correctly identifies opportunities to use optional chaining here, which would make the code more concise.
Apply optional chaining:
- if (error.response && error.response.headers) { - const retryAfter = - error.response.headers.get && - error.response.headers.get("Retry-After"); + const retryAfter = error.response?.headers?.get?.("Retry-After"); if (retryAfter) { const retryAfterMs = parseInt(retryAfter, 10) * 1000; if (!isNaN(retryAfterMs)) { delay = Math.max(delay, retryAfterMs); logger.info( `OKX API Retry-After header present, waiting ${delay}ms`, ); } } - }
604-608: Use optional chaining for consistency.Similar to the previous comment, optional chaining would improve readability here.
- hasData: !!data, - hasDataArray: !!(data && data.data), - dataLength: data && data.data ? data.data.length : 0, + hasData: !!data, + hasDataArray: !!data?.data, + dataLength: data?.data?.length ?? 0, code: data?.code, message: data?.msg || data?.message,packages/extension/src/ui/action/views/swap/libs/solana-gasvals.ts (1)
149-156: Consider making the default fee configurable.While 10000 lamports (0.00001 SOL) is a reasonable default for most Solana transactions, complex transactions might require higher fees. Consider making this configurable or deriving it from transaction size/complexity.
+ // Default fees based on typical Solana transaction costs + // Simple transfers: ~5000 lamports + // Token transfers: ~10000 lamports + // Complex swaps: ~15000-20000 lamports const defaultFee = 10000; // 0.00001 SOLConsider adding a comment to document the rationale for this default value, or potentially increase it slightly for swap transactions which tend to be more complex.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
packages/extension/src/ui/action/views/swap/libs/solana-gasvals.ts(3 hunks)packages/extension/src/ui/action/views/swap/libs/swap-txs.ts(1 hunks)packages/extension/src/ui/action/views/swap/views/swap-best-offer/index.vue(3 hunks)packages/swap/src/configs.ts(1 hunks)packages/swap/src/index.ts(2 hunks)packages/swap/src/providers/okx/index.ts(1 hunks)packages/swap/src/providers/okx/types.ts(1 hunks)packages/swap/src/types/index.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/swap/src/index.ts (1)
packages/swap/src/providers/okx/index.ts (1)
OKX(135-825)
🪛 Biome (1.9.4)
packages/swap/src/providers/okx/index.ts
[error] 95-95: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 97-98: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 604-604: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 605-605: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (10)
packages/swap/src/types/index.ts (1)
131-131: LGTM!The addition of the OKX provider to the enum follows the established naming convention and is consistent with the integration pattern used by other providers.
packages/swap/src/index.ts (2)
50-50: LGTM!The import statement follows the established pattern for other providers.
123-123: LGTM!The OKX provider is correctly added to the Solana providers list, following the same instantiation pattern as other providers (Jupiter, Rango, Changelly) with proper type casting and constructor parameters.
packages/swap/src/configs.ts (1)
68-77: Please verify OKX referrer addressesThe OKX entries in
packages/swap/src/configs.tsexactly mirror Jupiter’s referrer addresses:
- enkrypt:
HXWkRK9a4H1EuBiqP4sVfFsEpd2NasoQPScoXL1NgSE2- mew:
CmrkoXWiTW37ZqUZcfJtxiKhy9eRMBQHq1nm8HpmRXH4Confirm that OKX should indeed share these same referral addresses—or update them to the correct ones if they differ.
packages/extension/src/ui/action/views/swap/libs/swap-txs.ts (1)
112-127: No downstream impact—raw Solana transactions are fully supportedAll consumers of the new
isRawDataandhasThirdPartySignaturesproperties have been updated:• getSwapTransactions is only used for fee estimation; executeSwap continues to consume the original EnkryptSolanaTransaction objects.
• solana-gasvals.ts correctly branches onisRawData(skips blockhash updates, applies a default fee).
• send-transactions.ts deserializesserializedand handles third-party signatures as before.No further changes are needed.
packages/swap/src/providers/okx/index.ts (3)
49-128: Excellent implementation of rate limiting and retry logic!The rate limiting implementation with a 2-second minimum interval and the retry logic with exponential backoff are well-designed. The handling of 429 errors and respect for the Retry-After header demonstrates good API citizenship.
746-748: Good handling of the required parameters.The implementation correctly uses string "true" for
autoSlippageand includesmaxAutoSlippageBpsas noted in the API requirements. This aligns with the type definitions despite their optional marking.
639-663: Thorough base64 validation implementation!The multi-step validation of the transaction data (regex check, decode test, and logging) provides excellent debugging capabilities and ensures data integrity before returning to the caller.
packages/extension/src/ui/action/views/swap/libs/solana-gasvals.ts (2)
26-42: Well-structured addition of raw transaction support!The new
RawTransactiontype and its integration into theTaggedTransactionunion are properly implemented. The type includes all necessary fields for handling raw serialized transactions.
204-210: Good defensive programming for null message handling.The null check for
msgwith a fallback to the default fee ensures the fee calculation doesn't fail even in unexpected scenarios.
Fix okx tx issue
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
packages/swap/tests/okx.test.ts (3)
287-318: Remove console.log statements from testsTest files should not contain console.log statements. Consider using test framework's logging capabilities or remove them entirely.
- console.log("🚀 Testing USDC -> SOL swap with unwrapping detection"); const usdcToSolQuote = await okx.getQuote(usdcToSolQuoteOptions, { infiniteApproval: true, walletIdentifier: "enkrypt", slippage: "0.5", }); - console.log( - "🔍 USDC → SOL quote result:", - usdcToSolQuote ? "SUCCESS" : "FAILED", - ); if (usdcToSolQuote) { expect(usdcToSolQuote).not.toBeNull(); expect(usdcToSolQuote.provider).toBe(ProviderName.okx); - console.log("🔍 Getting USDC → SOL swap transaction..."); const usdcToSolSwap = await okx.getSwap(usdcToSolQuote.quote); - console.log( - "🔍 USDC → SOL swap result:", - usdcToSolSwap ? "SUCCESS" : "FAILED", - ); if (usdcToSolSwap) { expect(usdcToSolSwap).not.toBeNull(); expect(usdcToSolSwap.transactions.length).toBeGreaterThanOrEqual(1); - console.log( - `✅ USDC → SOL swap transaction created with ${usdcToSolSwap.transactions.length} transaction(s)`, - ); - console.log("✅ Unwrapping detection logic executed"); } }
359-389: Remove console.log statements from testsSimilar to the previous test, remove debugging console.log statements.
- console.log( - "🚀 Testing SOL -> USDC swap with Wrapped SOL account detection", - ); const solQuote = await okx.getQuote(solQuoteOptions, { infiniteApproval: true, walletIdentifier: "enkrypt", slippage: "0.5", }); - console.log("🔍 SOL quote result:", solQuote ? "SUCCESS" : "FAILED"); if (solQuote) { expect(solQuote).not.toBeNull(); expect(solQuote.provider).toBe(ProviderName.okx); - console.log("🔍 Getting SOL swap transaction..."); const solSwap = await okx.getSwap(solQuote.quote); - console.log("🔍 SOL swap result:", solSwap ? "SUCCESS" : "FAILED"); if (solSwap) { expect(solSwap).not.toBeNull(); expect(solSwap.transactions[0]).toHaveProperty("kind"); expect((solSwap.transactions[0] as SolanaTransaction).kind).toBe( "versioned", ); - console.log("✅ SOL swap transaction created successfully"); - console.log( - "✅ Wrapped SOL account detection and creation logic executed", - ); } }
452-469: Remove console.log statementsRemove debugging console.log statements from the test.
const serializedTx = (swap!.transactions[0] as SolanaTransaction) .serialized; - console.log("Serialized transaction length:", serializedTx.length); - console.log("First 100 chars:", serializedTx.substring(0, 100)); // Test if it's valid base64 let buffer: Buffer; try { buffer = Buffer.from(serializedTx, "base64"); - console.log("Decoded buffer length:", buffer.length); - console.log( - "First 20 bytes:", - Array.from(buffer.slice(0, 20)) - .map((b) => b.toString(16).padStart(2, "0")) - .join(" "), - ); } catch (e) { - console.error("Failed to decode base64:", e); throw e; }packages/swap/src/providers/okx/index.ts (3)
101-114: Use optional chaining for cleaner codeReplace the nested conditionals with optional chaining for better readability.
- if (error.response && error.response.headers) { - const retryAfter = - error.response.headers.get && - error.response.headers.get("Retry-After"); + const retryAfter = error.response?.headers?.get?.("Retry-After"); if (retryAfter) { const retryAfterMs = parseInt(retryAfter, 10) * 1000; if (!isNaN(retryAfterMs)) { delay = Math.max(delay, retryAfterMs); logger.info( `OKX API Retry-After header present, waiting ${delay}ms`, ); } } - }
56-76: Consider encapsulating rate limiting stateThe global mutable state for rate limiting (
lastRequestTime,requestCount) could cause issues if multiple OKX provider instances are used concurrently. Consider moving this state into the OKX class or using a more robust rate limiting solution.Consider moving the rate limiting state into the OKX class to avoid potential concurrency issues:
export class OKX extends ProviderClass { private lastRequestTime = 0; private requestCount = 0; private readonly MIN_REQUEST_INTERVAL = 2000; private async rateLimitedRequest(): Promise<void> { // Rate limiting logic here } }
632-636: Use optional chaining for cleaner null checksSimplify the nested property checks using optional chaining.
logger.info(`OKX: Swap API response structure:`, { hasData: !!data, - hasDataArray: !!(data && data.data), - dataLength: data && data.data ? data.data.length : 0, + hasDataArray: !!data?.data, + dataLength: data?.data?.length ?? 0, code: data?.code, message: data?.msg || data?.message, });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/swap/src/providers/okx/index.ts(1 hunks)packages/swap/tests/okx.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/swap/tests/okx.test.ts (3)
packages/swap/src/providers/okx/index.ts (1)
OKX(141-927)packages/swap/src/types/index.ts (2)
getQuoteOptions(142-148)BN(47-47)packages/swap/src/utils/solana.ts (1)
isValidSolanaAddressAsync(469-473)
🪛 Biome (1.9.4)
packages/swap/tests/okx.test.ts
[error] 547-547: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
[error] 34-34: Shouldn't redeclare 'isValidSolanaAddressAsync'. Consider to delete it or rename it.
'isValidSolanaAddressAsync' is defined here:
(lint/suspicious/noRedeclare)
packages/swap/src/providers/okx/index.ts
[error] 101-101: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 103-104: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 632-632: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 633-633: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 905-905: The catch clause that only rethrows the original error is useless.
An unnecessary catch clause can be confusing.
Unsafe fix: Remove the try/catch clause.
(lint/complexity/noUselessCatch)
[error] 908-908: Unnecessary continue statement
Unsafe fix: Delete the unnecessary continue statement
(lint/correctness/noUnnecessaryContinue)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: buildAll
- GitHub Check: test
- GitHub Check: test
🔇 Additional comments (4)
packages/swap/tests/okx.test.ts (2)
44-47: LGTM!Basic initialization test is correctly implemented.
49-95: Well-structured quote testThe test properly verifies quote retrieval with appropriate assertions and timeout handling.
packages/swap/src/providers/okx/index.ts (2)
1-46: LGTM! Well-organized importsImports are properly organized and all necessary dependencies are included.
168-205: Well-implemented initialization logicThe initialization properly validates the network, fetches OKX tokens, and sets up token mappings correctly.
packages/swap/tests/okx.test.ts
Outdated
| import { isValidSolanaAddressAsync } from "../src/utils/solana"; | ||
| import { | ||
| fromToken, | ||
| amount, | ||
| fromAddress, | ||
| toAddress, | ||
| toToken, | ||
| nodeURL, | ||
| } from "./fixtures/solana/configs"; | ||
| import { isValidSolanaAddressAsync } from "../src/utils/solana"; |
There was a problem hiding this comment.
Remove duplicate import statement
The isValidSolanaAddressAsync function is imported twice - once at line 25 and again at line 34.
import BN from "bn.js";
import { isValidSolanaAddressAsync } from "../src/utils/solana";
import {
fromToken,
amount,
fromAddress,
toAddress,
toToken,
nodeURL,
} from "./fixtures/solana/configs";
-import { isValidSolanaAddressAsync } from "../src/utils/solana";📝 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.
| import { isValidSolanaAddressAsync } from "../src/utils/solana"; | |
| import { | |
| fromToken, | |
| amount, | |
| fromAddress, | |
| toAddress, | |
| toToken, | |
| nodeURL, | |
| } from "./fixtures/solana/configs"; | |
| import { isValidSolanaAddressAsync } from "../src/utils/solana"; | |
| import BN from "bn.js"; | |
| import { isValidSolanaAddressAsync } from "../src/utils/solana"; | |
| import { | |
| fromToken, | |
| amount, | |
| fromAddress, | |
| toAddress, | |
| toToken, | |
| nodeURL, | |
| } from "./fixtures/solana/configs"; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 34-34: Shouldn't redeclare 'isValidSolanaAddressAsync'. Consider to delete it or rename it.
'isValidSolanaAddressAsync' is defined here:
(lint/suspicious/noRedeclare)
🤖 Prompt for AI Agents
In packages/swap/tests/okx.test.ts between lines 25 and 34, there is a duplicate
import of isValidSolanaAddressAsync from ../src/utils/solana. Remove the second
import statement at line 34 to avoid redundancy and potential confusion.
2 participants
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests