-
Notifications
You must be signed in to change notification settings - Fork 174
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release: v2.0.2 #591
base: main
Are you sure you want to change the base?
Release: v2.0.2 #591
Conversation
devop: switch solana asset handler to helius
WalkthroughThis pull request introduces several minor updates across different blockchain provider components in the Enkrypt extension. The changes primarily focus on improving error messaging, adding support for token icons, and refining input validation logic. Key modifications include updating version numbers, introducing an optional Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
💼 Build Files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/extension/src/providers/ethereum/libs/api.ts (1)
79-79
: Clarify placeholdericon
usage
In the catch block,icon
is always set toundefined
. Confirm there is no fallback icon scenario.packages/extension/src/providers/solana/libs/api.ts (2)
26-26
: Confirm necessity of emptyinit()
Thisinit()
method is empty. If unused, consider removing it or using it for any async setup logic (e.g., pre-fetching data).
100-109
: Conditional icon injection
Reading the first file URI for the icon is intuitive. Consider additional checks if multiple files are available.packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts (1)
338-341
: Consider extracting the fallback icon logic into a helper function for maintainability.The fallback logic is correct. However, centralizing it in a helper function or utility could improve consistency across the codebase, especially if this pattern for handling icons is replicated in multiple places.
Below is an example of how you might define and reuse a helper function. Note that this new function would need to reside outside the selected line range, so no diff is provided to encapsulate the entire change, but you can still see how to reference it in the changed lines:
// Move this function to a suitable location in your codebase. function getFallbackIcon( tokenInfo: Record<string, CGToken>, tInfo: { icon?: string }, network: BaseNetwork, tokenAddress: string ): string { return ( tokenInfo[tokenAddress]?.logoURI || tInfo.icon || network.icon ); }Then change lines 338-341 accordingly:
- icon: - tokenInfo[unknownTokens[idx]]?.logoURI || - tInfo.icon || - network.icon, + icon: getFallbackIcon(tokenInfo, tInfo, network, unknownTokens[idx]),
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
packages/extension/package.json
(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vue
(2 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/providers/ethereum/libs/api.ts
(1 hunks)packages/extension/src/providers/ethereum/libs/assets-handlers/assetinfo-mew.ts
(1 hunks)packages/extension/src/providers/ethereum/types/index.ts
(1 hunks)packages/extension/src/providers/ethereum/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/providers/kadena/ui/send-transaction/index.vue
(1 hunks)packages/extension/src/providers/solana/libs/api.ts
(3 hunks)packages/extension/src/providers/solana/types/sol-token.ts
(1 hunks)packages/extension/src/providers/solana/ui/send-transaction/index.vue
(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/extension/package.json
🔇 Additional comments (12)
packages/extension/src/providers/kadena/ui/send-transaction/index.vue (1)
325-325
: Consistent Error MessagingChanging the error message to "Not enough balance." helps maintain consistency across different transaction flows. This unified phrasing is clearer to the user and aligns with the convention used in other providers.
packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (1)
99-99
: Expose zero balance as a reusable propThe newly introduced
:is-balance-zero="UTXOBalance.isZero()"
prop helps the component correctly handle scenarios where the detected UTXO balance is zero. This is a good approach to ensure your UI can differentiate zero balances and provide a more tailored user message or experience.packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (2)
454-454
: Improved user feedback for insufficient fundsReplacing "Amount exceeds maximum value." with "Not enough balance." is more direct, aligning with usage across other providers. This phrasing better communicates the underlying cause of the error from the user's perspective.
565-566
: Validate numerical input before heavy processingThe introduction of:
const sendAmountBigNumber = new BigNumber(sendAmount.value); if (sendAmountBigNumber.isNaN()) return false;ensures that non-numeric inputs are caught early, preventing potential downstream errors in calculations. This is a good validation checkpoint that avoids unpredictable behavior.
packages/extension/src/providers/solana/ui/send-transaction/index.vue (1)
400-400
: Unify insufficient balance messageChanging the error message to "Not enough balance." maintains consistency with other providers. It clearly communicates the issue to end users and aligns with the project-wide error messaging guidelines.
packages/extension/src/providers/bitcoin/ui/send-transaction/components/send-alert.vue (2)
4-5
: Well-structured error message hierarchy!The new condition for zero balance takes precedence over other error states, and the message "Not enough balance" aligns well with the standardization effort across blockchain providers. The logical progression from zero balance → below dust → not enough funds is clear and maintainable.
28-28
: LGTM: Clean props interface additionThe new
isBalanceZero
prop is well-typed and follows the codebase's naming conventions.packages/extension/src/providers/solana/types/sol-token.ts (1)
6-6
: Check usage for undefinedcgId
MakingcgId
optional is fine. Please verify that all references tocgId
in the code handle the case where it is undefined.packages/extension/src/providers/ethereum/types/index.ts (1)
63-63
: Optional icon property
Addingicon?: string
helps provide a token icon when available with minimal disruption. Ensure any consumer of this interface checks forundefined
before using the icon.packages/extension/src/providers/solana/libs/api.ts (3)
36-36
: Good addition of'confirmed'
commitment
Providing a commitment level here helps ensure consistent transaction status. Nice improvement.
69-92
: Verify JSON-RPC approach
Fetching token info via a JSON-RPC call is a robust solution. Make sure the node supportsgetAsset
consistently in all environments.
98-98
: Graceful fallback on missing token info
Switching to directgetParsedAccountInfo
is a solid fallback for decoding basic token metadata.
Summary by CodeRabbit
Version Update
Bug Fixes
Improvements