fix(ui): wallet screen identity, state management, and input validation (#797)#800
Conversation
…627) During SPV reconciliation, per_address_sum only contains addresses with current UTXOs. Addresses whose funds were fully spent never had their balance reset to zero, causing the address table to display stale non-zero balances even though UTXO count correctly showed 0. Now explicitly zeroes address_balances for any known address absent from the refreshed UTXO map before applying current sums. Closes #571 Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…613) * fix: handle malformed YAML gracefully in load_testnet_nodes_from_yml Replace .expect() with match expression to avoid panicking when .testnet_nodes.yml contains malformed YAML. Instead, logs the error with tracing::error and returns None, allowing the application to continue without crashing. Closes #557 Co-authored-by: lklimek <lklimek@users.noreply.github.com> * fix: propagate YAML parse errors to UI and remove unwrap calls - Change load_testnet_nodes_from_yml to return Result<Option<TestnetNodes>, String> so parse errors display in the UI error banner instead of only logging - Use explicit type annotation on serde_yaml_ng::from_str::<TestnetNodes> - Replace .unwrap() with .and_then() in fill_random_hpmn/fill_random_masternode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: lklimek <lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* chore: move doc/ contents into docs/ and update references Consolidate documentation under a single docs/ directory. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: CLAUDE.md should write manual test scenarios for PRs --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
…tion (#603) * build(flatpak): use only-arches for dynamic protoc architecture selection Replace the fragile sed-based CI patching of the Flatpak manifest with Flatpak's native `only-arches` source selector. The protoc module now declares both x86_64 and aarch64 sources inline, and build-commands use a glob pattern (`protoc-*.zip`) so no per-arch fixup is needed. Changes: - flatpak manifest: add aarch64 protoc source with `only-arches`, use glob in unzip commands, remove stale CI-patching comment - CI workflow: remove `protoc-zip`/`protoc-sha256` matrix variables and the "Patch manifest for architecture" step https://claude.ai/code/session_015AD2pCWoJdV2VDydcqFHPG * fix(flatpak): use dest-filename for deterministic protoc extraction Use dest-filename to normalize both arch-specific protoc zips to a common name, avoiding glob expansion in build-commands. This ensures the unzip target is deterministic regardless of shell behavior in the Flatpak sandbox. https://claude.ai/code/session_015AD2pCWoJdV2VDydcqFHPG * build: update platform to b445b6f0 and remove rust-dashcore patches Update dashpay/platform dependency from d6f4eb9a to b445b6f0e0bd4863 (3.0.1 → 3.1.0-dev.1). Remove the [patch] section that pinned rust-dashcore crates to a separate rev, as the new platform commit resolves them correctly on its own. https://claude.ai/code/session_015AD2pCWoJdV2VDydcqFHPG --------- Co-authored-by: Claude <noreply@anthropic.com>
Resolve conflicts preserving shielded pool features from HEAD while adopting v1.0-dev improvements: refactored asset lock fee calculation, mine dialog hardening, network validation, and API signature updates. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…xtract/all-merged
The [patch] section referenced ../platform local paths that don't exist in CI. Since dash-sdk already depends on the feat/zk branch, all transitive platform crates resolve correctly without patches. Also fixes a formatting issue in address_table.rs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The remove_utxos tests were failing because `register_test_address` inserts into `wallet_addresses` which has a foreign key constraint on `wallet(seed_hash)`. Added the missing `store_wallet` call so the parent row exists before inserting child address rows. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rged # Conflicts: # Cargo.lock # Cargo.toml # src/backend_task/core/mod.rs # src/backend_task/wallet/fetch_platform_address_balances.rs # src/context/wallet_lifecycle.rs # src/database/wallet.rs # src/model/wallet/mod.rs # src/ui/wallets/wallets_screen/mod.rs
The `pub mod shielded;` declaration was accidentally removed from `src/model/wallet/mod.rs` during the v1.0-dev merge, causing build failures since the shielded.rs file still exists. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Initial plan * fix(test): restore store_wallet calls lost in merge Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com>
…rged Resolve conflicts in Cargo.toml (keep feat/zk branch), Cargo.lock (regenerate with pinned platform rev 4d7b9be5), and backend_task/mod.rs (combine TaskError wrapping with ShieldedTask). Fix post-merge integration issues: - SPV manager: remove stale .await on subscribe methods, add command_receiver channel for updated DashSpvClient::run() API - send_screen: update SendStatus::WaitingForResult to unit variant - network_chooser_screen: handle new SyncState::Initializing variant Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rged # Conflicts: # .github/workflows/clippy.yml # .github/workflows/tests.yml # CLAUDE.md # Cargo.lock # Cargo.toml # src/app.rs # src/backend_task/core/mod.rs # src/backend_task/error.rs # src/backend_task/mod.rs # src/context/mod.rs # src/database/initialization.rs # src/spv/manager.rs # src/ui/components/top_panel.rs # src/ui/components/wallet_unlock.rs # src/ui/contracts_documents/add_contracts_screen.rs # src/ui/contracts_documents/document_action_screen.rs # src/ui/contracts_documents/group_actions_screen.rs # src/ui/contracts_documents/register_contract_screen.rs # src/ui/contracts_documents/update_contract_screen.rs # src/ui/dashpay/contact_requests.rs # src/ui/dashpay/profile_screen.rs # src/ui/dashpay/profile_search.rs # src/ui/identities/add_new_identity_screen/by_wallet_qr_code.rs # src/ui/identities/identities_screen.rs # src/ui/identities/keys/key_info_screen.rs # src/ui/mod.rs # src/ui/tokens/destroy_frozen_funds_screen.rs # src/ui/tokens/direct_token_purchase_screen.rs # src/ui/tokens/freeze_tokens_screen.rs # src/ui/tokens/mint_tokens_screen.rs # src/ui/tokens/pause_tokens_screen.rs # src/ui/tokens/resume_tokens_screen.rs # src/ui/tokens/set_token_price_screen.rs # src/ui/tokens/tokens_screen/my_tokens.rs # src/ui/tokens/unfreeze_tokens_screen.rs # src/ui/tokens/update_token_config.rs # src/ui/tools/masternode_list_diff_screen.rs # src/ui/wallets/asset_lock_detail_screen.rs # src/ui/wallets/create_asset_lock_screen.rs # src/ui/wallets/single_key_send_screen.rs # src/ui/wallets/wallets_screen/mod.rs
Migrate from feat/zk to 3.1-dev branch of dashpay/platform, adapting to breaking API changes in dash-spv, key-wallet, and dpp: - FeeLevel removed; use FeeRate::normal() directly - DashSpvClientInterface/Command removed; use DashSpvClient directly - SyncState::Initializing removed; replaced with WaitForEvents - NetworkExt trait inlined into Network impl - OrchardProver now requires wrapper struct around ProvingKey - OrchardAddress::from_raw_bytes now returns Result - Builder functions gain fee/platform_version params - NullifierSyncConfig API uses NullifierSyncCheckpoint - WalletManager.create_unsigned_payment_transaction removed; use TransactionBuilder directly - Work around Send lifetime issues with spawn_blocking Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…kError Replace all Result<T, String> error patterns in the shielded pool module with typed TaskError variants, aligning with the codebase-wide typed error migration (PR #739). New TaskError variants: ShieldedNoUnspentNotes, ShieldedInsufficientBalance, PlatformAddressNotFound, ShieldedMerkleWitnessUnavailable, ShieldedTransitionBuildFailed, ShieldedBroadcastFailed, ShieldedInvalidRecipientAddress, ShieldedAssetLockTimeout, ShieldedSyncFailed, ShieldedTreeUpdateFailed, ShieldedNullifierSyncFailed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Merge origin/zk branch which adapts to dashpay/platform v3.1-dev API: - OrchardProver trait wrapper (CachedProver) replaces raw ProvingKey refs - OrchardAddress::from_raw_bytes now returns Result - FeeLevel removed; use FeeRate::normal() directly - WalletManager.create_unsigned_payment_transaction removed; use TransactionBuilder - SyncState::Initializing removed; replaced with WaitForEvents - SpvClient ownership model changed (no longer Arc-wrapped) Resolved conflicts preserving typed TaskError variants from our branch while adopting origin/zk's v3.1-dev API adaptations. Fixed branch name typo (3.1-dev → v3.1-dev) in Cargo.toml. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
backport: merge v1.0-dev (including extracted non-ZK fixes) back into zk
Reserve width for Save and Auto Update buttons so the password input doesn't consume all available space, pushing buttons off-screen. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nto zk-fixes # Conflicts: # Cargo.lock # Cargo.toml # src/app.rs # src/backend_task/core/mod.rs # src/backend_task/error.rs # src/database/initialization.rs # src/spv/manager.rs # src/ui/wallets/wallets_screen/mod.rs
…how details Add CoreRpcConnectionFailed variant to TaskError that includes the configured address in the user-facing message. Connection-refused errors are now detected via is_rpc_connection_error() and enriched with host:port at every RPC call site where the URL is known (AppContext::rpc_error_with_url helper). Details panel is now shown for all RPC-related errors regardless of developer mode, so users can always see the technical information they need to diagnose connectivity issues. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…gtest The Network Chooser screen had three bugs related to RPC password handling: 1. Password input was initialized from Regtest config only, ignoring the current network selection. 2. Password UI was hidden for all networks except Regtest, even though Mainnet/Testnet/Devnet also use RPC mode. 3. Save logic was hardcoded to update Regtest config and triggered a SwitchNetwork(Regtest) action, which disconnected the active network's ZMQ listener unnecessarily. Now the password input shows for any network in RPC mode, reads/writes the correct network config, reinits the RPC client in-place without triggering a network switch, and reloads the stored password when the user switches network tabs. The "Auto Update" (dashmate) button remains Regtest-only since dashmate is only relevant for local networks. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract the SPV is_ours override into a named function and add four unit tests covering: outgoing already-ours, incoming not-ours (the main override case), zero-amount edge case, and outgoing not-ours. A comment explains why bloom filter false positive testing requires mocking the SPV layer and is out of scope for unit tests. Fixes PROJ-002. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add WAL-021 through WAL-024 covering the tab-based navigation, developer-mode System tab, collapsible transaction history, and collapsible balance breakdown sections introduced in PR #791. Fixes PROJ-007. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Core addresses now show "(change)" suffix for BIP44 change addresses (m/44'/5'/0'/1/x). New with_exclude_change(true) builder method filters them out — send inputs should not display change addresses since users don't share them with others. Change detection uses the existing DerivationPathHelpers::is_bip44_change trait method from the wallet model. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Change addresses are internal wallet addresses not meant to be shared. The Send screen's destination AddressInput now uses with_exclude_change(true) to filter them out of the autocomplete. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add right-aligned compact buttons below the address table for generating new addresses on Core (BIP44) and Platform tabs. Core addresses use the async BackendTask path for SPV compatibility; Platform addresses derive synchronously. The button is disabled during generation to prevent double-clicks. Shielded tab already has its own button; System tab has no button (addresses are derived automatically). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
System addresses (Identity Registration, CoinJoin, Provider keys, etc.) are internal wallet infrastructure not meant for user-facing send/receive operations. Filter them out by checking each address against watched_addresses path_reference → AccountCategory. Only BIP44 (Core) and PlatformPayment addresses are shown. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Address type suffix (Core/Platform/Shielded/Identity) is now always shown in autocomplete entries, not just when multiple types are enabled. Users can type "platform", "core", "plat", etc. to filter entries by type — the filter matches against both short_label() and display_name(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Detect AddressInvalidNonceError from Platform and show an actionable message: "The transaction used an outdated sequence number. Please retry — the wallet will use the correct number automatically." Previously this fell through to the generic ShieldedBroadcastFailed message which gave no guidance. Upstream fix filed: dashpay/platform#3407 (SDK auto-retry on nonce mismatch). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Wildcard matches caused all shielded screens to be considered equal regardless of which wallet they belonged to, breaking multi-wallet navigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switching wallets left stale initializing/syncing flags, pending tasks, and address count from the previous wallet, causing UI glitches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ror variants
Add WalletInfoUnavailable, MissingBip44Account, and ChangeAddressDerivation
variants to TaskError, replacing ad-hoc WalletPaymentFailed { detail: String }
usage for these three recurring error patterns in SPV payment code.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The ~30s background thread for proving key generation is unnecessary during tests and slows down the test suite. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…nces is_platform_address now delegates to is_platform_address_string for consistent case-insensitive HRP matching. Address hint constants are rewritten as complete, i18n-extractable sentences. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Strip OS-level error details like "(os error 111)" and transport error wrappers before storing in ConnectionStatus, so users see clean messages instead of raw system error text. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis PR introduces wallet management and shielded transaction handling improvements, including tab-based account navigation, enhanced error handling with new TaskError variants, shielded wallet initialization and note synchronization pipelines, improved address input validation and truncation, state management refactoring in shielded screens, and SPV transaction classification logic updates. Documentation adds four new wallet UI user stories. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
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. Comment |
There was a problem hiding this comment.
Pull request overview
Fixes several wallet UI/state and validation issues called out in #797, spanning screen identity, shielded tab lifecycle, typed SPV payment errors, and improved RPC error presentation.
Changes:
- Make shielded
ScreenTypeequality wallet-specific (compareWalletSeedHash) to prevent cross-wallet screen reuse. - Reset all shielded tab transient state on wallet switch and standardize Platform address prefix detection/hints.
- Introduce typed SPV payment
TaskErrorvariants, add RPC error sanitization for Networks status, and skip Halo2 proving key warmup intestingbuilds.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/ui/wallets/shielded_tab.rs |
Resets additional transient shielded-tab fields when seed_hash changes. |
src/ui/mod.rs |
Fixes ScreenType equality for shielded screens to compare by WalletSeedHash. |
src/ui/helpers.rs |
Delegates Platform address detection to the canonical case-insensitive helper; updates user-facing hint/example strings. |
src/backend_task/error.rs |
Adds typed TaskError variants for common SPV payment failure cases. |
src/backend_task/core/mod.rs |
Uses new typed errors in SPV tx building/signing path; sanitizes RPC error strings for connection status display. |
src/app.rs |
Guards Halo2 proving key warmup thread so it doesn’t run under the testing feature. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| WalletInfoUnavailable, | ||
|
|
||
| /// Expected BIP44 account not found at the given index. | ||
| #[error("No BIP44 account found at the expected index. Please refresh your wallet.")] |
There was a problem hiding this comment.
Our regular user will not understand it. Check persona and adjust message accordingly.
There was a problem hiding this comment.
Fixed in 3c8466a — rewritten for everyday user persona: "Your wallet needs to be refreshed before sending. Please refresh and try again."
- WalletInfoUnavailable: "wallet is still loading" instead of technical jargon - MissingBip44Account: "needs to be refreshed" instead of "BIP44 account" - ChangeAddressDerivation: "could not prepare transaction" instead of "derive change address" - sanitize_rpc_error: fix doc/impl mismatch, strip "RPC error:" prefix, remove redundant "RPC: " output prefix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ues-797 # Conflicts: # src/backend_task/core/mod.rs
Summary
Fixes all 7 items from #797.
Changes
WalletSeedHashinstead of using wildcardsinitializing,syncing,pending_task,address_count)TaskErrorvariants replace string-basedWalletPaymentFailed#[cfg(not(feature = "testing"))]skips ~30s proving key build in testsis_platform_address_stringfor consistent case-insensitive validationCloses #797
Test plan
--features testing— verify no 30s Halo2 warmup delay🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements