-
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
feat: recent addresses #587
Conversation
WalkthroughThis pull request introduces a new Changes
Sequence DiagramsequenceDiagram
participant User
participant SendTransaction
participant RecentlySentAddressesState
participant BrowserStorage
User->>SendTransaction: Initiate Transaction
SendTransaction->>RecentlySentAddressesState: addRecentlySentAddress(network, address)
RecentlySentAddressesState->>BrowserStorage: Save/Update Addresses
SendTransaction->>RecentlySentAddressesState: getRecentlySentAddresses(network)
RecentlySentAddressesState->>BrowserStorage: Retrieve Addresses
BrowserStorage-->>RecentlySentAddressesState: Return Addresses
RecentlySentAddressesState-->>SendTransaction: Return Addresses
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: 2
🧹 Nitpick comments (5)
packages/extension/src/types/provider.ts (1)
132-132
: Potentially unused constructor.Per the static analysis hint, this constructor may be unnecessary if no custom initialization is performed. Consider removing it to reduce clutter.
- constructor(node: string, options?: unknown) { } + // Remove if not implementing any special logic🧰 Tools
🪛 Biome (1.9.4)
[error] 132-132132: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/extension/src/providers/common/ui/send-transaction/send-contacts-list.vue (2)
26-49
: Iteration over recently sent addresses
Displaying the “Recent” header and mapping overrecentlySentAddresses
is a clean approach. Consider truncating or limiting display if addresses become very long to preserve layout.-<send-address-item - v-for="(recentAddress, index) in recentlySentAddresses" - :key="index" - ... -/> +<send-address-item + v-for="(recentAddress, index) in recentlySentAddresses" + :key="index" + :truncatedAddress="recentAddress.substring(0, 6) + '...' + recentAddress.slice(-4)" + ... +/>
132-156
: Asynchronous load of recently sent addresses
The timeout mechanism provides a safeguard against indefinite waiting. Logging errors and clearing addresses on timeout or exception is good defensive programming. However, consider making the timeout configurable or providing a user-facing signal instead of just an error log.packages/extension/src/providers/polkadot/ui/send-transaction/index.vue (1)
501-502
: Instantiating recentlySentAddresses
Defining a local instance keeps logic scoped to the component. If you plan to share it in the future, consider a single state in a store or plugin.packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (1)
714-714
: InstantiatingRecentlySentAddressesState
.
Storing a dedicated instance allows its usage within this component. Make sure only one instance is used or that instances are shared where appropriate to avoid duplicating state.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
packages/extension/src/libs/recently-sent-addresses/index.ts
(1 hunks)packages/extension/src/libs/recently-sent-addresses/types.ts
(1 hunks)packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue
(3 hunks)packages/extension/src/providers/common/ui/send-transaction/send-address-item.vue
(2 hunks)packages/extension/src/providers/common/ui/send-transaction/send-contacts-list.vue
(7 hunks)packages/extension/src/providers/ethereum/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/providers/kadena/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/providers/polkadot/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/providers/solana/networks/solana.ts
(1 hunks)packages/extension/src/providers/solana/ui/send-transaction/index.vue
(2 hunks)packages/extension/src/types/provider.ts
(2 hunks)packages/swap/src/providers/rango/supported.ts
(1 hunks)packages/swap/tests/changelly.test.ts
(1 hunks)
✅ Files skipped from review due to trivial changes (3)
- packages/extension/src/libs/recently-sent-addresses/types.ts
- packages/swap/src/providers/rango/supported.ts
- packages/swap/tests/changelly.test.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/extension/src/types/provider.ts
[error] 132-132132: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🔇 Additional comments (29)
packages/extension/src/providers/solana/networks/solana.ts (2)
28-28
: Correct naming alignment.Renaming the exported instance to
solana
aligns with the actual network being Solana, preventing confusion in the codebase.
30-30
: Consistency check.Ensure any references to this default export are updated to
solana
throughout the codebase to avoid import errors.packages/extension/src/libs/recently-sent-addresses/index.ts (4)
7-14
: Initial constructor logic.Thank you for encapsulating the browser storage logic within the class. This helps reduce code duplication across the codebase.
16-30
: Enforce unique addresses and maximal length.This method ensures:
- No duplicate addresses are stored.
- Only the latest five addresses are retained.
This is a clean and efficient approach.
32-38
: Graceful fallback for absent state.Returning an empty array when there's no stored state prevents runtime errors.
40-40
: Class export verification.Double-check that import statements elsewhere in the codebase correctly reference this default export to avoid mismatch errors.
✅ Verification successful
Default export is correctly referenced across the codebase
The verification shows that
RecentlySentAddressesState
is properly imported as a default export in all its usages across the codebase:
- All imports consistently use the correct syntax:
import RecentlySentAddressesState from '@/libs/recently-sent-addresses'
- The class is used correctly in multiple send-transaction components for different chains (Solana, Ethereum, Kadena, Bitcoin, Polkadot)
- All instances properly instantiate the class using
new RecentlySentAddressesState()
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for RecentlySentAddressesState usage rg 'RecentlySentAddressesState' -A 5Length of output: 8290
packages/extension/src/providers/common/ui/send-transaction/send-address-item.vue (2)
10-10
: Conditional rendering for name.Checking for a null name before displaying ensures that the UI remains clean and avoids unnecessary empty headings.
42-45
: Flexible account type definition.Using an optional
name
property while requiringaddress
accommodates multiple use cases and prevents type errors.packages/extension/src/types/provider.ts (1)
54-54
: New namespace.Storing recently sent addresses under
recentlySentAddresses
provides a clear separation of concerns and helps keep the storage organized.packages/extension/src/providers/common/ui/send-transaction/send-contacts-list.vue (8)
12-15
: Conditional block for recently sent addresses
Good use ofv-if="recentlySentAddresses && !isMyAddress"
to conditionally display recently sent addresses only for the non-"my address" view. This helps avoid cluttering the interface when viewing one's own addresses.
58-62
: Checked state condition
Theis-checked
logic properly compares the displayed addresses to highlight the selected one. This is consistent with how different networks format addresses.
94-94
: Imports
ImportingonMounted
,PropType
, andref
from 'vue' is standard and aligns with the Composition API.
103-103
: RecentlySentAddressesState import
Nice approach to centralize recently sent addresses logic. This keeps code modular and more maintainable.
112-112
: Prop definitions
Explicitly defining types withPropType
improves type safety and ensures better Vue integration.
128-129
: Instantiation of RecentlySentAddressesState
Creating a state instance at the component level allows the component to retrieve or store addresses without re-instantiating. This is good for performance.
130-131
: Reactive reference for recentlySentAddresses
InitializingrecentlySentAddresses
withnull
is a good approach to differentiate between “not yet loaded” and “empty.”
157-157
: Close function
Theclose
function emits a'close', false
event to the parent. Straightforward and consistent with Vue’s event model.packages/extension/src/providers/kadena/ui/send-transaction/index.vue (3)
130-130
: Importing RecentlySentAddressesState
Good choice to keep consistent usage of the state class across different providers.
470-471
: Instantiation of RecentlySentAddressesState
Instantiating once per component is typically sufficient. Ensures the addresses are stored reliably and remain consistent.
473-477
: Add address to recently sent on sendAction
Inserting the address at send time is a sensible approach. Ensure the remainder of the function handles failures gracefully (e.g., if the address cannot be recorded for some reason).Would you like a try-catch around
addRecentlySentAddress
to capture potential storage errors or a fallback scenario?packages/extension/src/providers/bitcoin/ui/send-transaction/index.vue (4)
171-171
: Importing RecentlySentAddressesState
Consistent with other providers, keeping this pattern ensures a unified UX across networks.
317-318
: Semantics in isInputsValid
Using BigNumber checks is correct and safer than a direct numeric comparison for large values. The new lines clarify numeric checks.
431-432
: RecentlySentAddressesState instantiation
This promotes an easy way to track recently used addresses for Bitcoin transactions, consistent with other providers.
434-438
: Adding to recently sent addresses
InsertingaddressTo.value
into the state before processing the transaction is a good user experience—recent addresses are always up to date.packages/extension/src/providers/polkadot/ui/send-transaction/index.vue (2)
144-144
: Importing RecentlySentAddressesState
Again, consistent with the multi-network approach.
504-508
: Call to addRecentlySentAddress
Robust inclusion of the add address call insendAction
. Consider adding error handling if the storage fails, especially if the user is offline.Would you like to see an additional fallback if local storage is unavailable (e.g., storage quota exceeded)?
packages/extension/src/providers/ethereum/ui/send-transaction/index.vue (1)
174-174
: Good addition of the import statement.
Importing theRecentlySentAddressesState
class enables the functionality for tracking and managing recently sent addresses.packages/extension/src/providers/solana/ui/send-transaction/index.vue (2)
176-176
: ImportingRecentlySentAddressesState
.
This import provides the necessary support for storing and retrieving recently sent addresses.
721-721
: Instantiation in the Solana context.
Great job reusing the state logic here. This ensures consistent user experience across different networks.
Summary by CodeRabbit
New Features
Improvements
Technical Updates
RecentlySentAddressesState
class for managing address storage