Conversation
…n AppKit migration - Remove @rainbow-me/rainbowkit dependency - Add @reown/appkit and adapters for EVM, Bitcoin, and Solana - Add @solana/wallet-adapter-wallets for Phantom/Solflare support - Add @solana/web3.js for Solana transaction handling Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ll three adapters - Add AppKit initialization with WagmiAdapter, BitcoinAdapter, and SolanaAdapter - Configure EVM networks (mainnet, polygon, arbitrum, etc.) - Configure Bitcoin and Solana network support - Use PhantomWalletAdapter and SolflareWalletAdapter for Solana - Export helper functions for adapter creation and initialization - Follow Reown AppKit v1.8.15 patterns with correct import paths Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…export config - Removed @rainbow-me/rainbowkit dependency (getDefaultConfig) - Import networks from @reown/appkit/networks instead of wagmi/chains - Use WagmiAdapter from @reown/appkit-adapter-wagmi - Removed unsupported chains (monad, hyperEvm, plasma, katana) not available in AppKit networks - Export createWagmiAdapter() and getWagmiAdapter() for adapter access - Export createWagmiConfig() for backward compatibility with WagmiProvider Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…instead of RainbowKit - Replace RainbowKitProvider with AppKit initialization via initializeAppKit() - Use getWagmiAdapter() to get wagmi config from AppKit adapter - Replace RainbowKit's ConnectButton.Custom with custom button using useAppKit and useAppKitAccount hooks - Remove themeMode prop as AppKit handles theming internally - Add useCallback for handleClick optimization Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…of RainbowKit - Remove RainbowKit imports (@rainbow-me/rainbowkit, styles) - Add AppKit imports (useAppKit, useAppKitAccount from @reown/appkit/react) - Replace RainbowKitProvider with direct AppKit initialization pattern - Create custom ConnectButton using AppKit's useAppKit hook - Update DemoContent to use useAppKitAccount instead of useAccount - Use initializeAppKit and getWagmiAdapter from config/appkit Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…on flow
- Remove unused themeMode prop from InternalWalletProvider call
(AppKit handles theming through createAppKit configuration)
- Add enableWalletConnection={true} to SwapWidgetCore when using
internal wallet mode to ensure connect button is visible
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ain-agno Create chain-agnostic transaction status checking service with: - checkEvmStatus: EVM transaction status using viem publicClient - checkBitcoinStatus: Bitcoin tx status via mempool.space API - checkSolanaStatus: Solana signature status via connection - waitFor* variants for blocking confirmation waits - Unified checkTransactionStatus function for all chain types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ified status polling - Created useTransactionStatus hook for unified transaction status polling UI - Implements polling for EVM, Bitcoin (UTXO), and Solana transactions - Uses React Query for state management with configurable poll intervals - Provides useTransactionWatcher hook for simplified SwapWidget integration - Follows existing hook patterns from useSwapRates and useBalances Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…transact Create useBitcoinSigning.ts hook for Bitcoin/UTXO transaction signing using Reown AppKit's BitcoinAdapter with bip122 namespace. Features: - sendTransfer() for simple BTC transfers - signPsbt() for complex PSBT signing with optional broadcast - signMessage() for message signing - getAccountAddresses() to retrieve wallet addresses - checkTxStatus() for transaction status polling via mempool.space - Proper error handling for user rejections and failures - State management for loading/error/txid tracking Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… signing - Import and use useBitcoinSigning hook for Bitcoin transaction signing - Add UTXO chain detection using getChainType() === 'utxo' - Implement Bitcoin signing flow in handleExecuteSwap: - Support PSBT signing via signPsbt() - Support simple transfers via sendBitcoinTransfer() - Update button text and disabled state for UTXO chains - Enable swap rates fetching for UTXO chains - Display Bitcoin wallet address in sell section header - Use mempool.space for Bitcoin transaction explorer links - Update handleButtonClick to handle Bitcoin wallet connection state Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… transac
Create Solana signing hook with:
- useAppKitProvider<Provider>('solana') for wallet provider
- useAppKitConnection() for Solana connection
- sendTransaction() for sending transactions
- signTransaction() for signing without broadcast
- signMessage() for message signing with TextEncoder
- checkTxStatus() and waitForConfirmation() for tx monitoring
- Proper error handling for user rejections
- State management for loading/error/signature tracking
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…in signing flow - Import and use useSolanaSigning hook for Solana wallet integration - Add isSellAssetSolana and canExecuteSolana chain type detection - Implement Solana transaction execution flow in handleExecuteSwap: - Get quote from API with Solana addresses - Deserialize and send versioned transaction via wallet - Handle transaction status updates and error handling - Update button text and disabled states for Solana flow - Display Solana address badge when Solana asset is selected - Add Solscan explorer link for Solana transaction status Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…let prov Add type definitions for multi-chain wallet provider support: - Re-export BitcoinConnector from @reown/appkit-adapter-bitcoin - Re-export SolanaProvider from @reown/appkit-adapter-solana/react - Add TransactionStatus type for unified tx status tracking - Add TransactionStatusResult with confirmations and block info - Add BitcoinTransactionStatus for mempool.space API responses - Add WalletProviderNamespace type for namespace identification - Add MultiChainAddress and MultiChainWalletState utility types Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fixed import sorting with simple-import-sort - Fixed prettier formatting issues - Removed unused imports (useAppKitProvider, BitcoinConnector, ChainTypeResult) - Removed unused variable (queryClient) - Fixed async functions without await expressions - Changed getAccountAddresses to sync since no await needed - Prefixed unused parameter with underscore (_timeoutMs) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…types - Updated vite.config.ts to replace RainbowKit externals with AppKit packages - Updated README.md documentation to reflect migration to Reown AppKit - Updated props table to mention AppKit multi-chain support - Updated wallet connection example to remove RainbowKit import - Updated built-in wallet section with multi-chain capabilities - Updated Notes section to describe multi-chain swap support - Verified all RainbowKit references removed from codebase Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis PR replaces RainbowKit with Reown AppKit and implements multi-chain swap support (EVM, Bitcoin/UTXO, Solana): new appkit config, adapters, multi-chain balance hooks, Bitcoin/Solana signing hooks, cross-chain transaction status service, UI/provider updates, and build/dependency changes. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SwapWidget
participant SigningHook as Signing Hook (BTC/SOL/EVM)
participant AppKit as Reown AppKit
participant Chain as Blockchain Network
User->>SwapWidget: initiate swap
SwapWidget->>SigningHook: request sign/broadcast for target chain
SigningHook->>AppKit: request wallet client / connector / provider
AppKit-->>SigningHook: return WalletClient / BitcoinConnector / SolanaProvider
SigningHook->>Chain: build, sign and broadcast tx (PSBT / Solana tx / EVM tx)
Chain-->>SigningHook: txHash / txid / signature
SigningHook-->>SwapWidget: tx identifier
SwapWidget->>Chain: checkTransactionStatus (via service)
Chain-->>SwapWidget: status (pending / confirmed / failed)
SwapWidget->>SwapWidget: refresh multi-chain balances
SwapWidget-->>User: update UI
sequenceDiagram
participant User
participant TokenModal
participant BalanceHook as useMultiChainBalances
participant EvmRpc as EVM RPC
participant Mempool as Mempool API
participant SolanaRpc as Solana RPC
User->>TokenModal: open selector
TokenModal->>BalanceHook: fetch balances (assetIds)
BalanceHook->>EvmRpc: query ERC20 / getBalance
BalanceHook->>Mempool: query UTXO balances for BTC
BalanceHook->>SolanaRpc: query SOL / SPL token accounts
EvmRpc-->>BalanceHook: evm balances
Mempool-->>BalanceHook: utxo balances
SolanaRpc-->>BalanceHook: solana balances
BalanceHook-->>TokenModal: aggregated balances + refetchSpecific
TokenModal-->>User: render balances
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
5fa6340 to
bbf3f46
Compare
…let-connect-and-support-u
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@packages/swap-widget/package.json`:
- Around line 18-21: The package.json has mismatched `@reown/`* versions (most are
^1.8.15 while `@reown/appkit-adapter-solana` is ^1.8.14); align all `@reown`
packages to the same release to avoid peer/type mismatches by changing the
version strings for `@reown/appkit`, `@reown/appkit-adapter-bitcoin`,
`@reown/appkit-adapter-wagmi` (and any other `@reown/`* entries) to match
`@reown/appkit-adapter-solana`’s current published version (^1.8.14) so all `@reown`
packages share the identical X.Y.Z version.
In `@packages/swap-widget/src/components/SwapWidget.tsx`:
- Around line 304-374: executeUtxoSwap currently doesn't verify the user's
wallet has enough balance to cover sellAmountBaseUnit; add a preflight balance
check at the start of executeUtxoSwap (after verifying
isBitcoinConnected/bitcoinAddress and rateToUse) that compares
sellAmountBaseUnit against the user's available sell balance (e.g., sellBalance
or sellBalanceBaseUnit) and if insufficient, setTxStatus to an error (clear
isExecuting) and return early; ensure you do this before resetBitcoinState() and
before making the API quote so you don't clear UI state or call
apiClient.getQuote when funds are inadequate.
In `@packages/swap-widget/src/components/WalletProvider.tsx`:
- Around line 29-33: Move the side-effect initializeAppKit(projectId) out of the
useMemo and into a useEffect to ensure initialization is performed as an effect
rather than a pure memoized computation: call initializeAppKit(projectId) inside
a useEffect that depends on projectId (and keep its internal guard as-is), and
change the useMemo for wagmiConfig (the variable wagmiConfig returned from
getWagmiAdapter()) to only compute and return adapter?.wagmiConfig without
performing initialization; reference the existing initializeAppKit,
getWagmiAdapter, wagmiConfig, and projectId identifiers when making these edits.
In `@packages/swap-widget/src/config/wagmi.ts`:
- Around line 46-48: Remove the unused exported function createWagmiConfig by
deleting its declaration and export; replace any internal references with
getWagmiAdapter()?.wagmiConfig (as used in appkit.ts) if found, and ensure no
other modules import createWagmiConfig (remove or update those imports). After
removal, run the typecheck/build to confirm there are no dangling references and
update exports if necessary to preserve API surface.
In `@packages/swap-widget/src/hooks/useBalances.ts`:
- Around line 110-127: In fetchBitcoinBalance, the intermediate subtractions can
produce NaN if either operand is undefined; change the calculations to coerce
undefined to 0 before subtracting (e.g., use (data.chain_stats?.funded_txo_sum
?? 0) and (data.chain_stats?.spent_txo_sum ?? 0) for confirmedBalance, and
similarly for mempoolBalance) so confirmedBalance and mempoolBalance are always
numeric before summing and returning totalBalance.toString().
In `@packages/swap-widget/src/hooks/useBitcoinSigning.ts`:
- Around line 90-103: The comparisons in taprootByPurpose and
nativeSegwitAccount assume purpose can be numeric but AccountAddress.purpose is
typed as string | undefined; fix by normalizing the value to a string before
comparing (e.g., read purpose via const purpose = String(account.purpose) or
guard typeof account.purpose === 'string') and then compare against '86' and
'84' respectively, or update the AccountAddress type to include number if you
prefer; update the taprootByPurpose and nativeSegwitAccount predicates to use
the normalized purpose variable when testing equality so TS2367 is resolved.
🧹 Nitpick comments (9)
packages/swap-widget/src/config/appkit.ts (1)
74-79: Avoidas anyfor Solana wallet adapters.This discards type-safety around adapter capabilities. Prefer a typed wallets array so mismatches are caught at compile time.
♻️ Suggested typing for wallet adapters
-import { PhantomWalletAdapter, SolflareWalletAdapter } from '@solana/wallet-adapter-wallets' +import { PhantomWalletAdapter, SolflareWalletAdapter } from '@solana/wallet-adapter-wallets' +import type { WalletAdapter } from '@solana/wallet-adapter-base'- solanaAdapter = new SolanaAdapter({ - wallets: [new PhantomWalletAdapter(), new SolflareWalletAdapter()] as any, - }) + const wallets: WalletAdapter[] = [new PhantomWalletAdapter(), new SolflareWalletAdapter()] + solanaAdapter = new SolanaAdapter({ wallets })As per coding guidelines, avoid
anywhen a concrete adapter type is available.packages/swap-widget/src/hooks/useBalances.ts (3)
272-325:useBitcoinBalanceusesuseQueriesunnecessarily for a single query.Using
useQuerieswith a single-element array and then accessingquery[0]adds unnecessary complexity. Consider usinguseQuerydirectly for cleaner code.Suggested simplification
- const query = useQueries({ - queries: [ - { - queryKey: ['utxoBalance', address, assetId], - queryFn: async () => { + const query = useQuery({ + queryKey: ['utxoBalance', address, assetId], + queryFn: async () => { if (!address || !assetId || !isUtxo) return null try { const balance = await fetchBitcoinBalance(address) return { assetId, balance, precision, } } catch { return null } }, - enabled: !!address && !!assetId && isUtxo, - staleTime: 60_000, - refetchOnWindowFocus: false, - }, - ], - }) + enabled: !!address && !!assetId && isUtxo, + staleTime: 60_000, + refetchOnWindowFocus: false, + })
755-814:refetchSpecifichas complex query key matching logic.The predicate function performs deep equality checks on query keys with multiple chain-type branches. This is functional but verbose. Consider extracting the key-matching logic or using a simpler approach like storing assetId directly in the query key for easier matching.
373-374: Use structured parsing for SPL token balance.The hardcoded byte offsets are actually correct for the SPL token account layout, but manually slicing binary data is less maintainable than using a structured approach. Use
connection.getTokenAccountBalance(tokenAccountPubkey)(available via@solana/web3.js) to retrieve the amount, decimals, and pre-formatted uiAmount in a single RPC call. Alternatively, if you need the full account structure, use@solana/spl-token'sgetAccount()helper to deserialize the account data.Note: The current code also returns raw balance without accounting for token decimals, which may not be suitable for UI display.
packages/swap-widget/src/components/SwapWidget.tsx (4)
417-417: Unsafeanycast for Solana transaction metadata.Line 417 uses
(innerStep as any)?.solanaTransactionMetadatawhich bypasses type safety. Consider defining a proper type for the step structure or using a type guard.Suggested type improvement
type SolanaTransactionMetadata = { instructions: Array<{ keys: Array<{ pubkey: string; isSigner: boolean; isWritable: boolean }> programId: string data: { data: number[] } }> } type StepWithSolanaMetadata = { solanaTransactionMetadata?: SolanaTransactionMetadata } const solanaTransactionMetadata = (innerStep as StepWithSolanaMetadata | undefined)?.solanaTransactionMetadata
433-449: Dynamic import of@solana/web3.jsinside callback could cause issues.The dynamic import
await import('@solana/web3.js')on line 433 is executed every timeexecuteSolanaSwapruns. This is unnecessary overhead sincePublicKeyandConnectionare already imported at the top of useBalances.ts. Consider importing these at module level or memoizing the import.
781-814:isButtonDisableddoesn't check for wallet connection for EVM.For EVM swaps, the button is not disabled when
!walletClient && canExecuteDirectly, but clicking it triggersonConnectWallet. This is inconsistent with UTXO/Solana paths where the button is disabled when wallet is not connected. Consider aligning the behavior.
1129-1139: Transaction link IIFE in JSX is functional but verbose.The inline IIFE for generating explorer links works but reduces readability. Consider extracting to a memoized value.
Suggested extraction
const txExplorerLink = useMemo(() => { if (!txStatus?.txHash) return undefined if (isSellAssetUtxo) return `https://mempool.space/tx/${txStatus.txHash}` if (isSellAssetSolana) return `https://solscan.io/tx/${txStatus.txHash}` return `${sellAsset.explorerTxLink ?? 'https://etherscan.io/tx/'}${txStatus.txHash}` }, [txStatus?.txHash, isSellAssetUtxo, isSellAssetSolana, sellAsset.explorerTxLink])packages/swap-widget/src/services/transactionStatus.ts (1)
253-287:_timeoutMsparameter is unused.The
_timeoutMsparameter on line 257 is prefixed with underscore indicating it's intentionally unused, butconnection.confirmTransactionhas its own timeout mechanism vialastValidBlockHeight. Consider either removing the parameter or documenting why it's kept for API consistency.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/swap-widget/src/hooks/useBitcoinSigning.ts`:
- Around line 216-247: In signMessage, guard against a missing/empty address and
empty signatures: before calling provider.signMessage in the signMessage
function, verify address is non-empty and if not setState to isLoading:false and
error (e.g., 'Missing wallet address') and throw; after receiving signatureStr,
validate it's a non-empty string and if empty setState to isLoading:false and
error (e.g., 'Empty signature returned') and throw; ensure provider.signMessage
is only invoked with a validated address and preserve existing error/isLoading
state updates in both success and failure paths.
🧹 Nitpick comments (1)
packages/swap-widget/src/hooks/useBitcoinSigning.ts (1)
282-284: Add explicit return type tocheckTxStatus.
Helps align with the project’s explicit typing guideline.As per coding guidelines, please keep explicit return types for functions in TS.♻️ Suggested change
- const checkTxStatus = useCallback((txid: string, network: 'mainnet' | 'testnet' = 'mainnet') => { - return checkBitcoinStatus(txid, network) - }, []) + const checkTxStatus = useCallback( + (txid: string, network: 'mainnet' | 'testnet' = 'mainnet'): ReturnType<typeof checkBitcoinStatus> => { + return checkBitcoinStatus(txid, network) + }, + [], + )
|
@coderabbitai can you create 2 issues:
|
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 155
Issue 1: BTC Quotes API ProblemTitle: BTC quotes API not returning valid quotes - all swappers return UnsupportedChain errors Body: ## Description
The BTC quotes API is not returning valid quotes. All swappers are returning errors, primarily "UnsupportedChain" errors.
## API Endpoint
`https://api.shapeshift.com/v1/swap/rates?sellAssetId=bip122%3A000000000019d6689c085ae165831e93%2Fslip44%3A0&buyAssetId=eip155%3A42161%2Fslip44%3A60&sellAmountCryptoBaseUnit=100000`
## Example Response
All swappers (THORChain, 0x, CoW Swap, Jupiter, ButterSwap, Bebop) return rate "0" with errors:
- Most return: "UnsupportedChain" / "unsupported chainId"
- ButterSwap specifically returns: "BTC sells are currently unsupported"
## Context
This issue was identified during testing of PR `#11712` which adds Bitcoin support to the swap widget.
## Related
- PR: `#11712`
- Reported by: `@NeOMakinG`Assignee: NeOMakinG Issue 2: Swap Status EndpointTitle: Implement swap status endpoint for widget polling Body: ## Description
The swap widget needs a swap status endpoint to verify if swaps are successful or not. This will enable the widget to poll for status updates and provide real-time feedback to users.
## Requirements
- Endpoint should accept a transaction/swap identifier
- Should return the current status of the swap (pending, confirmed, failed, etc.)
- Should support multi-chain swaps (EVM, Bitcoin, Solana)
- Should be optimized for polling (efficient, rate-limited if necessary)
## Context
This functionality is needed to complete the multi-chain swap support added in PR `#11712`, which introduces Bitcoin and Solana wallet integration alongside EVM chains.
## Related
- PR: `#11712`
- Reported by: `@NeOMakinG`Assignee: NeOMakinG You can create these issues directly on GitHub. Would you like me to help with anything else related to these issues? 🧠 Learnings used |
- Align all @reown/* package versions to 1.8.14 - Move initializeAppKit side effect from useMemo to useEffect - Remove unused createWagmiConfig function - Fix Bitcoin balance calculation to handle undefined operands - Add balance validation to executeUtxoSwap before execution - Guard signMessage against missing address and empty signatures
The downgrade to 1.8.14 caused 'Invalid chain id' errors when signing Solana transactions. Updated all @reown/* packages to latest stable version 1.8.17 which has the Solana adapter fix.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/swap-widget/src/components/SwapWidget.tsx`:
- Around line 685-692: The redirectToShapeShift callback opens a new tab with
window.open without noopener/noreferrer which allows reverse-tabnabbing; update
redirectToShapeShift to supply the third window.open parameter with
'noopener,noreferrer' (and for extra safety set newWindow.opener = null after
opening if a reference is returned) so the opened page cannot access the opener;
refer to redirectToShapeShift and the window.open call when making the change.
- Around line 225-241: The effectiveReceiveAddress useMemo currently falls back
to walletAddress for all chains which can prefill invalid EVM addresses for
UTXO/Solana; update the logic in the effectiveReceiveAddress computation so that
walletAddress is only used when buyChainType === 'evm' (or when explicitly
appropriate), and return an empty string ('') for UTXO/Solana when no
chain-specific address exists; adjust the final return branch accordingly in the
useMemo that references customReceiveAddress, defaultReceiveAddress,
buyChainType, bitcoinAddress, solanaAddress, and walletAddress.
In `@packages/swap-widget/src/config/wagmi.ts`:
- Around line 32-41: The singleton createWagmiAdapter currently reuses
wagmiAdapterInstance without validating the incoming projectId; modify
createWagmiAdapter to store the projectId of the created instance (e.g.,
cachedProjectId) and on each call compare the new projectId to
cachedProjectId—if they differ either reinitialize wagmiAdapterInstance with the
new projectId (creating a new WagmiAdapter using SUPPORTED_CHAINS and the new
projectId) or throw a clear error indicating a projectId mismatch; ensure the
function returns the (possibly reinitialized) wagmiAdapterInstance.
- Fix effectiveReceiveAddress to not fall back to EVM address for UTXO/Solana - Add noopener,noreferrer to window.open for security
Track cached projectId and reinitialize adapter if projectId changes
gomesalexandre
left a comment
There was a problem hiding this comment.
Reviewed by Claude Code (Opus 4.5)
Summary
This PR migrates the swap widget from RainbowKit to Reown AppKit and adds Bitcoin/Solana wallet support. The feature scope is reasonable but the implementation has issues ranging from a real bug to significant code quality concerns.
🚨 Critical Bug (Not in diff - existing code touched by this PR)
Number precision issue in TokenSelectModal.tsx:372
(Number(balance.balance) / Math.pow(10, asset.precision)) * Number(marketData[asset.assetId].price)Number.MAX_SAFE_INTEGER is ~9e15. Any balance exceeding this in base units will silently lose precision. 1000 ETH in wei = 1e21 → corrupted display. This affects display only, not actual swap amounts.
Fix: Use bnOrZero from @shapeshiftoss/utils.
🔴 Serious Issues
- Dead code:
useTransactionStatus.ts(224 lines) is never imported anywhere - Duplicate function:
createWagmiAdapterexists in bothappkit.tsandwagmi.tswith different chain support - Missing validation:
executeSolanaSwaplacks balance validation (unlikeexecuteUtxoSwap) - Reinventing wheel: Custom
getChainType()intypes/index.ts:238just remapsChainNamespacefrom CAIP to slightly different strings, then code redundantly checks both
⚠️ Code Quality (Tech Debt)
- Same error detection logic copy-pasted 7 times across signing hooks
- Magic numbers for setTimeout delays throughout
- Empty strings
''used instead ofundefined - Slippage conversion copy-pasted 3 times
anytype casts instead of proper typing
Blocker Assessment
If it works functionally: The Number/Math.pow bug is the main real issue that will manifest (whale balance display corruption). Everything else is tech debt.
Recommendation: Can merge with follow-up ticket for the Number precision fix. See inline comments for specific locations.
gomesalexandre
left a comment
There was a problem hiding this comment.
Stamparaooni sandwich for the sake of progression - comments/review issues to tackle in this PR before mergy or in a follow-up PR
https://jam.dev/c/ff41c109-66fc-4b7d-aa96-7cd351d14064
Was able to:
- Connect Solana through Phantom
- Trade to/from Solana
- Trade from SOL to BTC happy
- Trade from BTC to SOL not happy (see around the end of jam)
Spotted issues:
- Able to click swap without a receive address
- The "Receive address" modal when connected with EVM only wrongly populates with EVM addy on Solana side
- "Enter Base58 address" isn't a very hooman-frenly placeholder
- Seeing "Swap Succesful" on Tx initiated
- The "Swap succesful" thingie doesn't go away on input change
Generally speaking, think we'll need a trade execution flow, similar to the one in Yield or e.g the swapper one in app but cleaner and simpler
- We probably should handle the "Add xyz wallet" functionality from reown in app if possible i.e something somewhere, if wallet supports e.g SOL and I'm receiving SOL, should be able to click connect or add and trigger the same method as reown modal does
- Able to "execute" swap with more than balance (and most likely invalid/no quote)
- Explorer link is explorer Tx link, not swapper one which can be confusing i.e swapping SOL -> BTC but looks like I wrongly got USDC
- Fix Number precision issue using bnOrZero instead of Number/Math.pow - Add balance validation to executeSolanaSwap and executeEvmSwap - Disable swap button when no receive address is available - Fix receive address modal to use chain-specific wallet address - Change 'Swap successful' to 'Transaction submitted' for clarity - Reset txStatus on sell amount input change - Change Solana placeholder from 'Base58 address' to 'Enter Solana address' - Remove dead code: useTransactionStatus.ts hook (never imported) - Remove duplicate createWagmiAdapter from wagmi.ts (kept in appkit.ts) - Resolve yarn.lock conflict from develop
b59cebe to
cbe931b
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/swap-widget/src/components/SwapWidget.tsx`:
- Line 461: The line creating a Buffer from ix.data.data can throw if ix.data or
ix.data.data is undefined; update the handling around the Buffer creation in
SwapWidget (the const data = Buffer.from(ix.data.data) site) to first validate
ix.data and ix.data.data, and either skip/continue when missing or provide a
safe default (e.g., an empty array/Buffer) before calling Buffer.from; ensure
any downstream logic expects the default or early-return so no runtime exception
occurs.
♻️ Duplicate comments (3)
packages/swap-widget/src/components/SwapWidget.tsx (3)
323-323: Slippage conversion duplicated across swap functions.This
parseFloat(slippage) / 100pattern appears inexecuteUtxoSwap,executeSolanaSwap, andexecuteEvmSwap. As noted in a previous review, consider extracting to a utility usingbninstead ofparseFloatfor consistency with the codebase's BigNumber patterns.
746-751: Silent return without user feedback persists.When UTXO/Solana wallet is not connected, clicking the button silently returns. While
buttonTextshows "Connect Bitcoin/Solana Wallet", the button click does nothing. Consider triggering the AppKit wallet connection modal or showing a toast.
1173-1183: Hardcoded explorer URLs noted in previous review.As flagged previously, consider using existing
getExplorerTxLinkhelper instead of hardcoding mempool.space and solscan.io. For cross-chain swaps, the current approach using sell asset's explorer is correct since the transaction originates on the sell chain.
🧹 Nitpick comments (1)
packages/swap-widget/src/components/SwapWidget.tsx (1)
436-436: Avoidanytype for Solana transaction metadata.Using
anybypasses TypeScript type checking. Consider defining a proper type forsolanaTransactionMetadatato catch potential runtime errors at compile time.Example type definition
type SolanaInstructionKey = { pubkey: string isSigner: boolean isWritable: boolean } type SolanaInstruction = { keys: SolanaInstructionKey[] programId: string data: { data: number[] } } type SolanaTransactionMetadata = { instructions: SolanaInstruction[] }
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/swap-widget/src/components/SwapWidget.tsx`:
- Line 1232: currentAssetIds is being passed as a fresh array literal on every
render which creates a new reference and can trigger unnecessary re-renders in
TokenSelectModal and its refetchSpecific effect; fix it by memoizing the array
using React.useMemo inside the SwapWidget component (e.g. compute a stable
currentAssetIdsMemo from sellAsset.assetId and buyAsset.assetId) and pass that
memoized value to the currentAssetIds prop so the reference stays stable across
renders unless either assetId changes.
- Line 354: The call signPsbt({ psbt, signInputs: {}, broadcast: true }) may
fail for wallets that require explicit input indices; update the signing logic
in SwapWidget (where signPsbt is called and psbt is available) to build
signInputs from the PSBT's input list (e.g., map psbt.inputs to an object with
input indices true/required keys) and pass that populated signInputs instead of
an empty object, and add fallback error handling to catch a signing rejection
and surface a user-friendly message indicating the wallet requires explicit
input specification.
In `@packages/swap-widget/src/components/TokenSelectModal.tsx`:
- Around line 167-171: The effect in TokenSelectModal uses optional chaining on
refetchSpecific and an unstable array dependency currentAssetIds; remove the
unnecessary optional chaining on refetchSpecific (it is always returned by
useMultiChainBalances) and replace the array dependency with a stable string key
(e.g., join or JSON.stringify of currentAssetIds) so the useEffect callback only
runs when the actual asset IDs change; update the useEffect that references
refetchSpecific and currentAssetIds accordingly to call
refetchSpecific(currentAssetIds) when isOpen is true and the stable key
indicates a change.
♻️ Duplicate comments (4)
packages/swap-widget/src/components/SwapWidget.tsx (4)
323-323: Slippage conversion duplicated and usesparseFloat.This pattern appears 3 times (lines 323, 424, 551). Per coding guidelines, prefer
bnoverparseFloatfor numeric conversions and extract to a utility.Suggested refactor
// At module level or in a utils file const toSlippageDecimal = (slippage: string): string => bn(slippage).div('100').toString() // Then replace all occurrences: const slippageDecimal = toSlippageDecimal(slippage)
1173-1183: Hardcoded explorer URLs - consider using asset'sexplorerTxLink.The UTXO and Solana explorers are hardcoded. For consistency and maintainability, consider using the asset's
explorerTxLinkproperty when available, with fallbacks.Also note: for cross-chain swaps, this uses the sell asset's chain type, but the transaction hash might correspond to a different chain depending on the swap stage.
454-468: Add defensive check for instruction data beforeBuffer.from.If
ix.dataorix.data.datais undefined,Buffer.from(undefined)will throw. The API response structure should be validated.Suggested fix
const instructions = solanaTransactionMetadata.instructions.map((ix: any) => { + if (!ix.data?.data) { + throw new Error(`Invalid instruction data for programId: ${ix.programId}`) + } + const keys = ix.keys.map((key: any) => ({ pubkey: new PublicKey(key.pubkey), isSigner: key.isSigner, isWritable: key.isWritable, })) const data = Buffer.from(ix.data.data)
746-751: Silent failure when UTXO/Solana wallet not connected.When the user clicks the swap button but UTXO/Solana wallet isn't connected, the function returns silently with no feedback. The button text says "Connect Bitcoin/Solana Wallet" but clicking doesn't trigger connection.
Consider opening the wallet connect modal or showing an error toast.
CodeRabbit Comments AddressedAll CodeRabbit review comments have been addressed:
|
|
Create issues for anything that we didn't tackle in this comment (check in the codebase to understand if we fixed it or not) |
3bf57c5 to
f5f9b20
Compare
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 14805 🏁 Script executed: Length of output: 31632 🏁 Script executed: Length of output: 705 🏁 Script executed: Length of output: 8040 🏁 Script executed: Length of output: 472 🏁 Script executed: Length of output: 941 🏁 Script executed: Length of output: 992 🏁 Script executed: Length of output: 46
✅ Already Fixed (2 items)
🆕 Issue Created (1 item)
All review items have been addressed! Two were already fixed in previous commits, and I've created a follow-up issue for the remaining item. 🧠 Learnings used |
…at-switch-widget-to-wallet-connect-and-support-u
…let-connect-and-support-u
…let-connect-and-support-u
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@packages/swap-widget/src/services/transactionStatus.ts`:
- Around line 191-222: Wrap the body of waitForEvmConfirmation in a try/catch
and validate the chain before creating the client (don’t silently fall back to
mainnet like checkEvmStatus); lookup chain via EVM_CHAINS_BY_ID[chainId] and if
missing return a TransactionStatusResult with status 'failed' and a descriptive
error (e.g., 'unsupported chain'); inside try call
client.waitForTransactionReceipt as before and on receipt.status === 'success'
return 'confirmed', otherwise return 'failed' with the revert error; in catch
return a TransactionStatusResult with status 'pending' or 'failed' depending on
the error (e.g., timeout -> 'pending', network/other -> 'failed') and include
the caught error.message in the error field.
- Around line 51-57: The code silently falls back to mainnet when
EVM_CHAINS_BY_ID[chainId] is missing; change the logic to detect unsupported
chain IDs and fail explicitly: check EVM_CHAINS_BY_ID[chainId] (using the
chainId variable) before calling createPublicClient, and if it's undefined (and
no existingClient was provided) either throw an Error or return a clearly typed
error status from the surrounding function (instead of defaulting to mainnet);
update references around createPublicClient, client, and mainnet so callers know
they must handle the error condition.
- Around line 119-124: Wrap the secondary block height fetch and JSON parse
(currentBlockResponse and currentBlockHeight) in its own try/catch and check
response.ok before reading json; on any fetch/parse error or non-OK response log
the error and fall back to a safe default for confirmations (e.g., set
confirmations = 1 when data.block_height exists) so a failed height lookup
doesn’t incorrectly mark a transaction as confirmed; update the code in
transactionStatus.ts around
currentBlockResponse/currentBlockHeight/confirmations to implement these checks
and fallback behavior.
♻️ Duplicate comments (2)
packages/swap-widget/src/components/SwapWidget.tsx (2)
1235-1235: MemoizecurrentAssetIdsto keep the prop stable.
The array literal creates a new reference each render and can trigger unnecessary downstream effects. As per coding guidelines, memoize derived arrays.♻️ Suggested refactor
+ const currentAssetIds = useMemo( + () => [sellAsset.assetId, buyAsset.assetId], + [sellAsset.assetId, buyAsset.assetId], + ) ... - currentAssetIds={[sellAsset.assetId, buyAsset.assetId]} + currentAssetIds={currentAssetIds}
347-355: Populate PSBTsignInputsfor stricter wallets.
Some wallets require explicit input indices; an empty object can cause signing to fail. Consider derivingsignInputsfrom the PSBT (or your existing PSBT parser) before callingsignPsbt, or surface a targeted error if the wallet rejects auto‑detection.💡 Possible fix
- if (psbt) { - txid = await signPsbt({ psbt, signInputs: {}, broadcast: true }) - } else if (recipientAddress) { + if (psbt) { + const { Psbt } = await import('bitcoinjs-lib') + const decoded = Psbt.fromBase64(psbt) + const signInputs = Object.fromEntries( + decoded.data.inputs.map((_, index) => [index, true]), + ) + txid = await signPsbt({ psbt, signInputs, broadcast: true }) + } else if (recipientAddress) {
🧹 Nitpick comments (2)
packages/swap-widget/src/components/SwapWidget.tsx (2)
435-456: Avoidanyfor Solana instruction metadata.
Usinganyhere removes type safety across the transaction build. Consider introducing explicit types (orunknown+ type guard) forsolanaTransactionMetadataandinstructions. As per coding guidelines, avoidanyin TSX.♻️ Suggested typing
+type SolanaInstructionKey = { + pubkey: string + isSigner: boolean + isWritable: boolean +} +type SolanaInstruction = { + programId: string + keys: SolanaInstructionKey[] + data: { data: string } +} +type SolanaTransactionMetadata = { + instructions: SolanaInstruction[] +} - const solanaTransactionMetadata = (innerStep as any)?.solanaTransactionMetadata + const solanaTransactionMetadata = + (innerStep as { solanaTransactionMetadata?: SolanaTransactionMetadata }) + ?.solanaTransactionMetadata - const instructions = solanaTransactionMetadata.instructions.map((ix: any) => { + const instructions = solanaTransactionMetadata.instructions.map((ix: SolanaInstruction) => {
771-820: Move new button text to i18n keys.
These newly added user‑facing strings are hardcoded. Please route them throughuseTranslate()with translation keys for consistency. As per coding guidelines, all copy should use i18n keys.
Description
This PR migrates the swap widget from RainbowKit to Reown AppKit and adds comprehensive multi-chain wallet support including Bitcoin (UTXO) and Solana. The migration enables a better multi-chain experience with improved wallet connection handling and cross-chain swap capabilities.
Key Changes
1. Multi-Chain Wallet Integration
useBitcoinSigninghook for Bitcoin wallet interactions (sendTransfer, signPSBT, signMessage)useSolanaSigninghook for Solana wallet interactions2. Balance Fetching Enhancements
useMultiChainBalanceshook supporting EVM, UTXO, and Solana chainshttps://api.mainnet-beta.solana.com) for balance fetching without wallet connectionqueryClient.invalidateQueries()API for better performance3. Cross-Chain Swap Support
canExecuteDirectlylogic to enable EVM → non-EVM swaps (e.g., Base → Solana with Phantom wallet)4. UI/UX Improvements
isButtonDisabledlogic with early returns for better readability5. Configuration Updates
types/index.tswith multi-chain types and utilitiesFiles Modified
Core Components:
src/components/SwapWidget.tsx- Main swap logic with multi-chain supportsrc/components/TokenSelectModal.tsx- Token selection with multi-chain balancesHooks:
src/hooks/useBalances.ts- Multi-chain balance fetchingsrc/hooks/useBitcoinSigning.ts- Bitcoin wallet integration (NEW)src/hooks/useSolanaSigning.ts- Solana wallet integration (NEW)Configuration:
src/config/appkit.ts- AppKit configuration with all adapterssrc/types/index.ts- Multi-chain types and utilitiesTechnical Details
Bitcoin Integration:
Solana Integration:
Balance Refresh Strategy:
['multiChainBalance', chainType, address, assetId, ...]staleTimesettings (60s for EVM/UTXO, 30s for Solana)Issue (if applicable)
N/A - Infrastructure improvement for multi-chain support
Risk
Medium Risk
This PR affects:
Risk Mitigation:
Testing
Engineering
Local Testing:
cd packages/swap-widget yarn install yarn devTest Scenarios:
EVM Wallet Connection:
Bitcoin Wallet Connection:
Solana Wallet Connection:
TokenSelectModal:
Cross-Chain Swaps:
Integration Points:
Operations
User-Facing Changes:
Wallet Connection:
Balance Display:
Cross-Chain Swaps:
Test Plan:
Screenshots (if applicable)
https://jam.dev/c/612daaf3-478a-430c-85aa-c2b3e9a37a37
https://jam.dev/c/132c2637-599c-4b9b-87db-4c9bda6788c3
Summary by CodeRabbit
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.