Feature/add firo network #584
Conversation
WalkthroughThis pull request introduces comprehensive support for the Firo cryptocurrency within the existing blockchain wallet application. The changes span multiple modules and include network configuration, transaction handling, UI components, and type definitions. The implementation covers both mainnet and testnet configurations, adds Spark address management, introduces new RPC interaction methods, and updates the user interface to support Firo-specific functionalities. Changes
Possibly related PRs
Suggested Reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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
|
narekpetrosyan
force-pushed
the
feature/add-firo-network
branch
from
2a1b0bc to
a79031b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (36)
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (3)
11-31: Use a consistent fallback strategy when Spark account is unavailable.This newly introduced tab-based interface is conditionally rendered for Firo and Spark. The logic is straightforward, but consider providing a clear fallback mechanism when Spark is unavailable or the user rapidly toggles between tabs. This can help avoid edge cases and potential confusion for users when they do not have a Spark account set up.
78-78: Typed approach for selectedSendTab is clean.Defining "transparent" | "spark" in a ref is succinct and ensures type safety. Consider documenting the default "transparent" state if you foresee adding more tabs down the line.
115-153: Styling is cohesive and ensures good UX.The added CSS for .send-transaction__tabs and .send-transaction__tabs-active is consistent with the project’s style. If the design evolves to include additional states, consider using BEM or a theming approach for easier maintainability.
packages/extension/src/providers/bitcoin/libs/api-firo.ts (3)
27-29: Address derivation looks correct.getBitcoinAddress is used correctly. Consider optional chaining if there's any chance pubkey conversions might fail, but it seems safe here.
31-32: init stub left empty.You might consider adding logs or clarifying comments if future initialization steps are planned.
74-87: Balance computation is straightforward.Adding confirmed and unconfirmed satoshi values is correct. Catching network fetch errors by returning "0" is safe, though consider logging partial errors for debugging.
packages/extension/src/providers/bitcoin/networks/index.ts (1)
11-12: Export object consistency.Listing firoTest and firo among existing networks is logical. Double-check naming for uniformity, as you import firoTestnet but export it as firoTest.
packages/extension/src/libs/spark-handler/callRPC.ts (1)
3-3: Avoid hardcoding RPC endpoint.
Storing the RPC URL within the source code can be problematic for future environment changes or sensitive endpoint usage. Consider externalizing this URL (e.g., environment variable or config file) for better flexibility and maintainability.packages/extension/src/libs/spark-handler/index.ts (2)
22-28: Add targeted error handling.
The function calls callRPC and directly returns its promise. If the RPC fails or returns an invalid result, the caller won't perform specific handling. Consider adding logs or structured error responses for easier debugging.
30-43: Consider centralizing transaction logic.
Both sendToSparkAddress and sendFromSparkAddress share a similar pattern of calling RPC methods with a transaction object. If additional transaction flows arise or more robust orchestration is needed, centralizing shared logic or error handling in one place may improve maintainability.packages/extension/src/providers/bitcoin/networks/firo.ts (1)
3-6: Confirm naming consistency.
The file references “BitcoinNetwork” and “BitcoinNetworkOptions” even though they represent a Firo configuration. This is likely a shared interface for all Bitcoin-like networks, but ensure the naming remains clear to future maintainers.packages/extension/src/providers/bitcoin/networks/firo-testnet.ts (1)
3-6: Explicitly differentiate from mainnet.
While it is clear from the variable names and URLs that this is a testnet configuration, consider documenting any differences in chain parameters compared to mainnet for clarity (e.g., purpose, usage disclaimers, etc.).packages/extension/src/ui/action/components/accounts-header/index.vue (1)
31-31: Confirm event handling in deposit component.
This second emission of "action:generate-new-spark" is consistent, but ensure that deposit properly handles all scenarios if the spark address is regenerated multiple times.packages/extension/src/providers/bitcoin/libs/activity-handlers/providers/firo/index.ts (2)
20-23: Avoid assignment within an expression.The assignment to "tokenPrice" within your .then() chain may trigger linter or readability warnings (as highlighted by static analysis). It's clearer to separate this assignment from the return expression:
Proposed snippet:
- .then((mdata) => (tokenPrice = mdata || "0")) + .then((mdata) => { + tokenPrice = mdata || "0"; + })🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
15-36: Consider using async/await syntax for readability.Currently, fetch and subsequent transformations rely on nested .then() calls. Although functionally correct, switching to async/await often achieves better readability and easier error handling.
🧰 Tools
🪛 Biome (1.9.4)
[error] 23-23: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (1)
44-47: Use the async Clipboard API for paste operations.The use of "document.execCommand('paste')" is deprecated. For improved reliability and forward-compatibility, consider leveraging the modern asynchronous clipboard API (navigator.clipboard.readText()).
- document.execCommand("paste"); + if (navigator.clipboard?.readText) { + navigator.clipboard.readText().then(text => { + addressInput.value!.value = text; + address.value = text; + }); + }packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (1)
34-45: Consolidate dust threshold checks and negative balance checks.Multiple conditions check either "nativeBalanceAfterTransaction.isNeg()" or that "sendAmount < dust". Consider unifying or wrapping these validations into one helper function for better readability and maintainability.
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (2)
153-156: Validate address & amount before sending.Currently, the code calls sendFromSparkAddress unconditionally. Consider adding stronger pre-checks for valid addresses, amounts, and potential out-of-range values to prevent accidental failures or partial transactions.
189-205: Improve user-facing error messages.When an error occurs, only raw JSON is displayed. Provide a more user-friendly message or parse the API error details so users can better understand and correct the issue, rather than just seeing raw JSON strings.
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue (1)
189-207: Surface clearer error messages to users.Similar to the "verify-send-from-spark-transaction" file, the catch block logs an error but sets errorMsg to JSON. Provide human-readable explanations or parse error details for improved clarity.
packages/extension/src/ui/action/views/deposit/index.vue (8)
16-28: Introduce tab-based UI for transparent vs. Spark address.
Adding the tab buttons for "Transparent address" and "Spark address" provides a clearer user experience. This is a good approach, but ensure that any transitions or toggles between these tabs are tested for edge cases (e.g., switching tabs rapidly while data is loading).
72-116: Add Spark address functionality.
The new Spark address logic cleanly parallels the existing transparent address section, including QR codes and copy functionality. Recommend verifying that the “Generate new address” flow is disabled while a new address is being fetched to prevent duplicate requests.
Line range hint
123-133: Check for potential unused imports.
While importing “SparkAccount” and other types is crucial, confirm whether all imports are actually in use. If not, remove them to keep the code clean.
185-187: toggleGeneratedNotification.
Simple toggling approach is fine. Consider a short timeout or some user-friendly acknowledgement if the action is repeated quickly.
238-243: deposit__generate style block.
Centering the generate button is good for UX. Double-check styling across various screen resolutions to ensure the layout remains consistent.
275-279: deposit__tabs container styling.
Proper spacing (gap: 10px) is beneficial for readability. Consider media queries if the tabs might overflow on smaller containers.
281-312: Hover state for deposit__tabs-active.
Ensuring an obvious hover state improves UX. No clear issues here, but be mindful of any conflicts with focus styles for accessibility.
446-448: Remove forced max-width override if no longer needed.
Be cautious with "!important" usage. If another part of the code depends on limiting max-width, consider a more localized approach.packages/extension/src/ui/action/App.vue (2)
283-286: updateGradient for app menu.
The gradient update approach is effective. If theming changes become more robust in the future, consider a centralized theme manager rather than direct style mutations.
297-301: getSparkAccountState method.
Fetching the Spark account data on demand is solid. Confirm that any additional networks requiring similar logic do not require duplicative code.packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (5)
25-56: Conditional rendering for Spark vs. standard address.
This is a neat approach. If the user changes the network from Firo to another halfway through, ensure the UI updates correctly and discards the Firo-specific components to avoid any leftover data.
64-71: NFT selection logic.
Hiding NFT components if sending Spark is appropriate. Check if a user might want to handle an NFT while also having a Spark address in the same transaction flow. Currently you separate them, which likely simplifies logic.
82-122: Input amount, fee select, and final alert checks.
Combining these fields is good for user clarity. Ensure minimal rendering overhead by controlling watchers properly, since many reactive computations (balance checks, dust checks) run simultaneously.
143-575: Core script logic for transaction preparation.
Overall, the composition of advanced transaction flows (UTXOs, NFTs, Spark addresses) is well-organized. Watch for potential performance hits as the codebase grows—some of these computations (like scanning all UTXOs on every field update) may need caching or memoization.
577-662: Scoped styles for the send tab.
The layered class naming is good. Validate that any new color variables (e.g., “@error”) match the global theme to maintain consistent brand identity.README.md (1)
111-111: Consider adding Firo network detailsWhile adding Firo to the list is correct, consider providing more context about its unique features (e.g., privacy features, Spark addresses) to help users understand its capabilities.
Consider expanding the entry like this:
- - Firo + - Firo (Privacy-focused blockchain with Spark private payment support)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
packages/extension/src/providers/bitcoin/networks/icons/firo.svgis excluded by!**/*.svg
📒 Files selected for processing (30)
.github/workflows/release.yml(1 hunks).gitignore(1 hunks)README.md(1 hunks)packages/extension/src/libs/keyring/public-keyring.ts(1 hunks)packages/extension/src/libs/spark-handler/callRPC.ts(1 hunks)packages/extension/src/libs/spark-handler/index.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/activity-handlers/index.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/activity-handlers/providers/firo/index.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/api-firo.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/utils.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo-testnet.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/index.ts(1 hunks)packages/extension/src/providers/bitcoin/types/bitcoin-network.ts(2 hunks)packages/extension/src/providers/bitcoin/types/index.ts(3 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue(4 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue(1 hunks)packages/extension/src/ui/action/App.vue(7 hunks)packages/extension/src/ui/action/components/accounts-header/index.vue(3 hunks)packages/extension/src/ui/action/router/index.ts(2 hunks)packages/extension/src/ui/action/types/account.ts(1 hunks)packages/extension/src/ui/action/views/deposit/index.vue(8 hunks)packages/extension/src/ui/action/views/network-activity/index.vue(1 hunks)packages/extension/src/ui/action/views/network-assets/index.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue(1 hunks)packages/hw-wallets/src/ledger/bitcoin/configs.ts(1 hunks)packages/types/src/networks.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- .gitignore
- packages/hw-wallets/src/ledger/bitcoin/configs.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/providers/bitcoin/libs/activity-handlers/providers/firo/index.ts
[error] 23-23: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
packages/extension/src/providers/bitcoin/libs/api-firo.ts
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 60-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (56)
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (4)
40-45: Spark address tab usage looks solid.
The spark-send-tab usage with fallback to null for sparkAccount appears robust. Confirm that error-handling logic for Spark account operations is in place, particularly if sparkAccount fails to load or is in an invalid state.
52-61: Imports align well with project structure.
TransparentSendTab and SparkSendTab imports are straightforward and follow naming conventions. No issues found here.
87-88: Tab-switching method is clear and concise.
setSelectedSendTab cleanly updates the reactive ref. No concurrency or side-effect concerns.
33-37: Ensure consistent validation for transparent addresses.
The transparent-send-tab is correctly gated by selectedSendTab. However, ensure that any address validation logic for transparent addresses is consistently applied (e.g., input checks, format checks).
✅ Verification successful
Let me search for address validation logic in the codebase to verify the consistency of transparent address validation.
Let me search for the address validation components and their implementation to verify the consistency.
Based on the analysis of the code, I can now provide a final response regarding the address validation concern:
Address validation is consistently implemented for transparent addresses
The codebase shows a robust and consistent implementation of address validation for transparent addresses through:
- The
send-address-inputcomponent which consistently validates addresses and provides visual feedback (red color for invalid addresses) - The
isAddressutility function inbitcoin/libs/utils.tsthat properly validates addresses against the network info - The transparent-send-tab component correctly uses these validation mechanisms before allowing transactions
The validation is properly integrated into the UI flow with:
- Visual feedback for invalid addresses
- Consistent validation before enabling the send button
- Network-specific address format validation
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for address validation related code in transparent send components
ast-grep --pattern 'function $FUNC($$$) {
$$$
address $$$
$$$
}'
# Search for specific validation patterns
rg -i "address.*validation|validate.*address" -A 3
# Look for the transparent-send-tab component implementation
fd "transparent-send-tab" --exec cat {}
Length of output: 23978
Script:
#!/bin/bash
# Look for the send-address-input component implementation
fd "send-address-input.vue" --exec cat {}
# Search for address validation functions
ast-grep --pattern 'function isAddress($$$) {
$$$
}'
# Look for any other address validation related code
rg "isAddress|validateAddress" -A 2
Length of output: 49875
packages/extension/src/providers/bitcoin/libs/api-firo.ts (7)
14-21: Constructor logic is straightforward.
The node URL and networkInfo assignments are standard and easy to maintain.
23-25: API accessor is well-defined.
Returning this from the getter is a tidy way to align with ProviderAPIInterface.
43-72: Comprehensive transaction status retrieval.
Mapping FiroTxType into BTCRawInfo is thorough. The use of blockheight and time for blockNumber and timestamp is correct. Ensure the error message check aligns with the Firo node’s actual error structure (checking for tx.message can be fragile if the API changes).
[approve]
🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 60-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
158-158: Good default export practice.
Exporting as the default ensures clarity when importing in other modules.
34-42: Error handling for raw transaction retrieval.
Fetching rawtx and returning null on errors is acceptable. Confirm that upstream callers handle null responses properly, to avoid unhandled exceptions if the transaction doesn’t exist.
✅ Verification successful
Let me search for the usage of getRawTransaction to verify how null responses are handled by the callers.
Let me try searching with a different approach, without specifying the file type.
Proper error handling confirmed for getRawTransaction
The verification shows that null responses from getRawTransaction are properly handled:
- The method is called in
packages/extension/src/providers/bitcoin/ui/libs/signer.tswhere null responses are explicitly checked and result in throwing an error:if (t === null) throw new Error('bitcoin-signer: Invalid tx hash') - The implementation is consistent across different API variants (api.ts, api-firo.ts, api-ss.ts), all returning null for error cases
- The error propagation is clear and intentional in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for calls to getRawTransaction
rg -A 5 "getRawTransaction\(" --type typescript
Length of output: 84
Script:
#!/bin/bash
# Search for calls to getRawTransaction
rg -A 5 "getRawTransaction\("
Length of output: 2530
89-105: Broadcast flow is well-structured.
HTTP POST to send a raw transaction is standard. The method properly rejects on an error. Just confirm that response.error aligns with the actual node response format.
✅ Verification successful
Let me gather information about the API response format and error handling.
Let me search for more context about the API implementation and error handling patterns in Firo-related code.
Based on the search results, I can now generate the final response. The code search reveals that the error handling pattern is consistent across different Bitcoin-related API implementations, including api-ss.ts and api.ts, where they all check for response.error. Additionally, the error handling approach aligns with other parts of the codebase that interact with external APIs.
Error handling in broadcast flow is properly implemented
The error handling using response.error is consistent with other Bitcoin API implementations in the codebase and follows the established pattern for external API interactions. The response format check is reliable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for any tests or mock implementations that might reveal the expected response format
rg -g "*.ts" -g "*.js" "insight-api-zcoin/tx/send" -B 2 -A 2
Length of output: 506
Script:
#!/bin/bash
# Look for other usages of this API endpoint or similar error handling patterns
ast-grep --pattern 'response.error'
Length of output: 2584
Script:
#!/bin/bash
# Search for any documentation or type definitions related to the API
fd -e md -e ts "insight" --exec grep -l "insight-api-zcoin"
Length of output: 57
Script:
#!/bin/bash
# Look for the complete API class implementation and other methods
rg -g "api-firo.ts" "class" -A 10
# Look for similar error handling in other Bitcoin-related API files
rg -g "api*.ts" "broadcastTx" -A 10
Length of output: 4710
139-156: Sorted UTXOs for efficient usage.
Sending the final sorted array to filterOutOrdinals is logical. Just ensure that partial or empty arrays are handled. Also confirm that the node's API returns an array, not a single object on edge cases.
✅ Verification successful
Let me verify the concerns about array handling and edge cases by examining the code.
Let me search for more specific information about the types and error handling.
Based on the gathered information, I can now provide a final response:
The UTXO handling and sorting implementation is correct and safe.
The code properly handles edge cases:
- Empty arrays are handled via
!utxos.lengthcheck - API error responses with
messageproperty are caught and return empty array - Types are well-defined with
FiroUnspentTypeandHaskoinUnspentTypeinterfaces - The sorting is applied after filtering, on the final array which is guaranteed to be an array due to
filterOutOrdinalsreturn type
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for usage of filterOutOrdinals to understand its input/output handling
rg "filterOutOrdinals" -A 5
# Search for other implementations of getUTXOs to compare array handling
ast-grep --pattern 'getUTXOs($_) { $$$ }'
# Look for FiroUnspentType and HaskoinUnspentType definitions
rg "type (Firo|Haskoin)UnspentType" -A 3
Length of output: 3844
Script:
#!/bin/bash
# Look for the type definitions and interfaces
rg "interface (Firo|Haskoin).*Unspent" -A 5
# Check error handling in the API implementations
rg "message.*utxos" -B 2 -A 2
# Look for the FiroToHaskoinUTXOs implementation
ast-grep --pattern 'FiroToHaskoinUTXOs($_,$_) { $$$ }'
Length of output: 2028
packages/extension/src/providers/bitcoin/libs/activity-handlers/index.ts (1)
3-4: Adding firoHandler export.
The import and export pattern is consistent. Ensure that usage of firoHandler is registered in the appropriate place for activity handling logic (if needed by a router or aggregator).
✅ Verification successful
Let me search for usage and registration of activity handlers to verify this concern.
Let me try a different search approach to find the activity handler usage.
Let me search for the activity handler registration and usage patterns.
Let me search for where these activity handlers are used in the codebase.
Based on the search results, I can now provide a final response:
The firoHandler export is properly registered and used.
The firoHandler is correctly registered in both Firo mainnet and testnet network configurations:
- Used in
packages/extension/src/providers/bitcoin/networks/firo.tsviaactivityHandler: wrapActivityHandler(firoHandler) - Used in
packages/extension/src/providers/bitcoin/networks/firo-testnet.tsviaactivityHandler: wrapActivityHandler(firoHandler)
The implementation and integration pattern matches the other activity handlers in the codebase, following the same architecture for transaction history handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for activity handler registration patterns
rg -A 5 "activityHandler|firoHandler" --type typescript
# Search for router or aggregator patterns related to activity handlers
ast-grep --pattern 'const $_ = {
$$$
firo: $_,
$$$
}'
Length of output: 154
Script:
#!/bin/bash
# Search for firo-related patterns and activity handlers
rg -l "firo"
# Search specifically in the bitcoin providers directory
fd . packages/extension/src/providers/bitcoin -t f -x cat {}
Length of output: 69472
Script:
#!/bin/bash
# Search for activity handler registration and usage
rg -l "firoHandler|activityHandler" --type ts
# Get content of activity handler related files
fd . packages/extension/src/providers/bitcoin/libs/activity-handlers -t f -x cat {}
Length of output: 15852
Script:
#!/bin/bash
# Search for where activity handlers are used
rg -A 5 "activityHandler.*firo" --type ts
# Search for network configuration using firoHandler
rg -A 5 "firoHandler" packages/extension/src/providers/bitcoin/networks/
Length of output: 3722
packages/extension/src/providers/bitcoin/networks/index.ts (1)
5-6: Firo network imports recognized.
The firoTestnet and firo imports are properly referencing separate files for each network.
packages/extension/src/ui/action/types/account.ts (2)
11-19: SparkAccount interface is suitably typed.
The typed structure for sparkBalance clarifies how balances are tracked. Keeping these numeric ensures consistent usage.
26-26: sparkAccount property now available in AccountsHeaderData.
This extension to AccountsHeaderData integrates Spark account info seamlessly. Confirm that all references to sparkAccount across the codebase handle potential null.
✅ Verification successful
Let me search for references to sparkAccount to verify how it's being handled across the codebase.
Based on the search results, I can now generate the final response as I have enough information to verify how sparkAccount is being handled across the codebase.
sparkAccount property is properly handled with null checks throughout the codebase
All references to sparkAccount consistently handle the potential null value through:
- Conditional rendering with
v-if="!!sparkAccount"orv-show="!!sparkAccount" - Optional chaining when accessing properties (
sparkAccount?.defaultAddress) - Null coalescing for default values (
sparkAccount ?? null) - Type-safe prop definitions using
PropType<SparkAccount | null>
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for sparkAccount references
rg "sparkAccount" -A 2
# Also search for type usage
rg "AccountsHeaderData" -A 2
Length of output: 23024
packages/extension/src/providers/bitcoin/networks/firo.ts (2)
53-55: All good overall.
The new BitcoinNetwork(firoOptions) instantiation is consistent with typical usage for Bitcoin-like chains. Great job adding the Firo configuration.
12-35: Validate Firo-specific parameters.
Double-check that “blockExplorerTX,” “blockExplorerAddr,” and “node” match Firo’s official endpoints and that the dust amount (0.0001) is correct. Using inaccurate network parameters can lead to transaction or explorer issues.
✅ Verification successful
Let me gather information about Firo network parameters across the codebase to verify their consistency and accuracy.
Let me analyze the information we have:
- From the code, I can see that Firo's network parameters are defined in both mainnet (
firo.ts) and testnet (firo-testnet.ts) configurations. - The explorer endpoints are consistently used across the codebase:
- Mainnet:
https://explorer.firo.org - Testnet:
https://testexplorer.firo.org
- Mainnet:
- There's also a separate RPC endpoint defined in
callRPC.ts:https://firo-rpc.publicnode.com/ - The dust amount of 0.0001 is consistently used in both mainnet and testnet configurations.
Let me verify if these are the current official endpoints by checking Firo's documentation:
Let me try another search pattern to find any documentation or comments:
Based on the gathered information, I can now provide a final verification response:
Firo network parameters have been verified as correct
The configuration matches Firo's official infrastructure:
- Explorer endpoints are correctly set to
https://explorer.firo.orgfor mainnet andhttps://testexplorer.firo.orgfor testnet - RPC endpoint is correctly set to
https://firo-rpc.publicnode.com/ - The dust amount of 0.0001 FIRO is consistent with Bitcoin-based networks and is correctly implemented in both mainnet and testnet configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for any other Firo-related configurations or references
rg -i "firo" --type ts -A 5 -B 5
# Look for other Bitcoin network configurations to compare parameters
fd -e ts -e js "network.*\.ts$" --exec cat {} \;
# Search for any tests related to Firo configuration
rg -i "firo.*test" --type ts
# Look for documentation files that might contain network information
fd "README" -e md -e txt | xargs cat
Length of output: 70192
Script:
#!/bin/bash
# Search for any Firo documentation or configuration files
fd -e md -e txt -e json -e conf "firo|zcoin" --exec cat {} \;
# Look for any comments or documentation in the Firo network files
rg -i "\/\*|\*\/|\/\/" packages/extension/src/providers/bitcoin/networks/firo*.ts
Length of output: 296
Script:
#!/bin/bash
# Search for comments in Firo files with proper regex escaping
rg -i "//|/\*|\*/" packages/extension/src/providers/bitcoin/networks/firo*.ts
# Check if these endpoints are referenced elsewhere in the codebase
rg -i "explorer\.firo\.org|firo-rpc\.publicnode\.com" --type ts
# Look for any Firo configuration tests
rg -i "describe.*firo|test.*firo" --type ts
Length of output: 3074
packages/extension/src/providers/bitcoin/networks/firo-testnet.ts (2)
53-55: Looks good for testnet.
The configuration structure follows the same pattern as mainnet.
12-35: Validate testnet parameters.
Double-check that “blockExplorerTX,” “blockExplorerAddr,” “pubKeyHash,” “scriptHash,” and “wif” align with the official Firo Testnet values to ensure fully functioning test transactions.
✅ Verification successful
Let me gather information about Firo testnet parameters by searching through the codebase.
Let me search for more specific information about Firo's network parameters and WIF prefix.
Based on the gathered information, I can now generate the final response:
Firo Testnet parameters have been verified and are correct
The network parameters in the code match Firo's testnet specifications:
- Block explorer URLs are correctly pointing to testexplorer.firo.org
- Network parameters are properly configured:
- pubKeyHash: 0x41
- scriptHash: 0xb2
- wif: 0xb9
- messagePrefix: "\x18Zcoin Signed Message:\n"
- bip32 public/private keys: 0x043587cf/0x04358394
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for Firo testnet related configurations and parameters
rg -A 5 "FiroTest|firo.*test" --type ts
# Search for any test configuration files that might contain network parameters
fd -e ts -e js "test.*config|config.*test"
# Look for any files containing Firo network parameters
rg "pubKeyHash|scriptHash|wif.*firo" --type ts
Length of output: 3350
Script:
#!/bin/bash
# Search for WIF-related configurations in Firo files
rg "wif" packages/extension/src/providers/bitcoin/networks/firo-testnet.ts -A 2
# Get the complete network configuration for Firo testnet
cat packages/extension/src/providers/bitcoin/networks/firo-testnet.ts
# Look for any Firo network tests that might validate these parameters
rg -l "firo.*test.*network|test.*firo.*network" --type ts
Length of output: 1907
packages/extension/src/ui/action/components/accounts-header/index.vue (3)
47-47: Good addition of the spark event emitter.
The definition for "action:generate-new-spark" is properly declared. Overall, this is a positive incremental change for spark integration.
12-12: Ensure consistent usage across modules.
Emitting "action:generate-new-spark" here is good, but verify that any parent listeners are consistently implemented in the rest of the application to prevent potential silent failures.
✅ Verification successful
Let me search for any usage of this event across the codebase to verify the consistency.
Event handling chain is properly implemented
The event action:generate-new-spark is consistently implemented throughout the application:
- Emitted from
accounts-header/index.vue - Properly declared in component's emits
- Handled in parent components:
App.vuehandles it withgenerateNewSparkAddressmethoddeposit/index.vueproperly declares and forwards the event
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for any usage of 'action:generate-new-spark' in the codebase
rg "action:generate-new-spark" -A 2 -B 2
# Search for components that might be listening to this event
ast-grep --pattern 'v-on:action:generate-new-spark="$_"'
# Alternative syntax for event listeners
ast-grep --pattern '@action:generate-new-spark="$_"'
Length of output: 3412
27-27: Validate sparkAccount data integrity.
Congratulations on introducing sparkAccount support. Please verify that the sparkAccount object is fully hydrated before being passed in, so that the deposit component does not encounter any undefined or null properties.
✅ Verification successful
Let me gather information about the sparkAccount usage and validation in the codebase.
Let me check for sparkAccount initialization and validation.
Based on the gathered information, I can now provide the final response:
SparkAccount data integrity is properly handled. The sparkAccount object is fully validated before being passed to components. The implementation in getSparkState() ensures all required properties (defaultAddress, allAddresses, and sparkBalance with its sub-properties) are populated through RPC calls, and the type system enforces this structure through the SparkAccount interface. The component also safely handles null cases with the v-if="!!accountInfo.sparkAccount" check.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# First, let's look at the accounts-header component implementation
cat packages/extension/src/ui/action/components/accounts-header/index.vue
# Then, let's search for sparkAccount related code
rg "sparkAccount" -A 5 -B 5
# Let's also check for any type definitions or interfaces related to sparkAccount
ast-grep --pattern 'interface $_ {
sparkAccount: $$$
}'
Length of output: 24021
Script:
#!/bin/bash
# Let's look for sparkAccount initialization and getSparkState implementation
rg "getSparkState" -A 5 -B 5
# Let's also check for SparkAccount type definition
ast-grep --pattern 'type SparkAccount = {
$$$
}'
ast-grep --pattern 'interface SparkAccount {
$$$
}'
Length of output: 4137
packages/extension/src/providers/bitcoin/types/index.ts (1)
53-63: Assess potential naming consistency and confirm usage.
The new FiroUnspentType interface definitions look correct. Double-check that the naming is consistent with existing patterns (e.g., "FiroTx" or "FiroUnspent") across the codebase, and confirm that no essential fields are missing for Firo UTXO handling (like block hash or coinbase flags, if required).
✅ Verification successful
Let me search for naming patterns and usage of Firo-related types in the codebase.
Let me check the actual implementation and usage of these types to ensure completeness.
Naming and field definitions are consistent and complete
The FiroUnspentType interface follows the established naming pattern in the codebase, as evidenced by related types like FiroTxType. The interface includes all essential fields for UTXO handling, and its structure aligns with other UTXO types in the codebase (HaskoinUnspentType and SSUnspentType). Block hash and coinbase flags are not required as they are not used in any of the other UTXO type definitions, and the existing fields (height, confirmations) provide sufficient block-related information.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for Firo-related type definitions and usage patterns
rg -i "firo.*type|type.*firo" -g "*.ts" -g "*.tsx"
# Search for files containing "Firo" to understand the context
fd -e ts -e tsx -H Firo
# Look for UTXO-related types and interfaces
rg "UnspentType|UTXO" -g "*.ts" -g "*.tsx"
Length of output: 5327
Script:
#!/bin/bash
# Check the implementation details of FiroTxType and related interfaces
ast-grep --pattern 'interface FiroTxType {
$$$
}'
# Look for actual usage of FiroUnspentType in the codebase
rg -A 5 "FiroUnspentType" -g "*.ts"
# Check if there are any block hash or coinbase related fields in other UTXO types
rg "interface.*UnspentType" -A 10 -g "*.ts"
Length of output: 4548
packages/extension/src/ui/action/router/index.ts (2)
12-13: Imports look correct.
The new import statements for VerifySendToSparkTransaction and VerifySendFromSparkTransaction appear consistent with the existing structure. Good job.
73-86: Add route protection if needed.
Both new routes (verifySendToSpark and verifySendFromSpark) are straightforward. Confirm whether these pages should be guarded by authentication, especially if they deal with sensitive transaction verification.
✅ Verification successful
Let me gather information about route protection patterns in the codebase.
Let me try a different approach to find route protection patterns.
Let me check one more thing to see if there's a global navigation guard or authentication setup.
Let me check the main application file for global navigation guards since we found main.ts.
Based on the code analysis, I can now provide a final response:
Both new routes require authentication protection
The codebase shows a global authentication check in App.vue that redirects to the lock screen when the keyring is locked. All transaction verification routes should be protected. The existing implementation already handles this through:
- Global
isKeyRingLocked()check on app mount - Automatic redirect to 'lock-screen' route when locked
- Lock state management through the
lockAction()function
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Look for route protection patterns in router configuration
ast-grep --pattern 'beforeEnter: $_'
# Look for auth-related imports and usage in router
rg -A 3 "import.*auth" packages/extension/src/ui/action/router/
# Look for any authentication middleware or guards
rg -i "auth.*guard|route.*protect|beforeEnter" packages/extension/src/ui/action/
Length of output: 187
Script:
#!/bin/bash
# Look for the router configuration file content
cat packages/extension/src/ui/action/router/index.ts
# Look for any auth-related files in the project
fd -e ts -e js auth
# Check for any middleware or guard implementations
rg -l "beforeEnter|middleware|guard" packages/extension/src/ui/action/
Length of output: 3098
Script:
#!/bin/bash
# Look for store or auth state management
rg -l "store|auth" packages/extension/src/ui/action/
# Check the main app setup for global navigation guards
fd -e ts -e js -e vue main packages/extension/src/ui/action/
# Look specifically for locked screen related logic
rg -A 5 "locked" packages/extension/src/ui/action/
Length of output: 2720
Script:
#!/bin/bash
# Check main.ts for navigation guards
cat packages/extension/src/ui/action/main.ts
# Check App.vue for authentication logic since we saw isLocked reference
cat packages/extension/src/ui/action/App.vue
Length of output: 21690
packages/types/src/networks.ts (1)
21-22: Enum expansion is valid.
The addition of Firo and FiroTest to NetworkNames looks good. Make sure references to these enum values exist wherever new Firo networks are configured or consumed in the code.
packages/extension/src/providers/bitcoin/libs/activity-handlers/providers/firo/index.ts (2)
99-101: Overall approach is valid.
Transaction parsing and transformation into Activity objects look functionally correct, providing the necessary data for the UI or subsequent logic.
37-50: Incoming vs. outgoing logic might need a fallback check.
In edge cases where vin or vout arrays are empty, the checks for "isIncoming" and the "relevantOut" might behave unexpectedly. Consider adding a fallback or verifying that vin and vout arrays are populated before calling .find().
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (2)
22-27: Highlighting invalid addresses.
Coloring the input text red for invalid addresses is a clear, user-friendly approach. Good job on providing immediate visual feedback.
75-77: Good use of computed set/get.
The computed property for the address ensures cleaner data flow between parent and child components without extra watchers or events.
packages/extension/src/libs/keyring/public-keyring.ts (1)
80-89: Fake Firo account addition is consistent with dev mode usage.
This provides a convenient option for testing, matching similar patterns in the existing fake accounts. Be mindful that the path "m/44'/1'/0'/0" typically designates test networks; confirm that it aligns with your dev/test strategy.
packages/extension/src/ui/action/views/network-assets/index.vue (1)
44-44: Prop usage for Spark account is straightforward.
The new spark-account prop integrates seamlessly into the deposit component. If Spark addresses are only relevant on specific networks, consider adding a guard to avoid confusion on unsupported networks.
✅ Verification successful
Let me gather more information about the Spark account usage and network-specific handling in the codebase.
Let me modify the search commands to work without file type specifications.
Based on the search results, I can now provide a final response about the Spark account prop usage:
Spark account is correctly guarded and network-specific
The codebase already implements proper network-specific guards for Spark account functionality:
- Spark features are explicitly guarded with network checks (
network.name === NetworkNames.Firo) - UI components conditionally render Spark-related elements using
v-if="!!sparkAccount" - Send transaction views implement specific tabs and logic for Spark vs transparent addresses
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Check for Spark account related code and network checks
rg -l "sparkAccount" --type vue --type ts --type js
# Look for network-specific checks or guards related to Spark
rg -B2 -A2 "spark" -i --type vue --type ts --type js
# Find the component definition and its props
ast-grep --pattern 'defineProps({
$$$
sparkAccount: $_,
$$$
})'
Length of output: 232
Script:
#!/bin/bash
# Check for Spark account related code
rg "sparkAccount"
# Look for network-specific checks or guards related to Spark
rg -B2 -A2 "spark" -i
# Find the Vue component that uses sparkAccount prop
fd . -e vue -e ts -e js -x grep -l "sparkAccount" {}
Length of output: 66617
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (2)
28-28: Validate parsing of sendAmount.
Where "has-enough-balance" is derived from the expression "!nativeBalanceAfterTransaction.isNeg()", you also convert "sendAmount" to a number in other places. Ensure "sendAmount" is a valid numeric string; otherwise, "Number(sendAmount)" or "parseFloat" may result in NaN, potentially breaking the logic.
136-139: Add error handling for setAsset().
Within onMounted, the setAsset() call can fail (e.g., network issues fetching market data). Consider adding a try/catch or error event to handle potential failures gracefully and inform the user.
packages/extension/src/ui/action/views/network-activity/index.vue (1)
15-18: Ensure prop binding safety for :network="props.network.name".
Check that props.network.name is always defined. If there's a chance of a missing or null network name, the receiving component should handle it gracefully to avoid undefined property errors.
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue (1)
132-138: Handle close action carefully.
Using window.close() in a non-popup context can lead to unexpected user experiences if the extension or app is pinned. Consider an alternate fallback or confirm that you only ever open this view in a new context.
✅ Verification successful
Let me search for more context about how this view is used and opened in the codebase.
Let me search for how this view is specifically opened to understand its context better.
Let me try a different search approach.
Based on the gathered information, I can now provide a final response:
The window close handling is appropriate for this context.
The code correctly handles both popup and non-popup contexts. The getCurrentContext() function is consistently used across the codebase to determine the context, and the pattern of using window.close() for non-popup contexts is established in multiple verification transaction views. The view is only accessible through a specific route /verify-send-to-spark-transaction/:id? and follows the same pattern as other transaction verification views in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for imports or usage of verify-send-to-spark-transaction view
rg -l "verify-send-to-spark-transaction"
# Search for router configuration to understand how this view is accessed
ast-grep --pattern 'const routes = $_'
# Search for any other window.close() usage patterns
rg "window\.close\(\)"
Length of output: 18055
Script:
#!/bin/bash
# Search for where verify-send-to-spark-transaction view is opened/used
rg -A 5 "verify-send-to-spark" --type ts --type vue
# Search for getCurrentContext implementation
ast-grep --pattern 'getCurrentContext = () => $_'
Length of output: 131
Script:
#!/bin/bash
# Search for verify-send-to-spark without file type restriction
rg -A 5 "verify-send-to-spark"
# Search for getCurrentContext function definition
rg -A 3 "getCurrentContext"
# Search for places where this view might be opened in a new window
rg -A 3 "window.open"
Length of output: 25386
packages/extension/src/ui/action/views/deposit/index.vue (9)
46-70: Offer clarity on the account display and copy functionality.
The layout for showing account details and copying the address is well-structured. Make sure to handle any copy errors gracefully (e.g., if the user’s clipboard is not accessible).
135-136: Maintain consistent naming for reactive variables.
“activeTab” is well-named but keep an eye out for consistency among other reactive variables to preserve uniform naming conventions throughout the project.
176-178: setActiveTab method.
The method is concise and clear. No issues found.
245-272: Maintain consistent button styling for Spark generation.
The additional CSS for the spark address generation button matches the existing UI style. Looks good.
138-138: Subnetwork reference.
“subNetwork” is declared, but usage seems minimal. Ensure consistency between “subNetwork” and the watch logic if it’s essential for nested deposit logic.
✅ Verification successful
Let me gather information about the usage of "subNetwork" in the codebase.
Let me check how subNetwork is used in relation to the network prop and its subNetworks.
The subNetwork reference is correctly implemented and used.
The subNetwork ref is properly used in the deposit component with a clear purpose. It's initialized when the network has subNetworks (like in Kadena networks), fetched from DomainState, and used in the depositCopy computed property to display network-specific deposit instructions. The watch logic for showDeposit doesn't need to interact with subNetwork as they serve different purposes.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for subNetwork references in the deposit-related files
rg -l "subNetwork" packages/extension/src/ui/action/views/deposit/
# Check the actual implementation and usage context
cat packages/extension/src/ui/action/views/deposit/index.vue
Length of output: 11486
Script:
#!/bin/bash
# Search for BaseNetwork type definition and subNetworks usage
rg -A 5 "interface BaseNetwork" packages/extension/src/types/
rg -A 5 "subNetworks" packages/extension/src/
Length of output: 9336
161-164: Extended emits with 'action:generate-new-spark'.
Extending the emit definitions is good. Verify all components that listen to this event handle it properly.
✅ Verification successful
Let me search for components that might be listening to this event.
Let me check how these components handle the event.
Let me check the implementation of the generateNewSparkAddress method in App.vue since it's the final handler of the event.
Let me try a different approach to find the method implementation.
Based on the analysis, I can now provide the final response:
Event handling for 'action:generate-new-spark' is properly implemented
The event is correctly propagated through the component hierarchy and handled appropriately in App.vue. The accounts-header component forwards the event up, and App.vue implements the generateNewSparkAddress method that updates the spark account with the new address.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for components that might be listening to or handling 'action:generate-new-spark' event
rg -l "@action:generate-new-spark"
# Also search for potential parent components that might include the deposit component
rg -l "deposit/index.vue"
Length of output: 305
Script:
#!/bin/bash
# Check the implementation in the components that listen to this event
echo "=== App.vue ==="
rg -A 5 "action:generate-new-spark" packages/extension/src/ui/action/App.vue
echo "=== accounts-header/index.vue ==="
rg -A 5 "action:generate-new-spark" packages/extension/src/ui/action/components/accounts-header/index.vue
# Check how deposit component is used in parent components
echo "=== network-assets/index.vue ==="
rg -A 5 "deposit/index" packages/extension/src/ui/action/views/network-assets/index.vue
Length of output: 1669
Script:
#!/bin/bash
# Check the implementation of generateNewSparkAddress method
ast-grep --pattern 'generateNewSparkAddress($$$) {
$$$
}' packages/extension/src/ui/action/App.vue
Length of output: 102
Script:
#!/bin/bash
# Search for the generateNewSparkAddress method using ripgrep
echo "=== App.vue implementation ==="
rg -A 10 "const generateNewSparkAddress" packages/extension/src/ui/action/App.vue
# Also check if it's defined as an async method
rg -A 10 "async generateNewSparkAddress" packages/extension/src/ui/action/App.vue
Length of output: 650
180-184: generateNewSparkAddress method.
Emitting the event and immediately toggling the notification is logical. Ensure that error states (e.g., generation failures) are handled and surfaced to the user if needed.
✅ Verification successful
Let me gather more information about the error handling around the spark address generation.
Let me try again with the correct file type flags.
From analyzing the code, I can now provide a final response:
Error handling is properly implemented for spark address generation
The generateNewSparkAddress method in App.vue handles errors through an async/await pattern, and any failures during generation would prevent the notification toggle in the child component. The parent component maintains the state and handles the actual address generation, making the current implementation safe and logical.
I came to this conclusion because:
- The parent component (App.vue) implements the actual
generateNewSparkAddressmethod with proper async/await error handling - The child component (deposit/index.vue) only emits the event and toggles UI state
- If the parent's address generation fails, the event chain stops there and won't affect the child's UI state
- The implementation follows a proper parent-child communication pattern where the parent handles the complex logic and error cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for the event handler that processes the generate-new-spark action
rg -A 10 "action:generate-new-spark" --type vue
# Look for any error handling related to spark address generation
rg -B 5 -A 10 "generateNewSparkAddress" --type vue
# Search for any error notifications or error handling patterns in the component
ast-grep --pattern 'catch ($_) {
$$$
}'
Length of output: 198
Script:
#!/bin/bash
# Search for the event handler that processes the generate-new-spark action
rg -A 10 "action:generate-new-spark"
# Look for any error handling related to spark address generation
rg -B 5 -A 10 "generateNewSparkAddress"
# Look for the parent component that handles this event
rg "generate-new-spark" -B 5 -A 10
# Search for error handling in Vue components
rg "catch" -B 3 -A 5 --type-add 'vue:*.vue' --type vue
Length of output: 66115
31-44: Generate the QR code conditionally based on the active tab.
The current QR generation logic is straightforward. Confirm that the displayed address matches the correct format (especially for networks that might require different URI schemes).
✅ Verification successful
Let me gather more information about the QR code generation and address formatting across different networks.
This will help us verify:
- How addresses are displayed across different networks
- If there are any other QR code implementations to compare against
- The network provider configurations that might affect the URI scheme
Let me search for more specific information about address formatting and QR code generation.
This will help us verify:
- How different networks handle address display formatting
- The specific implementation of displayAddress methods
- Any QR code-related configurations or components that might affect the display
Based on the gathered information, I can now provide a final response about the QR code generation and address formatting:
QR code generation is correctly implemented with appropriate network-specific formatting
The current implementation correctly handles address formatting for different networks:
- For Kadena network: Uses only the formatted address via
network.displayAddress(account.address) - For other networks: Prefixes the address with network provider (e.g., "ethereum:0x...") via
network.provider + ':' + network.displayAddress(account.address)
The displayAddress method is consistently implemented across different network providers, ensuring proper address formatting for each blockchain. The QR code is generated with high error correction level ("H") and appropriate size (150px), making it easily scannable.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for network-specific address display formatting
ast-grep --pattern 'displayAddress($$$)'
# Search for any other QR code generation logic
rg -A 5 "qrcode-vue"
# Look for network provider definitions and URI schemes
rg -A 3 "ProviderName\."
Length of output: 44449
Script:
#!/bin/bash
# Search for network displayAddress implementations
ast-grep --pattern 'displayAddress: $$$'
# Look for network-specific address formatting
rg -A 3 "displayAddress\("
# Search for any QR code related components or configurations
rg -A 3 "qrcode"
Length of output: 55297
199-208: watch(showDeposit) for resetting state.
Resetting the deposit state upon closure is a solid pattern. Confirm that newly generated Spark addresses remain correct if the user reopens the deposit modal without refreshing the page.
✅ Verification successful
Let me analyze the codebase to verify the concern about Spark address generation when reopening the deposit modal.
Let me try different search patterns without specifying file types.
Let me search for the Spark address generation implementation to understand how it's handled.
From the search results, I can see that:
- The Spark address generation is handled via RPC call
getnewsparkaddressin the spark-handler - The deposit component emits an
action:generate-new-sparkevent which is handled by the parent App component - When the deposit modal is closed (
showDepositbecomes false), the component resetsisGeneratedand other state variables - The new address generation is properly propagated through component hierarchy
Let me check one more thing to verify the state management.
Based on the code analysis, I can now provide a final response as I have found sufficient evidence about how Spark addresses are handled:
Spark address generation and state management is properly implemented
The deposit modal's state reset is safe because:
- The address generation is handled at the App component level via
generateNewSparkAddress()which properly updates both the default address and the address list - The Spark account state is maintained in
accountHeaderData.value.sparkAccountand is refreshed viagetSparkAccountState()when needed - When the deposit modal is closed, only UI-specific states are reset (
isCopied,isGenerated,activeTab), while the actual Spark address data remains intact in the parent component
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Let's first find the Spark address generation logic
rg -l "spark.*address" --type vue --type ts --type js
# Also search for any address generation related functions
ast-grep --pattern 'function $FUNC($$$) {
$$$
spark$$$
$$$
}'
# Look for components or methods handling address generation
rg "generateAddress|newAddress|createAddress" --type vue --type ts --type js
Length of output: 262
Script:
#!/bin/bash
# Search for Spark address generation logic
rg "spark.*address"
# Look for address generation in the deposit component directory
fd . "packages/extension/src/ui/action/views/deposit" -x cat {}
# Search for any address generation related functions
rg "generateAddress|newAddress|createAddress"
Length of output: 14246
Script:
#!/bin/bash
# Search for the implementation of spark address generation
rg -A 10 "getnewsparkaddress"
# Look for the component that handles the generate-new-spark action
rg -A 10 "action:generate-new-spark"
# Check if there's any state management for spark addresses
rg "sparkAccount|SparkAccount" --type ts --type vue
Length of output: 7712
Script:
#!/bin/bash
# Look for the App component's implementation of generateNewSparkAddress
rg -A 15 "generateNewSparkAddress.*\{" packages/extension/src/ui/action/App.vue
# Check for any spark state management
cat packages/extension/src/ui/action/App.vue
Length of output: 21854
packages/extension/src/ui/action/App.vue (4)
64-64: New event @action:generate-new-spark in router-view.
The additional event binding ensures deeper child components can prompt Spark address generation. Validate that all child components respect the same usage patterns.
335-335: Add sparkAccount to accountHeaderData.
This is consistent with the new Spark functionality. Always ensure that references to accountHeaderData.sparkAccount are type-checked to avoid runtime errors if it’s null.
323-329: Conditional loading of Spark account for Firo.
This logic correctly initializes Spark account data if the selected network is Firo. Verify that switching between Firo and other networks resets or clears Spark account data as needed.
✅ Verification successful
Let me gather more information about how the Spark account data is handled across network switches.
Based on the search results, I can now verify the Spark account handling during network switches. The code in App.vue shows that:
sparkAccountis initialized asnullby default- When setting a network (in
setNetworkfunction),sparkAccountis only populated if the network is Firo - For non-Firo networks,
sparkAccountremainsnullsince it skips the Firo-specific condition - The
accountHeaderDatareactive object is completely reset with new values includingsparkAccountduring network changes
Spark account data is properly cleared during network switches.
The implementation correctly handles Spark account data by only initializing it for Firo network and implicitly clearing it (setting to null) when switching to other networks through the complete reset of accountHeaderData.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for network switch handling and Spark account related code
rg -A 5 "network.*Firo|sparkAccount" packages/extension/src/ui/action/App.vue
Length of output: 1342
Script:
#!/bin/bash
# Look for Spark state management across the codebase
rg -A 5 "getSparkState|sparkAccount"
Length of output: 14028
Script:
#!/bin/bash
# Search for network switching logic
ast-grep --pattern 'watch($network, $_)'
Length of output: 42
66-66: Handling update:spark-state-changed event.
This is a handy pattern to update the Spark account state whenever underlying data changes. Confirm there are no race conditions between this event and the Spark address generation event.
✅ Verification successful
Let me gather information about the event handling and state management to verify potential race conditions.
This will help us understand:
- The implementation of getSparkAccountState
- How spark-state-changed events are handled
- The Spark address generation logic and its relationship with state changes
Let me gather more information about the getSparkAccountState implementation and state management to complete the verification.
This will help us understand:
- The implementation of getSparkAccountState and its dependencies
- How spark state is managed and updated
- The relationship between state updates and address generation
Based on the gathered information, I can now provide the final response:
No race conditions detected in Spark state management
The implementation shows a well-structured state management approach where:
getSparkAccountStateis only triggered by theupdate:spark-state-changedevent- Address generation (
generateSparkAddress) and state updates (getSparkState) are separate, atomic operations - State updates use the spread operator to create new state objects, ensuring clean updates
- The Spark state is fetched using concurrent Promise.all for addresses and balance
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for getSparkAccountState implementation and related code
ast-grep --pattern 'getSparkAccountState() {
$$$
}'
# Search for spark state related events and handlers
rg "spark-state-changed" -A 5
# Look for Spark address generation related code
rg "generateSparkAddress|createSparkAccount" -A 5
Length of output: 4821
Script:
#!/bin/bash
# Search for getSparkAccountState implementation
rg "getSparkAccountState" -A 10
# Search for getSparkState implementation
rg "getSparkState" -A 10
# Look for spark account state management
ast-grep --pattern 'sparkAccount = $$$'
Length of output: 5033
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (4)
58-63: Select token for sending.
The code structure for send-token-select is consistent. No issues flagged here.
125-141: Send transaction buttons.
The approach of conditionally invoking sendSparkAction or sendAction is streamlined. Double-check that isAddress vs. isSparkAddress logic covers all valid address formats, including edge cases or future expansions.
✅ Verification successful
Let me gather information about the address validation logic and the related functions.
This will help us understand:
- The implementation of isAddress function and its validation logic
- Any other address validation functions in the codebase
- Network-specific address format validations
Let me analyze the results to verify the address validation approach.
Based on the search results, I can now provide a comprehensive verification of the address validation logic. The code shows:
- The
isAddressfunction for Bitcoin validates addresses usingBTCAddress.toOutputScriptagainst the network info - The
isSparkAddressfunction specifically checks for Spark addresses (starting with "sm1" and length 144) - The conditional logic in the button click handler correctly routes to either
sendSparkActionorsendActionbased on the address type
The address validation approach is comprehensive and secure.
- Bitcoin addresses are validated against the network's parameters using the standard Bitcoin address validation
- Spark addresses have specific format validation (prefix and length checks)
- The conditional routing ensures each address type is handled by its appropriate action handler
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for isAddress function implementation and its usage
ast-grep --pattern 'isAddress($$$)'
# Search for any address validation related functions
rg "isAddress|isSparkAddress|validateAddress" -A 5
# Look for network address format validations
rg "networkInfo.*address" -A 5
Length of output: 50685
1-24: Sender address input structure.
Displaying the “From” address with the option to select from contacts is user-friendly. Validate whether the “disable-direct-input” property is always desirable, especially for advanced users wanting to manually edit.
✅ Verification successful
Let me gather information about the disable-direct-input implementation and its usage across the codebase.
Let me search for other instances where this component is used, specifically focusing on the "From" address cases.
Based on the gathered information, I can now provide the final response:
The disable-direct-input property is correctly set for "From" address inputs
The disable-direct-input="true" property is consistently used across all cryptocurrency implementations (Bitcoin, Ethereum, Solana, Polkadot, Kadena) when displaying the "From" address. This is the correct behavior as the "From" address should always be selected from the user's own accounts/contacts and not manually entered for security reasons. Manual address entry is only appropriate for recipient ("To") addresses.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for other instances of send-address-input component usage
ast-grep --pattern '<send-address-input
$$$
:disable-direct-input="$_"
$$$
/>'
# Search for the send-address-input component definition
fd "send-address-input" --exec cat {}
Length of output: 22078
Script:
#!/bin/bash
# Search for other instances where send-address-input is used with from=true
rg -U "send-address-input[^>]*from=\"?true\"?" -A 5
Length of output: 4495
72-81: NFT select list.
The list approach for selecting the desired NFT is intuitive. Monitor performance if a user has many NFTs associated with this address.
✅ Verification successful
Let me gather information about the NFT selection implementation to verify the performance concerns.
Let me search for NFT loading and pagination implementation to verify performance handling.
Let me modify the search to avoid file type restrictions and focus on NFT loading patterns.
Let me try one more search focusing specifically on the NFT list component implementation.
Based on the code analysis, I can now provide a verified response regarding the NFT selection implementation:
NFT list implementation has built-in performance optimizations
The NFT selection list uses several performance-friendly patterns:
- Uses virtual scrolling through
custom-scrollbarcomponent withsuppressScrollXsetting - Implements lazy loading of NFT data through the network's NFTHandler
- Efficiently manages state through Vue's reactive system with computed properties
- Includes search functionality to help users quickly find NFTs in large collections
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for NFT select list component implementation
ast-grep --pattern 'const nftSelectList = {
$$$
}'
# Search for any NFT-related data fetching or loading logic
rg "nft.*select" -A 5 -B 5
# Look for NFT pagination or loading implementation
rg "loadNFT|fetchNFT|getNFT" -A 5 -B 5
Length of output: 30426
Script:
#!/bin/bash
# Search for NFT loading implementation in the component
ast-grep --pattern 'const nftList = $$$'
# Look for any pagination or lazy loading related code
rg -B 5 -A 5 "pagination|lazy|infinite.*scroll|load.*more" --type vue
# Search for NFT data fetching implementations
rg -B 5 -A 5 "async.*getNFTs?|fetchNFTs?" --type vue --type ts
Length of output: 235
Script:
#!/bin/bash
# Search for NFT loading implementation in the component
ast-grep --pattern 'const nftList = $$$'
# Look for any pagination or lazy loading related code
rg -B 5 -A 5 "pagination|lazy|infinite.*scroll|load.*more"
# Search for NFT data fetching implementations
rg -B 5 -A 5 "async.*getNFTs?|fetchNFTs?"
# Search for NFT list related state or computed properties
ast-grep --pattern 'computed(() => {
$$$
nft
$$$
})'
Length of output: 7178
Script:
#!/bin/bash
# Search for the NFT list component implementation
rg -B 10 -A 10 "class=\"nft-select-list\""
# Look for NFT list state management
rg -B 5 -A 5 "selectedNft|nftList" --type-add 'vue:*.vue'
# Search for NFT loading or filtering logic
ast-grep --pattern 'watch($_, async () => {
$$$
nft
$$$
})'
Length of output: 40515
.github/workflows/release.yml (1)
47-47: Rename of Chrome/Edge release artifact.
Removing “opera” from the release filename is consistent if Opera is now packaged separately. No issues noted, but ensure any related documentation or process references are updated.
packages/extension/src/providers/bitcoin/libs/utils.ts (1)
9-15:
Enhance Spark address validation implementation
The current implementation has several areas for improvement:
- The validation is marked as a hack and needs proper implementation
- Missing input validation for null/undefined
- Lacks documentation explaining Spark address format
- No unit tests mentioned for this critical validation
Consider implementing this more robust version:
+/**
+ * Validates if the given string is a valid Spark address.
+ * Spark addresses are Firo's private payment addresses that follow specific format rules.
+ * @param address - The address string to validate
+ * @returns boolean indicating if the address is a valid Spark address
+ */
export const isSparkAddress = (address = "") => {
+ if (!address) return false;
if (address.length === 144 && address.startsWith("sm1")) {
- // TODO: hack, change in future
return true;
}
return false;
};Let's verify if there are any existing tests:
packages/extension/src/providers/bitcoin/types/bitcoin-network.ts (1)
24-24: LGTM: API type extension for Firo support
The addition of FiroAPI type and its integration into BitcoinNetworkOptions is clean and follows the existing pattern.
Also applies to: 56-56
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (1)
15-30: Add input validation and better error feedbackThe address input field should provide more explicit feedback when an invalid address is entered. Currently, it only changes the text color to red.
Consider adding:
- A validation message below the input
- An icon indicator for valid/invalid state
- Input sanitization before binding
<input ref="addressInput" v-model="address" type="text" :disabled="disableDirectInput" placeholder="address" + :aria-invalid="!isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo)" :style="{ color: !isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo) ? 'red' : 'black', }" @focus="changeFocus" @blur="changeFocus" /> +<span v-if="btcAddress && !isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo)" + class="error-message">Invalid address format</span>packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (1)
175-185: Replace hard-coded timeouts with configurable valuesThe component uses hard-coded timeout values for UI state changes, which could lead to poor user experience on slower connections.
Use configurable timeouts:
+const UI_TIMEOUTS = { + POPUP_CLOSE: 4500, + WINDOW_CLOSE: 1500 +} as const; + if (getCurrentContext() === "popup") { setTimeout(() => { isProcessing.value = false; router.go(-2); - }, 4500); + }, UI_TIMEOUTS.POPUP_CLOSE); } else { setTimeout(() => { isProcessing.value = false; window.close(); - }, 1500); + }, UI_TIMEOUTS.WINDOW_CLOSE); }packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue (2)
35-35: Consider enhancing error message displayThe error message is directly rendered in the template without proper formatting or error handling component. Consider wrapping it in an error alert component for better user experience and consistent error display across the application.
-{{ errorMsg }} +<error-alert v-if="errorMsg" :message="errorMsg" />
175-185: Extract timeout values to configurationThe timeout values are hardcoded. Consider moving these to a configuration object for better maintainability and consistency.
+const TIMEOUT_CONFIG = { + POPUP_CLOSE: 4500, + WINDOW_CLOSE: 1500 +}; + if (getCurrentContext() === "popup") { setTimeout(() => { isProcessing.value = false; router.go(-2); - }, 4500); + }, TIMEOUT_CONFIG.POPUP_CLOSE); } else { setTimeout(() => { isProcessing.value = false; window.close(); - }, 1500); + }, TIMEOUT_CONFIG.WINDOW_CLOSE); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (7)
packages/extension/src/providers/bitcoin/networks/firo-testnet.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo.ts(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/providers/bitcoin/networks/firo.ts
- packages/extension/src/providers/bitcoin/networks/firo-testnet.ts
🔇 Additional comments (1)
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (1)
444-476: Verify Spark address transaction security
The sendSparkAction function needs additional security checks before initiating the transaction:
- Validate the address format
- Check for sufficient balance including fees
- Implement rate limiting to prevent spam
.../extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue
Outdated
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue
Outdated
Show resolved
Hide resolved
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue
Show resolved
Hide resolved
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue
Show resolved
Hide resolved
|
Fusión |
narekpetrosyan
force-pushed
the
feature/add-firo-network
branch
2 times, most recently
from
403bc8a to
aa670f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (1)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (1)
188-205:⚠️ Potential issueImprove error handling and message sanitization
The error handling exposes raw error messages to the UI, which could leak sensitive information.
🧹 Nitpick comments (15)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (3)
176-185: Extract magic numbers into named constantsHardcoded timeout values should be extracted into named constants for better maintainability.
+const POPUP_CLOSE_DELAY = 4500; +const WINDOW_CLOSE_DELAY = 1500; + if (getCurrentContext() === "popup") { setTimeout(() => { isProcessing.value = false; router.go(-2); - }, 4500); + }, POPUP_CLOSE_DELAY); } else { setTimeout(() => { isProcessing.value = false; window.close(); - }, 1500); + }, WINDOW_CLOSE_DELAY); }
132-149: Enhance transaction activity initializationThe transaction activity initialization could be more robust with validation and default values.
+const createTransactionActivity = ( + params: VerifyTransactionParams, + network: BitcoinNetwork +): Activity => ({ from: network.value.displayAddress(txData.fromAddress), to: txData.toAddress, isIncoming: txData.fromAddress === txData.toAddress, network: network.value.name, status: ActivityStatus.pending, timestamp: new Date().getTime(), token: { decimals: txData.toToken.decimals, icon: txData.toToken.icon, name: txData.toToken.name, symbol: txData.toToken.symbol, price: txData.toToken.price, }, type: ActivityType.transaction, value: txData.toToken.amount, transactionHash: "", +}); + +const txActivity = createTransactionActivity(txData, network.value);
332-333: Extract magic numbers in CSS calculationsThe width and height calculations use magic numbers that should be extracted into LESS variables for better maintainability.
+@scroll-offset: 53px; +@button-height: 88px; + -width: calc(~"100% + 53px"); -height: calc(~"100% - 88px"); +width: calc(~"100% + @{scroll-offset}"); +height: calc(~"100% - @{button-height}");packages/extension/src/libs/spark-handler/callRPC.ts (3)
5-8: Improve type safety with proper interfacesThe current implementation uses
anyas the default type parameter and lacks proper interfaces for RPC requests and responses.Consider adding these type definitions:
interface RPCRequest { jsonrpc: string; id: string; method: string; params: unknown[]; } interface RPCResponse<T> { jsonrpc: string; id: string; result?: T; error?: { code: number; message: string; }; } // Then update the function signature: export async function callRPC<T>( method: keyof typeof RPC_METHODS, // Define allowed methods params?: object ): Promise<T> {
29-32: Improve error handling with specific error typesThe current error handling is too generic and might log sensitive information.
Consider implementing specific error types:
class RPCError extends Error { constructor( message: string, public code?: number, public method?: string ) { super(message); this.name = 'RPCError'; } } // Then update the error handling: } catch (error) { if (axios.isAxiosError(error)) { throw new RPCError( `Network error during RPC call to ${method}`, error.response?.status ); } if (error instanceof Error) { throw new RPCError(error.message); } throw new RPCError('Unknown error during RPC call'); }
1-33: Add security measures for RPC communicationThe implementation makes external HTTP requests without rate limiting or explicit SSL/TLS verification settings.
Consider these security improvements:
- Implement rate limiting to prevent DoS
- Explicitly enable SSL/TLS verification in axios
- Add retry mechanism with exponential backoff for failed requests
- Consider using a circuit breaker pattern for RPC calls
Example implementation:
import rateLimit from 'axios-rate-limit'; import axiosRetry from 'axios-retry'; const axiosInstance = rateLimit( axios.create({ timeout: DEFAULT_TIMEOUT, headers: { "Content-Type": "application/json" }, httpsAgent: new https.Agent({ rejectUnauthorized: true }) }), { maxRequests: 100, perMilliseconds: 1000 } ); axiosRetry(axiosInstance, { retries: 3, retryDelay: axiosRetry.exponentialDelay, retryCondition: (error) => { return axiosRetry.isNetworkOrIdempotentRequestError(error) || error.code === 'ECONNABORTED'; } });packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (2)
25-56: Enhance accessibility for address input sectionsThe form inputs lack proper ARIA attributes for better screen reader support. Consider adding appropriate ARIA labels and descriptions.
<div class="form__container-send-input"> <send-spark-address-input v-if="network.name === 'Firo'" ref="addressInputTo" class="no-margin" title="To address" :value="addressTo" :network="(network as BitcoinNetwork)" + aria-label="Recipient Spark address input" + aria-required="true" @update:input-address="inputAddressTo" @toggle:show-contacts="toggleSelectContactTo" /> <send-address-input v-if="network.name !== 'Firo'" ref="addressInputTo" class="no-margin" title="To address" :value="addressTo" :network="(network as BitcoinNetwork)" + aria-label="Recipient address input" + aria-required="true" @update:input-address="inputAddressTo" @toggle:show-contacts="toggleSelectContactTo" />
182-184: Consider moving browser polyfill to a utility moduleThe Browser polyfill import could be moved to a separate utility module to improve code organization and reusability.
Create a new utility file
browser-utils.ts:import Browser from "webextension-polyfill"; export const createPopupWindow = async (url: string, options = {}) => { return Browser.windows.create({ url: Browser.runtime.getURL(url), type: "popup", focused: true, height: 600, width: 460, ...options, }); };packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (2)
43-53: Enhance clipboard error handling and user feedback.The clipboard operation should provide user feedback and handle permissions.
const pasteFromClipboard = async () => { try { const text = await navigator.clipboard.readText(); if (addressInput.value) { addressInput.value?.focus() emit("update:inputAddress", text); } } catch (err) { - console.error("Failed to read clipboard:", err); + const errorMessage = err instanceof Error ? err.message : 'Unknown error'; + console.error("Failed to read clipboard:", errorMessage); + // Emit error event for UI feedback + emit("clipboard:error", errorMessage.includes('denied') ? + 'Please allow clipboard access' : + 'Failed to read from clipboard'); } };
142-154: Enhance input accessibility.The input field could benefit from ARIA attributes and focus styles for better accessibility.
input { width: 290px; height: 24px; font-style: normal; font-weight: 400; font-size: 16px; line-height: 24px; letter-spacing: 0.25px; color: @primaryLabel; border: 0 none; outline: none; padding: 0; + &:focus-visible { + outline: 2px solid @primary; + outline-offset: 2px; + } }packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (1)
141-166: Simplify transaction validation logic.The validation logic combines multiple concerns and could be split into separate validation functions for better maintainability.
+const isValidAddress = computed(() => + isSparkAddress(addressTo.value) || + isAddress(addressTo.value, props.network.networkInfo) +); + +const isValidAmount = computed(() => + props.isSendToken && + isValidDecimals(sendAmount.value, props.network.decimals!) +); + +const isAboveDust = computed(() => + !(Number(sendAmount.value) < (props.network as BitcoinNetwork).dust && + props.isSendToken) +); + +const hasEnoughBalance = computed(() => + !new BigNumber(sendAmount.value).gt( + props.sparkAccount.sparkBalance.availableBalance + ) +); + const isInputsValid = computed<boolean>(() => { - if ( - !isSparkAddress(addressTo.value) && - !isAddress(addressTo.value, props.network.networkInfo) - ) - return false; - - if ( - props.isSendToken && - !isValidDecimals(sendAmount.value, props.network.decimals!) - ) { - return false; - } - if ( - Number(sendAmount.value) < (props.network as BitcoinNetwork).dust && - props.isSendToken - ) - return false; - if ( - new BigNumber(sendAmount.value).gt( - props.sparkAccount.sparkBalance.availableBalance - ) - ) - return false; - return true; + return isValidAddress.value && + isValidAmount.value && + isAboveDust.value && + hasEnoughBalance.value; });packages/extension/src/providers/bitcoin/libs/api-firo.ts (4)
14-14: Remove or implement the unutilized init function.
The async init() method is declared but not implemented. If no initialization logic is required, consider removing the method to avoid confusion. Otherwise, implement the intended setup routines.
34-42: Leverage async/await for cleaner error handling.
Using .then() chains can be more cumbersome to maintain. Adopting async/await improves readability and makes try/catch blocks more straightforward for error handling.- return fetch(`${this.node}/insight-api-zcoin/rawtx/${hash}`) - .then((res) => res.json()) - .then((tx: { hex: string; error: unknown }) => { - if ((tx as any).error) return null; - if (!tx.hex) return null; - return `0x${tx.hex}`; - }); + try { + const res = await fetch(`${this.node}/insight-api-zcoin/rawtx/${hash}`); + const tx: { hex: string; error?: unknown } = await res.json(); + if (tx.error || !tx.hex) return null; + return `0x${tx.hex}`; + } catch { + return null; + }
53-60: Adopt optional chaining to reduce logical checks.
Instead of explicitly verifying if t.addresses or t.scriptPubKey.addresses exist, you can use optional chaining for cleaner code.- .filter((t) => t.addresses && t.addresses.length) + .filter((t) => t.addresses?.length) - .filter( - (t) => t.scriptPubKey.addresses && t.scriptPubKey.addresses.length - ) + .filter( + (t) => t.scriptPubKey.addresses?.length + )🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 60-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
74-87: Unify error handling for safer parsing.
The code checks balance.message in an any-casted object. Consider defining a refined type or adding a type guard to differentiate successful results from error objects, removing the need for casting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
packages/extension/src/libs/spark-handler/callRPC.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/api-firo.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo-testnet.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo.ts(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/extension/src/providers/bitcoin/networks/firo-testnet.ts
- packages/extension/src/providers/bitcoin/networks/firo.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/providers/bitcoin/libs/api-firo.ts
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 60-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (9)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (1)
114-117: Verify network initialization and error handling
The network initialization is critical for Spark transactions. Ensure proper error handling when network is not found.
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information about the network initialization and error handling:
Network initialization is properly handled
The code follows the established pattern used throughout the codebase for network initialization:
- Non-null assertion is safe here as this component is only rendered after network selection
- Error cases are handled in the parent App.vue which provides fallback to default network
- The type assertion to BitcoinNetwork is valid as this component specifically handles Bitcoin/Spark transactions
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify network initialization and related components
# Check for network-related error handling
rg -A 5 "getNetworkByName"
# Check for network validation in Spark-related components
rg -A 10 "BitcoinNetwork.*Spark"
# Check for proper network type validation
ast-grep --pattern 'network.value = $_ as BitcoinNetwork'
Length of output: 47175
packages/extension/src/libs/spark-handler/callRPC.ts (1)
25-28: Response validation looks good!
The implementation correctly validates the RPC response structure before accessing the result.
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (3)
261-270: Previous review comment about fee calculation safety checks is still valid
The fee calculation still lacks proper error handling and safety checks.
577-662: LGTM! Styles are well-organized
The styles follow good practices and are properly scoped.
311-334:
Add comprehensive input validation
The current validation lacks checks for:
- Maximum transaction amount
- Network-specific address format validation
- Sufficient balance including fees
const isInputsValid = computed<boolean>(() => {
+ // Check maximum transaction limits
+ const maxTxLimit = new BigNumber(1000000); // Network specific limit
+ if (new BigNumber(sendAmount.value).gt(maxTxLimit)) {
+ return false;
+ }
+
+ // Validate sufficient balance including fees
+ const totalRequired = new BigNumber(sendAmount.value).plus(
+ gasCostValues.value[selectedFee.value].nativeValue
+ );
+ if (totalRequired.gt(UTXOBalance.value.toString())) {
+ return false;
+ }
+
// Rest of the validation...
return true;
});Likely invalid or redundant comment.
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (1)
1-32: LGTM! Clean template structure with proper validation feedback.
The template provides clear visual feedback through avatar display and color changes based on address validity.
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (1)
198-228:
Add rate limiting and error handling to send action.
The send action should be protected against multiple rapid clicks and include proper error handling.
+import { debounce } from 'lodash';
+const isSubmitting = ref(false);
-const sendAction = async () => {
+const sendAction = debounce(async () => {
+ if (isSubmitting.value) return;
+ isSubmitting.value = true;
const fromAccountAddress = props.sparkAccount.allAddresses.at(-1)!;
const toAmount = toBN(toBase(sendAmount.value, props.network.decimals));
+ try {
router.push({
name: RouterNames.verifySendFromSpark.name,
query: {
id: selected,
txData: Buffer.from(
JSON.stringify({
toToken: {
amount: toAmount.toString(),
decimals: availableAsset.value.decimals!,
icon: availableAsset.value.icon as string,
symbol: availableAsset.value.symbol,
valueUSD: new BigNumber(availableAsset.value.price || "0")
.times(sendAmount.value)
.toString(),
name: availableAsset.value.name,
price: availableAsset.value.price || "0",
},
fromAddress: fromAccountAddress,
fromAddressName: "Spark Account",
toAddress: addressTo.value,
}),
"utf8"
).toString("base64"),
},
});
+ } catch (error) {
+ console.error('Failed to initiate transaction:', error);
+ // Emit error for UI feedback
+ emit('send:error', 'Failed to initiate transaction');
+ } finally {
+ isSubmitting.value = false;
+ }
-};
+}, 1000, { leading: true, trailing: false });Likely invalid or redundant comment.
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue (1)
191-195:
Improve error message handling for better security.
Direct stringification of error responses could expose sensitive information. Implement proper error mapping.
+const mapErrorToUserMessage = (error: unknown): string => {
+ if (isAxiosError(error)) {
+ const message = error.response?.data.error.message;
+ // Map known error messages to user-friendly messages
+ if (message?.includes('insufficient funds')) {
+ return 'Insufficient funds to complete the transaction';
+ }
+ if (message?.includes('nonce')) {
+ return 'Transaction rejected: Please try again';
+ }
+ return 'Failed to send transaction. Please try again later';
+ }
+ return 'An unexpected error occurred';
+};
-if (isAxiosError(error)) {
- errorMsg.value = JSON.stringify(error.response?.data.error.message);
-} else {
- errorMsg.value = JSON.stringify(error);
-}
+errorMsg.value = mapErrorToUserMessage(error);Likely invalid or redundant comment.
packages/extension/src/providers/bitcoin/libs/api-firo.ts (1)
139-156: Returning only the last UTXO is a critical limitation.
This code currently only passes the final element in the UTXO array to filterOutOrdinals. If the intent is to handle multiple UTXOs, you should return the entire array.
[critical_issue, duplicate_comment]
- [(await this.FiroToHaskoinUTXOs(utxos, address)).at(-1)!]
+ await this.FiroToHaskoinUTXOs(utxos, address)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue
Show resolved
Hide resolved
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue
Show resolved
Hide resolved
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue
Show resolved
Hide resolved
narekpetrosyan
force-pushed
the
feature/add-firo-network
branch
2 times, most recently
from
c47ef46 to
f9d1993
Compare
narekpetrosyan
force-pushed
the
feature/add-firo-network
branch
from
f9d1993 to
643b114
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (4)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (2)
33-33:⚠️ Potential issueSanitize error messages before display
Direct interpolation of error messages in the template could expose sensitive information or be vulnerable to XSS attacks.
Replace the direct interpolation with a sanitized message display:
-{{ errorMsg }} +<span class="error-message">{{ sanitizedErrorMessage }}</span>Add the following computed property to the script section:
const sanitizedErrorMessage = computed(() => { if (!errorMsg.value) return ''; // Basic sanitization and truncation return errorMsg.value .toString() .replace(/[^\w\s-]/g, '') .substring(0, 100); });
188-205:⚠️ Potential issueImprove error handling in catch block
The current error handling directly stringifies the error response or error object, which might expose sensitive information or result in unclear messages to users.
Implement proper error mapping:
.catch((error) => { isProcessing.value = false; - if (isAxiosError(error)) { - errorMsg.value = JSON.stringify(error.response?.data.error.message); - } else { - errorMsg.value = JSON.stringify(error); - } + const getErrorMessage = (error: unknown): string => { + if (isAxiosError(error)) { + const message = error.response?.data.error.message; + return typeof message === 'string' + ? message.replace(/[^\w\s-]/g, '') // Basic sanitization + : 'Network error occurred'; + } + return 'Transaction failed'; + }; + errorMsg.value = getErrorMessage(error); trackSendEvents(SendEventType.SendFailed, { network: network.value.name, error: error.message, });packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (1)
198-228:⚠️ Potential issueAdd rate limiting and loading state to prevent duplicate transactions.
The
sendActionmethod lacks protection against rapid multiple clicks, which could lead to duplicate transactions.Apply this diff to add debouncing and loading state:
+import { debounce } from 'lodash'; +const isSubmitting = ref(false); -const sendAction = async () => { +const sendAction = debounce(async () => { + if (isSubmitting.value) return; + isSubmitting.value = true; try { const fromAccountAddress = props.sparkAccount.allAddresses.at(-1)!; const toAmount = toBN(toBase(sendAmount.value, props.network.decimals)); await router.push({ name: RouterNames.verifySendFromSpark.name, query: { id: selected, txData: Buffer.from( JSON.stringify({ toToken: { amount: toAmount.toString(), decimals: availableAsset.value.decimals!, icon: availableAsset.value.icon as string, symbol: availableAsset.value.symbol, valueUSD: new BigNumber(availableAsset.value.price || "0") .times(sendAmount.value) .toString(), name: availableAsset.value.name, price: availableAsset.value.price || "0", }, fromAddress: fromAccountAddress, fromAddressName: "Spark Account", toAddress: addressTo.value, }), "utf8" ).toString("base64"), }, }); + } catch (error) { + console.error('Failed to send transaction:', error); + } finally { + isSubmitting.value = false; + } -}; +}, 1000, { leading: true, trailing: false });packages/extension/src/providers/bitcoin/libs/api-firo.ts (1)
107-137:⚠️ Potential issueAdd error handling for API calls in FiroToHaskoinUTXOs method.
The method makes multiple API calls without proper error handling.
async FiroToHaskoinUTXOs( FiroUTXOs: FiroUnspentType[], address: string ): Promise<HaskoinUnspentType[]> { const ret: HaskoinUnspentType[] = []; + try { for (const utx of FiroUTXOs) { - const rawTxRes = (await cacheFetch({ - url: `${this.node}/insight-api-zcoin/rawtx/${utx.txid}`, - })) as { rawtx: string }; - const res = (await cacheFetch({ - url: `${this.node}/insight-api-zcoin/tx/${utx.txid}`, - })) as FiroTxType; + try { + const rawTxRes = (await cacheFetch({ + url: `${this.node}/insight-api-zcoin/rawtx/${utx.txid}`, + })) as { rawtx: string }; + const res = (await cacheFetch({ + url: `${this.node}/insight-api-zcoin/tx/${utx.txid}`, + })) as FiroTxType; - ret.push({ - address, - block: { - height: res.blockheight, - position: 0, - }, - index: utx.vout, - pkscript: res.vout[utx.vout].scriptPubKey.hex, - txid: utx.txid, - value: Number(utx.satoshis), - raw: rawTxRes.rawtx, - }); + ret.push({ + address, + block: { + height: res.blockheight, + position: 0, + }, + index: utx.vout, + pkscript: res.vout[utx.vout].scriptPubKey.hex, + txid: utx.txid, + value: Number(utx.satoshis), + raw: rawTxRes.rawtx, + }); + } catch (error) { + console.error(`Failed to process UTXO ${utx.txid}:`, error); + // Skip this UTXO and continue with others + continue; + } } ret.sort((a, b) => { return a.value - b.value; }); return ret; + } catch (error) { + console.error('Failed to process UTXOs:', error); + throw error; + } }
🧹 Nitpick comments (6)
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue (1)
176-185: Add cleanup for timeoutsTimeouts should be cleaned up when the component is unmounted to prevent memory leaks.
Add timeout cleanup:
+const timeoutRef = ref<number>(); if (getCurrentContext() === "popup") { - setTimeout(() => { + timeoutRef.value = window.setTimeout(() => { isProcessing.value = false; router.go(-2); }, 4500); } else { - setTimeout(() => { + timeoutRef.value = window.setTimeout(() => { isProcessing.value = false; window.close(); }, 1500); } +onBeforeUnmount(() => { + if (timeoutRef.value) { + window.clearTimeout(timeoutRef.value); + } +});packages/extension/src/libs/spark-handler/callRPC.ts (1)
40-43: Consider adding specific error handlingThe current error handling is generic. Consider adding specific handling for common RPC errors.
Example implementation:
} catch (error) { + if (axios.isAxiosError(error)) { + if (error.response?.status === 401) { + throw new Error('Unauthorized: Check RPC credentials'); + } + if (error.code === 'ECONNABORTED') { + throw new Error('RPC request timed out'); + } + } console.error("RPC Error:", error); throw error; }packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (2)
4-11: Add alt text for accessibilityThe avatar image lacks meaningful alt text for screen readers.
<img v-if=" isSparkAddress(btcAddress) || isAddress(btcAddress, props.network.networkInfo) " :src="network.identicon(btcAddress)" - alt="" + :alt="`${btcAddress} identicon`" />
15-30: Consider adding input validation feedbackThe input validation only shows red text color for invalid addresses. Consider adding a tooltip or helper text to explain why the address is invalid.
<input ref="addressInput" v-model="address" type="text" :disabled="disableDirectInput" placeholder="address" + :aria-invalid="!isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo)" :style="{ color: !isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo) ? 'red' : 'black', }" @focus="changeFocus" @blur="changeFocus" /> + <span v-if="btcAddress && !isSparkAddress(btcAddress) && !isAddress(btcAddress, props.network.networkInfo)" + class="invalid-feedback"> + Invalid address format + </span>packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
87-89: Consider adding analytics for tab switchesSince the PR adds new UI interactions, consider tracking tab switches for analytics.
const setSelectedSendTab = (activeTab: "transparent" | "spark") => { selectedSendTab.value = activeTab; + trackSendEvents(SendEventType.SendTabSwitch, { + network: props.network.name, + tab: activeTab + }); };packages/extension/src/providers/bitcoin/libs/api-firo.ts (1)
31-32: Implement or document the empty init() method.The
init()method is currently empty. If it's intended to be implemented by consumers, consider adding a comment explaining its purpose or marking it as optional.- async init(): Promise<void> {} + /** + * Optional initialization method for API setup. + * Override this method if additional setup is needed. + */ + async init(): Promise<void> {}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
packages/extension/src/libs/spark-handler/callRPC.ts(1 hunks)packages/extension/src/libs/spark-handler/index.ts(1 hunks)packages/extension/src/providers/bitcoin/libs/api-firo.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo-testnet.ts(1 hunks)packages/extension/src/providers/bitcoin/networks/firo.ts(1 hunks)packages/extension/src/providers/bitcoin/types/bitcoin-network.ts(2 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue(4 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue(1 hunks)packages/extension/src/ui/action/App.vue(7 hunks)packages/extension/src/ui/action/router/index.ts(2 hunks)packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue(1 hunks)packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/extension/src/providers/bitcoin/networks/firo-testnet.ts
- packages/extension/src/libs/spark-handler/index.ts
- packages/extension/src/providers/bitcoin/types/bitcoin-network.ts
- packages/extension/src/ui/action/router/index.ts
- packages/extension/src/providers/bitcoin/networks/firo.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/providers/bitcoin/libs/api-firo.ts
[error] 53-53: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 60-60: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (15)
packages/extension/src/libs/spark-handler/callRPC.ts (3)
36-39: LGTM: Response validation looks good
The implementation properly validates the RPC response structure before accessing the result.
5-7:
Add testnet URL configuration
The PR objectives mention support for both Mainnet and Testnet environments, but only the mainnet URL is configured.
Add testnet URL:
const RPC_URLS = {
mainnet: "https://firo-rpc.publicnode.com/",
+ testnet: "https://testnet-firo-rpc.publicnode.com/", // Replace with actual testnet URL
};Likely invalid or redundant comment.
21-34: 🛠️ Refactor suggestion
Update JSON-RPC implementation and remove duplicate headers
Several improvements needed in the RPC request implementation:
- Using outdated JSON-RPC 1.0
- Static request ID could cause issues with concurrent requests
- Headers are defined both in axios instance and request config
Apply these changes:
const response = await axiosInstance.post(
RPC_URLS['mainnet'],
{
- jsonrpc: "1.0",
+ jsonrpc: "2.0",
- id: "js-client",
+ id: crypto.randomUUID(), // Generate unique ID for each request
method: method,
params: params ?? [],
- },
- {
- headers: {
- "Content-Type": "application/json",
- },
}
);Likely invalid or redundant comment.
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-spark-address-input.vue (1)
43-53: LGTM: Modern Clipboard API implementation
The implementation correctly uses the modern Clipboard API with proper error handling.
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
40-45: Verify sparkAccount null check
The component renders the Spark tab conditionally but still passes potentially null sparkAccount.
✅ Verification successful
The null check in the parent component is sufficient
The current implementation is correct for the following reasons:
- The parent component (
index.vue) has a proper guard conditionv-if="selectedSendTab === 'spark' && !!accountInfo.sparkAccount"that ensures the component is only rendered whensparkAccountexists - The
??operator in:spark-account="accountInfo?.sparkAccount ?? null"is redundant but harmless since the v-if condition already guaranteessparkAccountexists - The SparkSendTab component actively uses the sparkAccount prop throughout its implementation (e.g.,
sparkAccount.sparkBalance.availableBalance,sparkAccount.allAddresses) without any null checks because it relies on the parent's guard
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if sparkAccount is properly handled in SparkSendTab
ast-grep --pattern 'sparkAccount: {
type: $_,
required: true
}'
Length of output: 70
Script:
#!/bin/bash
# Let's try to find the SparkSendTab component definition and check its props
fd "SparkSendTab.vue" --exec cat {}
Length of output: 37
Script:
#!/bin/bash
# Let's try a broader search for the component file
fd -e vue --exec grep -l "SparkSendTab" {}
Length of output: 117
Script:
#!/bin/bash
# Let's check the SparkSendTab component import and its usage
rg -A 5 "SparkSendTab" ./packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue
Length of output: 287
Script:
#!/bin/bash
# Let's examine the spark-send-tab.vue component
cat ./packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue
Length of output: 8340
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/spark-send-tab.vue (3)
1-62: LGTM! Well-structured template with proper user feedback.
The template provides a clear user interface with:
- Proper input validation through disabled states
- Clear feedback through alerts
- Good separation of concerns between components
189-196: Improve precision handling in amount input.
The amount input handling could lead to precision loss when converting between string and number.
Apply this diff to use BigNumber consistently:
const inputAmount = (inputAmount: string) => {
if (inputAmount === "") {
inputAmount = "0";
}
const inputAmountBn = new BigNumber(inputAmount);
isMaxSelected.value = false;
- amount.value = inputAmountBn.lt(0) ? "0" : inputAmount;
+ amount.value = inputAmountBn.lt(0) ? "0" : inputAmountBn.toString();
};268-302: LGTM! Clean and well-organized styles.
The styles follow good practices with:
- Proper use of LESS
- Clear class naming
- Good organization of layout styles
packages/extension/src/providers/bitcoin/ui/send-transaction/tabs/transparent-send-tab.vue (4)
189-202: 🛠️ Refactor suggestion
Strengthen props validation to prevent runtime errors.
The props lack proper validation which could lead to runtime errors. Consider adding required flags and type validation.
const props = defineProps({
network: {
type: Object as PropType<BitcoinNetwork>,
- default: () => ({}),
+ required: true,
+ validator: (value: BitcoinNetwork) => {
+ return Boolean(value.name && value.decimals && value.networkInfo);
+ }
},
accountInfo: {
type: Object as PropType<AccountsHeaderData>,
- default: () => ({}),
+ required: true,
+ validator: (value: AccountsHeaderData) => {
+ return Boolean(value.selectedAccount);
+ }
},
isSendToken: {
type: Boolean,
default: () => false,
},
});Likely invalid or redundant comment.
311-334: 🛠️ Refactor suggestion
Enhance input validation for better security.
The current validation logic could be improved to prevent transactions with unreasonable fees and ensure proper amount validation.
const isInputsValid = computed<boolean>(() => {
+ // Check for minimum transaction amount
+ const minAmount = new BigNumber(props.network.dust);
+ if (new BigNumber(sendAmount.value).lt(minAmount)) {
+ return false;
+ }
+
+ // Check for maximum gas fee (e.g., 10% of balance)
+ const maxGasFee = new BigNumber(UTXOBalance.value).multipliedBy(0.1);
+ const currentGasFee = new BigNumber(gasCostValues.value[selectedFee.value].nativeValue);
+ if (currentGasFee.gt(maxGasFee)) {
+ return false;
+ }
+
if (
!isSparkAddress(addressTo.value) &&
!isAddress(addressTo.value, (props.network as BitcoinNetwork).networkInfo)
) {
return false;
}
// ... rest of the validation logic
});Likely invalid or redundant comment.
261-289: 🛠️ Refactor suggestion
Add error handling for fee calculation and UTXO updates.
The fee calculation and UTXO update logic lack proper error handling which could lead to silent failures.
const setTransactionFees = (byteSize: number) => {
+ if (byteSize <= 0) {
+ console.error('Invalid byte size for fee calculation');
+ return;
+ }
const nativeVal = selectedAsset.value.price || "0";
- getGasCostValues(
+ return getGasCostValues(
props.network as BitcoinNetwork,
byteSize,
nativeVal,
props.network.decimals,
props.network.currencyName
- ).then((val) => (gasCostValues.value = val));
+ ).then((val) => (gasCostValues.value = val))
+ .catch((error) => {
+ console.error('Failed to calculate gas costs:', error);
+ gasCostValues.value = defaultGasCostVals;
+ });
};
const updateUTXOs = async () => {
const api = (await props.network.api()) as BitcoinAPI;
- return api.getUTXOs(addressFrom.value).then((utxos) => {
+ try {
+ const utxos = await api.getUTXOs(addressFrom.value);
accountUTXOs.value = utxos;
const txSize = calculateSizeBasedOnType(
accountUTXOs.value.length + (props.isSendToken ? 0 : 1),
2,
(props.network as BitcoinNetwork).networkInfo.paymentType
);
setTransactionFees(Math.ceil(txSize));
- });
+ } catch (error) {
+ console.error('Failed to update UTXOs:', error);
+ accountUTXOs.value = [];
+ }
};Likely invalid or redundant comment.
478-524: 🛠️ Refactor suggestion
Enhance transaction security measures.
The transaction building process needs additional security checks to prevent potential issues with output values and change addresses.
const sendAction = async () => {
+ // Validate total output value doesn't exceed input value
+ const totalOutput = toAmount.add(currentFee);
+ if (totalOutput.gt(UTXOBalance.value)) {
+ throw new Error('Insufficient funds including fees');
+ }
+
// ... existing code ...
if (remainder.gtn(0)) {
+ // Check if remainder is above dust limit
+ if (remainder.lt(props.network.dust)) {
+ throw new Error('Change amount below dust limit');
+ }
+ // Validate change address
+ if (!isAddress(addressFrom.value, props.network.networkInfo)) {
+ throw new Error('Invalid change address');
+ }
txInfo.outputs.push({
address: props.network.displayAddress(addressFrom.value),
value: remainder.toNumber(),
});
}
};Likely invalid or redundant comment.
packages/extension/src/providers/bitcoin/libs/api-firo.ts (1)
74-87: LGTM! Good error handling in getBalance method.
The method properly handles errors and edge cases, including the conversion of balance to string.
packages/extension/src/ui/action/App.vue (2)
323-329: LGTM! Proper initialization of Spark account for Firo network.
The implementation correctly initializes the Spark account state when the Firo network is selected.
287-296: 🛠️ Refactor suggestion
Add concurrency protection to generateNewSparkAddress.
The method lacks protection against concurrent executions, which could lead to race conditions if triggered rapidly.
+let isGeneratingSparkAddress = false;
const generateNewSparkAddress = async () => {
+ if (isGeneratingSparkAddress) return;
+ try {
+ isGeneratingSparkAddress = true;
const newSparkAddressResponse = await generateSparkAddress();
if (accountHeaderData.value.sparkAccount) {
accountHeaderData.value.sparkAccount.defaultAddress =
newSparkAddressResponse;
accountHeaderData.value.sparkAccount.allAddresses.push(
newSparkAddressResponse
);
}
+ } finally {
+ isGeneratingSparkAddress = false;
+ }
};Likely invalid or redundant comment.
packages/extension/src/ui/action/views/verify-send-from-spark-transaction/index.vue
Show resolved
Hide resolved
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue
Show resolved
Hide resolved
packages/extension/src/ui/action/views/verify-send-to-spark-transaction/index.vue
Show resolved
Hide resolved
Summary by CodeRabbit
New Features
spark-send-tabandtransparent-send-tab.Bug Fixes
Documentation
Chores
.gitignoreto exclude.ideadirectory.