feat: support FoxWallet #514
Conversation
WalkthroughThe changes introduce two new wallet strategies, Changes
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
packages/wallet-ts/src/strategies/wallet-strategy/WalletStrategy.tsOops! Something went wrong! :( ESLint: 7.32.0 Error: Error while loading rule 'jest/unbound-method': You have used a rule which requires parserServices to be generated. You must therefore provide a value for the "parserOptions.project" property for @typescript-eslint/parser. Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
# Conflicts: # packages/wallet-ts/src/strategies/wallet-strategy/WalletStrategy.ts # packages/wallets/wallet-evm/src/strategy/strategy.ts # packages/wallets/wallet-evm/src/strategy/utils/index.ts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Outside diff range and nitpick comments (21)
packages/wallet-ts/src/types/enums.ts (1)
22-23: Consider maintaining alphabetical ordering of enum entries.The new entries are added at the end, which breaks the alphabetical ordering of the enum. Consider reordering them to maintain consistency.
export enum Wallet { BitGet = 'BitGet', Cosmostation = 'cosmostation', CosmostationEth = 'cosmostation-eth', + FoxWallet = 'fox-wallet', + FoxWalletCosmos = 'fox-wallet-cosmos', Keplr = 'keplr', Leap = 'leap', Ledger = 'ledger', LedgerCosmos = 'ledger-cosmos', LedgerLegacy = 'ledger-legacy', Magic = 'magic', Metamask = 'metamask', Ninji = 'ninji', OkxWallet = 'okx-wallet', Phantom = 'phantom', PrivateKey = 'private-key', Torus = 'torus', Trezor = 'trezor', TrustWallet = 'trust-wallet', WalletConnect = 'wallet-connect', - FoxWallet = 'fox-wallet', - FoxWalletCosmos = 'fox-wallet-cosmos', }packages/wallets/wallet-evm/src/strategy/utils/foxwallet.ts (2)
11-21: Add JSDoc documentation for better code maintainabilityConsider adding JSDoc documentation to explain the function's purpose, parameters, and return type.
+/** + * Retrieves the Fox Wallet provider, waiting for initialization if necessary. + * @param {Object} options - The options object + * @param {number} [options.timeout=3000] - Maximum time in milliseconds to wait for provider initialization + * @returns {Promise<BrowserEip1993Provider>} The Fox Wallet provider + */ export async function getFoxWalletProvider({ timeout } = { timeout: 3000 }) {
23-38: Consider adding cleanup mechanism for premature unmountsWhile the timeout cleanup is handled, there's a potential memory leak if the component unmounts before the timeout occurs. Consider adding a way to cancel the listener externally.
-async function listenForFoxWalletInitialized({ timeout } = { timeout: 3000 }) { +async function listenForFoxWalletInitialized({ timeout } = { timeout: 3000 }, abortSignal?: AbortSignal) { return new Promise((resolve) => { + if (abortSignal?.aborted) { + resolve(null); + return; + } + const handleInitialization = () => { resolve(getFoxWalletFromWindow()) } + const cleanup = () => { + $window.removeEventListener('foxwallet#initialized', handleInitialization) + resolve(null) + } + + abortSignal?.addEventListener('abort', cleanup) $window.addEventListener('foxwallet#initialized', handleInitialization, { once: true, }) - setTimeout(() => { - $window.removeEventListener('foxwallet#initialized', handleInitialization) - resolve(null) - }, timeout) + setTimeout(cleanup, timeout) }) }packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/utils.ts (3)
1-6: Consider adding runtime type checking for window accessWhile the type casting provides compile-time safety, consider adding runtime checks to ensure type safety when accessing window properties.
-const $window = (isServerSide() - ? {} - : window) as unknown as WindowWithEip1193Provider +const $window = (() => { + if (isServerSide()) { + return {} as WindowWithEip1193Provider; + } + if (typeof window === 'undefined') { + return {} as WindowWithEip1193Provider; + } + return window as unknown as WindowWithEip1193Provider; +})();
8-18: Add JSDoc documentation and explicit return typeThe function would benefit from documentation explaining the timeout parameter and its behavior.
+/** + * Attempts to get the FoxWallet provider, waiting for initialization if necessary. + * @param {Object} options - Configuration options + * @param {number} [options.timeout=3000] - Maximum time in milliseconds to wait for provider initialization + * @returns {Promise<BrowserEip1993Provider>} The FoxWallet provider or null if not available + */ -export async function getFoxWalletProvider({ timeout } = { timeout: 3000 }) { +export async function getFoxWalletProvider({ timeout } = { timeout: 3000 }): Promise<BrowserEip1993Provider | null> {
1-66: Consider adding retry mechanism and logging for better debuggingThe current implementation could benefit from:
- A retry mechanism for temporary provider unavailability
- Debug logging to help troubleshoot wallet detection issues
- Metrics collection for timeout frequency and initialization success rate
This would improve the reliability and maintainability of the wallet integration.
Would you like me to provide an example implementation with these improvements?
packages/wallets/wallet-strategy/src/strategy/WalletStrategy.ts (1)
93-97: Consider documenting wallet integration patternsThe FoxWallet implementation correctly follows the established patterns. To maintain this consistency for future wallet integrations, consider documenting:
- The required steps for adding new EVM wallets
- The standard argument structure
- The error handling patterns
packages/wallets/wallet-evm/src/strategy/strategy.ts (2)
364-365: Consider refactoring provider selection for better maintainabilityThe growing chain of ternary operators makes the code harder to maintain. Consider refactoring to a more maintainable pattern, such as a map of wallet types to provider functions.
Here's a suggested refactor:
- private async getEthereum(): Promise<BrowserEip1993Provider> { - const provider = - this.wallet === Wallet.Metamask - ? await getMetamaskProvider() - : this.wallet === Wallet.Phantom - ? await getPhantomProvider() - : this.wallet === Wallet.BitGet - ? await getBitGetProvider() - : this.wallet === Wallet.OkxWallet - ? await getOkxWalletProvider() - : this.wallet === Wallet.TrustWallet - ? await getTrustWalletProvider() - : this.wallet === Wallet.FoxWallet - ? await getFoxWalletProvider() - : undefined + private readonly walletProviders = { + [Wallet.Metamask]: getMetamaskProvider, + [Wallet.Phantom]: getPhantomProvider, + [Wallet.BitGet]: getBitGetProvider, + [Wallet.OkxWallet]: getOkxWalletProvider, + [Wallet.TrustWallet]: getTrustWalletProvider, + [Wallet.FoxWallet]: getFoxWalletProvider, + } + + private async getEthereum(): Promise<BrowserEip1993Provider> { + const getProvider = this.walletProviders[this.wallet] + const provider = getProvider ? await getProvider() : undefined if (!provider) { throw this.EvmWalletException( new Error(`Please install the ${this.wallet} wallet extension.`), { code: UnspecifiedErrorCode, type: ErrorType.WalletNotInstalledError, contextModule: WalletAction.GetAccounts, }, ) } return provider }
Line range hint
74-98: Add FoxWallet case in EvmWalletException methodThe
EvmWalletExceptionmethod needs to handleWallet.FoxWalletspecifically, similar to how other wallets are handled.Add the following case:
public EvmWalletException( error: Error, context?: ErrorContext, ): ThrownException { if (this.wallet === Wallet.Metamask) { return new MetamaskException(error, context) } if (this.wallet === Wallet.BitGet) { return new BitGetException(error, context) } if (this.wallet === Wallet.OkxWallet) { return new OkxWalletException(error, context) } if (this.wallet === Wallet.Phantom) { return new MetamaskException(error, context) } if (this.wallet === Wallet.TrustWallet) { return new TrustWalletException(error, context) } + if (this.wallet === Wallet.FoxWallet) { + return new FoxWalletException(error, context) + } return new WalletException(error, context) }packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/foxwallet/index.ts (5)
43-43: Improve error handling by avoiding casting errors toanyIn several catch blocks, errors are cast to
anyto access themessageproperty:throw new CosmosWalletException(new Error((e as any).message))Casting to
anyundermines type safety. Instead, you can check ifeis an instance ofErrorand handle it accordingly. Ifeis not anError, you can convert it to a string to capture the error message.Apply the following changes to improve error handling:
- throw new CosmosWalletException(new Error((e as any).message)) + throw new CosmosWalletException(e instanceof Error ? e : new Error(String(e)))This approach maintains type safety and ensures that meaningful error messages are propagated.
Also applies to: 54-54, 72-72, 87-87, 125-125, 163-163
95-95: Fix grammatical errors in commentsThere are minor grammatical errors in the code comments:
- Line 95: "after its broadcasted" should be "after it's broadcasted" or "after it has been broadcasted."
- Line 134: "Since it uses the
'Block'mode, and it will wait..." The word "and" is unnecessary and can be removed for clarity.Apply this diff to correct the typos:
- * so we have to make sure the transaction is included in a block after its broadcasted + * so we have to make sure the transaction is included in a block after it's broadcasted - * Since it uses the `Block` mode, and it will wait for the transaction to be included in a block, + * Since it uses the `Block` mode, it will wait for the transaction to be included in a block,Also applies to: 134-134
101-130: Refactor duplicate code inbroadcastTxmethodsThe
broadcastTxandbroadcastTxBlockmethods share similar logic, differing only in theBroadcastModeused (Syncvs.Block). Refactoring them into a single method will reduce code duplication and enhance maintainability.You can create a private method to handle transaction broadcasting:
private async broadcastTransaction( txRaw: CosmosTxV1Beta1Tx.TxRaw, mode: BroadcastMode ): Promise<string> { const { chainId } = this const foxwallet = await this.getFoxWallet() try { const result = await foxwallet.sendTx( chainId, CosmosTxV1Beta1Tx.TxRaw.encode(txRaw).finish(), mode, ) if (!result || result.length === 0) { throw new TransactionException( new Error('Transaction failed to be broadcasted'), { contextModule: 'FoxWallet' }, ) } return Buffer.from(result).toString('hex') } catch (e) { if (e instanceof TransactionException) { throw e } throw new CosmosWalletException( e instanceof Error ? e : new Error(String(e)), { context: 'broadcast-tx', contextModule: 'FoxWallet', }, ) } } async broadcastTx(txRaw: CosmosTxV1Beta1Tx.TxRaw): Promise<string> { return this.broadcastTransaction(txRaw, BroadcastMode.Sync) } async broadcastTxBlock(txRaw: CosmosTxV1Beta1Tx.TxRaw): Promise<string> { return this.broadcastTransaction(txRaw, BroadcastMode.Block) }This refactoring consolidates the common code and makes it easier to manage any future changes.
Also applies to: 139-168
85-85: Simplify unnecessary double type assertionIn
getOfflineSigner(), there is an unnecessary double type assertion:) as unknown as OfflineDirectSignerSince
getOfflineSigner(chainId)likely returns a compatible type, you can simplify the assertion:- ) as unknown as OfflineDirectSigner + ) as OfflineDirectSignerEnsure that the
getOfflineSignermethod returns the correct type or adjust the types appropriately.
186-195: Review the check for$windowbeing falsyThe check for
$windowbeing falsy may not be necessary:if (!$window) { throw new CosmosWalletException( new Error('Please install FoxWallet extension'), { code: UnspecifiedErrorCode, type: ErrorType.WalletNotInstalledError, contextModule: 'FoxWallet', }, ) }Since
$windowis initially assigned as an object ({}) whenwindowis undefined, this condition will never betrue.Consider removing the check or modifying it to accurately reflect the intended condition:
- if (!$window) { + if (typeof window === 'undefined') {Alternatively, since you handle the absence of
foxwallet?.cosmosin the next check, this initial check might be redundant and can be safely removed.packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/index.ts (2)
45-45: Simplify the return statement ingetWalletDeviceTypeSince the method is
async, you can directly return the value without wrapping it inPromise.resolve().Apply this diff to simplify the return statement:
- return Promise.resolve(WalletDeviceType.Browser) + return WalletDeviceType.Browser
74-78: Simplify the return statement ingetSessionOrConfirmSince the method is
async, you can directly return the value without usingPromise.resolve().Apply this diff to simplify the return statement:
- return Promise.resolve( - `0x${Buffer.from( - `Confirmation for ${address} at time: ${Date.now()}`, - ).toString('hex')}`, - ) + return `0x${Buffer.from( + `Confirmation for ${address} at time: ${Date.now()}`, + ).toString('hex')}`packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/index.ts (5)
30-32: Remove unnecessary constructorThe constructor does not add any additional logic beyond calling the superclass's constructor and can be safely removed to simplify the code.
Apply this diff to remove the unnecessary constructor:
- constructor(args: EthereumWalletStrategyArgs) { - super(args) - }🧰 Tools
🪛 Biome
[error] 30-32: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
34-36: Simplify return statement by removingPromise.resolveSince the method is declared as
async, you can return the value directly without wrapping it inPromise.resolve.Apply this diff to simplify the return statement:
- return Promise.resolve(WalletDeviceType.Browser) + return WalletDeviceType.Browser
38-40: Simplify return statement by removingPromise.resolveAs this method is
async, you can return the boolean value directly withoutPromise.resolve.Apply this diff to simplify the return statement:
- return Promise.resolve(true) + return true
72-78: Simplify return statement by removingPromise.resolveSince the method is declared as
async, you can return the value directly without wrapping it inPromise.resolve.Apply this diff to simplify the return statement:
- return Promise.resolve( - `0x${Buffer.from( - `Confirmation for ${address} at time: ${Date.now()}`, - ).toString('hex')}`, - ) + return `0x${Buffer.from( + `Confirmation for ${address} at time: ${Date.now()}`, + ).toString('hex')}`
255-259: Provide more context in the error messageWhen throwing an exception in
getPubKey, consider providing additional context or guidance on how to obtain the public key if needed.Optionally, you can include suggestions in the error message:
throw new WalletException( - new Error('You can only fetch PubKey from Cosmos native wallets'), + new Error('Public keys are only available from Cosmos native wallets. Please use a compatible wallet to access the public key.'), )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (15)
packages/wallet-ts/src/strategies/wallet-strategy/WalletStrategy.ts(2 hunks)packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/index.ts(1 hunks)packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/utils.ts(1 hunks)packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/foxwallet/index.ts(1 hunks)packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/index.ts(1 hunks)packages/wallet-ts/src/strategies/wallet-strategy/types.ts(2 hunks)packages/wallet-ts/src/strategies/wallet-strategy/utils.ts(1 hunks)packages/wallet-ts/src/types/enums.ts(1 hunks)packages/wallets/wallet-base/src/types/enums.ts(1 hunks)packages/wallets/wallet-base/src/types/provider.ts(2 hunks)packages/wallets/wallet-base/src/utils/wallet.ts(1 hunks)packages/wallets/wallet-evm/src/strategy/strategy.ts(3 hunks)packages/wallets/wallet-evm/src/strategy/utils/foxwallet.ts(1 hunks)packages/wallets/wallet-evm/src/strategy/utils/index.ts(1 hunks)packages/wallets/wallet-strategy/src/strategy/WalletStrategy.ts(1 hunks)
🧰 Additional context used
🪛 Biome
packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/index.ts
[error] 30-32: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (20)
packages/wallets/wallet-base/src/types/provider.ts (2)
10-10: LGTM! Property addition follows established patterns.
The isFoxWallet boolean flag is consistent with other wallet identification properties in the interface.
20-20: LGTM! Verify property name matches FoxWallet's documentation.
The foxwallet provider injection follows the same pattern as other supported wallets.
Let's verify the property name matches FoxWallet's official documentation:
✅ Verification successful
The initial search shows extensive usage of foxwallet property in the codebase, but let's verify the documentation and actual implementation to confirm the property name matches FoxWallet's conventions.
The previous searches didn't yield documentation, so let's examine the actual implementation to verify how the provider is detected and accessed in the code.
Property name foxwallet.ethereum is correctly implemented and matches FoxWallet's injection pattern
The implementation shows consistent usage of foxwallet.ethereum across the codebase:
- The provider detection code in
getFoxWalletFromWindow()specifically checks for$window.foxwallet?.ethereum - The property is used consistently in both wallet-evm and wallet-ts packages
- The code handles both direct injection (
foxwallet.ethereum) and legacy injection (viaethereum.isFoxWallet) patterns
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for FoxWallet provider documentation references
# Expected: Find references confirming the correct property name
# Search for FoxWallet documentation or examples in the codebase
rg -i "foxwallet" -g "*.md" -g "*.txt" -g "*.js" -g "*.ts"
# Search for any test files that might verify the provider detection
rg -i "foxwallet.*provider" -g "*test*.ts" -g "*spec*.ts"
Length of output: 13702
Script:
#!/bin/bash
# Let's search for FoxWallet's documentation or examples in the repository
rg -i "foxwallet" -g "*.md" -g "README*" -g "CHANGELOG*" -g "docs/*"
# Let's also check the implementation to see how the provider is accessed
ast-grep --pattern 'foxwallet?.ethereum'
Length of output: 111
Script:
#!/bin/bash
# Let's check the actual implementation of FoxWallet provider detection
rg -A 5 "getFoxWalletFromWindow"
# Let's also check for any window property access patterns
ast-grep --pattern '$window.foxwallet'
Length of output: 4448
packages/wallet-ts/src/strategies/wallet-strategy/utils.ts (2)
18-18: Verify EIP-712 compatibility for FoxWallet
FoxWallet is added as an Ethereum wallet but not included in isEip712V2OnlyWallet. Please verify if FoxWallet supports other EIP-712 versions or if it should be restricted to v2 only.
18-18: Clarify multi-chain support architecture
The PR objectives mention FoxWallet as a multi-chain wallet, and the AI summary indicates a separate FoxWalletCosmos class exists. However, adding FoxWallet to isEthWallet means isCosmosWallet(Wallet.FoxWallet) will return false. Please clarify if this is the intended architecture for handling multi-chain support.
packages/wallets/wallet-base/src/utils/wallet.ts (3)
18-18: LGTM! Addition of FoxWallet to Ethereum wallets list.
The change correctly identifies FoxWallet as an Ethereum-compatible wallet.
18-18: Verify EIP-712 v2 support for FoxWallet.
Since some wallets only support EIP-712 v2, we should verify if FoxWallet needs to be added to the isEip712V2OnlyWallet list.
18-18: Consider multi-chain support implications.
Since FoxWallet is described as a multi-chain wallet compatible with Injective protocol, should it also be added to the COSMOS_WALLETS array? This would ensure proper handling of both Ethereum and Cosmos transactions.
Also applies to: 34-41
packages/wallet-ts/src/types/enums.ts (1)
22-23: LGTM! The new wallet entries follow the established naming conventions.
The new entries FoxWallet and FoxWalletCosmos are well-integrated and follow the project's conventions:
- Uses hyphenated values consistent with other entries
- Follows the established pattern for Cosmos variants
packages/wallets/wallet-base/src/types/enums.ts (1)
23-24: LGTM! Verify string literal usage.
The new enum entries follow the established naming patterns and correctly represent both EVM and Cosmos variants of FoxWallet.
Let's verify the consistent usage of these string literals across the codebase:
✅ Verification successful
String literals are used consistently across the codebase
The verification shows that:
- The enum values
'fox-wallet'and'fox-wallet-cosmos'are only used in the enum definition - All references to FoxWallet in the codebase use the proper enum values through
Wallet.FoxWalletandWallet.FoxWalletCosmos - The implementation includes proper strategy handling for both variants in switch statements
- No hardcoded string literals are used where the enum should be used
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the wallet string literals are used consistently
# Check for any hardcoded string literals that should use the enum
rg -i "fox.?wallet" --type ts
# Check for proper enum usage in strategy creation
ast-grep --pattern 'case Wallet.FoxWallet: $$$'
ast-grep --pattern 'case Wallet.FoxWalletCosmos: $$$'
Length of output: 13721
packages/wallet-ts/src/strategies/wallet-strategy/types.ts (2)
12-12: LGTM! Property follows established pattern
The isFoxWallet boolean property follows the same pattern as other wallet detection flags in the interface.
22-22: LGTM! Window extension properly typed
The foxwallet property is correctly structured with optional chaining and proper typing, consistent with other wallet provider implementations.
packages/wallets/wallet-evm/src/strategy/utils/foxwallet.ts (2)
1-10: LGTM: Proper setup for cross-environment compatibility
The imports and window setup are well-structured with appropriate type safety and server-side rendering consideration.
51-57: Verify Fox Wallet provider authenticity
The current implementation trusts the isFoxWallet flag without additional verification. Consider adding checks to verify the authenticity of the provider to prevent potential security risks from malicious providers.
packages/wallets/wallet-strategy/src/strategy/WalletStrategy.ts (1)
93-97: LGTM! FoxWallet implementation follows established patterns
The implementation correctly uses EvmWalletStrategy and follows the same pattern as other EVM wallets, ensuring consistency in the codebase.
packages/wallets/wallet-evm/src/strategy/strategy.ts (2)
43-43: LGTM: Import follows established patterns
The addition of getFoxWalletProvider follows the existing import pattern and naming convention.
52-52:
Add FoxWalletException class for consistent error handling
While Wallet.FoxWallet is added to supported wallets, there's no corresponding FoxWalletException class in the error handling section (like other wallets have MetamaskException, BitGetException, etc.). This should be added to maintain consistent error handling across all wallet types.
Let's verify the error handling for other wallets:
packages/wallet-ts/src/strategies/wallet-strategy/WalletStrategy.ts (2)
106-109: Implementation follows established patterns correctly
The integration of FoxWallet and FoxWalletCosmos follows the existing patterns:
- FoxWallet uses ethWalletArgs like other EVM wallets
- FoxWalletCosmos uses full args like other Cosmos wallets
Let's verify the wallet type definitions and utility functions:
#!/bin/bash
# Description: Verify wallet enum types and utility functions
# Expected: FoxWallet and FoxWalletCosmos should be defined in enums.js
# and properly categorized in utils.js
# Check wallet enum definitions
rg -A 2 "export enum Wallet" "packages/wallet-ts/src/types/enums.js"
# Check wallet type categorization
rg "FoxWallet" "packages/wallet-ts/src/strategies/wallet-strategy/utils.js"22-23: Verify the existence of FoxWallet strategy files
The imports look consistent with the project's structure. Let's verify that the referenced strategy files exist.
✅ Verification successful
Import paths for FoxWallet strategy files are correct and files exist
The verification confirms that both strategy files exist in the correct locations:
packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/index.tspackages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/index.ts
The import paths in the code match the actual file structure, following the same pattern as other wallet implementations in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the FoxWallet strategy files exist
# Expected: Both files should exist in the strategies directory
fd -t f "FoxWallet/index.js" "packages/wallet-ts/src/strategies"
fd -t f "FoxWalletCosmos/index.js" "packages/wallet-ts/src/strategies"
Length of output: 319
Script:
#!/bin/bash
# Let's try searching with .ts extension and broader path patterns
fd -t f "FoxWallet/index.ts" "packages/wallet-ts"
fd -t f "FoxWalletCosmos/index.ts" "packages/wallet-ts"
# Let's also check if these files exist in the wallet-strategy directory
ls -la packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/
ls -la packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/
# Let's verify the import paths by checking the directory structure
tree packages/wallet-ts/src/strategies/wallet-strategy/
Length of output: 2005
packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWalletCosmos/foxwallet/index.ts (1)
2-2: Verify compatibility between Keplr types and FoxWallet implementation
The import statement aliases Keplr as Fox:
import type { Keplr as Fox } from '@keplr-wallet/types'This assumes that FoxWallet's API is fully compatible with Keplr's types. If FoxWallet does not implement all methods and properties defined in the Keplr interface, this could lead to runtime errors or type mismatches. Ensure that FoxWallet's cosmos API conforms to the Keplr type definitions or consider defining specific types for FoxWallet to accurately represent its API.
packages/wallet-ts/src/strategies/wallet-strategy/strategies/FoxWallet/index.ts (1)
106-112: Ensure endpoints.grpc is defined before using it
While you check if endpoints is defined, endpoints.grpc might still be undefined, which could lead to runtime errors.
Please verify that endpoints.grpc is always defined when endpoints is provided. Consider adding a check for endpoints.grpc:
if (!endpoints || !endpoints.grpc) {
throw new WalletException(
new Error(
'You have to pass endpoints with a valid grpc endpoint within the options for using Ethereum native wallets',
),
)
}| @@ -3,3 +3,4 @@ export { getBitGetProvider } from './bitget.js' | |||
| export { getPhantomProvider } from './phantom.js' | |||
| export { getMetamaskProvider } from './metamask.js' | |||
| export { getTrustWalletProvider } from './trustWallet.js' | |||
| export { getFoxWalletProvider } from './foxwallet.js' | |||
There was a problem hiding this comment.
💡 Codebase verification
The export statement needs to be updated to .ts extension
The file being imported exists as foxwallet.ts, but the import statement incorrectly uses .js extension. The export statement should be:
export { getFoxWalletProvider } from './foxwallet.ts'packages/wallets/wallet-evm/src/strategy/utils/foxwallet.tsexists and contains thegetFoxWalletProviderfunction- Current import uses
.jsextension:from './foxwallet.js' - All other files in the directory are
.tsfiles
🔗 Analysis chain
LGTM! Verify foxwallet.js exists
The export statement follows the established pattern for wallet providers and is appropriately placed.
Let's verify the existence and content of the referenced file:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and basic structure of foxwallet.js
# Check if the file exists in the same directory
fd -p "wallet-evm/src/strategy/utils/foxwallet.js$"
# Verify the exported function exists
rg -A 5 "export function getFoxWalletProvider"
Length of output: 102
Script:
#!/bin/bash
# Let's try a broader search to understand the structure
# 1. Find any file with "fox" in the name
fd -t f "fox" packages/wallets/wallet-evm
# 2. Check the directory structure of the wallet-evm package
fd . packages/wallets/wallet-evm/src/strategy/utils
# 3. Look for any references to FoxWallet in the codebase
rg -i "foxwallet" packages/wallets/wallet-evm
Length of output: 2530
| function getFoxWalletFromWindow() { | ||
| const injectedProviderExist = | ||
| typeof window !== 'undefined' && | ||
| (typeof $window.ethereum !== 'undefined' || | ||
| typeof $window.foxwallet !== 'undefined') | ||
|
|
||
| // No injected providers exist. | ||
| if (!injectedProviderExist) { | ||
| return | ||
| } | ||
|
|
||
| if ($window.foxwallet?.ethereum) { | ||
| return $window.foxwallet?.ethereum | ||
| } | ||
|
|
||
| if ($window.ethereum.isFoxWallet) { | ||
| return $window.ethereum | ||
| } | ||
|
|
||
| if ($window.providers) { | ||
| return $window.providers.find((p) => p.isFoxWallet) | ||
| } | ||
|
|
||
| return | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Type safety suggestion can be improved
The review suggestion is good but needs refinement. All wallet utilities in the codebase already import and use the BrowserEip1993Provider type, and the pattern of checking $window.providers.find() is consistently used across all wallet implementations. However, the array check is still a valid concern.
if ($window.providers) {
- return $window.providers.find((p) => p.isFoxWallet)
+ return Array.isArray($window.providers)
+ ? $window.providers.find((p) => p.isFoxWallet)
+ : undefined
}🔗 Analysis chain
Enhance type safety and array verification
While the provider detection logic is thorough, consider these improvements:
- Add type safety for the providers array
- Verify array access before using find
if ($window.providers) {
- return $window.providers.find((p) => p.isFoxWallet)
+ return Array.isArray($window.providers)
+ ? $window.providers.find((p: BrowserEip1993Provider) => p.isFoxWallet)
+ : undefined
}Let me check the implementation patterns in other wallet utilities to make a more informed decision.
Let me check if there are any type definitions or interfaces used in these wallet utilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify if there are any other wallet implementations that might have similar provider detection patterns
rg -l "providers?.find\(" --type ts
Length of output: 723
Script:
#!/bin/bash
# Check implementation patterns in other wallet utilities
rg "providers?.find\(" -A 5 --type ts
Length of output: 4214
Script:
#!/bin/bash
# Check for type definitions and interfaces
rg "type|interface.*Provider" --type ts packages/wallets/wallet-evm/src/
rg "BrowserEip1993Provider" --type ts
Length of output: 11490
| function getFoxWalletFromWindow() { | ||
| const injectedProviderExist = | ||
| typeof window !== 'undefined' && | ||
| (typeof $window.ethereum !== 'undefined' || | ||
| typeof $window.foxwallet !== 'undefined') | ||
|
|
||
| // No injected providers exist. | ||
| if (!injectedProviderExist) { | ||
| return | ||
| } | ||
|
|
||
| if ($window.foxwallet?.ethereum) { | ||
| return $window.foxwallet.ethereum | ||
| } | ||
|
|
||
| if ($window.ethereum.isFoxWallet) { | ||
| return $window.ethereum | ||
| } | ||
|
|
||
| if ($window.providers) { | ||
| return $window.providers.find((p) => p.isFoxWallet) | ||
| } | ||
|
|
||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance type safety and return consistency
The function could benefit from improved type safety and consistent return types.
-function getFoxWalletFromWindow() {
+function getFoxWalletFromWindow(): BrowserEip1993Provider | undefined {
const injectedProviderExist =
- typeof window !== 'undefined' &&
- (typeof $window.ethereum !== 'undefined' ||
+ (typeof $window.ethereum !== 'undefined' ||
typeof $window.foxwallet !== 'undefined')
// No injected providers exist.
if (!injectedProviderExist) {
return
}
if ($window.foxwallet?.ethereum) {
return $window.foxwallet.ethereum
}
if ($window.ethereum?.isFoxWallet) {
return $window.ethereum
}
- if ($window.providers) {
+ if (Array.isArray($window.providers)) {
return $window.providers.find((p) => p.isFoxWallet)
}
return
}📝 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.
| function getFoxWalletFromWindow() { | |
| const injectedProviderExist = | |
| typeof window !== 'undefined' && | |
| (typeof $window.ethereum !== 'undefined' || | |
| typeof $window.foxwallet !== 'undefined') | |
| // No injected providers exist. | |
| if (!injectedProviderExist) { | |
| return | |
| } | |
| if ($window.foxwallet?.ethereum) { | |
| return $window.foxwallet.ethereum | |
| } | |
| if ($window.ethereum.isFoxWallet) { | |
| return $window.ethereum | |
| } | |
| if ($window.providers) { | |
| return $window.providers.find((p) => p.isFoxWallet) | |
| } | |
| return | |
| } | |
| function getFoxWalletFromWindow(): BrowserEip1993Provider | undefined { | |
| const injectedProviderExist = | |
| (typeof $window.ethereum !== 'undefined' || | |
| typeof $window.foxwallet !== 'undefined') | |
| // No injected providers exist. | |
| if (!injectedProviderExist) { | |
| return | |
| } | |
| if ($window.foxwallet?.ethereum) { | |
| return $window.foxwallet.ethereum | |
| } | |
| if ($window.ethereum?.isFoxWallet) { | |
| return $window.ethereum | |
| } | |
| if (Array.isArray($window.providers)) { | |
| return $window.providers.find((p) => p.isFoxWallet) | |
| } | |
| return | |
| } |
| async function listenForFoxWalletInitialized( | ||
| { timeout } = { timeout: 3000 }, | ||
| ) { | ||
| return new Promise((resolve) => { | ||
| const handleInitialization = () => { | ||
| resolve(getFoxWalletFromWindow()) | ||
| } | ||
|
|
||
| $window.addEventListener('foxwallet#initialized', handleInitialization, { | ||
| once: true, | ||
| }) | ||
|
|
||
| setTimeout(() => { | ||
| $window.removeEventListener( | ||
| 'foxwallet#initialized', | ||
| handleInitialization, | ||
| ) | ||
| resolve(null) | ||
| }, timeout) | ||
| }) | ||
| } |
There was a problem hiding this comment.
Improve error handling and prevent potential memory leaks
The current implementation has several potential issues:
- No error handling for invalid event data
- Possible memory leak if timeout occurs after event resolution
- No Promise rejection for error cases
async function listenForFoxWalletInitialized(
{ timeout } = { timeout: 3000 },
) {
- return new Promise((resolve) => {
+ return new Promise((resolve, reject) => {
+ let isResolved = false;
const handleInitialization = () => {
- resolve(getFoxWalletFromWindow())
+ if (isResolved) return;
+ isResolved = true;
+ try {
+ const provider = getFoxWalletFromWindow();
+ resolve(provider);
+ } catch (error) {
+ reject(error);
+ }
}
$window.addEventListener('foxwallet#initialized', handleInitialization, {
once: true,
})
- setTimeout(() => {
+ const timeoutId = setTimeout(() => {
+ if (isResolved) return;
+ isResolved = true;
$window.removeEventListener(
'foxwallet#initialized',
handleInitialization,
)
resolve(null)
}, timeout)
})
}📝 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.
| async function listenForFoxWalletInitialized( | |
| { timeout } = { timeout: 3000 }, | |
| ) { | |
| return new Promise((resolve) => { | |
| const handleInitialization = () => { | |
| resolve(getFoxWalletFromWindow()) | |
| } | |
| $window.addEventListener('foxwallet#initialized', handleInitialization, { | |
| once: true, | |
| }) | |
| setTimeout(() => { | |
| $window.removeEventListener( | |
| 'foxwallet#initialized', | |
| handleInitialization, | |
| ) | |
| resolve(null) | |
| }, timeout) | |
| }) | |
| } | |
| async function listenForFoxWalletInitialized( | |
| { timeout } = { timeout: 3000 }, | |
| ) { | |
| return new Promise((resolve, reject) => { | |
| let isResolved = false; | |
| const handleInitialization = () => { | |
| if (isResolved) return; | |
| isResolved = true; | |
| try { | |
| const provider = getFoxWalletFromWindow(); | |
| resolve(provider); | |
| } catch (error) { | |
| reject(error); | |
| } | |
| } | |
| $window.addEventListener('foxwallet#initialized', handleInitialization, { | |
| once: true, | |
| }) | |
| const timeoutId = setTimeout(() => { | |
| if (isResolved) return; | |
| isResolved = true; | |
| $window.removeEventListener( | |
| 'foxwallet#initialized', | |
| handleInitialization, | |
| ) | |
| resolve(null) | |
| }, timeout) | |
| }) | |
| } |
| case Wallet.FoxWallet: | ||
| return new EvmWalletStrategy({ | ||
| ...ethWalletArgs, | ||
| wallet: Wallet.FoxWallet, | ||
| }) |
There was a problem hiding this comment.
💡 Codebase verification
FoxWallet is not registered in wallet-base package
FoxWallet needs to be added to both:
Walletenum inpackages/wallets/wallet-base/src/types/enums.tsisEthWalletfunction inpackages/wallets/wallet-base/src/utils/wallet.ts
🔗 Analysis chain
Verify FoxWallet registration in wallet-base package
Ensure that FoxWallet is properly registered in the wallet-base package's isEthWallet function to maintain consistent wallet type checking.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify FoxWallet registration in wallet-base package
# Check if FoxWallet is properly registered in isEthWallet function
rg -A 5 "isEthWallet.*=.*" packages/wallets/wallet-base/src
# Check if FoxWallet is defined in Wallet enum
rg -A 10 "export enum Wallet" packages/wallets/wallet-base/src
Length of output: 2223
| try { | ||
| return await ethereum.request({ | ||
| method: 'eth_signTypedData_v4', | ||
| params: [eip712json, address], |
There was a problem hiding this comment.
Correct parameter order for eth_signTypedData_v4
The parameters for eth_signTypedData_v4 should be [address, typedData], but they are currently [typedData, address], which will cause incorrect behavior.
Apply this diff to fix the parameter order:
method: 'eth_signTypedData_v4',
- params: [eip712json, address],
+ params: [address, eip712json],📝 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.
| params: [eip712json, address], | |
| params: [address, eip712json], |
| try { | ||
| const signature = await ethereum.request({ | ||
| method: 'personal_sign', | ||
| params: [toUtf8(data), signer], |
There was a problem hiding this comment.
Convert data to a hex string for personal_sign
The personal_sign method expects the message parameter to be a hex-encoded string. Currently, toUtf8(data) returns a Uint8Array. You should convert it to a hex string prefixed with 0x.
Apply this diff to convert the data to a hex string:
- params: [toUtf8(data), signer],
+ params: [`0x${Buffer.from(data).toString('hex')}`, signer],Ensure that data is properly converted to a hex 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.
| params: [toUtf8(data), signer], | |
| params: [`0x${Buffer.from(data).toString('hex')}`, signer], |
| const transactionReceiptRetry = async () => { | ||
| const receipt = await ethereum.request({ | ||
| method: 'eth_getTransactionReceipt', | ||
| params: [txHash], | ||
| }) | ||
|
|
||
| if (!receipt) { | ||
| await sleep(interval) | ||
| await transactionReceiptRetry() | ||
| } | ||
|
|
||
| return receipt | ||
| } |
There was a problem hiding this comment.
Avoid potential stack overflow due to recursive retry logic
The transactionReceiptRetry function recursively calls itself when receipt is not found, which could lead to a stack overflow after many retries.
Refactor the function to use a loop instead of recursion:
const transactionReceiptRetry = async () => {
- const receipt = await ethereum.request({
- method: 'eth_getTransactionReceipt',
- params: [txHash],
- })
-
- if (!receipt) {
- await sleep(interval)
- await transactionReceiptRetry()
- }
-
- return receipt
+ let receipt = null
+ while (!receipt) {
+ receipt = await ethereum.request({
+ method: 'eth_getTransactionReceipt',
+ params: [txHash],
+ })
+ if (!receipt) {
+ await sleep(interval)
+ }
+ }
+ return receipt
}📝 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 transactionReceiptRetry = async () => { | |
| const receipt = await ethereum.request({ | |
| method: 'eth_getTransactionReceipt', | |
| params: [txHash], | |
| }) | |
| if (!receipt) { | |
| await sleep(interval) | |
| await transactionReceiptRetry() | |
| } | |
| return receipt | |
| } | |
| const transactionReceiptRetry = async () => { | |
| let receipt = null | |
| while (!receipt) { | |
| receipt = await ethereum.request({ | |
| method: 'eth_getTransactionReceipt', | |
| params: [txHash], | |
| }) | |
| if (!receipt) { | |
| await sleep(interval) | |
| } | |
| } | |
| return receipt | |
| } |
| async sendEthereumTransaction( | ||
| transaction: unknown, | ||
| _options: { address: AccountAddress; ethereumChainId: EthereumChainId }, | ||
| ): Promise<string> { | ||
| const ethereum = await this.getEthereum() | ||
|
|
||
| try { | ||
| return await ethereum.request({ | ||
| method: 'eth_sendTransaction', | ||
| params: [transaction], | ||
| }) | ||
| } catch (e: unknown) { | ||
| throw new CosmosWalletException(new Error((e as any).message), { | ||
| code: UnspecifiedErrorCode, | ||
| type: ErrorType.WalletError, | ||
| contextModule: WalletAction.SendEthereumTransaction, | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify the type of the transaction parameter
The transaction parameter is currently typed as unknown. Providing a specific type will improve type safety and code readability.
Please specify the appropriate type for transaction. For example:
- async sendEthereumTransaction(
- transaction: unknown,
+ async sendEthereumTransaction(
+ transaction: EthereumTransactionRequest,Replace EthereumTransactionRequest with the actual type that matches the expected transaction object.
Committable suggestion skipped: line range outside the PR's diff.
| throw new CosmosWalletException(new Error((e as any).message), { | ||
| code: UnspecifiedErrorCode, | ||
| type: ErrorType.WalletError, | ||
| contextModule: WalletAction.GetChainId, | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Use appropriate exception type for Ethereum-related errors
In Ethereum-related methods, wrapping errors in CosmosWalletException might be misleading. Consider using a more appropriate exception type like WalletException to accurately reflect the context.
Update the exception to use WalletException:
- throw new CosmosWalletException(new Error((e as any).message), {
+ throw new WalletException(new Error((e as any).message), {Apply similar changes in other Ethereum-related methods where CosmosWalletException is used.
Committable suggestion skipped: line range outside the PR's diff.
Hi, We are FoxWallet team, and we want to add supporting FoxWallet in
wallet-tsandwallet-evm.FoxWallet is multi-chain wallet that support Injective protocol. You can find more information about FoxWallet here.
Summary by CodeRabbit
New Features
FoxWalletandFoxWalletCosmos.Bug Fixes
Documentation
Refactor