Feat/add owallet #70
Feat/add owallet #70
Conversation
WalkthroughThe changes introduce a new Vue component for an OWallet icon, enhance wallet connection functionality with a new method to connect to the OWallet, and implement a validation function for the connected OWallet address. Additionally, a new entry for the OWallet icon is added to the shared icon library. These modifications collectively expand the wallet features and improve the integration of the OWallet within the application. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WalletStore
participant OWalletBase
User->>WalletStore: connectOWallet()
WalletStore->>WalletStore: connectWallet(Wallet.OWallet)
WalletStore->>WalletStore: retrieve Injective addresses
WalletStore->>WalletStore: confirm session
WalletStore->>OWalletBase: create instance
OWalletBase->>WalletStore: get public key
WalletStore->>WalletStore: validate address
WalletStore->>User: update wallet state
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (4)
packages/ui-shared/playground/icons.json (1)
142-142: LGTM! Consider alphabetical ordering.The addition of "wallet/owallet" is consistent with the existing naming convention for wallet icons and aligns with the PR objective. Good job!
For improved readability and easier maintenance, consider ordering the wallet icons alphabetically. In this case, "wallet/owallet" could be placed between "wallet/ninji" and "wallet/phantom".
layer/icons/wallet-new/owallet.vue (1)
5-20: SVG implementation looks good. Consider adding accessibility attributes.The SVG implementation for the OWallet icon is well-structured and visually appealing. The use of opacity and gradients enhances the icon's appearance.
To improve accessibility, consider adding the following attributes to the
<svg>element:- <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-label="OWallet icon">This change will help screen readers identify and describe the icon to users with visual impairments.
layer/icons/wallet/owallet.vue (1)
5-21: Enhance accessibility by adding ARIA attributes to the SVG.The current SVG lacks accessibility attributes, which can make it difficult for screen readers to interpret the icon.
Add appropriate ARIA attributes to improve accessibility:
<template> - <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg" role="img" aria-labelledby="owalletIconTitle"> + <title id="owalletIconTitle">OWallet Icon</title> <!-- ... SVG content ... --> </svg> </template>These changes will help screen readers understand the purpose of the icon, improving the overall accessibility of your application.
layer/store/wallet.ts (1)
438-460: LGTM: NewconnectOWalletmethod addedThe new
connectOWalletmethod follows the established pattern for wallet connections in this file and correctly integrates OWallet support. However, consider the following suggestions for improvement:
- Add error handling to manage potential failures during the connection process.
- Include comments explaining the purpose and flow of the method for better maintainability.
Consider adding error handling and comments:
async connectOWallet() { const walletStore = useSharedWalletStore() + try { + // Connect to OWallet await walletStore.connectWallet(Wallet.OWallet) + // Retrieve Injective addresses and select the first one const injectiveAddresses = await getAddresses() const [injectiveAddress] = injectiveAddresses const session = await walletStrategy.getSessionOrConfirm() + // Validate the Injective address await confirmCorrectOWalletAddress(injectiveAddress) + // Update the store with the new wallet information walletStore.$patch({ injectiveAddress, addresses: injectiveAddresses, address: getEthereumAddress(injectiveAddress), addressConfirmation: await walletStrategy.getSessionOrConfirm( injectiveAddress ), session }) + // Finalize the connection process await walletStore.onConnect() + } catch (error) { + console.error('Failed to connect OWallet:', error) + // Handle the error appropriately (e.g., show an error message to the user) + } },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
layer/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (5)
- layer/icons/wallet-new/owallet.vue (1 hunks)
- layer/icons/wallet/owallet.vue (1 hunks)
- layer/store/wallet.ts (2 hunks)
- layer/wallet/cosmos.ts (2 hunks)
- packages/ui-shared/playground/icons.json (2 hunks)
🔇 Additional comments (5)
layer/icons/wallet-new/owallet.vue (1)
1-20: Overall, the OWallet icon component is well-implemented with room for minor improvements.The component successfully creates an SVG icon for OWallet, which aligns with the PR objective of adding an icon for OWallet. The implementation is generally good, with a clean structure and visually appealing design.
To further enhance this component:
- Improve the UUID generation in the script section for better uniqueness and unpredictability.
- Add accessibility attributes to the SVG element in the template section.
These improvements will make the component more robust and accessible while maintaining its core functionality.
layer/wallet/cosmos.ts (2)
9-9: LGTM: Import statement for OWalletBaseThe import statement for
OWalletBaseis correctly added and is consistent with other wallet-related imports in the file. This import is necessary for the newconfirmCorrectOWalletAddressfunction.
Line range hint
1-56: Summary: OWallet support successfully implementedThe changes in this file successfully implement support for OWallet, which aligns with the PR objectives to integrate OWallet support in the Injective UI. The new
confirmCorrectOWalletAddressfunction mirrors the existing Keplr wallet validation, maintaining consistency in the codebase.While the implementation is generally good, consider the suggested improvements for error handling and async operation consistency to enhance robustness and maintainability.
Overall, these changes effectively contribute to the goal of supporting OWallet in the application.
layer/store/wallet.ts (2)
23-23: LGTM: Import addition for OWallet supportThe addition of
confirmCorrectOWalletAddressimport is consistent with the existing code style and supports the new OWallet connection functionality.
Line range hint
1-1043: Overall assessment: OWallet integration looks goodThe changes to add OWallet support in this file are well-implemented and consistent with the existing code patterns. The new
connectOWalletmethod and the related import have been added correctly. With the suggested improvements for error handling and comments, this implementation will be robust and maintainable.
| <script lang="ts" setup> | ||
| const uuid = `${Math.random()}` | ||
| </script> |
There was a problem hiding this comment.
Consider using a more robust UUID generation method.
The current implementation uses Math.random() to generate a UUID, which doesn't guarantee uniqueness or unpredictability. This could potentially lead to conflicts if multiple instances of the component are rendered.
Consider using a dedicated UUID library or a more robust method for generating UUIDs. Here's an example using the uuid package:
<script lang="ts" setup>
-const uuid = `${Math.random()}`
+import { v4 as uuidv4 } from 'uuid';
+const uuid = uuidv4();
</script>Make sure to install the uuid package:
npm install uuid
npm install --save-dev @types/uuid
This approach ensures a more reliable and standardized UUID generation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <script lang="ts" setup> | |
| const uuid = `${Math.random()}` | |
| </script> | |
| <script lang="ts" setup> | |
| import { v4 as uuidv4 } from 'uuid'; | |
| const uuid = uuidv4(); | |
| </script> |
| <script lang="ts" setup> | ||
| const uuid = `${Math.random()}` | ||
| </script> |
There was a problem hiding this comment.
Consider using a more robust UUID generation method.
The current implementation uses Math.random() to generate a UUID, which doesn't guarantee uniqueness or unpredictability. This could potentially lead to conflicts if multiple instances of the component are rendered.
Consider using a dedicated UUID library or a more robust method for generating UUIDs. Here's an example using the uuid package:
<script lang="ts" setup>
-const uuid = `${Math.random()}`
+import { v4 as uuidv4 } from 'uuid';
+const uuid = uuidv4();
</script>Make sure to install the uuid package:
npm install uuid
npm install --save-dev @types/uuid
This approach will provide a more reliable and standardized way of generating UUIDs.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <script lang="ts" setup> | |
| const uuid = `${Math.random()}` | |
| </script> | |
| <script lang="ts" setup> | |
| import { v4 as uuidv4 } from 'uuid'; | |
| const uuid = uuidv4(); | |
| </script> |
| <template> | ||
| <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg"> | ||
| <path opacity="0.51" d="M145.129 56.7787H94.4052C91.9231 56.7787 89.8652 54.7633 89.8652 52.2388V30.5363C89.8652 28.0542 91.8806 25.9963 94.4052 25.9963H147.399C148.672 25.9963 149.669 26.9934 149.669 28.2663V52.2812C149.627 54.7633 147.611 56.7787 145.129 56.7787Z" fill="#AD83FF"/> | ||
| <path opacity="0.5" d="M132.442 46.2768H85.5575C84.1574 46.2768 83.0754 45.1312 83.0754 43.7947V17.9977C83.0754 16.5976 84.221 15.5156 85.5575 15.5156H132.484C133.884 15.5156 134.966 16.6612 134.966 17.9977V43.7947C134.987 45.1948 133.842 46.2768 132.442 46.2768Z" fill="#AD83FF"/> | ||
| <path opacity="0.9" d="M163.352 68.5518H108.343C107.028 68.5518 105.946 67.5122 105.946 66.1545V39.4666C105.946 38.1513 106.985 37.0693 108.343 37.0693H163.352C164.668 37.0693 165.75 38.1088 165.75 39.4666V66.1333C165.771 67.4486 164.668 68.5518 163.352 68.5518Z" fill="#AD83FF"/> | ||
| <path d="M145.32 129.311C130.809 149.698 106.985 163 80.0638 163C35.8526 163 0 127.147 0 82.9358C0 40.6552 32.7977 6.01179 74.3358 3.08418C74.6752 3.08418 74.9935 3.04175 75.3329 3.04175C77.9635 3.12661 80.0638 5.09956 80.0638 7.53924V36.4123C54.3305 36.4123 33.4978 57.2662 33.4978 82.9782C33.4978 108.69 54.3517 129.544 80.0638 129.544C105.776 129.544 126.63 108.69 126.63 82.9782C126.63 82.9782 152.66 93.734 145.32 129.311Z" fill="#925AFF"/> | ||
| <path d="M159.916 89.1109C159.513 85.6529 156.415 82.9799 152.618 82.9799H85.474C82.5039 82.9799 80.0855 80.5826 80.0855 77.5914V7.54086C80.0855 5.10119 77.9852 3.12823 75.3546 3.04338C75.6516 3.04338 75.9698 3.00094 76.2668 3.00094C79.6824 2.95851 81.5068 4.35868 81.9948 4.80418L82.0372 4.84661L82.1221 4.93147L154.23 77.0398C160.34 83.192 159.916 89.1109 159.916 89.1109Z" fill="#AD83FF"/> | ||
| <path opacity="0.91" d="M159.873 89.0451C159.831 89.9362 159.746 90.806 159.661 91.6333C158.155 105.571 153.063 118.449 145.32 129.31C152.66 93.7123 126.63 82.9778 126.63 82.9778H152.597C156.415 82.9778 159.513 85.6508 159.873 89.0451Z" fill="url(#paint0_linear_4242_32575)"/> | ||
| <defs> | ||
| <linearGradient id="paint0_linear_4242_32575" x1="143.252" y1="131.138" x2="143.252" y2="83.1802" gradientUnits="userSpaceOnUse"> | ||
| <stop offset="8.52446e-08" stop-color="#925AFF"/> | ||
| <stop offset="1" stop-color="#713BBB"/> | ||
| </linearGradient> | ||
| </defs> | ||
| </svg> | ||
|
|
||
| </template> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider extracting the SVG to a separate file.
While inline SVGs offer easy manipulation, they can impact performance if the component is frequently used. For a more optimized approach, consider extracting the SVG to a separate file.
- Create a new file
owallet-icon.svgin the appropriate assets directory. - Move the SVG content to this new file.
- Import and use the SVG file in your component:
<script lang="ts" setup>
const uuid = `${Math.random()}`
+import OWalletIcon from '@/assets/icons/owallet-icon.svg'
</script>
<template>
- <svg width="166" height="165" viewBox="0 0 166 165" fill="none" xmlns="http://www.w3.org/2000/svg">
- <!-- ... SVG content ... -->
- </svg>
+ <img :src="OWalletIcon" alt="OWallet Icon" width="166" height="165" />
</template>This approach can improve performance, especially if the icon is used in multiple places.
Committable suggestion was skipped due to low confidence.
| export const confirmCorrectOWalletAddress = async (injectiveAddress: string) => { | ||
| // We only perform this check for OWallet addresses | ||
| if (walletStrategy.getWallet() !== Wallet.OWallet) { | ||
| return | ||
| } | ||
|
|
||
| const owallet = new OWalletBase(CHAIN_ID) | ||
| const key = await owallet.getKey() | ||
| const publicKey = PublicKey.fromBase64( | ||
| Buffer.from(key.pubKey).toString('base64') | ||
| ) | ||
|
|
||
| const { address: derivedAddress } = publicKey.toAddress() | ||
|
|
||
| if (derivedAddress !== injectiveAddress) { | ||
| throw new CosmosWalletException( | ||
| new Error( | ||
| 'Connected OWallet address is wrong. Please update Injective on OWallet.' | ||
| ) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and provide more informative error messages
The confirmCorrectOWalletAddress function is a good addition and mirrors the existing confirmCorrectKeplrAddress function. However, there are a few suggestions for improvement:
- Add
awaitto theowallet.getKey()call on line 42, as it's an asynchronous operation. - Implement error handling for potential exceptions from
owallet.getKey()orpublicKey.toAddress(). - Enhance the error message to include the mismatched addresses for easier debugging.
Consider refactoring the function as follows:
export const confirmCorrectOWalletAddress = async (injectiveAddress: string) => {
if (walletStrategy.getWallet() !== Wallet.OWallet) {
return
}
try {
const owallet = new OWalletBase(CHAIN_ID)
const key = await owallet.getKey()
const publicKey = PublicKey.fromBase64(
Buffer.from(key.pubKey).toString('base64')
)
const { address: derivedAddress } = publicKey.toAddress()
if (derivedAddress !== injectiveAddress) {
throw new CosmosWalletException(
new Error(
`Connected OWallet address is incorrect. Expected: ${injectiveAddress}, Got: ${derivedAddress}`
)
)
}
} catch (error) {
if (error instanceof CosmosWalletException) {
throw error
}
throw new CosmosWalletException(
new Error(`Failed to validate OWallet address: ${error.message}`)
)
}
}This refactored version:
- Correctly awaits the asynchronous
owallet.getKey()call. - Adds error handling for potential exceptions.
- Provides a more informative error message that includes both the expected and actual addresses.
Summary by CodeRabbit
New Features
owallet) for enhanced visual representation.Improvements
owalletentry.