Add Solana support #57
Conversation
WalkthroughThis update introduces comprehensive support for the Solana blockchain within the project. New dependencies and configurations have been added across multiple packages. The diff includes the implementation of Solana-specific features such as atomic swaps via HTLC and relay mechanisms, updated error handling in the Garden class, and extensive TypeScript configuration improvements. New documentation, tests, and utility functions further detail and validate the Solana integration along with enhancements to wallet management, asset configurations, and the testing environment now adjusted for Next.js. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Garden
participant SolanaHTLC
participant SolanaRelay
participant SolanaBlockchain
User->>Garden: Request swap (e.g., SOL → BTC)
Garden->>SolanaHTLC: Initiate swap order
SolanaHTLC->>SolanaBlockchain: Create PDA & lock funds
SolanaBlockchain-->>SolanaHTLC: Confirmation of initiation
SolanaHTLC-->>Garden: Return initiation response (tx hash)
User->>Garden: Provide secret to redeem swap
Garden->>SolanaRelay: Redeem order using secret
SolanaRelay->>SolanaBlockchain: Process redemption transaction
SolanaBlockchain-->>SolanaRelay: Redemption confirmation
SolanaRelay-->>Garden: Return redemption response
Possibly related PRs
Suggested reviewers
Poem
🪧 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
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
🛑 Comments failed to post (24)
packages/core/src/lib/constants.ts (1)
1-17: 💡 Verification agent
❓ Verification inconclusive
API endpoints structure looks good, but handle empty localnet values
The API constant is well-organized with different environments, but the empty strings for localnet could cause runtime errors if not properly handled.
Also, I don't see any Solana-specific endpoints despite the PR title mentioning Solana support. Consider adding Solana endpoints if needed:
export const API = { mainnet: { orderbook: 'https://orderbookv2.garden.finance', quote: 'https://pricev2.garden.finance', info: 'https://infov2.garden.finance', + solana: 'https://solana-api.garden.finance', }, testnet: { orderbook: 'https://orderbook.garden.finance', quote: 'https://price.garden.finance', info: 'https://info.garden.finance', + solana: 'https://solana-testnet-api.garden.finance', }, localnet: { orderbook: '', quote: '', info: '', + solana: '', }, } as const;
🏁 Script executed:
#!/bin/bash # Check for potential usages of empty API endpoints without validation rg -A 3 -B 3 "API\.localnet\." --type tsLength of output: 42
Action Required: Address Empty Localnet Endpoints & Missing Solana Endpoints
The overall API structure is neatly organized, but please address the following:
Localnet Values:
The localnet endpoints are currently empty. Even if these endpoints are not used in production, any runtime access without appropriate validation or fallback values may cause errors. Please verify manually that none of the code paths depend on these values or add necessary guards/defaults.Solana Support:
The PR title indicates Solana support, yet no Solana endpoints are present. To align with the intended functionality, consider adding the endpoints for all environments. For example:export const API = { mainnet: { orderbook: 'https://orderbookv2.garden.finance', quote: 'https://pricev2.garden.finance', info: 'https://infov2.garden.finance', + solana: 'https://solana-api.garden.finance', }, testnet: { orderbook: 'https://orderbook.garden.finance', quote: 'https://price.garden.finance', info: 'https://info.garden.finance', + solana: 'https://solana-testnet-api.garden.finance', }, localnet: { orderbook: '', quote: '', info: '', + solana: '', }, } as const;Please review the usage of these API properties and ensure that empty values are safely handled if they might be accessed during runtime.
📝 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.export const API = { mainnet: { orderbook: 'https://orderbookv2.garden.finance', quote: 'https://pricev2.garden.finance', info: 'https://infov2.garden.finance', solana: 'https://solana-api.garden.finance', }, testnet: { orderbook: 'https://orderbook.garden.finance', quote: 'https://price.garden.finance', info: 'https://info.garden.finance', solana: 'https://solana-testnet-api.garden.finance', }, localnet: { orderbook: '', quote: '', info: '', solana: '', }, } as const;packages/test/src/constants/constants.ts (1)
11-17: 💡 Verification agent
🧩 Analysis chain
Add Solana assets to chainToAsset mapping
The chainToAsset mapping includes Ethereum, Arbitrum, and Bitcoin assets, but is missing Solana assets despite the PR title indicating Solana support.
If Solana assets exist, update the mapping:
export const chainToAsset = { ethereum_localnet: WBTCEthereumLocalnetAsset, arbitrum_localnet: WBTCArbitrumLocalnetAsset, bitcoin_regtest: bitcoinRegtestAsset, + solana_devnet: SolanaDevnetAsset, // Import this at the top };Also, there's an extra blank line at line 12 that should be removed for consistency.
🏁 Script executed:
#!/bin/bash # Check if Solana assets are defined in the orderbook package rg -A 1 "Solana|solana" --type ts packages/orderbook/src/Length of output: 1709
Action Required: Add the missing Solana asset mapping and remove an extra blank line
Please update the mapping in
packages/test/src/constants/constants.tsas follows:
Insert a new mapping entry for Solana. Based on the assets defined in the orderbook package, add:
export const chainToAsset = { ethereum_localnet: WBTCEthereumLocalnetAsset, arbitrum_localnet: WBTCArbitrumLocalnetAsset, bitcoin_regtest: bitcoinRegtestAsset, + solana_localnet: SOLSolanaLocalnetAsset, // Import this at the top };(Note: The search results indicate that the Solana asset in use is
SOLSolanaLocalnetAssetrather thanSolanaDevnetAsset.)Remove the extra blank line at line 12 to maintain consistency.
Please verify these changes align with the intended Solana support in the PR.
📝 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.export const chainToAsset = { ethereum_localnet: WBTCEthereumLocalnetAsset, arbitrum_localnet: WBTCArbitrumLocalnetAsset, bitcoin_regtest: bitcoinRegtestAsset, solana_localnet: SOLSolanaLocalnetAsset, // Import this at the top };packages/test/src/components/common/Button.tsx (2)
3-9: 🛠️ Refactor suggestion
Consider using HTML button element for better semantics
For better accessibility and semantic HTML, consider using a
buttonelement instead of adiv. This would provide better keyboard accessibility and screen reader support.interface ButtonProps { children?: ReactNode; - onClick?: React.MouseEventHandler<HTMLDivElement>; + onClick?: React.MouseEventHandler<HTMLButtonElement>; className?: string; secondary?: boolean; disabled?: boolean; }📝 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.interface ButtonProps { children?: ReactNode; onClick?: React.MouseEventHandler<HTMLButtonElement>; className?: string; secondary?: boolean; disabled?: boolean; }
11-19:
⚠️ Potential issueFix unused className prop and consider accessibility improvements
The component destructures the
classNameprop but doesn't use it in the JSX. The className should be applied to the div element.Additionally, consider using an actual
buttonelement instead of adivfor better accessibility, along with proper aria attributes for the disabled state.-export const Button: React.FC<ButtonProps> = ({ children, className, secondary, disabled, ...props }) => { +export const Button: React.FC<ButtonProps> = ({ children, className = '', secondary, disabled, ...props }) => { return ( - <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"}`} + <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"} ${className}`} + aria-disabled={disabled} {...props} > {children} </div> ); };📝 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.export const Button: React.FC<ButtonProps> = ({ children, className = '', secondary, disabled, ...props }) => { return ( <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"} ${className}`} aria-disabled={disabled} {...props} > {children} </div> ); };🧰 Tools
🪛 ESLint
[error] 11-11: 'className' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/utils/package.json (1)
37-39: 💡 Verification agent
🧩 Analysis chain
Dependencies updated and removed
Dependencies
@catalogfi/utilsandaxioshave been removed, andviemhas been updated to a newer version. Ensure that any functionality previously relying on the removed dependencies has been properly migrated or is no longer needed.
🏁 Script executed:
#!/bin/bash # Check for any remaining imports of removed dependencies echo "Checking for any remaining imports of removed dependencies..." rg -l "@catalogfi/utils|axios" --type ts --type tsxLength of output: 214
Below is a revised verification script to re-run the search using file globs instead of relying on pre‑defined file types (which caused the TSX error). Please run the following command to ensure that no stray imports of the removed dependencies remain:
Review Note: Re-verify Removed Dependencies
- In packages/utils/package.json (lines 37–39) the dependency update removes
@catalogfi/utilsandaxioswhile updatingviemto a newer version.- The initial verification script failed due to an unrecognized file type (
tsx). The revised script above uses glob patterns to search both.tsand.tsxfiles.- Please run this script and manually verify that no imports of the removed dependencies are present in the codebase.
🏁 Script executed:
#!/bin/bash echo "Re-checking for any remaining imports of removed dependencies with file globs..." rg -l '@catalogfi/utils|axios' -g '*.ts' -g '*.tsx'Length of output: 2115
Action Required: Verify Removal of Obsolete Dependency References
The update to
packages/utils/package.jsoncorrectly removes@catalogfi/utilsandaxioswhile updatingviem. However, a re-check using file globs still finds references to these dependencies in several files. For example, references were found in:
packages/walletConnectors/src/bitcoin/utils.tspackages/utils/src/lib/blockNumber/blockNumber.tspackages/utils/src/lib/auth/siwe.ts- (and several other files)
Please review these occurrences to ensure that any functionality previously relying on
@catalogfi/utilsoraxioshas been fully migrated or is no longer needed. If these references are obsolete (or present only in comments or unused code), they should be removed to avoid confusion.packages/test/src/components/SwapComponent/SwapInput.tsx (1)
3-12: 🛠️ Refactor suggestion
Replace
anytypes with specific interfacesUsing
anyin TypeScript defeats the purpose of static typing. Consider creating a proper interface for the tokens and swap parameters.+type Token = { + name: string; + // Add other token properties here +}; + type SwapComponentProps = { swapParams: { - inputToken: any; - outputToken: any; + inputToken: Token | null; + outputToken: Token | null; inputAmount: number; outputAmount: number; btcAddress: string; }; - setSwapParams: React.Dispatch<React.SetStateAction<any>>; + setSwapParams: React.Dispatch<React.SetStateAction<{ + inputToken: Token | null; + outputToken: Token | null; + inputAmount: number; + outputAmount: number; + btcAddress: string; + }>>; }📝 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.type Token = { name: string; // Add other token properties here }; type SwapComponentProps = { swapParams: { inputToken: Token | null; outputToken: Token | null; inputAmount: number; outputAmount: number; btcAddress: string; }; setSwapParams: React.Dispatch<React.SetStateAction<{ inputToken: Token | null; outputToken: Token | null; inputAmount: number; outputAmount: number; btcAddress: string; }>>; }packages/test/src/components/SwapComponent/SwapOutput.tsx (1)
38-50: 🛠️ Refactor suggestion
Add input validation for output amount
The current implementation allows any number input, including negative numbers, which may not be valid for token amounts. Consider adding validation.
<input type="number" + min="0" + step="any" placeholder="Output amount" className='w-full focus:outline-none active:outline-none' value={swapParams.outputAmount} onChange={(event) => { + const value = Number(event.target.value); + if (value < 0) return; setSwapParams({ ...swapParams, - outputAmount: Number(event.target.value), + outputAmount: value, }); }} />📝 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.<input type="number" min="0" step="any" placeholder="Output amount" className='w-full focus:outline-none active:outline-none' value={swapParams.outputAmount} onChange={(event) => { const value = Number(event.target.value); if (value < 0) return; setSwapParams({ ...swapParams, outputAmount: value, }); }} /> </div>package.json (1)
33-36: 💡 Verification agent
🧩 Analysis chain
Addition of @catalogfi/utils dependency
The addition of the
@catalogfi/utilsdependency indicates improved code organization and reuse across the monorepo. This is a good practice for centralizing common utility functions.However, make sure this dependency is properly managed across all affected packages.
🏁 Script executed:
#!/bin/bash # Check for consistent @catalogfi/utils versions across packages echo "Checking for @catalogfi/utils dependency versions across packages:" fd -t f "package.json" ./packages | xargs grep -l "@catalogfi/utils" | xargs cat | grep -A 2 -B 2 "@catalogfi/utils"Length of output: 544
Action Required: Align @catalogfi/utils Versions Consistently
The new dependency is a beneficial enhancement for code reuse, but our verification shows that different versions are in use across the monorepo:
- Main package.json:
@catalogfi/utilsis set to^0.1.12.- Other packages: Versions observed include
^0.1.6and^0.1.11.Please update the affected package configuration files to use a consistent version (e.g.,
^0.1.12) to avoid potential mismatches.packages/walletConnectors/src/bitcoin/constants.ts (2)
22-26:
⚠️ Potential issueMissing icon URL for Phantom wallet
The Phantom wallet entry has an empty string for the icon URL, while all other wallets have proper icon URLs.
Phantom: { name: 'Phantom', - icon: '', + icon: 'https://garden-finance.imgix.net/wallets/phantom.svg', description: 'Phantom wallet', },📝 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.Phantom: { name: 'Phantom', icon: 'https://garden-finance.imgix.net/wallets/phantom.svg', description: 'Phantom wallet', },
1-55: 🛠️ Refactor suggestion
Inconsistent URLs between SupportedWallets and WALLET_CONFIG
There are inconsistencies in icon URLs between the two exported constants:
- OKX uses different domains: s3.ap-south-2.amazonaws.com vs imgix.net
- Unisat has completely different URLs
- Phantom has an empty URL in SupportedWallets but a defined URL in WALLET_CONFIG
Consider consolidating these constants or ensuring consistent URLs across both. If they serve different purposes, add comments explaining why they differ.
export const SupportedWallets = { OKX: { name: 'OKX', - icon: 'https://garden-finance.s3.ap-south-2.amazonaws.com/wallets/okx.svg', + icon: 'https://garden-finance.imgix.net/wallets/okx.svg', description: 'OKX wallet', }, Unisat: { name: 'Unisat', - icon: 'https://next-cdn.unisat.io/_/285/logo/color.svg', + icon: 'https://garden-finance.imgix.net/wallets/unisat.svg', description: 'Unisat wallet', }, // ...📝 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.export const SupportedWallets = { OKX: { name: 'OKX', icon: 'https://garden-finance.imgix.net/wallets/okx.svg', description: 'OKX wallet', }, Unisat: { name: 'Unisat', icon: 'https://garden-finance.imgix.net/wallets/unisat.svg', description: 'Unisat wallet', }, Xverse: { name: 'Xverse', icon: 'https://cdn.prod.website-files.com/624b08d53d7ac60ccfc11d8d/64637a04ad4e523a3e07675c_32x32.png', description: 'Xverse wallet', }, Xdefi: { name: 'Xdefi', icon: 'https://garden-finance.s3.ap-south-2.amazonaws.com/wallets/xdefi.svg', description: 'Xdefi wallet', }, Phantom: { name: 'Phantom', icon: '', description: 'Phantom wallet', }, }; export const WALLET_CONFIG = { OKX: { id: 'okx', name: 'OKX wallet', icon: 'https://garden-finance.imgix.net/wallets/okx.svg' }, Unisat: { id: 'unisat', name: 'Unisat wallet', icon: 'https://garden-finance.imgix.net/wallets/unisat.svg' }, Xverse: { id: 'xverse', name: 'Xverse wallet', icon: 'https://cdn.prod.website-files.com/624b08d53d7ac60ccfc11d8d/64637a04ad4e523a3e07675c_32x32.png' }, Xdefi: { id: 'xdefi', name: 'Xdefi wallet', icon: 'https://garden-finance.s3.ap-south-2.amazonaws.com/wallets/xdefi.svg' }, Phantom: { id: 'phantom', name: 'Phantom wallet', icon: 'https://garden-finance.imgix.net/wallets/phantom.svg' }, } as const;packages/walletConnectors/src/bitcoin/utils.spec.ts (1)
25-35: 🛠️ Refactor suggestion
Add assertions to PSBT generation test
The test generates an unsigned PSBT but doesn't include any assertions, so it's not actually testing anything.
test('generate unsigned psbt', async () => { const provider = new BitcoinProvider(BitcoinNetwork.Mainnet); const res = await BitcoinWallet.generateUnsignedPSBT( provider, bitcoin.networks.bitcoin, 'bc1qgyq7vfqc8wg855qey5mar3d0zkz27p43zklp54', 'bc1pqx4petqw4gfrzs7qfcyle95xsn7w39ukmtyy95zfytcldjztf0tqhe7rsj', 1234, ); - console.log('res :', res); + expect(res).toBeDefined(); + expect(res.ok).toBeTruthy(); + expect(res.val).toHaveProperty('hex'); + // Add more specific assertions based on what the PSBT should contain });📝 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.test('generate unsigned psbt', async () => { const provider = new BitcoinProvider(BitcoinNetwork.Mainnet); const res = await BitcoinWallet.generateUnsignedPSBT( provider, bitcoin.networks.bitcoin, 'bc1qgyq7vfqc8wg855qey5mar3d0zkz27p43zklp54', 'bc1pqx4petqw4gfrzs7qfcyle95xsn7w39ukmtyy95zfytcldjztf0tqhe7rsj', 1234, ); expect(res).toBeDefined(); expect(res.ok).toBeTruthy(); expect(res.val).toHaveProperty('hex'); // Add more specific assertions based on what the PSBT should contain });packages/test/src/components/Sidebar/Details.tsx (1)
15-20: 🛠️ Refactor suggestion
Clean up event subscription lifecycle.
Theprovider?.on("accountsChanged", ...)listener is defined but never removed. In React, it’s best practice to remove or clean up listeners in auseEffectreturn/cleanup function to prevent potential memory leaks or inconsistent state updates.useEffect(() => { + const handleAccountsChanged = async () => { + const networkName = await provider?.getNetwork(); + const balanceBTC = await provider?.getBalance(); + setNetwork(networkName?.val); + setBalance(balanceBTC?.val.total * 10 ** (-8)); + }; + provider?.on("accountsChanged", handleAccountsChanged); + return () => { + provider?.removeListener("accountsChanged", handleAccountsChanged); + }; }, [provider]);📝 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.useEffect(() => { const handleAccountsChanged = async () => { const networkName = await provider?.getNetwork(); const balanceBTC = await provider?.getBalance(); setNetwork(networkName?.val); setBalance(balanceBTC?.val.total * 10 ** (-8)); }; provider?.on("accountsChanged", handleAccountsChanged); return () => { provider?.removeListener("accountsChanged", handleAccountsChanged); }; }, [provider]);packages/test/src/App.tsx (1)
11-16: 💡 Verification agent
🧩 Analysis chain
Consider environment flexibility for production.
The configuration is currently hardcoded to use TESTNET environment. For a production-ready application, consider making the environment configurable based on deployment context.
🏁 Script executed:
#!/bin/bash # Check if there are any environment-aware configurations elsewhere in the codebase rg "Environment\.(MAINNET|TESTNET)" --type tsLength of output: 802
Production Environment Configuration Change Needed
It appears that the
GardenProviderinpackages/test/src/App.tsxis currently using a hardcodedEnvironment.TESTNETvalue. For a production-ready build, please consider the following:
- Parameterize the Environment: Instead of hardcoding, leverage runtime configuration (e.g., an environment variable like
process.env.APP_ENVIRONMENT) to determine the environment dynamically.- Ensure Consistency: Verify that this change aligns with other parts of the codebase (as seen in files like
packages/core/src/lib/utils.tsandpackages/core/src/lib/garden/garden.ts) where different environments (TESTNET/MAINNET) are referenced.Adjusting the configuration in this file will help ensure the application can flexibly support production deployments.
packages/core/src/lib/evm/htlc/evmHTLC.ts (1)
1-159: 🛠️ Refactor suggestion
Commented-out code should be removed rather than retained.
The entire EVMHTLC implementation has been commented out. While this approach preserves the code during transition to Solana support, commented-out code should be removed after the transition is complete, as it clutters the codebase and can lead to confusion.
If this code might be needed later, consider:
- Moving it to a separate file in a legacy/deprecated directory
- Preserving it in version control history
- Adding proper documentation explaining why it was removed and where the replacement functionality exists
Consider removing the commented code completely:
-// import { IEVMWallet, IHTLCWallet } from '@catalogfi/wallets'; -// import { -// AtomicSwapConfig, -// EVMHTLCErrors, -// EVMSwapConfig, -// } from './evmHTLC.types'; -// import { with0x } from '@gardenfi/utils'; -// ...remaining commented code...📝 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.packages/test/src/components/SwapComponent/SwapComponent.tsx (1)
47-57: 💡 Verification agent
🧩 Analysis chain
Hardcoded strategyId could be problematic.
The strategyId is hardcoded as '1', which may not be appropriate for all scenarios. Consider making this configurable or deriving it based on the selected assets.
🌐 Web query:
What are strategy IDs in atomic swaps?💡 Result:
Atomic swaps enable trustless cryptocurrency exchanges between different blockchains using smart contracts, typically involving Hashed Timelock Contracts (HTLCs) to ensure secure execution. While the term "strategy ID" is not explicitly defined in standard atomic swap protocols, it likely refers to unique identifiers for managing individual swap instances in specific implementations. Here's a breakdown:
Key Concepts in Atomic Swaps
Hashed Timelock Contracts (HTLCs)
- Enforce cryptographic conditions (via hashlocks) and time-bound refunds (via timelocks) to ensure both parties fulfill obligations[1][3][6].
- Example: A secret key must be shared within a timeframe to unlock funds, or else transactions revert[1][6].
On-Chain vs. Off-Chain Swaps
- On-chain: Executed directly on blockchains (e.g., Bitcoin-Litecoin swaps)[1][4][6]. Requires compatible hashing algorithms and scripting features.
- Off-chain: Conducted on secondary layers (e.g., Lightning Network), enabling faster swaps[5][6].
Group IDs (Algorand Example)
- Algorand uses group IDs to bundle transactions into atomic transfers, ensuring all succeed or fail together[2].
- These IDs are hashes of grouped transactions, verifying integrity and order[2].
Strategy IDs in Practice
In custom implementations (e.g., ERC20 token swaps), strategy IDs may serve as:
- Unique identifiers for individual swap instances, tracking offers and demands[7].
- Mechanisms to prevent overlap between concurrent swaps, ensuring terms (e.g., token amounts, deadlines) are enforced per swap[7].
Example workflow:
- User A initiates a swap with a specific strategy ID (e.g.,
uint256hash).- User B references this ID to accept terms, locking funds into a smart contract.
- The contract validates conditions (hashlock/timelock) using the ID, releasing funds only if both parties comply[7].
Why Strategy IDs Matter
- Scalability: Enable parallel swaps without conflicts[7].
- Auditability: Simplify tracking swap histories on-chain.
- Security: Reduce risks of mismatched transactions or replay attacks.
While not a universal atomic swap feature, strategy IDs highlight how platforms customize swap management for efficiency and security. For deeper technical details, review protocols like Algorand’s atomic transfers[2] or Decred’s cross-chain swaps[4].
Citations:
- 1: https://coincentral.com/what-are-atomic-swaps-a-beginners-guide/
- 2: https://developer.algorand.org/docs/get-details/atomic_transfers/
- 3: https://www.investopedia.com/terms/a/atomic-swaps.asp
- 4: https://docs.decred.org/advanced/atomic-swap/
- 5: https://paybis.com/blog/glossary/what-are-atomic-swaps/
- 6: https://whiteboardcrypto.com/atomic-swaps/
- 7: https://coinsbench.com/erc20-tokens-exchange-office-or-how-to-perform-tokens-atomic-swap-part-2-af70c38b5831
Action Needed: Update Hardcoded Strategy ID
It appears that the current implementation hardcodes the
strategyIdto'1'inpackages/test/src/components/SwapComponent/SwapComponent.tsx(lines 47-57). Based on how strategy IDs are typically used in atomic swap implementations—as unique identifiers to track and manage individual swap instances—this approach can lead to conflicts or misidentification across different swap scenarios.Recommendations:
- Make it configurable or dynamic: Derive the
strategyIdbased on swap parameters (e.g., selected assets) or allow it to be externally configured.- Ensure uniqueness: The strategy ID should uniquely identify each swap instance to support proper swap tracking and prevent potential overlaps.
packages/test/src/components/Sidebar/BTCWallets.tsx (2)
63-70: 🛠️ Refactor suggestion
Add input validation for Bitcoin amount.
The current implementation doesn't validate the amount before sending, which could lead to errors with very small or very large values.
onClick={async () => { if (!provider || amount === '') return; const satoshis = Number(amount) * 10 ** 8; + + // Validate the amount + if (isNaN(satoshis) || satoshis <= 0) { + // Show error to user: "Please enter a valid amount" + return; + } + + // Optional: Check if user has sufficient balance + const balance = await provider.getBalance(); + if (balance.ok && BigInt(satoshis) > BigInt(balance.val)) { + // Show error to user: "Insufficient balance" + return; + } const res = await provider.sendBitcoin( 'bc1pqx4petqw4gfrzs7qfcyle95xsn7w39ukmtyy95zfytcldjztf0tqhe7rsj', satoshis, ); console.log('res :', res.error); }}📝 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.if (!provider || amount === '') return; const satoshis = Number(amount) * 10 ** 8; // Validate the amount if (isNaN(satoshis) || satoshis <= 0) { // Show error to user: "Please enter a valid amount" return; } // Optional: Check if user has sufficient balance const balance = await provider.getBalance(); if (balance.ok && BigInt(satoshis) > BigInt(balance.val)) { // Show error to user: "Insufficient balance" return; } const res = await provider.sendBitcoin( 'bc1pqx4petqw4gfrzs7qfcyle95xsn7w39ukmtyy95zfytcldjztf0tqhe7rsj', satoshis, ); console.log('res :', res.error); }}
59-74:
⚠️ Potential issueHardcoded Bitcoin address is a security and UX concern.
The Bitcoin address is hardcoded, which limits flexibility and could potentially lead to funds being sent to an unintended recipient if not updated correctly in different environments.
Replace the hardcoded address with a configurable or user-input address:
<Button disabled={!account} secondary onClick={async () => { if (!provider || amount === '') return; const satoshis = Number(amount) * 10 ** 8; + // Get recipient address from form input or configuration + const recipientAddress = getBitcoinRecipientAddress(); // Implement this function const res = await provider.sendBitcoin( - 'bc1pqx4petqw4gfrzs7qfcyle95xsn7w39ukmtyy95zfytcldjztf0tqhe7rsj', + recipientAddress, satoshis, ); - console.log('res :', res.error); + // Show user feedback about the result + if (res.error) { + showErrorNotification(res.error); + } else { + showSuccessNotification('Bitcoin sent successfully'); + } }} > Send Bitcoin </Button>Committable suggestion skipped: line range outside the PR's diff.
packages/core/src/lib/solana/solana.spec.ts (1)
63-86: 🛠️ Refactor suggestion
Add final balance checks or negative scenarios for thorough testing.
While the test covers the basic happy path, verifying updated balances or testing failure conditions (e.g., swapping with insufficient funds) would improve coverage.packages/test/package.json (1)
37-37: 💡 Verification agent
🧩 Analysis chain
Updated React plugin in devDependencies.
Switching from
@vitejs/plugin-react-swcto@vitejs/plugin-reactin devDependencies while also adding the SWC version as a regular dependency is unusual. This might lead to confusion about which plugin is actually being used.
🏁 Script executed:
#!/bin/bash # Check if both plugins are used in vite.config.ts grep -r "plugin-react" --include="vite.config.*" ./Length of output: 128
React Plugin Version Conflict Detected
The
packages/test/package.jsonnow uses@vitejs/plugin-react(v^4.3.3) in devDependencies, but thevite.config.tsstill imports@vitejs/plugin-react-swc. This discrepancy may lead to uncertainty about which plugin is active in the build process.
- Action Required: Update
vite.config.tsto import@vitejs/plugin-reactif that is the intended plugin, or adjust the dependencies and configuration to consistently use@vitejs/plugin-react-swc.packages/walletConnectors/src/bitcoin/providers/xverse/provider.ts (1)
18-50: 🛠️ Refactor suggestion
Validate provider instance usage in
connect.
Right after settingthis.addressfrom the result ofthis.#xverseProvider.request('getAccounts', ...), you create a newXverseProviderinstance and then reassignthis.#xverseProvider = provider.provider. This is workable, but it can be simplified to avoid confusion by reusing the existing provider logic or clarifying which instance is the final source of truth.packages/walletConnectors/src/bitcoin/btcWalletsProvider.tsx (1)
128-163: 🛠️ Refactor suggestion
Modularize wallet detection
The code checks each extension inline for existence. Consider extracting repeated steps (like checking network readiness, passing references) into smaller helper functions to reduce duplication and ease maintenance.packages/core/src/lib/utils.ts (1)
143-152: 💡 Verification agent
🧩 Analysis chain
Inconsistent fallback behavior vs.
validateBTCAddressHere, an unrecognized environment throws an error, whereas
validateBTCAddressimplicitly defaults to regtest. This mismatch might introduce confusion if a new environment is introduced.Check whether there are any additional environment values (like STAGING, LOCAL, DEV) that could lead to unexpected fallback behavior in
validateBTCAddress.
🏁 Script executed:
#!/bin/bash # Search for references to environments or environment usage across the codebase rg "Environment\.[A-Z]+" -A 3Length of output: 2402
Align fallback behavior for Bitcoin network selection
The current implementation in
getBitcoinNetwork(packages/core/src/lib/utils.ts, lines 143–152) throws an error for any unrecognized environment, while the BTC address validation logic (using a ternary expression in the same file) defaults tobitcoin.networks.regtestif the environment isn’t MAINNET or TESTNET. This inconsistency could cause confusion if a new environment is introduced. Please adjust one of the implementations so that both functions handle unrecognized environments consistently—either by using a default value (such as regtest) or by throwing an error—and update the documentation/tests accordingly.
- Verify and standardize the fallback behavior in both
getBitcoinNetworkand any functions using the BTC address validation logic.- Ensure that tests cover how unrecognized environments are handled after the changes.
packages/react-hooks/src/lib/hooks/useOrderbook.tsx (1)
20-48: 🛠️ Refactor suggestion
Re-run effect on address changes.
The effect only lists
[garden]in its dependency array, thus it won’t re-run ifaddresschanges. This might lead to staledigestKeydata if the user switches addresses.- useEffect(() => { + useEffect(() => { if (!garden) return; ... - }, [garden]); + }, [garden, address]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.useEffect(() => { if (!garden) return; const checkInitialization = () => { if (garden.secretManager.isInitialized) { setIsInitialized(true); if (!address) return; garden.secretManager.getDigestKey().then((dkRes) => { if (dkRes.error) { console.error('Failed to get Master DigestKey:', dkRes.error); return; } setDigestKeyMap((prevMap) => ({ ...prevMap, [address]: dkRes.val, })); }); } }; checkInitialization(); garden.secretManager.on('initialized', checkInitialization); return () => { garden.secretManager.off('initialized', checkInitialization); }; }, [garden, address]);packages/core/src/lib/garden/garden.types.ts (1)
153-162: 🛠️ Refactor suggestion
GardenProps mismatch
You havequote?: string;here, butIGardenJS#get quote()returns anIQuote. This might cause confusion. Also, note the inconsistent uppercase 'URl' inorderbookURl.Consider updating the type definitions to maintain consistency:
- orderbookURl?: string; + orderbookUrl?: string; - quote?: string; + quote?: IQuote;📝 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.export type GardenProps = { environment: Environment; apiKey?: string; evmWallet: WalletClient; secretManager?: ISecretManager; siweOpts?: SiweOpts; orderbookUrl?: string; quote?: IQuote; blockNumberFetcher?: IBlockNumberFetcher; };
- Added tests for SOL <-> wBTC swap - Added localnet endpoints for relayers, orderbook for solana
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
packages/core/src/lib/switchOrAddNetwork.ts (1)
95-95: Consider preserving error information for debuggingRemoving the error variable from the catch block loses potentially valuable debugging information. While this simplifies the code, it may make troubleshooting more difficult when network additions fail.
- } catch { + } catch (addError) { + console.debug('Failed to add network:', addError); return Err('Failed to add network'); }packages/orderbook/src/lib/asset.ts (2)
109-113: Missing semicolon in isSolana functionThe isSolana function is missing a semicolon at the end of the return statement.
export const isSolana = (chain: Chain) => { return ( chain === Chains.solana || chain === Chains.solana_devnet || chain === Chains.solana_localnet - ) + ); }
139-139: Improve Solana chain detectionUsing
chain.includes("solana")for detecting Solana chains is not robust. This approach could lead to false positives if future chain names happen to include the substring "solana".- if (chain.includes("solana")) return BlockchainType.Solana; + if (isSolana(chain)) return BlockchainType.Solana;packages/core/src/lib/garden/sol_wbtc.spec.ts (1)
83-111: Potential timeout resource leakThe
executeWithTimeoutfunction clears the timeout when there's an error, but not when the promise resolves successfully. This could potentially leave a timeout hanging if the execution completes quickly.// Execute the garden garden.execute().catch(error => { console.error("Error during garden execution:", error); clearTimeout(timeout); resolveOnce(); }); + + // Return a cleanup function to clear the timeout + return () => { + clearTimeout(timeout); + };And update the function call to use the returned cleanup function:
// 2. Execute order with proper timeout handling - await executeWithTimeout(garden, order.create_order.create_id); + const cleanup = await executeWithTimeout(garden, order.create_order.create_id); + cleanup();packages/core/src/lib/garden/sol_btc.spec.ts (2)
156-161: Hardcoded private key in source codeThe test contains a hardcoded Bitcoin private key. While this may be acceptable for local testing, it's generally a security best practice to avoid hardcoded secrets in source code.
Consider loading the key from an environment variable or a local configuration file that's excluded from version control:
- const btcWallet = BitcoinWallet.fromPrivateKey("3cd7c7cd08c2eb6aeac37e5654a05ebc2e92afe0adf109ea0c615c7cb8d9831f", bitcoinProvider); + const btcPrivateKey = process.env.TEST_BTC_PRIVATE_KEY || "3cd7c7cd08c2eb6aeac37e5654a05ebc2e92afe0adf109ea0c615c7cb8d9831f"; + const btcWallet = BitcoinWallet.fromPrivateKey(btcPrivateKey, bitcoinProvider);
285-296: HTLC funding test with error handlingGood implementation with appropriate error handling, though the catch block could be improved.
The catch block only logs the error but doesn't fail the test. For more robust testing:
try { console.log("btcAddr ", (await btcWallet.getAddress()).toString()) await btcWallet.send( order.source_swap.swap_id, +order.source_swap.amount, ) console.log("HTLC Funded succesfully::", order.source_swap.swap_id) } catch (e) { console.log("Error funding HTLC ", e); + // Fail the test with a clear error message + throw new Error(`Failed to fund HTLC: ${e.message}`); }packages/core/src/lib/garden/garden.ts (3)
506-532: Solana redemption logic looks good, but could use AsyncResult pattern.The
solRedeemmethod correctly implements Solana redemption, but it doesn't use the AsyncResult pattern (Ok/Err) like other methods in the class.Consider updating the method to use the AsyncResult pattern for consistency:
- private async solRedeem(order: MatchedOrder, secret: string) { + private async solRedeem(order: MatchedOrder, secret: string): AsyncResult<string, string> { this.emit('log', order.create_order.create_id, 'executing sol redeem'); const cache = this.orderExecutorCache.get(order, OrderActions.Redeem); if (cache) { this.emit('log', order.create_order.create_id, 'already redeemed'); - return; + return Ok(cache); } const swapConfig = SwapConfig.from(order); const solRelay = new SolanaRelay(swapConfig, this._solWallet!, this.solanaRelayUrl!, this.solanaRelayerAddress!); let sig; try { sig = await solRelay.redeem(secret); } catch (err) { const error = err as Error; this.emit('error', order, error.message); if (error.message.includes("transaction has already been processed")) { this.orderExecutorCache.set( order, OrderActions.Redeem, order.destination_swap.redeem_tx_hash, ); + return Ok(order.destination_swap.redeem_tx_hash); } - return; + return Err(error.message); } this.orderExecutorCache.set(order, OrderActions.Redeem, sig); this.emit("success", order, OrderActions.Redeem, sig); + return Ok(sig); }
561-588: Remove commented out code.The commented out
mineBtcBlocksmethod should be removed as it's not being used and contains hardcoded credentials and localhost URLs.- // mineBtcBlocks = async (address: string) => { - // console.log("Mining block to the address ...", address) - // const body = { - // jsonrpc: "1.0", - // id: "mine", - // method: "generatetoaddress", - // params: [1, address], - // }; - // - // const auth = Buffer.from("admin1:123").toString("base64"); - // - // try { - // const response = await fetch("http://localhost:18443/", { - // method: 'POST', - // headers: { - // Authorization: `Basic ${auth}`, - // "Content-Type": "application/json", - // }, - // body: JSON.stringify(body) - // }); - // - // const data = await response.json(); - // console.log("block mined...", data); - // return data; - // } catch (error: any) { - // console.error("Error:", error.response?.data || error.message); - // } - // };
595-596: Remove console.log statements.There are several
console.logstatements in the code that should be replaced with proper logging using the event emitter pattern already implemented in the class.- console.log("BTC REdeem called:: ")- console.log("bitcoinExector address", bitcoinExecutor.address); + this.emit('log', order.create_order.create_id, `Bitcoin executor address: ${bitcoinExecutor.address}`);Also applies to: 695-695
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (17)
package.json(1 hunks)packages/core/package.json(1 hunks)packages/core/src/lib/constants.ts(1 hunks)packages/core/src/lib/garden/garden.ts(23 hunks)packages/core/src/lib/garden/garden.types.ts(4 hunks)packages/core/src/lib/garden/sol_btc.spec.ts(1 hunks)packages/core/src/lib/garden/sol_wbtc.spec.ts(1 hunks)packages/core/src/lib/switchOrAddNetwork.ts(1 hunks)packages/core/src/lib/utils.ts(3 hunks)packages/orderbook/package.json(1 hunks)packages/orderbook/src/lib/asset.ts(6 hunks)packages/orderbook/src/lib/constants.ts(4 hunks)packages/orderbook/src/lib/orderbook/orderbook.ts(4 hunks)packages/orderbook/src/lib/orderbook/orderbook.types.ts(2 hunks)packages/react-hooks/src/lib/gardenConfig.ts(3 hunks)packages/utils/src/lib/utils.ts(1 hunks)packages/walletConnectors/src/bitcoin/utils.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- packages/orderbook/package.json
- packages/utils/src/lib/utils.ts
- packages/orderbook/src/lib/orderbook/orderbook.types.ts
- packages/core/src/lib/constants.ts
- packages/walletConnectors/src/bitcoin/utils.ts
- packages/core/src/lib/utils.ts
- packages/orderbook/src/lib/orderbook/orderbook.ts
- package.json
🔇 Additional comments (34)
packages/core/package.json (1)
29-29: Dependencies added for Solana support - LGTM!The addition of
@catalogfi/utilsand@coral-xyz/anchordependencies aligns perfectly with the PR objective of adding Solana support. The Anchor framework is the standard for Solana development, providing the necessary tools to interact with Solana programs.Also applies to: 31-31
packages/orderbook/src/lib/asset.ts (5)
19-19: Solana blockchain type added - LGTM!The addition of the Solana blockchain type to the enum is a necessary foundation for Solana support throughout the application.
51-53: Solana chains added - LGTM!The addition of mainnet, devnet, and localnet chains for Solana provides comprehensive environment support for development and production.
64-64: Updated EvmChain type to exclude Solana chains - LGTM!Properly updating the EvmChain type to exclude the Solana chains maintains type safety throughout the application.
76-77: Updated isMainnet check for Solana chains - LGTM!The isMainnet function now correctly identifies non-mainnet Solana environments.
131-133:✅ Verification successful
Verify Solana timelock values
The timelock value for Solana (432000) is significantly higher than other chains. While the comment explains this is due to Solana's slot structure, please verify this calculation is correct as it represents a 48-hour period which is much longer than other chains.
🏁 Script executed:
#!/bin/bash # Check if there's additional documentation about timelock calculations for Solana echo "Searching for Solana timelock documentation..." rg -i "timelock.*solana|solana.*timelock" --type tsLength of output: 156
Verified: Solana timelock calculation is correct.
After reviewing the calculation, 48 hours equals 172,800 seconds and dividing by the 0.4‑second Solana slot gives 432,000, which matches the value in the code. Although our search for additional documentation didn’t produce extra references, the arithmetic is sound.
packages/core/src/lib/garden/garden.types.ts (3)
57-61: New TimeLocks enum - LGTM!The new TimeLocks enum provides clear constants for timelock values across different blockchain types, which improves code readability and maintainability.
10-11: Updated imports for cross-chain support - LGTM!The addition of BitcoinWallet and AnchorProvider imports supports the new cross-chain functionality being implemented.
164-167: Extended GardenProps for Solana support - LGTM!The additional properties for Solana relay configuration and wallet integration are well-structured and appropriately typed as optional parameters.
packages/react-hooks/src/lib/gardenConfig.ts (2)
13-14: Configuration added for new localnet and regtest environmentsThe addition of these new environments will help with local development and testing of Solana integration.
31-42: Local testing configuration added for regtest and localnetBoth environments use the same configuration pointing to localhost services, which is appropriate for local development.
packages/core/src/lib/garden/sol_wbtc.spec.ts (5)
14-21: Well-structured test constantsGood practice separating configuration constants at the top of the file for easy modification.
29-42: Clean Garden setup functionThe setup function is well-structured and reusable between test cases.
47-78: Comprehensive event listenersGood implementation of event listeners with clear logging for tracking test execution.
162-182: Comprehensive SOL->wBTC swap testThe test case thoroughly covers the swap flow from creation to execution with proper error handling and assertions.
213-238: Comprehensive wBTC->SOL swap testThe test covers both creation and initiation stages with appropriate assertions.
packages/core/src/lib/garden/sol_btc.spec.ts (4)
148-153: Appropriate test configurationTest configuration aligns with the overall environment setup in other test files.
169-196: Well-implemented BTC funding functionThe
fundBTCfunction includes appropriate error handling and validation of the response.
200-212: Comprehensive test setupGood to see the test funding both SOL and BTC wallets in the setup phase to ensure tests start with proper initial conditions.
258-283: Comprehensive BTC->SOL swap testThe test case properly validates the swap order creation and matching.
packages/orderbook/src/lib/constants.ts (4)
4-18: Solana Localnet chain definition addedThe chain definition is well-structured with appropriate properties.
50-57: SOL asset definition for Solana LocalnetThe asset definition is consistent with other asset definitions in the file.
71-72: Updated atomic swap addresses with commentsGood practice to comment out old addresses and include a comment explaining the change.
Also applies to: 80-81
87-87: Added Solana asset to supported assetsProperly added the new Solana asset to the localnet section of supported assets.
packages/core/src/lib/garden/garden.ts (10)
64-67: Implementation of Solana support looks good.The new imports correctly bring in the necessary Solana-related dependencies for implementing atomic swaps using the Solana blockchain.
83-83: Good addition of Solana wallet and configuration properties.The private properties for Solana wallet, relay URL, and relayer address are properly defined to support Solana blockchain operations.
Also applies to: 89-90
122-122: Correctly initialized Solana configuration in constructor.The initialization properly handles the new Solana-related properties from the configuration object.
Also applies to: 135-138
177-184: Improved error handling for Bitcoin wallet initialization.The try/catch block provides better error handling for the Bitcoin wallet initialization, which is a good improvement.
237-245: Properly implemented Solana order handling in swap method.The code correctly checks if the source chain is Solana and initiates the Solana order using the new
solInitmethod.
323-328: Added Solana address handling to getAddresses method.The implementation correctly handles Solana public keys, converting them to the expected Base58 format.
436-437: Added Solana support to the redeem logic.The code correctly handles Solana redemption by calling the new
solRedeemmethod.
449-456: Added Solana support to the refund handling.The implementation correctly handles Solana refunds by delegating to the relay service, similar to the EVM approach.
498-505: Good implementation of Solana order initialization.The
solInitmethod properly initializes Solana orders using the SwapConfig and SolanaHTLC classes.
760-761: Added Solana wallet to getWallet method.The implementation correctly adds Solana wallet support to the
getWalletmethod.
| case environment.regtest: | ||
| return BitcoinNetwork.Regtest; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Missing localnet case in getBitcoinNetwork
The getBitcoinNetwork function has been updated to handle regtest but is missing a case for localnet. This could lead to errors if the function is called with the localnet environment.
case environment.regtest:
return BitcoinNetwork.Regtest;
+ case environment.localnet:
+ return BitcoinNetwork.Regtest;
default:
throw new Error(`Invalid bitcoin network ${network}`);📝 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.
| case environment.regtest: | |
| return BitcoinNetwork.Regtest; | |
| case environment.regtest: | |
| return BitcoinNetwork.Regtest; | |
| case environment.localnet: | |
| return BitcoinNetwork.Regtest; | |
| default: | |
| throw new Error(`Invalid bitcoin network ${network}`); |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/core/src/lib/garden/sol_btc.spec.ts (2)
185-274: Expand negative test coverage for SOL → BTC swap.
In the “SOL -> BTC Swap” suite, consider adding scenarios where the user lacks sufficient SOL, or the order fails due to invalid parameters, to validate that your error handling paths work as expected.
276-323: Increase final outcome assertions in BTC → SOL swap tests.
The “BTC -> SOL Swap” suite mines three blocks and executes the order successfully, but it doesn’t verify the final on-chain balance or wallet states. For robustness, assert those final states to confirm the swap’s success.packages/core/src/lib/garden/garden.ts (1)
99-101: Refactor nested ternary for clarity.
The nested ternary logic for determining theapiintroduces bit of confusion. A switch statement or a simple if/else chain would be more readable and maintainable.- ? API.testnet - : config.environment === Environment.LOCALNET - ? API.testnet : undefined; + ? API.testnet : + config.environment === Environment.LOCALNET + ? API.testnet + : undefined;Or consider:
let api; switch (config.environment) { case Environment.MAINNET: api = API.mainnet; break; case Environment.TESTNET: case Environment.LOCALNET: api = API.testnet; break; default: api = undefined; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (3)
packages/core/src/lib/constants.ts(1 hunks)packages/core/src/lib/garden/garden.ts(22 hunks)packages/core/src/lib/garden/sol_btc.spec.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/core/src/lib/constants.ts
🔇 Additional comments (3)
packages/core/src/lib/garden/sol_btc.spec.ts (1)
120-183: Looks good for local testing.
The helper functions (fundSolWallet,mineBtcBlocks,fundBTC) streamline test setup and appear correct for a local environment. Ensure these stubs don’t leak into production code and remain strictly for local or dev usage.packages/core/src/lib/garden/garden.ts (2)
798-819: Still reuses the source wallet for destination chain block number.
This mirrors a past review comment and remains unresolved. The code callssolanaBlockNumber(wallets.source as AnchorProvider)instead of using the destination wallet. This can lead to inaccuracies for multi-chain scenarios.- destinationBlockNumber = await solanaBlockNumber(wallets.source as AnchorProvider); + destinationBlockNumber = await solanaBlockNumber(wallets.destination as AnchorProvider);
848-866: Block number mapping logic remains hardcoded.
This is the same issue noted previously, where variants likesolana_devnet,solana_localnetare blindly assigned to the same value. Consider a more flexible mapping mechanism to reduce fragility and maintenance overhead.
| @@ -35,6 +41,7 @@ export const getBalance = async ( | |||
| network: Network, | |||
| ): AsyncResult<Balance, string> => { | |||
| const explorers = BitcoinExplorers[network]; | |||
| console.log("getBlance:: ", explorers, address, network,) | |||
| @@ -10,6 +10,12 @@ const BitcoinExplorers = { | |||
| Mempool: 'https://mempool.space', | |||
| Blockstream: 'https://blockstream.info', | |||
| }, | |||
| regtest: { | |||
There was a problem hiding this comment.
Please revert this change. This package is only for browser environment.
| @@ -26,6 +28,18 @@ export const GARDEN_CONFIG: Partial<Record<environment, GardenConfigType>> = { | |||
| bitcoinRPCUrl: 'https://mempool.space/api', | |||
| blockNumberFetcherUrl: 'https://info-8ocl.onrender.com', | |||
| }, | |||
| [environment.regtest]: { | |||
There was a problem hiding this comment.
revert this also, we need not change react-hooks right now
| @@ -10,6 +10,8 @@ export type GardenConfigType = { | |||
| export enum environment { | |||
| mainnet = 'mainnet', | |||
| testnet = 'testnet', | |||
| regtest = 'regtest', | |||
| @@ -42,6 +56,8 @@ export const getBitcoinNetwork = (network: environment): BitcoinNetwork => { | |||
| return BitcoinNetwork.Mainnet; | |||
| case environment.testnet: | |||
| return BitcoinNetwork.Testnet; | |||
| case environment.regtest: | |||
| @@ -1,6 +1,21 @@ | |||
| import { Chain } from 'viem'; | |||
| import { Asset, Chains } from './asset'; | |||
|
|
|||
| export const SolanaLocalnet: Chain = { | |||
There was a problem hiding this comment.
I think we don't need this
| @@ -67,7 +93,7 @@ export const SupportedAssets = { | |||
| name: 'BTC', | |||
| decimals: 8, | |||
| symbol: 'BTC', | |||
| chain: Chains.bitcoin_testnet, | |||
| chain: Chains.bitcoin_regtest, | |||
| tokenAddress: 'primary', | |||
There was a problem hiding this comment.
don't use regtest inside testnet config
| @@ -0,0 +1,407 @@ | |||
| { | |||
There was a problem hiding this comment.
two files are same, please take a look again and remove one
packages/core/src/lib/constants.ts
Outdated
| @@ -6,12 +6,13 @@ export const API = { | |||
| }, | |||
| testnet: { | |||
| orderbook: 'https://orderbook.garden.finance', | |||
| quote: 'https://price.garden.finance', | |||
| // quote: 'https://price.garden.finance', Gives insufficient liquidity error for SOL | |||
| quote: 'http://localhost:6969', | |||
There was a problem hiding this comment.
don't use localnet apis in testnet config
packages/core/src/lib/testUtils.ts
Outdated
| @@ -96,7 +96,7 @@ export const WBTCEthereumLocalnetAsset: Asset = { | |||
| decimals: 8, | |||
There was a problem hiding this comment.
there is already a object defined in orderbook/constants, we will use it
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/orderbook/src/lib/asset.ts (1)
109-113: Add missing semicolon to isSolana functionThe isSolana function is missing a semicolon at the end of line 112.
export const isSolana = (chain: Chain) => { return ( chain === Chains.solana || chain === Chains.solana_devnet || chain === Chains.solana_localnet - ) + ); }packages/core/src/lib/garden/sol_btc.spec.ts (2)
16-21: Consider using environment variables for sensitive valuesThe test file contains hardcoded values for private keys and addresses which could accidentally be committed to version control. While this may be acceptable for testing, it's better practice to use environment variables.
-const TEST_PRIVATE_KEY = "0x8fe869193b5010d1ee36e557478b43f2ade908f23cac40f024d4aa1cd1578a61"; -const TEST_RELAYER_ADDRESS = "AKnL4NNf3DGWZJS6cPknBuEGnVsV4A4m5tgebLHaRSZ9"; +const TEST_PRIVATE_KEY = process.env.TEST_PRIVATE_KEY || "0x8fe869193b5010d1ee36e557478b43f2ade908f23cac40f024d4aa1cd1578a61"; +const TEST_RELAYER_ADDRESS = process.env.TEST_RELAYER_ADDRESS || "AKnL4NNf3DGWZJS6cPknBuEGnVsV4A4m5tgebLHaRSZ9";
186-324: Consider extracting common test utilities to a shared fileThere's significant duplication between this file and
sol_wbtc.spec.tsin terms of setup functions, constants, and testing patterns. Consider extracting common utilities to a shared test utility file.You could create a
garden.test.utils.tsfile with shared utilities:// garden.test.utils.ts import { Garden } from './garden'; import * as anchor from "@coral-xyz/anchor"; import { web3 } from '@coral-xyz/anchor'; import { WalletClient } from 'viem'; import { MatchedOrder } from '@gardenfi/orderbook'; import { Environment } from '@gardenfi/utils'; import { BlockNumberFetcher } from "../blockNumberFetcher/blockNumber"; // Shared constants export const TEST_RPC_URL = "http://localhost:8899"; export const TEST_RELAY_URL = new URL("http://localhost:5014/relay"); export const TEST_SWAPPER_RELAYER = "http://localhost:4426"; export const TEST_RELAYER_ADDRESS = process.env.TEST_RELAYER_ADDRESS || "AKnL4NNf3DGWZJS6cPknBuEGnVsV4A4m5tgebLHaRSZ9"; export const TEST_BLOCKFETCHER_URL = "http://localhost:3008"; // Timeout constants export const EXECUTE_TIMEOUT = 90000; export const CREATE_ORDER_TIMEOUT = 30000; // Helper functions export function setupGarden(...) { ... } export function setupGardenListeners(...) { ... } export async function executeWithTimeout(...) { ... } export const fundSolWallet = async (...) => { ... }Then import and use these in your test files:
import { TEST_RPC_URL, setupGarden, executeWithTimeout, fundSolWallet, // ... other imports } from './garden.test.utils';packages/core/src/lib/garden/sol_wbtc.spec.ts (3)
14-20: Consider using environment variables for sensitive valuesSimilar to the
sol_btc.spec.tsfile, this file contains hardcoded private keys and addresses which should be loaded from environment variables instead.-const TEST_PRIVATE_KEY = "0x8fe869193b5010d1ee36e557478b43f2ade908f23cac40f024d4aa1cd1578a61"; -const TEST_RELAYER_ADDRESS = "AKnL4NNf3DGWZJS6cPknBuEGnVsV4A4m5tgebLHaRSZ9"; +const TEST_PRIVATE_KEY = process.env.TEST_PRIVATE_KEY || "0x8fe869193b5010d1ee36e557478b43f2ade908f23cac40f024d4aa1cd1578a61"; +const TEST_RELAYER_ADDRESS = process.env.TEST_RELAYER_ADDRESS || "AKnL4NNf3DGWZJS6cPknBuEGnVsV4A4m5tgebLHaRSZ9";
142-146: Add a comment explaining the use of arbitrumSepolia for wBTC testsThe test is for SOL -> wBTC but uses arbitrumSepolia as the chain, which might be confusing. Consider adding a comment explaining this relationship.
// Setup Arbitrum wallet arbitrumWalletClient = createWalletClient({ account, + // Using arbitrumSepolia for wBTC token interactions in the test environment chain: arbitrumSepolia as Chain, transport: http(), });
114-241: Consider extracting common test utilitiesAs mentioned in the review of
sol_btc.spec.ts, there's significant code duplication between these test files. Consider extracting common utilities, constants, and setup functions to a shared test utility file.Create a shared utility file that can be used by both test files to reduce duplication and make maintenance easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
📒 Files selected for processing (8)
packages/core/src/lib/constants.ts(1 hunks)packages/core/src/lib/garden/sol_btc.spec.ts(1 hunks)packages/core/src/lib/garden/sol_wbtc.spec.ts(1 hunks)packages/core/src/lib/testUtils.ts(0 hunks)packages/orderbook/src/lib/asset.ts(6 hunks)packages/orderbook/src/lib/constants.ts(2 hunks)packages/react-hooks/src/lib/gardenConfig.ts(1 hunks)packages/walletConnectors/src/bitcoin/utils.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/core/src/lib/testUtils.ts
✅ Files skipped from review due to trivial changes (1)
- packages/react-hooks/src/lib/gardenConfig.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/walletConnectors/src/bitcoin/utils.ts
- packages/core/src/lib/constants.ts
🧰 Additional context used
🧬 Code Definitions (1)
packages/orderbook/src/lib/asset.ts (1)
packages/orderbook/src/index.ts (7) (7)
Chains(5:5)Chain(3:3)TimeLocks(12:12)getBlockchainType(10:10)isBitcoin(8:8)BlockchainType(6:6)isEVM(9:9)
🔇 Additional comments (12)
packages/orderbook/src/lib/asset.ts (6)
19-19: Addition of Solana blockchain type is correctThe addition of Solana to the BlockchainType enum is appropriate for supporting Solana blockchain in the codebase, consistent with the PR objective of adding Solana support.
51-53: Solana chains added correctlyThe addition of the Solana chains (mainnet, devnet, and localnet) follows the established pattern for other chains in the system.
64-64: Appropriate update to EvmChain typeCorrectly updated the EvmChain type to exclude the newly added Solana chains, ensuring type safety when working with EVM-specific functionality.
76-77: Non-mainnet Solana chains properly identifiedThe isMainnet function has been appropriately updated to identify Solana devnet and localnet as non-mainnet environments.
131-133: Timelock values for Solana chains with helpful commentThe addition of timelock values for Solana chains is correct, and the comment explaining the calculation (based on Solana's 0.4s time slots) provides valuable context for future developers.
139-139: getBlockchainType updated to use the new isSolana functionThe getBlockchainType function correctly uses the newly added isSolana function to identify and return the appropriate blockchain type for Solana chains.
packages/orderbook/src/lib/constants.ts (5)
35-42: SOLSolanaLocalnetAsset definition looks goodThe definition of the Solana localnet asset follows the established pattern for asset definitions, with appropriate values for name, decimals, symbol, chain, and placeholder values for addresses.
56-57: Updated atomic swap address with helpful commentThe update to the WBTCArbitrumLocalnetAsset's atomic swap address includes a comment that clarifies the change, which is helpful for understanding why the address was modified.
65-66: Updated atomic swap address with helpful commentSimilarly, the update to the WBTCEthereumLocalnetAsset's atomic swap address includes a comment that clarifies the change, enhancing code readability and maintainability.
72-72: Solana localnet asset added to SupportedAssetsThe SOLSolanaLocalnetAsset has been correctly added to the localnet section of SupportedAssets, making it available for use in localnet environments.
75-75: Bitcoin regtest asset added to localnet sectionThe bitcoin_regtest_BTC asset is appropriately added to the localnet section, addressing previous concerns about mixing regtest assets in testnet configurations.
packages/core/src/lib/garden/sol_btc.spec.ts (1)
45-45: Use environment variables instead of a hardcoded API keyHardcoding sensitive tokens is risky even in test environments, as they can accidentally leak into version control or logs.
- apiKey: "AAAAAGghjwU6Os1DVFgmUXj0GcNt5jTJPbBmXKw7xRARW-qivNy4nfpKVgMNebmmxig2o3v-6M4l_ZmCgLp3vKywfVXDYBcL3M4c", + apiKey: process.env.TEST_API_KEY || "test_api_key"
| // 2. Initiate swap | ||
| const initResult = await garden.evmRelay.init(ethereumWalletClient, order); | ||
| expect(initResult.ok).toBeTruthy(); | ||
| console.log('Order initiated ✅'); |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Add a comment explaining why evmRelay.init is needed only for wBTC->SOL flow
The wBTC->SOL test has an explicit evmRelay.init() call that isn't present in the SOL->wBTC test. Consider adding a comment explaining why this step is necessary only for this direction of the swap.
🏁 Script executed:
#!/bin/bash
# Check the Garden class to understand why evmRelay.init is needed for wBTC->SOL
rg -A 10 "evmRelay" ./packages/core/src/lib/garden/garden.tsLength of output: 2311
Action: Add a clarifying inline comment above the explicit evmRelay.init() call
When reviewing the wBTC→SOL test, it’s important to note that the EVM side of the swap workflow must be explicitly initialized before proceeding. This manual initialization (via evmRelay.init()) is required because—in contrast to the SOL→wBTC flow—the EVM relay needs to be prepped (e.g., establishing swap-specific context or performing necessary validations) for the wBTC→SOL swap. In the SOL→wBTC scenario, the swap initiation logic is handled internally by the Solana relay, so no additional initialization is needed on the EVM side.
Please add a comment along these lines at the corresponding lines (232–235) in packages/core/src/lib/garden/sol_wbtc.spec.ts so that future readers understand the rationale behind this design decision.
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (1)
packages/test/src/config/index.tsx (1)
13-23: 💡 Verification agent🧩 Analysis chain
Potential naming conflict with existing config
There appears to be another
configobject defined inpackages/test/src/main.tsx. Having two different configurations with the same name could lead to confusion or errors.
🏁 Script executed:
#!/bin/bash # Check how these config objects are used in the codebase echo "Checking for imports of config from both files:" rg -A 2 "import .* from '[^']*main'" | grep -i config rg -A 2 "import .* from '[^']*config'" | grep -i config echo "Checking if both configs are used in the same files:" rg "from '[^']*main'" --files-with-matches | xargs rg "from '[^']*config'" --files-with-matches || echo "No files import from both locations"Length of output: 693
Naming conflict detected between config objects
It appears that two distinctconfigobjects exist—one exported frompackages/test/src/config/index.tsx(aswagmiAdapter.wagmiConfig) and another defined inpackages/test/src/main.tsx. Our verification shows that files such asvitest.workspace.jsandpackages/test/src/providers/Providers.tsxare importing from modules that resolve to these configurations, which could lead to confusion or errors when both are used in the same context. Please consider renaming one of the objects (e.g., toadapterConfigorappConfig) to clearly differentiate their roles.
♻️ Duplicate comments (2)
packages/core/src/lib/garden/sol_btc.spec.ts (1)
57-58:⚠️ Potential issueStore sensitive credentials in environment variables.
Embedding an API key directly in the code risks accidental exposure. Move the API key to a secure environment variable, e.g.,
process.env.RELAYER_API_KEY, to reduce security risks:- apiKey: "AAAAAGmzpY46OpuOQc2zg4nnkFPt2mzzRVb2eifR9syXRB6TxiWOSdTnHlNGrYw80CWPSz993TSMVzJSo7uppDtY51zOR3rvuUcc", + apiKey: process.env.RELAYER_API_KEY || "test_api_key_here",packages/core/src/lib/garden/garden.ts (1)
837-843:⚠️ Potential issueDestination block number for Solana is using the source wallet.
This issue was flagged in a past review comment and remains unresolved.Use
wallets.destinationfor Solana’s destination block number to avoid incorrect or stale values. For example:- destinationBlockNumber = await solanaBlockNumber(wallets.source as AnchorProvider); + destinationBlockNumber = await solanaBlockNumber(wallets.destination as AnchorProvider);
🧹 Nitpick comments (69)
packages/test/src/components/SwapComponent/SwapComponent.tsx (1)
8-12: Address Unused Solana Imports
The newly added imports from@reown/appkit/reactand@reown/appkit-adapter-solana/react(useAppKitAccount,useAppKitProvider,Provider,useAppKitConnection) are currently not used within the component. If these are placeholders for future Solana support, please add a clarifying comment or a TODO note explaining their intended usage. Otherwise, consider removing them to clean up the code and avoid ESLint warnings.🧰 Tools
🪛 ESLint
[error] 8-8: 'useAppKitAccount' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'useAppKitProvider' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'Provider' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'useAppKitConnection' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/test/.gitignore (1)
1-2: Header Comment Clarity
The header reference to GitHub's articles is clear; however, given the project's recent transition to a Next.js framework (which affects file generation), consider adding a brief note here to contextualize the adjustments made to this ignore list.packages/test/src/app/layout.tsx (3)
1-9: Use consistent import pathsYou're using an absolute import path for SolanaWalletProvider but a relative path for Providers. For better maintainability, consider using consistent import paths throughout the codebase.
-import SolanaWalletProvider from '@/providers/SolanaWalletProvider'; -import Providers from '../providers/Providers'; +import SolanaWalletProvider from '@/providers/SolanaWalletProvider'; +import Providers from '@/providers/Providers';
5-19: Add metadata to the layout componentThe layout component is missing metadata like title, description, which is a common practice in Next.js App Router for SEO optimization.
export default async function RootLayout({ children, }: { children: React.ReactNode; }) { return ( <html lang="en"> + <head> + <title>Garden Solana Swaps</title> + <meta name="description" content="Garden Solana atomic swaps interface" /> + </head> <body> <Providers> <SolanaWalletProvider>{children}</SolanaWalletProvider> </Providers> </body> </html> ); }
5-19: Add explicit React importWhile Next.js automatically imports React, it's a good practice to explicitly import it when using React types like React.ReactNode.
+import React from 'react'; export default async function RootLayout({ children, }: { children: React.ReactNode; }) {packages/test/src/common/AssetChainLogos.tsx (2)
4-7: Consider making chainLogo optionalSince your component already handles conditional rendering when chainLogo is absent, the type definition should reflect this by making chainLogo optional.
type AssetChainLogosProps = HTMLAttributes<HTMLDivElement> & { tokenLogo: string; - chainLogo: string; + chainLogo?: string; };
21-27: Improve accessibility with dynamic alt textThe current alt text is generic. Consider accepting alt text as props to provide more descriptive alternatives for screen readers.
type AssetChainLogosProps = HTMLAttributes<HTMLDivElement> & { tokenLogo: string; chainLogo: string; + tokenAlt?: string; + chainAlt?: string; }; export const AssetChainLogos: FC<AssetChainLogosProps> = ({ tokenLogo, chainLogo, + tokenAlt = "Token Logo", + chainAlt = "Chain Logo", ...rest }) => { // ... <Image src={tokenLogo} - alt="Token Logo" + alt={tokenAlt} // ... /> {chainLogo && ( <Image src={chainLogo} - alt="Chain Logo" + alt={chainAlt} // ... /> )}packages/test/src/common/SwapInfo.tsx (2)
31-33: Format currency amounts for better readabilityThe component displays raw amount values without formatting, which could be difficult to read for large numbers or values with many decimal places.
Consider using a formatter function:
+const formatAmount = (amount: string | number) => { + if (typeof amount === 'number') { + return amount.toLocaleString(undefined, { + maximumFractionDigits: 8 + }); + } + return amount; +}; // ... <p> - {sendAmount} + {formatAmount(sendAmount)} </p> // ... <p> - {receiveAmount} + {formatAmount(receiveAmount)} </p>Also applies to: 41-43
39-39: Add proper color contrast for the arrow iconThe ArrowRightIcon is using
fill-whitewhich might have insufficient contrast on light backgrounds.- <ArrowRightIcon className="fill-white"/> + <ArrowRightIcon className="fill-white dark:fill-white fill-gray-700"/>packages/test/src/providers/GardenProviderWrapper.tsx (3)
7-19: Memoize the mock storage objectCreating a new mock storage object on every render when on the server is inefficient. Consider memoizing it.
+const mockStorage: Storage = { + getItem: () => null, + setItem: () => {}, + removeItem: () => {}, + clear: () => {}, + length: 0, + key: () => null, +}; const getStorage = (): Storage => { if (typeof window !== "undefined") { return localStorage; } - return { - getItem: () => null, - setItem: () => {}, - removeItem: () => {}, - clear: () => {}, - length: 0, - key: () => null, - }; + return mockStorage; };
21-35: Make environment configurable and add error handlingThe environment is hardcoded to TESTNET and there's no error handling for the wallet client.
+import { useState, useEffect } from "react"; +interface GardenProviderWrapperProps { + children: React.ReactNode; + environment?: Environment; +} -function GardenProviderWrapper({ children }: { children: React.ReactNode }) { +function GardenProviderWrapper({ + children, + environment = Environment.TESTNET +}: GardenProviderWrapperProps) { const { data: walletClient } = useWalletClient(); + const [isClientReady, setIsClientReady] = useState(false); + + useEffect(() => { + setIsClientReady(true); + }, []); + + if (!isClientReady) { + return null; // Or a loading spinner + } return ( <GardenProvider config={{ store: getStorage(), - environment: Environment.TESTNET, + environment, walletClient: walletClient, }} >
22-22: Fix code formattingThere's an unnecessary blank line after the useWalletClient hook.
function GardenProviderWrapper({ children }: { children: React.ReactNode }) { const { data: walletClient } = useWalletClient(); - return (packages/test/src/store/blockNumberStore.ts (1)
7-7: Consider validating BASE_URL before useThe
BASE_URLis used directly in the API call without checking if it's defined. If the environment variable is not set, this could lead to an error.-const BASE_URL = process.env.NEXT_PUBLIC_DATA_URL; +const BASE_URL = process.env.NEXT_PUBLIC_DATA_URL || ''; + +if (!BASE_URL) { + console.warn('NEXT_PUBLIC_DATA_URL is not defined. API calls may fail.'); +}packages/test/src/components/swap/InputField.tsx (1)
28-36: Enhance accessibility and input type handlingThe input field lacks some accessibility attributes and doesn't specify input types for different use cases. Consider enhancing the component with ARIA attributes and supporting different input types.
<input id={id} placeholder={placeholder} value={value || ""} - type="text" + type={type || "text"} readOnly={readOnly} onChange={(e) => onChange?.(e.target.value)} - className={`w-full rounded-md bg-gray-700 text-white outline-none`} + className={`w-full rounded-md bg-gray-700 text-white outline-none ${readOnly ? 'cursor-not-allowed opacity-70' : ''}`} + aria-invalid={!!error} + aria-describedby={error ? `${id}-error` : undefined} /> {error && ( - <p className="text-sm text-red-500">{error}</p> + <p id={`${id}-error`} className="text-sm text-red-500" role="alert">{error}</p> )}Also, update the props interface to include the type property:
type InputFieldProps = { id: string; label: string; value: string | null; readOnly?: boolean; error?: string | null; onChange?: (value: string) => void; placeholder?: string; + type?: 'text' | 'email' | 'password' | 'number'; };packages/test/src/config/index.tsx (1)
1-1: File extension could be .ts instead of .tsxThis file doesn't contain any JSX code, so it could use the
.tsextension instead of.tsx.packages/test/src/providers/Providers.tsx (2)
6-14: Remove unused importsSeveral imported networks are not used in this file:
- mainnet
- arbitrum
- sepolia
- solana
- solanaTestnet
- solanaDevnet
Remove these unused imports to improve code clarity and avoid unnecessary imports.
import { - mainnet, - arbitrum, - sepolia, - solana, - solanaTestnet, - solanaDevnet, defineChain, } from '@reown/appkit/networks';🧰 Tools
🪛 ESLint
[error] 7-7: 'mainnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'arbitrum' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 9-9: 'sepolia' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'solana' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'solanaTestnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: 'solanaDevnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
54-56: Consider more graceful error handling for missing projectIdThrowing an error directly if projectId is not defined might cause application crashes. Consider implementing a more graceful fallback or showing a more user-friendly error message.
if (!projectId) { - throw new Error('Project ID is not defined'); + console.error('Project ID is not defined. Some features may not work correctly.'); + // Consider alternative approaches like redirecting to an error page or using a default value }packages/test/src/providers/SolanaWalletProvider.tsx (3)
4-11: Remove unused importsTwo imports are declared but never used:
clusterApiUrlfrom '@solana/web3.js'UnsafeBurnerWalletAdapterfrom '@solana/wallet-adapter-wallets'Remove these unused imports to improve code clarity.
import { ConnectionProvider, WalletProvider, } from '@solana/wallet-adapter-react'; import { WalletAdapterNetwork } from '@solana/wallet-adapter-base'; import { WalletModalProvider } from '@solana/wallet-adapter-react-ui'; - import { clusterApiUrl } from '@solana/web3.js'; - import { UnsafeBurnerWalletAdapter } from '@solana/wallet-adapter-wallets';🧰 Tools
🪛 ESLint
[error] 10-10: 'clusterApiUrl' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'UnsafeBurnerWalletAdapter' is defined but never used.
(@typescript-eslint/no-unused-vars)
16-16: Remove redundant commentThe comment "imports here" appears to be a leftover note and doesn't provide any value.
- // imports here
27-33: The wallets array is empty and dependency array includes unused variableThe wallets array is empty with just a commented out wallet adapter. Additionally, the dependency array includes
networkwhich isn't used in the calculation.const wallets = useMemo( () => [ // manually add any legacy wallet adapters here // new UnsafeBurnerWalletAdapter(), ], - [network], + [], // No dependencies since network isn't used in the calculation );Consider adding actual wallet adapters if they're needed, or document why they're intentionally left empty.
packages/test/tsconfig.json (1)
2-25: Good TypeScript configuration setup for Next.jsThe TypeScript configuration is well-structured for a Next.js project with appropriate settings:
- Modern JavaScript target (ES2017)
- Necessary DOM and ESNext libraries
- Strict type checking enabled
- Next.js plugin configured
- Path aliases for easier imports
One small improvement to consider:
"strict": true, "noEmit": true, - "noImplicitAny": false, + "noImplicitAny": true, "esModuleInterop": true,Setting
noImplicitAnyto true would improve type safety by requiring explicit types or type inference for all variables.packages/test/src/utils/getLatestUpdatedOrder.ts (2)
15-27: Add validation for the default case in getLatestUpdatedOrderThe current implementation returns
newOrderwhen either priority is not found. This might not always be the desired behavior and could lead to unexpected results.export const getLatestUpdatedOrder = ( newOrder: OrderWithStatus, oldOrder: OrderWithStatus ) => { const newOrderPriority = priorityList[newOrder.status]; const oldOrderPriority = priorityList[oldOrder.status]; - if (!newOrderPriority || !oldOrderPriority) { + if (!newOrderPriority && !oldOrderPriority) { + // Neither order has a known priority, default to newOrder return newOrder; + } else if (!newOrderPriority) { + // Only old order has a known priority + return oldOrder; + } else if (!oldOrderPriority) { + // Only new order has a known priority + return newOrder; } return newOrderPriority >= oldOrderPriority ? newOrder : oldOrder; };
29-41: Add null/undefined check and handle multiple matching ordersThe current implementation doesn't handle null/undefined inputs and doesn't address the case where multiple old orders might have the same create_id.
export const getLatestUpdatedOrders = ( newOrders: OrderWithStatus[], oldOrders: OrderWithStatus[] ) => { + if (!newOrders || !oldOrders) { + return newOrders || []; + } return newOrders.map((newOrder) => getLatestUpdatedOrder( newOrder, - oldOrders.find( - (o) => o.create_order.create_id === newOrder.create_order.create_id - ) ?? newOrder + oldOrders + .filter((o) => o.create_order.create_id === newOrder.create_order.create_id) + .reduce((latestOrder, currentOrder) => + getLatestUpdatedOrder(currentOrder, latestOrder), + newOrder + ) ) ); };This implementation first filters all matching old orders, then reduces them to find the one with the highest priority.
packages/test/src/utils/solana/connectSolanaWallet.ts (2)
1-5: Consider assigning string values to each enum member.Currently, the enum members rely on their numeric indices for identification. It might be clearer to assign them string values, ensuring each wallet can be referenced correctly (e.g.,
phantom = "phantom") and avoiding confusion when debugging.enum SolWallets { - phantom, - solflare, - backpack + phantom = "phantom", + solflare = "solflare", + backpack = "backpack" }
23-33: Implement or remove placeholder connection methods.These functions are currently empty shells. For maintainability, either implement the logic for connecting to each wallet or remove them if they're not yet ready for production.
packages/core/src/lib/solana/relayer/solanaRelay.ts (2)
59-80: Handle potential signing or wallet errors ininit()method.If
this.provider.wallet.signTransaction(tx)fails (e.g., the wallet is locked or absent), the current implementation will throw unhandled errors. Consider wrapping this call in a try-catch block or verifying the wallet state beforehand.try { await this.provider.wallet.signTransaction(tx); } catch (err) { // gracefully handle wallet errors here throw new Error("Unable to sign transaction: " + err.message); }
87-107: Reuse common transaction logic betweeninit()andredeem()methods.Both methods set up, sign (in
init), and post transactions to the relayer. Extracting shared logic (e.g., building the transaction, encoding, posting) into a helper function may reduce duplication and make future iterations easier.packages/core/src/lib/solana/idl/solana_native_swaps.ts (1)
372-412: Check consistency between 'expiresIn' and 'expirySlot'.Within this IDL, the
initiateinstruction uses aexpiresInargument of typeu32, while theswapAccountstoresexpirySlotas au64. If these two fields represent the same concept, consider aligning them to a single numeric type for clarity.packages/test/src/utils/utils.ts (2)
7-7: Use optional chaining for readability.Instead of using a logical AND to check
assets, apply optional chaining to safely access the asset property.Example fix:
-return assets && assets[`${swap.chain}_${swap.asset.toLowerCase()}`]; +return assets?.[`${swap.chain}_${swap.asset.toLowerCase()}`];🧰 Tools
🪛 Biome (1.9.4)
[error] 7-7: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
27-37: Proceed with caution when converting to native Number.Using
Number(...)aftertoFixed(...)may suffice here but can discard precision. If the application requires exact fractional amounts beyondtoFixed, consider returning the string or usingBigNumberdirectly.packages/test/src/store/swapStore.ts (2)
5-28: Potential improvement for readability.Holding multiple booleans for swap directions can become error-prone at scale. Consider using a single enumerated state or a union type. This would simplify toggling logic and ensure only valid states are represented.
58-139: Readability improvements in toggleSwapDirection.The toggle logic is logically grouped but repetitive. Consider extracting a helper function or table-driven approach to reduce the repeated boolean resets.
- // Repetitive toggles for different pairs ... + // Example approach: + // 1. Map current booleans to a mode. + // 2. Flip the mode. + // 3. Assign fromAsset/toAsset from the new mode.packages/test/src/components/transactions/Transactions.tsx (3)
32-40: Consider returning a default status instead ofundefined.When
parseStatusreturnsundefineddue to missing block numbers, downstream components might attempt to handle an undefined status. Returning a safe fallback (e.g., a placeholder status) could reduce potentialundefinedchecks later.
42-69: Add error handling to the fetch logic.In the
useEffect, any error thrown byfetchAndSetBlockNumbers()orfetchAndSetOrders()will bypass settingisLoadingOrdersto false. Consider adding acatchblock to ensure loading states are always cleared even on failure, and to display errors to users if needed.
80-80: Use a stable unique key for mapping orders instead of the array index.Using
indexaskeymay produce unexpected behaviors if the order list changes. If possible, use a unique property from eachorder, such asorder.create_order.create_id.packages/test/src/components/swap/TokenSwap.tsx (4)
8-8: Remove the unusedBigNumberimport.Static analysis tools report that
BigNumberis imported but never used. To keep the codebase clean and avoid confusion, remove it if it's no longer needed.- import BigNumber from 'bignumber.js';🧰 Tools
🪛 ESLint
[error] 8-8: 'BigNumber' is defined but never used.
(@typescript-eslint/no-unused-vars)
33-47: Remove or utilize the unused variables and imports.
TEST_RPC_URLis assigned but never used (line 33).publicKeyfromuseWallet()is never used (line 47).Unused variables can cause confusion, so consider removing or making use of them if they're still relevant.
🧰 Tools
🪛 ESLint
[error] 33-33: 'TEST_RPC_URL' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 47-47: 'publicKey' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
126-135: Useconstinstead ofletfor variables that are never reassigned.
bitcoinProvider(line 126)btcWalletx(line 130)BTC_ADDRESS(line 131)Switching to
consthelps prevent accidental reassignments and aligns with best practices.- let bitcoinProvider = new BitcoinProvider( + const bitcoinProvider = new BitcoinProvider(🧰 Tools
🪛 ESLint
[error] 126-126: 'bitcoinProvider' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 130-130: 'btcWalletx' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 131-131: 'BTC_ADDRESS' is never reassigned. Use 'const' instead.
(prefer-const)
160-271: Remove or refactor the unusedswapSOLTowBTCfunction.Currently,
swapSOLTowBTCis declared at line 160 but never invoked. If no future usage is planned, remove it. Otherwise, ensure it’s called or integrated into the code.- const swapSOLTowBTC = async () => { - // ... - };🧰 Tools
🪛 ESLint
[error] 160-160: 'swapSOLTowBTC' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/store/ordersStore.ts (1)
60-70: Add error tracking forfetchOrders.In case
orderBook.fetchOrders()fails for any reason besides the HTTP error (e.g., network outage, JSON parse error), ensure that you capture/log unexpected exceptions as well. Currently, errors are set in state ifres.okis false, but runtime exceptions could be missed.packages/core/src/lib/solana/relayer/solana_relayer.spec.ts (4)
1-5: Document the test with more complete JSDoc comments.The file starts with a comment block that briefly explains the purpose of the tests, but it would be more helpful with a more structured format that includes @description, @author, and other relevant tags.
/** - * We'll be testing if the relayer is able to - * 1. Initate a transaction - * 2. Redeem a transaction + * @description Test suite for the SolanaRelay class + * Verifies that the relayer can: + * 1. Initiate a transaction + * 2. Redeem a transaction + * @see SolanaRelay */
53-53: Add a longer timeout for the redemption test.The first test has a 120000ms timeout but the second test doesn't specify any timeout. Since blockchain operations can be slow, especially on test networks, it's a good practice to set appropriate timeouts for all tests.
it("Relayer should redeem the funds as per the secret hash provided", async () => { console.log("Redeeming the transaction"); const res = await userRelay.redeem(secretHash); expect(res).toBeDefined(); console.log("Relayer redeemed successfully!"); -}) +}, 120000);
30-47: Add more assertions to verify successful outcomes.The test only verifies that the transaction signatures are defined, but doesn't check if the transactions were actually successful. Consider adding more assertions to validate the state changes.
You could add checks to verify:
- The on-chain state after initiation (e.g., balance changes)
- The outcome after redemption
Example addition:
// After initiation const swapAccountInfo = await connection.getAccountInfo(swapAccount); expect(swapAccountInfo).toBeDefined(); expect(swapAccountInfo?.lamports).toBeGreaterThan(0); // After redemption const redeemerBalanceAfter = await connection.getBalance(new web3.PublicKey("Cw2NgGzGfdhPvWyTHitxZmgB4sRUJCXr8cUfdFNMheTN")); expect(redeemerBalanceAfter).toBeGreaterThan(redeemerBalanceBefore);
38-41: Use constants or test helpers for test values.The values for amount and expiresIn should be extracted as constants or test helper functions to make it easier to adjust tests in the future.
+const TEST_AMOUNT = BigInt(web3.LAMPORTS_PER_SOL * 0.00001); +const TEST_EXPIRY = 100; const randomSwapId = () => hex.encode(crypto.randomBytes(32)); const secret = crypto.randomBytes(32); secretHash = hex.encode(crypto.createHash("sha256").update(secret).digest()); -const amount = BigInt(web3.LAMPORTS_PER_SOL * 0.00001); -const expiresIn = 100; +const amount = TEST_AMOUNT; +const expiresIn = TEST_EXPIRY;packages/core/src/lib/solana/solana.spec.ts (6)
1-7: Improve test documentation with detailed JSDoc comments.The comment block provides a good overview, but could be enhanced with more structured JSDoc including expected outcomes and prerequisites.
/** + * @description Integration test for Solana atomic swap functionality. * This testcase performs a SOL - SOL atomic swap between "User" and "Filler". * The "User" utilizes (thus tests) the @see SolanaRelay class, whereas * the "Filler" utilizes (thus tests) the @see SolanaHTLC class * The orderId is random as the on-chain program just accepts it * as-is during init() and doesn't care about its value, + * @prerequisites Requires a running relayer service at the specified endpoint + * @see SolanaRelay + * @see SolanaHTLC */
19-19: Ensure URL protocol consistency.The relay URL is using HTTP rather than HTTPS which can be insecure for production scenarios.
-const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay"); +const TEST_RELAY_URL = new URL("https://solana-relayer-staging.hashira.io/relay");
53-65: Consider enabling airdrop functionality for robust testing.The airdrop functionality is commented out, which might cause tests to fail if the accounts don't have sufficient funds.
Either:
- Enable the airdrop code for local development
- Use mock providers for testing
- Add clear documentation about prerequisites (funded accounts)
If the intention is to keep the airdrop disabled in CI but enable it for local testing, consider:
const ENABLE_AIRDROP = process.env.ENABLE_AIRDROP === "true"; if (ENABLE_AIRDROP) { beforeAll(async () => { // Existing airdrop code }); }
48-51: Add error handling for initialization.The initialization of SwapConfig and SolanaRelay could throw exceptions if the inputs aren't valid, but there's no error handling here.
-const userSwapConfig = new SwapConfig(randomSwapId(), filler.publicKey.toBase58(), secretHash, amount, expiresIn); -const userRelay = new SolanaRelay(userSwapConfig, userProvider, TEST_RELAY_URL, "8jiuEDT8T4Eqd38hiXRHJxRMvMkBWpEPVM3uuAn6bj93"); +try { + const userSwapConfig = new SwapConfig(randomSwapId(), filler.publicKey.toBase58(), secretHash, amount, expiresIn); + const userRelay = new SolanaRelay(userSwapConfig, userProvider, TEST_RELAY_URL, "8jiuEDT8T4Eqd38hiXRHJxRMvMkBWpEPVM3uuAn6bj93"); +} catch (error) { + console.error("Error initializing swap configuration:", error); + throw error; +}
67-90: Add test isolation and cleanup.The test initiates real on-chain transactions but doesn't include any cleanup or isolation between test runs, which could lead to test failures or side effects when run multiple times.
Consider adding:
- Unique identifiers for each test run
- Proper cleanup in an
afterAllhook- Skip logic if prerequisites aren't met
For example:
afterAll(async () => { // Clean up resources or reset state if needed console.log("Test cleanup completed"); }); // Add skip logic if relayer isn't available it.runIf(process.env.TEST_WITH_RELAYER === "true")("Testing SOL - SOL atomic swap flow", async () => { // Existing test code });
81-83: Add additional validation for successful redemption.The test only verifies that a transaction signature is returned but doesn't validate that the funds were actually transferred successfully.
console.log("User redeems via the relayer..."); const userRedeemSig = await userRelay.redeem(hex.encode(secret)); expect(userRedeemSig).toBeDefined(); +// Verify the transaction was confirmed +const txStatus = await connection.getSignatureStatus(userRedeemSig); +expect(txStatus?.value?.confirmationStatus).toBe("confirmed"); console.log("User successfully redeemed with transaction:", userRedeemSig);packages/core/src/lib/solana/idl/solana_native_swaps.json (3)
1-8: Document program specification and contract behavior.While the IDL provides technical details of the contract structure, it lacks higher-level documentation explaining the overall contract behavior, design decisions, and security considerations.
Consider adding additional documentation in a separate README.md file or enhanced comments that explain:
- The swap protocol flow and expected behavior
- Security considerations and mitigation strategies
- Fee structures and edge cases
- Design decisions and contract invariants
148-169: Enhance error handling with more specific error codes.The error codes are minimal and could be expanded to provide more granular error information for different failure scenarios.
Consider adding more specific error codes for:
- Insufficient funds
- Already redeemed/refunded swaps
- Invalid time parameters
- Other edge cases that might occur
For example:
{ "code": 6004, "name": "InsufficientFunds", "msg": "The initiator's balance is insufficient for the requested swap amount" }, { "code": 6005, "name": "AlreadyRedeemed", "msg": "This swap has already been redeemed and cannot be redeemed again" }
124-129: Expand account structure to include more fields.The
SwapAccountdefinition in the accounts section is minimal and missing the detailed fields that are defined in the types section.Consider enhancing the account section to fully document the account structure directly:
{ "name": "SwapAccount", - "discriminator": [53, 126, 9, 14, 14, 197, 105, 182] + "discriminator": [53, 126, 9, 14, 14, 197, 105, 182], + "fields": [ + {"name": "swap_id", "type": {"array": ["u8", 32]}}, + {"name": "initiator", "type": "pubkey"}, + {"name": "redeemer", "type": "pubkey"}, + {"name": "secret_hash", "type": {"array": ["u8", 32]}}, + {"name": "expiry_slot", "type": "u64"}, + {"name": "amount", "type": "u64"} + ] }packages/test/src/components/transactions/TransactionRow.tsx (6)
29-52: Make order status handling more robust.The
getOrderStatusLabelfunction doesn't handle all possibleOrderStatusvalues comprehensively, and could result in unexpected behavior for unhandled statuses.Ensure all possible enum values are handled explicitly and add a fallback that logs unexpected values:
const getOrderStatusLabel = (status: OrderStatus) => { switch (status) { case OrderStatus.Matched: return StatusLabel.ShouldInitiate; case OrderStatus.DeadLineExceeded: return StatusLabel.Expired; case OrderStatus.InitiateDetected: return StatusLabel.InitiateDetected; case OrderStatus.Initiated: return StatusLabel.Initiated; case OrderStatus.CounterPartyInitiateDetected: case OrderStatus.CounterPartyInitiated: return StatusLabel.Redeeming; case OrderStatus.Redeemed: case OrderStatus.RedeemDetected: case OrderStatus.Refunded: case OrderStatus.RefundDetected: case OrderStatus.CounterPartyRedeemed: case OrderStatus.CounterPartyRedeemDetected: return StatusLabel.Completed; default: + console.warn(`Unhandled OrderStatus: ${status}`); return StatusLabel.Pending; } };Also consider adding a test to ensure all enum values are handled:
// In a test file test('getOrderStatusLabel handles all OrderStatus values', () => { Object.values(OrderStatus).forEach(status => { expect(getOrderStatusLabel(status)).toBeDefined(); }); });
92-99: Add error handling for the transaction click handler.The
handleTransactionClickfunction doesn't have any error handling for the window.open operation, which could fail in certain environments.const handleTransactionClick = () => { if (order?.create_order?.create_id) { - window.open( - `https://gardenexplorer.hashira.io/order/${create_order.create_id}`, - "_blank" - ); + try { + const newWindow = window.open( + `https://gardenexplorer.hashira.io/order/${create_order.create_id}`, + "_blank" + ); + + // Check if window was blocked by popup blocker + if (!newWindow || newWindow.closed || typeof newWindow.closed === 'undefined') { + console.warn("Popup window was blocked. Please check your browser settings."); + } + } catch (error) { + console.error("Failed to open transaction details:", error); + } } };
94-96: Use environment variables for explorer URL.The explorer URL is hardcoded, which could make it difficult to support different environments (development, staging, production).
+// At the top of the file, import from configuration +import { EXPLORER_URL } from "../../config/constants"; const handleTransactionClick = () => { if (order?.create_order?.create_id) { window.open( - `https://gardenexplorer.hashira.io/order/${create_order.create_id}`, + `${EXPLORER_URL}/order/${create_order.create_id}`, "_blank" ); } };
103-107: Improve accessibility.The component lacks appropriate ARIA attributes for accessibility, and the clickable div could be made more accessible by using semantic HTML.
return ( <div + role="button" + tabIndex={0} + aria-label={`Transaction details for ${sendAsset.symbol} to ${receiveAsset.symbol}`} className={`flex flex-col gap-1 p-4 ${isFirst ? "rounded-t-2xl" : ""} ${isLast ? "rounded-b-2xl" : ""} ${"cursor-pointer hover:bg-gray-900" }`} onClick={handleTransactionClick} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + handleTransactionClick(); + } + }} >
115-123: Add semantic styling to status labels.The status labels are displayed without any visual indication of their meaning, which could make it difficult for users to quickly understand the transaction status.
Consider adding color coding or icons to make the status more visually apparent:
<div className="flex justify-between"> - <p> + <p className={` + ${statusLabel === StatusLabel.Completed ? 'text-green-500' : ''} + ${statusLabel === StatusLabel.Pending ? 'text-yellow-500' : ''} + ${statusLabel === StatusLabel.Expired ? 'text-red-500' : ''} + ${statusLabel === StatusLabel.ShouldInitiate ? 'text-blue-500' : ''} + flex items-center gap-1 + `}> + {statusLabel === StatusLabel.Completed && <CheckCircleIcon className="w-4 h-4" />} + {statusLabel === StatusLabel.Pending && <ClockIcon className="w-4 h-4" />} + {statusLabel === StatusLabel.Expired && <ExclamationCircleIcon className="w-4 h-4" />} {statusLabel} </p> <p> {dayDifference} </p> </div>
63-90: Optimize memoization by combining related calculations.The component uses multiple separate
useMemocalls for related calculations, which could be combined to reduce unnecessary recalculations.-const sendAsset = useMemo( - () => getAssetFromSwap(source_swap, assets), - [source_swap, assets] -); -const receiveAsset = useMemo( - () => getAssetFromSwap(destination_swap, assets), - [destination_swap, assets] -); -const statusLabel = useMemo( - () => status && getOrderStatusLabel(status), - [status] -); -const sendAmount = useMemo( - () => formatAmount(create_order.source_amount, sendAsset?.decimals ?? 0), - [create_order.source_amount, sendAsset?.decimals] -); -const receiveAmount = useMemo( - () => - formatAmount( - create_order.destination_amount, - receiveAsset?.decimals ?? 0 - ), - [create_order.destination_amount, receiveAsset?.decimals] -); -const dayDifference = useMemo( - () => getDayDifference(create_order.updated_at), - [create_order.updated_at] -); +// Combine related calculations in a single useMemo +const { + sendAsset, + receiveAsset, + sendAmount, + receiveAmount +} = useMemo(() => { + const sendAsset = getAssetFromSwap(source_swap, assets); + const receiveAsset = getAssetFromSwap(destination_swap, assets); + + return { + sendAsset, + receiveAsset, + sendAmount: sendAsset ? formatAmount(create_order.source_amount, sendAsset.decimals ?? 0) : 0, + receiveAmount: receiveAsset ? formatAmount(create_order.destination_amount, receiveAsset.decimals ?? 0) : 0 + }; +}, [source_swap, destination_swap, assets, create_order.source_amount, create_order.destination_amount]); + +const statusLabel = useMemo( + () => status && getOrderStatusLabel(status), + [status] +); + +const dayDifference = useMemo( + () => getDayDifference(create_order.updated_at), + [create_order.updated_at] +);packages/test/src/components/swap/TokenSwapBak.tsx (3)
14-14: Unused import
anchoris imported but never referenced. Removing it will help keep the code clean.- import * as anchor from '@coral-xyz/anchor';🧰 Tools
🪛 ESLint
[error] 14-14: 'anchor' is defined but never used.
(@typescript-eslint/no-unused-vars)
18-18: Unused variable
walletConnectedis assigned but never used. Remove or use it to avoid confusion.- const { isConnected: walletConnected, address } = useAppKitAccount(); + const { address } = useAppKitAccount();🧰 Tools
🪛 ESLint
[error] 18-18: 'walletConnected' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
283-295: Inline debug usage
The button currently logs connection data instead of initiating the swap. Combining debug logs with the actual swap flow can lead to confusion. Consider removing or gating debug logs behind a debug mode.packages/core/src/lib/garden/sol_btc.spec.ts (2)
17-25: Consider removing or clarifying large blocks of commented-out code.Maintaining large commented sections can introduce confusion and overhead. If these blocks are no longer needed, remove them. If they will be used in the near future, leave a reference or TODO to clarify their intended usage.
Also applies to: 134-198
298-298: Revisit skipped test.You are skipping “BTC -> SOL Swap” test. If it’s intentionally deferred, consider adding a TODO explaining when or why it should be re-enabled. Otherwise, remove the skip block to ensure the test runs.
packages/test/package.json (1)
60-60: Clean up unused Vite dependencies.You appear to have fully migrated to Next.js, but keep references to Vite in devDependencies. Remove them if no longer required to streamline maintenance.
packages/core/src/lib/garden/garden.ts (4)
172-195: Remove outdated commented code if no longer needed.Long commented-out code can clutter the file and confuse maintainers. Consider removing or moving it into version control documentation.
- // ... - // Old approach for initializeSMandBTCWallet - // ... + // Remove or move this section to version control notes/documentation
198-208: Wrap critical wallet creation in a try/catch for better error handling.Previously, the code had a try/catch block. If
BitcoinWallet.fromPrivateKeythrows an error (e.g., invalid key, provider issues), it won’t be caught here. Consider restoring that pattern:try { + const provider: IBitcoinProvider = new BitcoinProvider(getBitcoinNetwork(this.environment)); + this._btcWallet = BitcoinWallet.fromPrivateKey(digestKey.val, provider); + } catch (error) { + return Err("Failed to initialize Bitcoin wallet: " + error); + }
251-251: Avoid usingconsole.login production code & fix minor typos.Consider using a unified logger or a monitoring tool. Also correct “qutoe” to “quote.”
- console.log("Result of attested qutoe:: ", quoteRes.val); + console.debug("[Garden] Attested quote result:", quoteRes.val); - console.log("Result of createOrderRes::", createOrderRes.val); + console.debug("[Garden] createOrder response:", createOrderRes.val);Also applies to: 253-253, 257-257
894-895: Additional debuggingconsole.logfound.For a cleaner production build, you may want to remove or replace it with a structured logger.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro (Legacy)
⛔ Files ignored due to path filters (7)
packages/test/public/file.svgis excluded by!**/*.svgpackages/test/public/globe.svgis excluded by!**/*.svgpackages/test/public/next.svgis excluded by!**/*.svgpackages/test/public/vercel.svgis excluded by!**/*.svgpackages/test/public/window.svgis excluded by!**/*.svgpackages/test/src/app/favicon.icois excluded by!**/*.icoyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (65)
packages/core/src/lib/garden/garden.ts(23 hunks)packages/core/src/lib/garden/sol_btc.spec.ts(1 hunks)packages/core/src/lib/solana/htlc/solanaHTLC.ts(1 hunks)packages/core/src/lib/solana/htlc/solana_htlc.spec.ts(1 hunks)packages/core/src/lib/solana/idl/solana_native_swaps.json(1 hunks)packages/core/src/lib/solana/idl/solana_native_swaps.ts(1 hunks)packages/core/src/lib/solana/relayer/solanaRelay.ts(1 hunks)packages/core/src/lib/solana/relayer/solana_relayer.spec.ts(1 hunks)packages/core/src/lib/solana/solana.spec.ts(1 hunks)packages/orderbook/src/lib/asset.ts(6 hunks)packages/orderbook/src/lib/constants.ts(2 hunks)packages/orderbook/src/lib/orderbook/orderbook.ts(6 hunks)packages/test/.gitignore(1 hunks)packages/test/README.md(1 hunks)packages/test/eslint.config.js(0 hunks)packages/test/eslint.config.mjs(1 hunks)packages/test/index.html(0 hunks)packages/test/next.config.ts(1 hunks)packages/test/package.json(1 hunks)packages/test/postcss.config.js(1 hunks)packages/test/postcss.config.mjs(1 hunks)packages/test/src/App.css(1 hunks)packages/test/src/App.tsx(1 hunks)packages/test/src/app/globals.css(1 hunks)packages/test/src/app/layout.tsx(1 hunks)packages/test/src/app/page.tsx(1 hunks)packages/test/src/common/AssetChainLogos.tsx(1 hunks)packages/test/src/common/SwapInfo.tsx(1 hunks)packages/test/src/components/Sidebar/BTCWallets.tsx(1 hunks)packages/test/src/components/Sidebar/Details.tsx(1 hunks)packages/test/src/components/Sidebar/EVMWallets.tsx(1 hunks)packages/test/src/components/Sidebar/LogoutButtons.tsx(1 hunks)packages/test/src/components/Sidebar/Sidebar.tsx(1 hunks)packages/test/src/components/Sidebar/SupportedChains.tsx(1 hunks)packages/test/src/components/Swap.tsx(1 hunks)packages/test/src/components/SwapComponent/SwapComponent.tsx(1 hunks)packages/test/src/components/SwapComponent/SwapInput.tsx(1 hunks)packages/test/src/components/SwapComponent/SwapOutput.tsx(1 hunks)packages/test/src/components/common/Button.tsx(1 hunks)packages/test/src/components/swap/InputField.tsx(1 hunks)packages/test/src/components/swap/TokenSwap.tsx(1 hunks)packages/test/src/components/swap/TokenSwapBak.tsx(1 hunks)packages/test/src/components/transactions/TransactionRow.tsx(1 hunks)packages/test/src/components/transactions/Transactions.tsx(1 hunks)packages/test/src/config/index.tsx(1 hunks)packages/test/src/constants/constants.ts(1 hunks)packages/test/src/index.css(1 hunks)packages/test/src/main.tsx(1 hunks)packages/test/src/providers/GardenProviderWrapper.tsx(1 hunks)packages/test/src/providers/Providers.tsx(1 hunks)packages/test/src/providers/SolanaWalletProvider.tsx(1 hunks)packages/test/src/store/assetInfoStore.ts(1 hunks)packages/test/src/store/blockNumberStore.ts(1 hunks)packages/test/src/store/ordersStore.ts(1 hunks)packages/test/src/store/swapStore.ts(1 hunks)packages/test/src/utils/getLatestUpdatedOrder.ts(1 hunks)packages/test/src/utils/solana/connectSolanaWallet.ts(1 hunks)packages/test/src/utils/utils.ts(1 hunks)packages/test/src/vite-env.d.ts(0 hunks)packages/test/tailwind.config.js(1 hunks)packages/test/tailwind.config.ts(1 hunks)packages/test/tsconfig.app.json(0 hunks)packages/test/tsconfig.json(1 hunks)packages/test/tsconfig.node.json(0 hunks)packages/test/vite.config.ts(0 hunks)
💤 Files with no reviewable changes (6)
- packages/test/src/vite-env.d.ts
- packages/test/vite.config.ts
- packages/test/eslint.config.js
- packages/test/tsconfig.app.json
- packages/test/tsconfig.node.json
- packages/test/index.html
✅ Files skipped from review due to trivial changes (19)
- packages/test/tailwind.config.js
- packages/test/postcss.config.mjs
- packages/test/src/App.css
- packages/test/src/components/Sidebar/Sidebar.tsx
- packages/test/src/components/Swap.tsx
- packages/test/src/components/Sidebar/BTCWallets.tsx
- packages/test/src/main.tsx
- packages/test/src/index.css
- packages/test/src/components/SwapComponent/SwapOutput.tsx
- packages/test/src/components/Sidebar/Details.tsx
- packages/test/eslint.config.mjs
- packages/test/src/constants/constants.ts
- packages/test/tailwind.config.ts
- packages/test/src/components/Sidebar/SupportedChains.tsx
- packages/test/README.md
- packages/test/src/app/globals.css
- packages/test/src/components/Sidebar/LogoutButtons.tsx
- packages/test/src/components/Sidebar/EVMWallets.tsx
- packages/test/src/components/SwapComponent/SwapInput.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/orderbook/src/lib/orderbook/orderbook.ts
🧰 Additional context used
🧬 Code Definitions (15)
packages/test/src/common/SwapInfo.tsx (2)
packages/test/src/store/assetInfoStore.ts (1)
assetInfoStore(53-138)packages/test/src/common/AssetChainLogos.tsx (1)
AssetChainLogos(9-40)
packages/test/src/providers/SolanaWalletProvider.tsx (1)
packages/test/src/store/blockNumberStore.ts (1)
network(5-5)
packages/core/src/lib/solana/solana.spec.ts (3)
packages/core/src/lib/solana/solanaTypes.ts (1)
SwapConfig(8-68)packages/core/src/lib/solana/relayer/solanaRelay.ts (1)
SolanaRelay(14-108)packages/core/src/lib/solana/htlc/solanaHTLC.ts (1)
SolanaHTLC(12-85)
packages/test/src/config/index.tsx (1)
packages/test/src/main.tsx (1)
config(20-31)
packages/core/src/lib/solana/htlc/solana_htlc.spec.ts (1)
packages/core/src/lib/solana/htlc/solanaHTLC.ts (1)
SolanaHTLC(12-85)
packages/test/src/components/swap/TokenSwapBak.tsx (1)
packages/test/src/store/swapStore.ts (1)
swapStore(30-139)
packages/test/src/components/swap/TokenSwap.tsx (1)
packages/test/src/store/swapStore.ts (1)
swapStore(30-139)
packages/core/src/lib/solana/relayer/solana_relayer.spec.ts (1)
packages/core/src/lib/solana/relayer/solanaRelay.ts (1)
SolanaRelay(14-108)
packages/test/src/components/transactions/TransactionRow.tsx (3)
packages/test/src/store/assetInfoStore.ts (1)
assetInfoStore(53-138)packages/test/src/utils/utils.ts (3)
getAssetFromSwap(6-8)formatAmount(27-37)getDayDifference(10-25)packages/test/src/common/SwapInfo.tsx (1)
SwapInfo(14-51)
packages/orderbook/src/lib/asset.ts (1)
packages/orderbook/src/index.ts (7)
Chains(5-5)Chain(3-3)TimeLocks(12-12)getBlockchainType(10-10)isBitcoin(8-8)BlockchainType(6-6)isEVM(9-9)
packages/core/src/lib/solana/relayer/solanaRelay.ts (2)
packages/core/src/lib/solana/idl/solana_native_swaps.ts (1)
SolanaNativeSwaps(7-414)packages/core/src/lib/garden/garden.ts (1)
swap(216-276)
packages/orderbook/src/lib/constants.ts (2)
packages/orderbook/src/index.ts (7)
SOLSolanaLocalnetAsset(24-24)Asset(3-3)Chains(5-5)WBTCEthereumLocalnetAsset(20-20)SupportedAssets(25-25)WBTCArbitrumLocalnetAsset(19-19)bitcoinRegtestAsset(23-23)packages/orderbook/src/lib/asset.ts (2)
Asset(14-14)Chains(39-58)
packages/core/src/lib/solana/htlc/solanaHTLC.ts (2)
packages/core/src/lib/solana/idl/solana_native_swaps.ts (1)
SolanaNativeSwaps(7-414)packages/core/src/lib/garden/garden.ts (1)
swap(216-276)
packages/test/src/providers/Providers.tsx (1)
packages/test/src/config/index.tsx (2)
projectId(5-5)wagmiAdapter(13-20)
packages/core/src/lib/garden/garden.ts (7)
packages/core/src/lib/garden/cache/executorCache.ts (1)
Cache(36-46)packages/orderbook/src/lib/asset.ts (3)
getBlockchainType(136-141)isBitcoin(84-90)isEVM(92-107)packages/core/src/lib/garden/garden.types.ts (1)
SwapParams(13-55)packages/orderbook/src/lib/orderbook/orderbook.types.ts (1)
MatchedOrder(234-241)packages/core/src/lib/solana/htlc/solanaHTLC.ts (1)
SolanaHTLC(12-85)packages/core/src/lib/solana/relayer/solanaRelay.ts (1)
SolanaRelay(14-108)packages/utils/src/lib/blockNumber/blockNumber.ts (2)
fetchBitcoinBlockNumber(50-59)fetchEVMBlockNumber(17-30)
🪛 ESLint
packages/test/src/providers/SolanaWalletProvider.tsx
[error] 10-10: 'clusterApiUrl' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'UnsafeBurnerWalletAdapter' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/components/swap/TokenSwapBak.tsx
[error] 14-14: 'anchor' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 18-18: 'walletConnected' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 164-164: 'handleSwap' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/components/swap/TokenSwap.tsx
[error] 8-8: 'BigNumber' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 33-33: 'TEST_RPC_URL' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 47-47: 'publicKey' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 126-126: 'bitcoinProvider' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 130-130: 'btcWalletx' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 131-131: 'BTC_ADDRESS' is never reassigned. Use 'const' instead.
(prefer-const)
[error] 160-160: 'swapSOLTowBTC' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 405-405: 'setSwapParams' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 406-406: 'setInputAmount' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 408-408: 'setErrorMessage' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 409-409: 'setLoading' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 423-423: 'W3MButton' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
[error] 426-426: 'validateInput' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/providers/Providers.tsx
[error] 7-7: 'mainnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'arbitrum' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 9-9: 'sepolia' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'solana' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'solanaTestnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 12-12: 'solanaDevnet' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/components/SwapComponent/SwapComponent.tsx
[error] 8-8: 'useAppKitAccount' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 8-8: 'useAppKitProvider' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 10-10: 'Provider' is defined but never used.
(@typescript-eslint/no-unused-vars)
[error] 11-11: 'useAppKitConnection' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/components/common/Button.tsx
[error] 11-11: 'className' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 Biome (1.9.4)
packages/test/src/utils/solana/connectSolanaWallet.ts
[error] 14-14: expected ) but instead found SolWallets
Remove SolWallets
(parse)
[error] 14-14: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 14-18: This block statement doesn't serve any purpose and can be safely removed.
Standalone block statements without any block-level declarations are redundant in JavaScript and can be removed to simplify the code.
Safe fix: Remove redundant block.
(lint/complexity/noUselessLoneBlockStatements)
packages/test/src/utils/utils.ts
[error] 7-7: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (69)
packages/test/src/components/SwapComponent/SwapComponent.tsx (1)
1-7: Component Structure & Integration Validation
The rest of the component, including state setup, event handlers (handleSwap,handleBtcInput, andhandleInitialize), and JSX rendering, remains clear and well-organized. The existing functionality integrates seamlessly with the added capabilities planned for Solana support, ensuring that the overall feature remains robust as future Solana-specific logic is introduced.Also applies to: 13-124
packages/test/.gitignore (9)
3-12: Dependency Ignorance for Modern Package Managers
The section handling dependency directories is well updated. Including Yarn’s Plug'n'Play (PnP) files and the related exceptions is good practice. Make sure that this configuration aligns with your project's package managers and that no essential files are inadvertently ignored.
13-15: Testing Artifacts Management
Adding/coverageto ignore testing output is appropriate. This ensures that build or coverage artifacts do not clutter the repository.
16-19: Next.js Build Directories
The inclusion of/.next/and/out/is well-suited for a Next.js project. These entries help ensure that build artifacts are not committed.
20-22: Production Build Exclusions
Ignoring the production build directory (i.e.,/build) is correctly set up. This keeps deployment-related artifacts out of version control.
23-26: Miscellaneous File Exclusions
Entries like.DS_Storeand*.pemare necessary to prevent unwanted or sensitive files from being tracked. Just verify that no required PEM files are needed in any deployment context.
27-32: Debug Log Patterns Update
The updated debug log patterns—includingnpm-debug.log*,yarn-debug.log*,yarn-error.log*, and.pnpm-debug.log*—are comprehensive. This should help keep the repository clean from transient debug outputs, which is especially useful in a complex multi-framework project.
33-35: Environment Files
Ignoring.env*files appropriately secures any local configuration or sensitive environment data from accidental commits.
36-38: Vercel Configuration Exclusion
The inclusion of.vercelin the ignore list is standard for projects deployed with Vercel. This prevents deployment configuration files from being committed unintentionally.
39-41: TypeScript Build Artifacts
Excluding*.tsbuildinfoandnext-env.d.tsis proper as these files are generated and can be re-created as needed. This helps maintain a clean repository.packages/test/postcss.config.js (1)
3-4: Updated PostCSS plugins for Solana supportThe PostCSS configuration has been updated with the correct Tailwind plugin identifier and added nesting support. These changes align well with the Next.js configuration and Solana integration requirements.
packages/test/next.config.ts (1)
3-16: Well-configured Next.js setup for WebAssembly and Bitcoin librariesThe Next.js configuration is properly set up to enable WebAssembly and top-level await features needed for Solana integration. The resolve alias for tiny-secp256k1 correctly enables Bitcoin-related cryptography operations.
To future-proof your configuration, consider adding a comment explaining why these specific settings are needed for Solana/Bitcoin integration, as this would be helpful for future developers.
packages/test/src/app/page.tsx (2)
5-13: Good use of dynamic imports with SSR disabledUsing
dynamicimports withssr: falseis appropriate here since these components likely require client-side APIs, such as wallet connections for Solana and other blockchains.
15-28: Well-structured responsive layoutThe component uses a responsive grid layout that adapts based on screen size, with a 3:2 ratio for the TokenSwap and Transactions components on medium screens and above. The layout structure is clean and follows best practices.
packages/test/src/store/blockNumberStore.ts (1)
16-33: Proper state management for API callsThe Zustand store implementation correctly handles loading states, error management, and asynchronous operations. The pattern of setting loading state before the request, handling both success and error cases, and finally resetting the loading state is a good practice.
packages/test/src/config/index.tsx (2)
5-9: Good practice for environment variable validationChecking for the presence of required environment variables at initialization time is a good practice that helps catch configuration issues early.
11-11: Appropriate inclusion of Solana in network listIncluding Solana in the list of supported networks aligns with the PR objective to add Solana support. This ensures that Solana blockchain functionality is properly integrated with the existing network configurations.
packages/test/src/providers/Providers.tsx (3)
24-46: Good implementation of the Solana localnet chain configurationThe chain definition is thorough and includes all necessary parameters for local development:
- Appropriate network identifiers
- Currency details
- RPC URLs for HTTP and WebSocket connections
- Block explorer configuration
68-76: Good implementation of the Providers componentThe component correctly wraps children with necessary providers:
- WagmiProvider for Ethereum wallet integration
- QueryClientProvider for data fetching
- GardenProviderWrapper for application-specific context
The implementation follows best practices for React context providers.
58-66:❓ Verification inconclusive
Verify Solana network configuration
The application is configured to use only
solanaLocalnetas both the network and default network. Ensure this is intentional and that other networks like mainnet or testnet aren't required for your application.
🏁 Script executed:
#!/bin/bash # Check for network configurations in the codebase echo "Checking for network configurations in the codebase..." # Look for other createAppKit usage to see what networks are used elsewhere echo "Looking for createAppKit usage patterns:" rg "createAppKit" -A 5 -B 5 # Check for environment-dependent network configuration echo "Looking for environment-based network configuration:" rg "networks.*process\.env" --include="*.{ts,tsx,js,jsx}"Length of output: 2231
Action Required: Verify Solana Network Configuration in Providers.tsx
- In
packages/test/src/providers/Providers.tsx(lines 58–66), thecreateAppKitcall is configured to use onlysolanaLocalnetfor bothnetworksanddefaultNetwork.- However, the file also imports and references other network constants (e.g.,
mainnet,arbitrum,sepolia,solana), which suggests that multiple environments might be supported elsewhere.- No environment-based configuration (e.g., via
process.env) was detected to override this static setting.- Please confirm that the exclusive use of
solanaLocalnetis intentional for your use case and that omitting additional networks is not an inadvertent oversight.packages/test/src/providers/SolanaWalletProvider.tsx (1)
35-41: Good implementation of the Solana wallet providersThe component correctly sets up the Solana wallet infrastructure with appropriate providers:
- ConnectionProvider for Solana RPC connection
- WalletProvider with autoConnect enabled
- WalletModalProvider for UI components
packages/test/tsconfig.json (1)
26-27: Comprehensive file inclusion configurationThe include and exclude patterns are properly configured:
- Includes TypeScript declaration files, all TS and TSX files
- Includes Next.js type definitions
- Properly excludes node_modules
packages/test/src/utils/getLatestUpdatedOrder.ts (1)
3-13: Good implementation of order status priority listThe priority list is well-structured with a clear progression of order statuses from matching to completion. The use of a partial record with OrderStatus as keys and numeric priorities as values is a good approach.
packages/test/src/utils/solana/connectSolanaWallet.ts (1)
35-43: Provide an implementation forconnectSolanaWallet.The
connectSolanaWalletfunction is declared but not implemented. Consider invoking one of the three specialized connect methods based on the selected wallet type, or removing it until it is ready to be utilized.Would you like me to propose a basic implementation based on your placeholders?
packages/core/src/lib/solana/htlc/solana_htlc.spec.ts (1)
47-52: Looks good.The test confirms that the HTLC
initflow returns a valid transaction signature. No immediate issues are detected here.packages/core/src/lib/solana/relayer/solanaRelay.ts (1)
30-45: Constructor logic looks solid.The constructor properly initializes the program, validates the relayer’s public key, and computes the PDA. Overall, this approach to setting up references is clear and straightforward.
packages/core/src/lib/solana/idl/solana_native_swaps.ts (2)
16-28: Looks good.The instruction definitions for
initiateappear correct and well-structured, reflecting typical anchor conventions (with the correct accounts & arguments).
7-9:❓ Verification inconclusive
Confirm the on-chain program address.
The commented-out address in line 8 differs from the assigned address in line 9. Ensure that the specified public key
6eksgdCnSjUaGQWZ6iYvauv1qzvYPF33RTGTM1ZuyENxis valid and deployed.
🏁 Script executed:
#!/bin/bash # Verify the Solana program deployment status via Solana CLI # (assuming local or test network connectivity). solana program show 6eksgdCnSjUaGQWZ6iYvauv1qzvYPF33RTGTM1ZuyENxLength of output: 163
Action Required: Manually Verify the On-Chain Program Deployment
The automated check using the Solana CLI failed because the
solanacommand isn’t available in this environment. Please manually verify that the program public key
6eksgdCnSjUaGQWZ6iYvauv1qzvYPF33RTGTM1ZuyENx
is valid and corresponds to a deployed on-chain program. Confirm whether the commented-out address should be removed permanently.
- Next Steps:
- Ensure you have the Solana CLI installed locally.
- Run
solana program show 6eksgdCnSjUaGQWZ6iYvauv1qzvYPF33RTGTM1ZuyENxin your environment to check the program’s status.- Update the code comments accordingly once verified.
packages/test/src/utils/utils.ts (2)
10-25: Handle invalid or future dates gracefully.If
dateis invalid or occurs in the future, the function may produce misleading values. Consider returning a clearer message or handling negative differences.Would you like a helper function that returns a guard or logs a warning if the date is invalid or in the future?
39-40: Check for SSR usage or trailing slashes.
window.locationis undefined on server-side. If this code runs in a Next.js environment, ensure it only runs client-side. Also, consider normalizing trailing slashes in paths, if relevant.packages/test/src/store/swapStore.ts (2)
30-48: Initial store setup looks good.Default values are clearly assigned, and references to
SupportedAssetsappear correct for a test environment.
49-57: Adequate setter methods.Each setter function is simple and clear. No concerns.
packages/test/src/store/ordersStore.ts (2)
25-45: Confirm that all order statuses are handled.
filterPendingOrdersexcludes orders withOrderStatus.RedeemDetectedonly. Ensure other statuses (e.g., canceled, expired) are intentionally included in thependingOrderslist or filter them as well if necessary.
80-90: Avoid returningundefinedin mapped arrays.When block numbers are missing, the mapped callback can return
undefined, which is then filtered out. Consider logging or capturing these cases in a separate fallback to track incomplete data or highlight missing chain info.packages/core/src/lib/solana/idl/solana_native_swaps.json (2)
1-281: This seems to be a duplicate file.According to previous review comments, this file appears to be a duplicate. Please verify if this is intentional or if one version should be removed.
#!/bin/bash # Check for potential duplicates of this file find packages/core/src -name "*.json" | xargs grep -l "solana_native_swaps" | grep -v "solana_native_swaps.json"
2-2:❓ Verification inconclusive
Verify hardcoded program address.
The program address is hardcoded in the IDL, which means if the contract is redeployed or upgraded, this IDL file would need to be updated.
Verify that this is the correct and up-to-date program address, and consider implementing a more flexible approach to handle multiple environments.
🏁 Script executed:
#!/bin/bash # Check if this program address exists and is a valid program solana config set --url https://api.devnet.solana.com solana account 6eksgdCnSjUaGQWZ6iYvauv1qzvYPF33RTGTM1ZuyENx --output json | grep programDataLength of output: 344
Address Verification Manual Check Required
The automated check couldn’t determine whether the program address is valid because the
solanaCLI isn’t available in the verification environment. Please manually verify that the hardcoded program address inpackages/core/src/lib/solana/idl/solana_native_swaps.json(line 2) is correct for the intended deployment. Additionally, consider implementing a dynamic configuration approach to avoid the need for manual updates if the contract is later redeployed or upgraded.packages/orderbook/src/lib/asset.ts (8)
19-19: Add Solana blockchain type
The addition ofSolanato theBlockchainTypeenum cleanly extends your set of supported blockchains.
51-53: New Solana chain constants
Introducingsolana,solana_testnet, andsolana_localnetinChainsis consistent with your existing pattern for network definitions.
64-64: Excluding Solana from EVM
Removing Solana chains fromEvmChainis correct, since they are not EVM-compatible.
76-77: Solana testnet/localnet classification
Explicitly treatingsolana_testnetandsolana_localnetas non-mainnet is correct.
109-113: New isSolana function
This utility function clearly identifies Solana-based chains. It aligns well with analogous functions likeisEVMandisBitcoin.
139-139: Mapping Solana to new enum
MappingisSolanachains toBlockchainType.Solanacompletes the Solana integration. Looks good.
131-133:❓ Verification inconclusive
High time lock for Solana
432000 time slots (~48 hours if each slot is ~0.4s) may be quite large. Confirm it meets your business requirements and user experience.You can search references to ensure no unexpected wait times:
🏁 Script executed:
#!/bin/bash rg 'solana_testnet\]|solana_localnet\]|solana\]' -A 5Length of output: 1088
Confirm Solana High Time Lock Duration
The configuration sets a time lock of 432000 time slots (~48 hours, assuming each slot is ~0.4s), which is noticeably high. Please verify that this duration aligns with your business requirements and does not negatively impact the user experience. The search confirms that this value is consistently applied in
packages/orderbook/src/lib/asset.tswithout any conflicting references.
- File:
packages/orderbook/src/lib/asset.ts(lines 131-133)- Snippet:
[Chains.solana]: 432000, // In Solana, timeslots are in chunks of 0.4s (48 hours when calculated as sec / 0.4) [Chains.solana_localnet]: 432000, [Chains.solana_testnet]: 432000Please double-check this value against your design and operational expectations.
118-118:❓ Verification inconclusive
Time lock update for bitcoin_regtest
You increased the regtest lock from 144 to 200. If this was intentional, confirm no breakage in regtest-based tests or logic.Run this script to detect code references expecting 144 for regtest:
🏁 Script executed:
#!/bin/bash rg 'TimeLocks\[Chains\.bitcoin_regtest'Length of output: 41
Action Required: Confirm Regtest Lock Update Impact
Your change for
Chains.bitcoin_regtestupdates the time lock from 144 to 200. Automated searches for references to the old value didn’t return any hits, which is encouraging. However, the absence of output from our grep check isn’t fully conclusive. Please manually verify that:
- There are no tests or logic sections expecting the old value (144) for bitcoin_regtest.
- The regtest-based flows run as intended with the updated lock value.
packages/orderbook/src/lib/constants.ts (5)
35-42: Introducing SOLSolanaLocalnetAsset
This provides a distinct localnet configuration for Solana, matching your pattern for other localnet assets.
56-57: Revised atomicSwapAddress
You replaced a commented-out address with a new one. Just ensure the new address remains valid for your localnet environment.
72-72: Register Solana localnet asset
Addingsolana_localnet_SOLto thelocalnetsection aligns with the newChains.solana_localnet.
75-75: Including bitcoin_regtest_BTC under localnet
Placing a regtest Bitcoin asset in localnet could be confusing if users expect a “testnet” environment. Confirm naming/usage.
78-85: New solana_testnet_SOL asset
Configuring the Solana testnet asset is consistent with existing code patterns for test networks.packages/test/src/store/assetInfoStore.ts (8)
1-4: Imports and environment usage
The imports for Zustand, axios, and your custom packages look appropriate. No immediate issues.
6-10: API URL constant
DefiningASSETS_API_URLfromNEXT_PUBLIC_DATA_URLis fine. Confirm this environment variable is set in your Next.js environment.
12-15: IOType enum
The enum cleanly distinguishes input vs. output usage. Straightforward and readable.
17-19: Networks type
Mapping chain keys to their data and assets ensures type safety. Good approach to strongly type this structure.
21-28: ChainData structure
TheChainDatafields cover essential network properties: chainId, explorer, logos, etc. This is consistent with the rest of your design.
30-31: Assets and Chains type
These mapped record types help separate chain data from asset data. Good to see them strongly typed.
33-51: AssetInfoState shape
The shape captures assets, chains, selector UI, and strategies. This is adequately comprehensive for your store.
53-138: Comprehensive store implementation
This Zustand store manages all asset and chain data, including asynchronous fetching, error handling, and strategies. Good structure overall.packages/core/src/lib/garden/sol_btc.spec.ts (1)
200-346: Well-structured test suites.Overall, the SOL <-> BTC tests cover environment setup, order creation, and execution. The structure and usage of helper functions like
executeWithTimeouthandle asynchronous flows nicely.packages/core/src/lib/solana/htlc/solanaHTLC.ts (3)
61-69: Good use of secret validation.Your call to
validateSecret(secret)helps ensure correct length and format before sending the transaction. This consistently aligns with your on-chain checks (e.g., in the Anchor program).
75-84:❓ Verification inconclusive
Confirm refund logic is invoked after expiry.
While the on-chain program enforces expiry, ensure your off-chain flow or additional validations don’t inadvertently trigger refunds prematurely. Verify your tests cover various timing scenarios.
🏁 Script executed:
#!/bin/bash # Attempt to locate and check any refund calls or relevant tests that might forcibly call 'refund' too soon. rg -A 5 "refund("Length of output: 84
Refund Logic Timing Check – Manual Verification Needed
The initial regex search encountered an error due to an unescaped parenthesis. Please re-run the verification with a corrected pattern (e.g., using
rg -A 5 "refund\(") to examine if any off-chain flows or tests trigger the refund too early. In parallel, ensure that your test suite covers scenarios verifying that refunds are only invoked after the expiry, given that the on-chain program enforces this condition.
25-32:❓ Verification inconclusive
Ensure the program address is correct and verify PDA derivation.
findProgramAddressSyncmust match the anchor IDL’s seeds. If the providedswapIdis shorter or longer than expected, the program address may be invalid. Consider verifying correct usage ofswapIdacross the codebase.
🏁 Script executed:
#!/bin/bash # This script checks for references to swapId usage to confirm consistent length across the codebase. ast-grep --pattern $'function $_(...) {\n $$$\n}'Length of output: 49
PDA Derivation Seed Length Verification Needed
The automated search for consistent usage of
swapIddidn’t yield any conclusive results. This means we couldn’t automatically confirm that theswapIdvalue (used in converting to a seed viaBuffer.from(swap.swapId)) consistently meets the length requirements expected by the Anchor IDL. Please manually verify that:
- The length of
swap.swapIdis always as required (i.e., it matches the expected seed format for the PDA derivation).- There’s no transformation or variation of
swapIdelsewhere in the codebase that could lead to an invalid program address.Keep an eye on the usage in:
- File:
packages/core/src/lib/solana/htlc/solanaHTLC.ts(lines 25–32)Once you’ve confirmed the consistency of
swapIdusage, we can proceed accordingly.
To further assist, here is an additional script that can be run to search for any occurrences of
swapIdusage in the repository:#!/bin/bash # Search for occurrences of swapId across the codebase in TypeScript files rg -n "swapId" .packages/test/package.json (1)
55-55:✅ Verification successful
Verify the React 19 dependency availability.
React version “^19.0.0” may not be stable or officially released. Ensure your environment supports this version, or switch to the latest production release.
🌐 Web query:
Is React 19.0.0 officially released, and is it stable?💡 Result:
React 19.0.0 was officially released on December 5, 2024, and is stable, marking a significant update with new features, performance improvements, and breaking changes to deprecated APIs[1][5][6].
Key details about the release:
- Stability: The React team confirmed stability in their official release post, providing an upgrade guide and codemods for migration[1][11].
- Breaking changes include:
- Removal of legacy APIs like string refs,
propTypes,findDOMNode, andReact.createFactory[1][11]- Requirement for the new JSX transform for performance optimizations[1][11]
- Error handling improvements with
onCaughtError/onUncaughtErrorroot APIs[1][5]Adoption status:
- React Native 0.78 added full React 19 support in February 2025[4]
- Next.js and major libraries like
react-day-pickerhave released compatibility updates[3][10]- The latest npm version (
19.0.0) shows 4+ months of production use with no critical stability issues reported[6][14]Developers should review the upgrade guide to address breaking changes, especially migrating from deprecated APIs and enabling the new JSX transform[1][11]. Most apps using modern React patterns can upgrade with minimal code changes[5][8].
Citations:
- 1: https://github.com/facebook/react/releases
- 2: https://react.dev/versions
- 3: https://react.statuscode.com/latest
- 4: https://www.lucentinnovation.com/blogs/it-insights/react-native-0-78-react-19-version-update-guide
- 5: https://react.dev/blog/2024/12/05/react-19
- 6: https://www.npmjs.com/package/react
- 7: https://github.com/facebook/react/blob/main/CHANGELOG.md
- 8: https://www.ifourtechnolab.com/blog/react-18-vs-react-19-key-differences-to-know-for-2024
- 9: https://en.wikipedia.org/wiki/React_(software)
- 10: support react 19 gpbl/react-day-picker#2152
- 11: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
- 12: https://legacy.reactjs.org/versions/
- 13: https://www.npmjs.com/package/react/v/19.0.0-rc-915b914b3a-20240515
- 14: https://www.npmjs.com/package/react?activeTab=versions
- 15: https://github.com/facebook/react/milestone/40
React 19.0.0 is now officially released and confirmed stable.
The dependency
"react": "^19.0.0"in the package.json is valid. No changes are needed; however, please ensure you follow the React 19 upgrade guide to address any breaking changes (such as deprecated APIs and requirements for the new JSX transform) if you haven't already.packages/core/src/lib/garden/garden.ts (7)
20-20: No concerns with the reference toisEVM.
42-42: Import statement looks fine.
84-84: Good addition of_solWallet,solanaRelayUrl, andsolanaRelayerAddress.Introducing specialized fields for Solana is appropriate. Ensure these fields are consistently named in documentation and well-initialized in all target environments.
Also applies to: 90-91
100-102: Clarify environment handling logic.Currently,
LOCALNETis mapped toAPI.testnet. Verify if this is the intended structure or if localnet should point to a different API. If you need a separate environment handler, you might consider a distinctAPI.localnetor a fallback block as shown:- ? API.testnet - : config.environment === Environment.LOCALNET - ? API.testnet : undefined; + ? API.testnet + : config.environment === Environment.LOCALNET + ? API.localnet : undefined;
113-116: New SIWE configuration.No immediate concerns. The initialization logic for
Siwelooks consistent. Make sureconfig.siweOptsis always valid or gracefully handled if undefined.
266-273: Verify Solana source chain check.You’re calling
solInitonly ifgetBlockchainType(orderRes.val.source_swap.chain) === BlockchainType.Solana. However, if the source chain is not Solana but the destination chain is, you may need similar initialization logic. Confirm the intended requirement.
873-890: Repeated manual block number mappings.This approach was previously noted as fragile. If you frequently map mainnet block numbers to local/test variants, consider a more robust mapping mechanism or a utility function that automatically applies all needed mappings based on environment context.
| export const Button: React.FC<ButtonProps> = ({ children, className, secondary, disabled, ...props }) => { | ||
| return ( | ||
| <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"}`} | ||
| {...props} | ||
| > | ||
| {children} | ||
| </div> | ||
| ); |
There was a problem hiding this comment.
Unused className prop in Button component
The Button component correctly accepts a className prop but never applies it to the rendered div. This would prevent users from customizing the button with additional classes.
Apply this fix to incorporate the user-provided className:
- <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"}`}
+ <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"} ${className || ""}`}📝 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.
| export const Button: React.FC<ButtonProps> = ({ children, className, secondary, disabled, ...props }) => { | |
| return ( | |
| <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"}`} | |
| {...props} | |
| > | |
| {children} | |
| </div> | |
| ); | |
| export const Button: React.FC<ButtonProps> = ({ children, className, secondary, disabled, ...props }) => { | |
| return ( | |
| <div className={`px-3 py-1 hover:text-white flex rounded-md bg-[#E36492] hover:bg-opacity-90 transition-all duration-200 ease-in-out cursor-pointer w-full ${disabled ? "border-gray-300 bg-gray-300 text-gray-500 pointer-events-none" : ""} ${secondary ? "border-none bg-opacity-10" : "bg-opacity-60 border border-[#E36492]"} ${className || ""}`} | |
| {...props} | |
| > | |
| {children} | |
| </div> | |
| ); | |
| } |
🧰 Tools
🪛 ESLint
[error] 11-11: 'className' is defined but never used.
(@typescript-eslint/no-unused-vars)
packages/test/src/App.tsx
Outdated
| environment: Environment.LOCALNET, | ||
| walletClient: walletClient, | ||
| orderBookUrl: 'http://localhost:4426', | ||
| quoteUrl: 'http://localhost:6969', | ||
| bitcoinRPCUrl: 'http://localhost:30000', | ||
| blockNumberFetcherUrl: 'http://localhost:3008', | ||
| apiKey: | ||
| 'AAAAAGghjwU6Os1DVFgmUXj0GcNt5jTJPbBmXKw7xRARW-qivNy4nfpKVgMNebmmxig2o3v-6M4l_ZmCgLp3vKywfVXDYBcL3M4c', | ||
| }} |
There was a problem hiding this comment.
Environment changed to LOCALNET with hardcoded credentials
The switch from TESTNET to LOCALNET with local development URLs is appropriate for testing Solana integration. However, there's a potential security issue with the hardcoded API key.
Consider moving the API key to an environment variable:
- apiKey:
- 'AAAAAGghjwU6Os1DVFgmUXj0GcNt5jTJPbBmXKw7xRARW-qivNy4nfpKVgMNebmmxig2o3v-6M4l_ZmCgLp3vKywfVXDYBcL3M4c',
+ apiKey: process.env.REACT_APP_GARDEN_API_KEY || '',Make sure to update your documentation to include this environment variable in the setup instructions.
📝 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.
| environment: Environment.LOCALNET, | |
| walletClient: walletClient, | |
| orderBookUrl: 'http://localhost:4426', | |
| quoteUrl: 'http://localhost:6969', | |
| bitcoinRPCUrl: 'http://localhost:30000', | |
| blockNumberFetcherUrl: 'http://localhost:3008', | |
| apiKey: | |
| 'AAAAAGghjwU6Os1DVFgmUXj0GcNt5jTJPbBmXKw7xRARW-qivNy4nfpKVgMNebmmxig2o3v-6M4l_ZmCgLp3vKywfVXDYBcL3M4c', | |
| }} | |
| environment: Environment.LOCALNET, | |
| walletClient: walletClient, | |
| orderBookUrl: 'http://localhost:4426', | |
| quoteUrl: 'http://localhost:6969', | |
| bitcoinRPCUrl: 'http://localhost:30000', | |
| blockNumberFetcherUrl: 'http://localhost:3008', | |
| apiKey: process.env.REACT_APP_GARDEN_API_KEY || '', | |
| }} |
| <Image | ||
| src={tokenLogo} | ||
| alt="Token Logo" | ||
| width={20} | ||
| height={20} | ||
| className="absolute left-0 z-30 rounded-full" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for missing image sources
The component doesn't handle cases where token logos might be empty strings. If tokenLogo is an empty string, it could lead to a broken image.
<Image
src={tokenLogo}
alt="Token Logo"
width={20}
height={20}
+ onError={(e) => {
+ e.currentTarget.src = '/fallback-token-logo.png';
+ }}
className="absolute left-0 z-30 rounded-full"
/>Apply the same change to the chainLogo Image component as well.
📝 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.
| <Image | |
| src={tokenLogo} | |
| alt="Token Logo" | |
| width={20} | |
| height={20} | |
| className="absolute left-0 z-30 rounded-full" | |
| /> | |
| <Image | |
| src={tokenLogo} | |
| alt="Token Logo" | |
| width={20} | |
| height={20} | |
| onError={(e) => { | |
| e.currentTarget.src = '/fallback-token-logo.png'; | |
| }} | |
| className="absolute left-0 z-30 rounded-full" | |
| /> | |
| <Image | |
| src={chainLogo} | |
| alt="Chain Logo" | |
| width={20} | |
| height={20} | |
| onError={(e) => { | |
| e.currentTarget.src = '/fallback-chain-logo.png'; | |
| }} | |
| className="absolute right-0 z-20 rounded-full" | |
| /> |
| const { chains } = assetInfoStore(); | ||
| const sendChain = | ||
| chains && !isBitcoin(sendAsset.chain) ? chains[sendAsset.chain] : undefined; | ||
| const receiveChain = | ||
| chains && !isBitcoin(receiveAsset.chain) | ||
| ? chains[receiveAsset.chain] | ||
| : undefined; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add null check for chains
The component retrieves chain information but doesn't handle the case when chains is null, which could happen during initial loading.
- const { chains } = assetInfoStore();
+ const { chains, isLoading } = assetInfoStore();
+
+ if (isLoading || !chains) {
+ return (
+ <div className="flex justify-between items-center">
+ <p>Loading swap details...</p>
+ </div>
+ );
+ }
const sendChain =
- chains && !isBitcoin(sendAsset.chain) ? chains[sendAsset.chain] : undefined;
+ !isBitcoin(sendAsset.chain) ? chains[sendAsset.chain] : undefined;
const receiveChain =
- chains && !isBitcoin(receiveAsset.chain)
+ !isBitcoin(receiveAsset.chain)
? chains[receiveAsset.chain]
: undefined;📝 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.
| const { chains } = assetInfoStore(); | |
| const sendChain = | |
| chains && !isBitcoin(sendAsset.chain) ? chains[sendAsset.chain] : undefined; | |
| const receiveChain = | |
| chains && !isBitcoin(receiveAsset.chain) | |
| ? chains[receiveAsset.chain] | |
| : undefined; | |
| const { chains, isLoading } = assetInfoStore(); | |
| if (isLoading || !chains) { | |
| return ( | |
| <div className="flex justify-between items-center"> | |
| <p>Loading swap details...</p> | |
| </div> | |
| ); | |
| } | |
| const sendChain = !isBitcoin(sendAsset.chain) ? chains[sendAsset.chain] : undefined; | |
| const receiveChain = | |
| !isBitcoin(receiveAsset.chain) | |
| ? chains[receiveAsset.chain] | |
| : undefined; |
| it("Relayer should redeem the funds as per the secret hash provided", async () => { | ||
| console.log("Redeeming the transaction"); | ||
| const res = await userRelay.redeem(secretHash); | ||
| expect(res).toBeDefined(); | ||
| console.log("Relayer redeemed successfully!"); |
There was a problem hiding this comment.
Fix secret parameter in redemption test.
You're using secretHash for redemption, but the redeem method should receive the actual secret, not the hash. This test will likely fail.
Looking at the implementation of SolanaRelay.redeem(), it expects a secret that will be validated against the stored hash.
it("Relayer should redeem the funds as per the secret hash provided", async () => {
console.log("Redeeming the transaction");
- const res = await userRelay.redeem(secretHash);
+ // Create a hex-encoded secret for redemption
+ const hexEncodedSecret = hex.encode(secret);
+ const res = await userRelay.redeem(hexEncodedSecret);
expect(res).toBeDefined();
console.log("Relayer redeemed successfully!");
})Note: I notice that the secret variable isn't defined in this test's scope. You should either define a new secret or modify the test to properly handle this scenario.
Committable suggestion skipped: line range outside the PR's diff.
| const TEST_RPC_URL = "https://solana-devnet.g.alchemy.com/v2/FyJVAXC6zO6bCojXG19p-sCpQaXWexBi"; | ||
| const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay"); | ||
| const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172] |
There was a problem hiding this comment.
Remove API key from source code.
The RPC URL contains what appears to be an API key, which is a security risk even if it's for a test environment.
-const TEST_RPC_URL = "https://solana-devnet.g.alchemy.com/v2/FyJVAXC6zO6bCojXG19p-sCpQaXWexBi";
+const TEST_RPC_URL = process.env.SOLANA_RPC_URL || "https://api.devnet.solana.com";
const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay");📝 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.
| const TEST_RPC_URL = "https://solana-devnet.g.alchemy.com/v2/FyJVAXC6zO6bCojXG19p-sCpQaXWexBi"; | |
| const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay"); | |
| const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172] | |
| const TEST_RPC_URL = process.env.SOLANA_RPC_URL || "https://api.devnet.solana.com"; | |
| const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay"); | |
| const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172]; |
|
|
||
| const TEST_RPC_URL = "https://solana-devnet.g.alchemy.com/v2/FyJVAXC6zO6bCojXG19p-sCpQaXWexBi"; | ||
| const TEST_RELAY_URL = new URL("http://solana-relayer-staging.hashira.io/relay"); | ||
| const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172] |
There was a problem hiding this comment.
Securely handle private keys.
Similar to the previous file, hardcoding private keys in source code is a significant security risk.
-const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172]
+// Use environment variables for private keys
+const PRIV = process.env.TEST_PRIVATE_KEY ?
+ Uint8Array.from(JSON.parse(process.env.TEST_PRIVATE_KEY)) :
+ new Uint8Array(64).fill(1); // Default test key for CI📝 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.
| const PRIV = [73, 87, 221, 5, 63, 180, 104, 26, 64, 41, 225, 50, 165, 84, 157, 74, 187, 105, 53, 112, 214, 236, 175, 55, 86, 247, 214, 120, 101, 90, 62, 178, 103, 156, 200, 13, 24, 181, 121, 93, 15, 85, 202, 164, 4, 30, 165, 77, 244, 66, 207, 78, 179, 255, 45, 233, 17, 131, 203, 187, 120, 110, 176, 172] | |
| // Use environment variables for private keys | |
| const PRIV = process.env.TEST_PRIVATE_KEY ? | |
| Uint8Array.from(JSON.parse(process.env.TEST_PRIVATE_KEY)) : | |
| new Uint8Array(64).fill(1); // Default test key for CI |
| }; | ||
|
|
||
| // Main swap function to handle swap click | ||
| const handleSwap = async () => { |
There was a problem hiding this comment.
HandleSwap function not invoked
The handleSwap function is defined but never called, leading to the static-analysis error. Replace the inline button click handler with handleSwap.
- <button
- onClick={() => {
- console.log({
- isConnected,
- address,
- connection,
- walletProvider,
- });
- }}
+ <button
+ onClick={handleSwap}
className={`w-full p-2 cursor-pointer text-white rounded-xl ${
loading
? 'bg-gray-700 cursor-not-allowed'
: 'bg-gray-900 hover:bg-gray-700'
}`}
disabled={loading}
>
{loading ? 'Processing...' : 'Swap'}
</button>📝 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.
| const handleSwap = async () => { | |
| <button | |
| onClick={handleSwap} | |
| className={`w-full p-2 cursor-pointer text-white rounded-xl ${ | |
| loading | |
| ? 'bg-gray-700 cursor-not-allowed' | |
| : 'bg-gray-900 hover:bg-gray-700' | |
| }`} | |
| disabled={loading} | |
| > | |
| {loading ? 'Processing...' : 'Swap'} | |
| </button> |
🧰 Tools
🪛 ESLint
[error] 164-164: 'handleSwap' is assigned a value but never used.
(@typescript-eslint/no-unused-vars)
| BitcoinNetwork.Testnet, | ||
| 'https://mempool.space/testnet4/api/' | ||
| ); | ||
| btcWallet = BitcoinWallet.fromPrivateKey("02438b87e7153f29c954b21d9019118fc40d88a51943a7b5e19e82a32a308206", bitcoinProvider); |
There was a problem hiding this comment.
Protect private keys using environment variables.
Avoid hardcoding private keys in source code to prevent leaks or unauthorized access. Store sensitive data in environment variables or a secure vault:
- btcWallet = BitcoinWallet.fromPrivateKey("02438b87e7153f29c954b21d9019118fc40d88a51943a7b5e19e82a32a308206", bitcoinProvider);
+ btcWallet = BitcoinWallet.fromPrivateKey(process.env.TEST_BTC_PRIVATE_KEY || "<fallback_key>", bitcoinProvider);📝 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.
| btcWallet = BitcoinWallet.fromPrivateKey("02438b87e7153f29c954b21d9019118fc40d88a51943a7b5e19e82a32a308206", bitcoinProvider); | |
| btcWallet = BitcoinWallet.fromPrivateKey(process.env.TEST_BTC_PRIVATE_KEY || "<fallback_key>", bitcoinProvider); |
|
/release-beta |
|
/release-beta |
…ance approval from maxint to swap amount
|
/release-beta |
|
/release-beta |
|
/release-beta |
|
/release-beta |
|
/release-beta |
…ds and update IDL for new structure
|
/release-beta |
…nclude initiator's public key in transaction accounts
|
/release-beta |
|
/release-beta |
1 similar comment
|
/release-beta |
Summary by CodeRabbit