-
Notifications
You must be signed in to change notification settings - Fork 177
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
devop: add litecoin api support #593
Conversation
WalkthroughThis pull request introduces several small modifications across different files in the Enkrypt extension codebase. The changes primarily focus on expanding network support, specifically adding Solana and Litecoin to the existing network configurations. Additionally, type safety improvements are made in the Bitcoin provider injection mechanism, and the Changes
Sequence DiagramsequenceDiagram
participant Client
participant EnkryptWindow
participant BitcoinProvider
Client->>EnkryptWindow: Request network information
EnkryptWindow->>BitcoinProvider: Inject provider
BitcoinProvider-->>EnkryptWindow: Confirm injection
Client->>BitcoinProvider: Get network details
BitcoinProvider-->>Client: Return network (Bitcoin/Litecoin)
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
💼 Build Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/extension/src/libs/utils/networks.ts (1)
Line range hint
1-99
: Consider adding Litecoin network constants.If we're adding Litecoin support, we should consider adding the following constants to maintain consistency with other networks:
const DEFAULT_BTC_NETWORK_NAME = NetworkNames.Bitcoin; const DEFAULT_KADENA_NETWORK_NAME = NetworkNames.Kadena; const DEFAULT_SOLANA_NETWORK_NAME = NetworkNames.Solana; +const DEFAULT_LITECOIN_NETWORK_NAME = NetworkNames.Litecoin; const DEFAULT_BTC_NETWORK = Bitcoin; const DEFAULT_KADENA_NETWORK = Kadena; const DEFAULT_SOLANA_NETWORK = Solana; +const DEFAULT_LITECOIN_NETWORK = Litecoin; // ... and in the exports: export { // ... DEFAULT_LITECOIN_NETWORK, DEFAULT_LITECOIN_NETWORK_NAME, };
🧹 Nitpick comments (6)
packages/extension/src/types/globals.ts (1)
10-10
: Consider using a more specific type instead ofany
The
unisat
property is redundant since the interface already has an index signature[key: string]: any
. If this property needs to be explicitly declared, consider using a more specific type to improve type safety.packages/extension/src/providers/bitcoin/methods/btc_getNetwork.ts (2)
30-31
: Consider using enum or constants for network identifiersThe network identifiers ('livenet', 'testnet', 'litecoin') are used as string literals. Consider defining these as constants or an enum to prevent typos and improve maintainability.
export enum NetworkIdentifier { BITCOIN = 'livenet', BITCOIN_TEST = 'testnet', LITECOIN = 'litecoin' }
32-32
: Consider explicit error handling for unknown networksThe function silently returns an empty string for unknown networks. Consider throwing an error or returning a specific error code for better error handling.
packages/extension/src/providers/bitcoin/inject.ts (2)
139-140
: Add error handling for settings accessConsider adding error handling in case the settings object or
btc
property is undefined.- if (settings.btc.injectUnisat) + if (settings?.btc?.injectUnisat)
142-142
: Consider adding null check for enkrypt providersAdd a null check before accessing the providers object to prevent potential runtime errors.
- (document as EnkryptWindow)['enkrypt']['providers'][options.name] = provider; + const enkryptWindow = document as EnkryptWindow; + if (!enkryptWindow.enkrypt?.providers) { + enkryptWindow.enkrypt = { providers: {} }; + } + enkryptWindow.enkrypt.providers[options.name] = provider;packages/extension/src/libs/utils/networks.ts (1)
82-82
: Consider adding Litecoin to POPULAR_NAMES.Since we're adding Litecoin support, consider adding it to the POPULAR_NAMES array if it's intended to be prominently displayed in the UI.
const POPULAR_NAMES = [ NetworkNames.Bitcoin, NetworkNames.Ethereum, NetworkNames.Matic, NetworkNames.Polkadot, NetworkNames.Binance, NetworkNames.Rootstock, NetworkNames.Optimism, NetworkNames.Kadena, NetworkNames.Solana, + NetworkNames.Litecoin, ];
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/extension/src/libs/utils/networks.ts
(1 hunks)packages/extension/src/providers/bitcoin/inject.ts
(1 hunks)packages/extension/src/providers/bitcoin/methods/btc_getNetwork.ts
(1 hunks)packages/extension/src/providers/bitcoin/types/index.ts
(1 hunks)packages/extension/src/types/globals.ts
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: buildAll
🔇 Additional comments (1)
packages/extension/src/providers/bitcoin/types/index.ts (1)
8-8
: Verify Litecoin network parametersThe addition of Litecoin to
BitcoinNetworks
looks good. However, please verify if Litecoin requires different network parameters (likemessagePrefix
,bech32
,bip32
, etc.) in theBitcoinNetworkInfo
interface.✅ Verification successful
Litecoin network parameters are properly configured ✅
The Litecoin network configuration in
packages/extension/src/providers/bitcoin/networks/litecoin.ts
correctly implements all required parameters from theBitcoinNetworkInfo
interface with Litecoin-specific values.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for Litecoin network configuration rg -l "NetworkNames.Litecoin.*BitcoinNetworkInfo"Length of output: 51
Script:
#!/bin/bash # Find BitcoinNetworkInfo interface definition ast-grep --pattern 'interface BitcoinNetworkInfo { $$$ }' # Search for network configuration files fd -e ts -e js bitcoin network config --exec rg -l "NetworkNames\.(Bitcoin|Litecoin)" # Find Litecoin-related configurations rg -l "litecoin" --type tsLength of output: 1979
Script:
#!/bin/bash # Check Litecoin network configuration implementation cat packages/extension/src/providers/bitcoin/networks/litecoin.ts # Check networks index file for network registration cat packages/extension/src/providers/bitcoin/networks/index.tsLength of output: 1907
Summary by CodeRabbit
Release Notes
New Features
Improvements
Technical Updates
EnkryptWindow
interface to support additional provider properties