refactor: create FundingWidget to top up identity and add funds with QR code on wallet page#416
refactor: create FundingWidget to top up identity and add funds with QR code on wallet page#416lklimek wants to merge 54 commits into
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
c3ca58a to
2302750
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a centralized FundingWidget component to streamline funding operations across the application, replacing multiple scattered funding implementations with a single, reusable component. The refactor consolidates wallet funding, asset lock usage, and QR code generation into one cohesive interface.
Key changes include:
- Created a new
FundingWidgetcomponent that handles all funding scenarios (wallet balance, asset locks, QR codes) - Replaced separate funding screens with the unified widget in identity creation and top-up flows
- Added wallet address top-up functionality with QR code support in the wallets screen
Reviewed Changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
src/ui/components/funding_widget.rs |
New centralized funding component with comprehensive funding method support |
src/ui/wallets/wallets_screen/mod.rs |
Added funding widget integration and top-up modal for wallet addresses |
src/ui/identities/top_up_identity_screen/mod.rs |
Refactored to use the new funding widget instead of custom implementations |
src/ui/identities/add_new_identity_screen/mod.rs |
Simplified identity creation flow using the funding widget |
Multiple by_*.rs files |
Removed redundant funding method implementations |
| .show(ui, |ui| { | ||
| ui.vertical(|ui| { | ||
| // Render the widget and get response | ||
| let funding_widget = self.funding_widget.as_mut().expect("Checked above"); |
There was a problem hiding this comment.
[nitpick] Using expect with a custom message is good, but consider using a more descriptive message that explains the actual condition being checked (e.g., "FundingWidget should be Some when funding_widget.is_some() returns true").
| let funding_widget = self.funding_widget.as_mut().expect("Checked above"); | |
| let funding_widget = self.funding_widget.as_mut().expect("FundingWidget should be Some because self.funding_widget.is_none() was checked above"); |
| /// Track existing UTXOs at the time the address was set up | ||
| /// This is used when ignore_existing_utxos is true to filter out pre-existing UTXOs |
There was a problem hiding this comment.
The existing_utxos_snapshot field needs documentation explaining its purpose in filtering pre-existing UTXOs when ignore_existing_utxos is enabled.
| /// Track existing UTXOs at the time the address was set up | |
| /// This is used when ignore_existing_utxos is true to filter out pre-existing UTXOs | |
| /// A snapshot of existing UTXOs (Unspent Transaction Outputs) at the time the funding address was set up. | |
| /// This field is used in conjunction with the `ignore_existing_utxos` flag. When `ignore_existing_utxos` is set to true, | |
| /// the `existing_utxos_snapshot` is used to filter out UTXOs that existed prior to the funding process, ensuring that | |
| /// only newly created UTXOs are considered for funding. This helps prevent double-counting or unintended reuse of | |
| /// pre-existing funds. |
…let-address-top-up
NOTE: Requires #419 to be merged first.
Known bugs
Bugs that are also present on v1.0-dev, to be handled separately:
Copilot's summary
This pull request introduces significant changes to the wallet and UI components, primarily focusing on simplifying the codebase by removing unused features and improving modularity. Key changes include the addition of a new method for calculating wallet balances, removal of multiple UI screens related to funding methods, and enhancements to the
WalletFundedScreenStepenum.Wallet Enhancements:
max_balancemethod to theWalletmodel to calculate the total balance by summing all UTXOs (src/model/wallet/mod.rs).UI Simplification:
by_using_unused_asset_lock.rsby_using_unused_balance.rsby_wallet_qr_code.rsby_using_unused_asset_lock.rsundertop_up_identity_screen.funding_widgetmodule to the UI components (src/ui/components/mod.rs).Codebase Improvements:
WalletFundedScreenStepenum by:is_processingmethod to check if the wallet is in a processing state.Displaytrait for better string representation (src/ui/identities/funding_common.rs).funding_commonto a public module for broader accessibility (src/ui/identities/mod.rs).These changes collectively streamline the codebase, reduce redundancy, and improve the maintainability of wallet-related functionalities.