fix: dpns name voting#5
Merged
Merged
Conversation
…mac_os" This reverts commit 15cb91e.
Feat/register usernames2
Contributor
|
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 (
|
QuantumExplorer
approved these changes
Oct 17, 2024
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
- 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>
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
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 13, 2026
Critical #5: replace 4 Mutex::lock().unwrap() calls with proper error handling. - store(): propagates MutexPoisoned error via the new Result return - flush() take path: propagates MutexPoisoned - flush() error recovery path: propagates MutexPoisoned - flush_inner() DB conn lock: propagates MutexPoisoned The load() method already handled poisoning correctly — now all 4 production lock sites are consistent. Test store() calls updated to .expect("store"). 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.