Feat/register usernames2#4
Merged
Merged
Conversation
QuantumExplorer
changed the base branch from
feat/register-usernames
to
master
QuantumExplorer
changed the base branch from
master
to
feat/register-usernames
thepastaclaw
added a commit
to thepastaclaw/dash-evo-tool
that referenced
this pull request
Feb 17, 2026
1. CRITICAL: Fix BitOrAssign dropping tasks in fetch_unresolved_profiles()
- Changed from action |= AppAction::BackendTask(task) loop (which only
kept the last task) to collecting all tasks into a Vec and dispatching
via AppAction::BackendTasks with Concurrent execution mode.
2. HIGH: Remove dummy [0x02; 33] pubkey in load_payment_history()
- Changed PaymentRecord.to_address from Address to Option<Address>
- Historical records loaded from DB now use None instead of generating
a fake-but-valid P2PKH address from a dummy public key.
3. HIGH: Fix i64 as u64 wrapping for amounts (payments.rs and dashpay.rs)
- Added bounds checking with warning logs for negative amounts,
clamping to 0 instead of silently wrapping.
4. MEDIUM: Eliminate duplicate payment history logic in dashpay.rs
- LoadPaymentHistory task handler now calls the shared
payments::load_payment_history() and converts results, instead of
reimplementing the same logic with different return types.
5. MEDIUM: Fix N+1 database queries in resolve_names_from_local_cache()
- Pre-load all contacts once before the loop and build a HashMap
for O(1) lookups instead of calling load_dashpay_contacts() and
load_dashpay_profile() per request inside the loop.
6. MEDIUM: Fix created_at: None breaking filters in contacts_list.rs
- Contacts from DashPayContactsWithInfo results now get current
timestamp as fallback instead of None, so they work with
'Recent' filter and 'Date added' sort.
7. LOW: Fix 'failed' as failure reason in payment status
- Changed PaymentStatus::Failed to use descriptive 'Transaction failed'
string instead of echoing the literal status column value 'failed'.
8. LOW: Fix i64 as u64 for timestamps in payments.rs
- Added bounds checking with warning for negative timestamps,
consistent with the amount fix.
9. LOW: Both i64 as u64 locations fixed (dashpay.rs duplicate removed
entirely by fix dashpay#4, payments.rs fixed by fixes dashpay#3/dashpay#8).
lklimek
added a commit
that referenced
this pull request
Feb 24, 2026
- Replace assume_checked() with require_network() for address validation (CodeRabbit #2) - Use styled Frame-with-dismiss error display matching Send dialog pattern (CodeRabbit #3) - Don't open dialog when no wallet selected; show MessageBanner instead (CodeRabbit #4) - Extract shared load_bip44_external_addresses() helper to eliminate near-duplicate code between mine and receive dialogs (CodeRabbit #5) - Add backend-side network guard (Regtest/Devnet) for defense-in-depth (CodeRabbit #6) - Rename shadowed ctx binding to refresh_ctx for clarity (CodeRabbit #7) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lklimek
added a commit
that referenced
this pull request
Feb 24, 2026
… mode (#638) * feat(wallet): add Mine Blocks dialog for Regtest/Devnet dev mode Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add manual test scenarios for mine blocks dialog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): close Mine Blocks dialog after Cancel/Mine click The dialog stayed open (in a broken state) after clicking Mine or Cancel because the local `open` variable was written back to `is_open` after the dialog state had already been reset. Pass `is_open` directly to egui's `.open()` and use a separate `close` flag for button-triggered dismissal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): address audit findings for Mine Blocks dialog - Wrap `generate_to_address` in `spawn_blocking` to avoid blocking the async runtime thread (HIGH) - Replace `.expect()` on core client lock with `.map_err()?` for graceful error handling instead of panic (HIGH) - Cap block count at 1000 to prevent resource exhaustion on the Core node (HIGH) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): filter non-numeric input in Mine Blocks block count field Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): address review comments on Mine Blocks dialog - Replace assume_checked() with require_network() for address validation (CodeRabbit #2) - Use styled Frame-with-dismiss error display matching Send dialog pattern (CodeRabbit #3) - Don't open dialog when no wallet selected; show MessageBanner instead (CodeRabbit #4) - Extract shared load_bip44_external_addresses() helper to eliminate near-duplicate code between mine and receive dialogs (CodeRabbit #5) - Add backend-side network guard (Regtest/Devnet) for defense-in-depth (CodeRabbit #6) - Rename shadowed ctx binding to refresh_ctx for clarity (CodeRabbit #7) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): address remaining review comments on Mine Blocks dialog - Change MineBlocksSuccess(usize) to MineBlocksSuccess(u64) for type consistency with block_count parameter (Claudius #5) - Align dialog close pattern with Send/Receive: use local `open` variable for egui X button, reset state inside closure for Cancel/Mine buttons (Claudius #6) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
11 tasks
lklimek
added a commit
that referenced
this pull request
Feb 25, 2026
- Remove dead `_is_sync_operation` parameter from `Database::set_platform_address_info` and all call sites (Finding #6) - Add doc comment on `set_platform_sync_info` explaining column name drift: `last_platform_sync_checkpoint` now stores SDK sync height (Finding #4) - Remove trivial `set_platform_address_info_from_sync` delegate and update callers to use `set_platform_address_info` directly (Finding #7) - Combine unnecessary `let provider` + `let mut provider = provider` rebinding into single `let mut` block (Finding #10) - Document UTXO selection race window on `broadcast_and_commit_asset_lock`: std::sync::RwLock guard is !Send so it cannot span async broadcast Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lklimek
added a commit
that referenced
this pull request
Feb 25, 2026
…rmSyncMode (#635) * refactor(wallet): simplify platform sync by removing PlatformSyncMode Remove the PlatformSyncMode enum (Auto/ForceFull/TerminalOnly) and terminal sync logic (apply_recent_balance_changes, last_terminal_block, last_full_sync_balance). The SDK now handles incremental sync internally via AddressProvider::current_balances() and last_sync_height(). Key changes: - Remove PlatformSyncMode enum from backend_task::wallet - Simplify fetch_platform_address_balances to use new SDK API with stored state (with_stored_state, current_balances, last_sync_height) - Change CoreTask::RefreshWalletInfo to use bool instead of Option<PlatformSyncMode> - Remove last_full_sync_balance from PlatformAddressInfo - Simplify database sync info to 2-tuple (timestamp, height) - Remove set_last_terminal_block from database - Simplify RefreshMode enum (remove PlatformFull, PlatformTerminal, CoreAndPlatformFull, CoreAndPlatformTerminal variants) Note: requires updated dash-sdk with new sync_address_balances API. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * chore: update platform SDK to rev 0fa82e6652 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs: add manual test scenarios for platform sync simplification Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): address PR #635 audit findings and extract broadcast helper - DB schema v28: drop obsolete columns (last_terminal_block, last_full_sync_balance), rename last_platform_sync_checkpoint → last_platform_sync_height, with SQLite ≥3.35 runtime check - Store asset lock TX before broadcast to prevent SPV InstantSend race - Defer UTXO removal until after successful broadcast - Replace .unwrap() on RwLock with .map_err() to avoid panics - Remove unused _is_sync_operation param and set_platform_address_info_from_sync wrapper - Fix redundant let-mut rebinding in fetch_platform_address_balances - Extract broadcast_and_commit_asset_lock() on AppContext to consolidate the store→broadcast→cleanup→UTXO-removal pattern from 5 code paths Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(db): schema v28 — drop obsolete sync columns and clean up DB interface (#653) - Bump DEFAULT_DB_VERSION 27 → 28 - Drop last_terminal_block from wallet table (unused after sync simplification) - Drop last_full_sync_balance from platform_address_balances table - Rename last_platform_sync_checkpoint → last_platform_sync_height - Add runtime SQLite ≥3.35 check (required for DROP COLUMN) - Idempotent migration: checks column existence before each ALTER - Remove unused _is_sync_operation param from set_platform_address_info() - Remove set_platform_address_info_from_sync() wrapper - Fix redundant let-mut rebinding in fetch_platform_address_balances Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> * revert(db): remove schema v28 migration from this branch The v28 schema changes (drop obsolete sync columns, rename last_platform_sync_checkpoint → last_platform_sync_height) will be applied separately and should not ship on this branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: address PR review comments — remove dead code and document risks - Remove dead `_is_sync_operation` parameter from `Database::set_platform_address_info` and all call sites (Finding #6) - Add doc comment on `set_platform_sync_info` explaining column name drift: `last_platform_sync_checkpoint` now stores SDK sync height (Finding #4) - Remove trivial `set_platform_address_info_from_sync` delegate and update callers to use `set_platform_address_info` directly (Finding #7) - Combine unnecessary `let provider` + `let mut provider = provider` rebinding into single `let mut` block (Finding #10) - Document UTXO selection race window on `broadcast_and_commit_asset_lock`: std::sync::RwLock guard is !Send so it cannot span async broadcast Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style: apply nightly fmt Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
2 tasks
shumkov
added a commit
that referenced
this pull request
Apr 12, 2026
Addresses all critical and important findings from the M2 code review. Critical #1: write_core SPV height Changed `WHERE seed_hash = ?2` to `WHERE wallet_id = ?2` in the wallet.last_terminal_block UPDATE. The persister passes wallet_id bytes; the old SQL matched against the seed_hash column which holds different bytes — 0 rows matched, sync height was silently lost on every restart. Critical #2: handle_wallet_unlocked shielded init After register_with_platform_wallet_manager (which may re-key the map), use wallet_id from the Wallet struct for subsequent lookups (initialize_shielded_wallet, queue_shielded_sync) instead of the stale seed_hash variable. Critical #3: WalletDerivationPath stores wrong key Changed qualified_identity_public_key.rs to populate wallet_seed_hash with wallet.wallet_id() instead of wallet.seed_hash(). Post-v40, determine_wallet_info() returns wallet_id bytes, matching the map key. Important #4/#5: wallet selection + UI validation wallets_screen uses wallet_id for persist_selected_wallet_hash and the arc-matches validation check. Finding #6: shielded_wallet_meta in v40 DELETE sweep Added to the cache nuke table list. Wallet.wallet_id is now non-optional (WalletId, not Option<WalletId>). The wallet migration screen (to be implemented) ensures every wallet has wallet_id before the main UI loads. WalletArcRef.seed_hash renamed to wallet_id. No more map_key() fallback — wallet_id is always the canonical key. get_wallets() uses [0u8; 32] as sentinel for NULL wallet_id rows (password wallets pre-migration). The migration screen detects this sentinel and prompts for unlock. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov
added a commit
that referenced
this pull request
Apr 12, 2026
… stale docs Review finding #1 (Critical): Silent data loss on proof encoding write_asset_locks used unwrap_or_default() on bincode encode failure, silently writing an empty blob. Changed to propagate the error via SqlitePersistError::Encode so the flush fails visibly instead of losing the proof. Review finding #4 (Important): Dead code cleanup Deleted store_asset_lock_transaction and update_asset_lock_chain_locked_height from Database — all callers were removed in Item 8.1d. Removed unused imports (Hash, serialize). Review finding #5 (Important): Stale doc comment Updated platform_wallet_bridge.rs module docs to reflect the current state: WalletId = SHA256(root_pub_key || chain_code), both AppContext and PlatformWalletManager keyed consistently. Review finding #2 (FK mismatch) acknowledged as pre-existing: asset_lock_transaction.wallet FK references wallet(seed_hash) but stores wallet_id bytes. FKs are off at runtime. Proper fix deferred to the wallet table PK migration. Review finding #3 (no round-trip test) acknowledged: adding a test for write_asset_locks + load_asset_locks is a follow-up. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov
added a commit
that referenced
this pull request
Apr 12, 2026
Critical #4: register_with_platform_wallet_manager called db.set_wallet_id(&wallet_id, &wallet_id) — passing wallet_id for both the seed_hash (PK lookup) and wallet_id (value to set). The SQL WHERE seed_hash = ?2 matched 0 rows because wallet_id != seed_hash. Root cause: the bulk rename changed the parameter name from seed_hash to wallet_id, but the VALUE is still the real seed_hash (from wallet_seed_snapshot). Renamed parameter back to seed_hash for clarity. Now correctly passes seed_hash for the PK lookup and platform_wallet.wallet_id() for the value. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.