Skip to content

Fix matrix strategy for ARM and AMD Builds#2

Closed
vivekgsharma wants to merge 2 commits into
masterfrom
fix_matrix
Closed

Fix matrix strategy for ARM and AMD Builds#2
vivekgsharma wants to merge 2 commits into
masterfrom
fix_matrix

Conversation

@vivekgsharma
Copy link
Copy Markdown
Collaborator

No description provided.

@QuantumExplorer
Copy link
Copy Markdown
Member

Latte used this. Thanks Vivek.

@pauldelucia pauldelucia deleted the fix_matrix branch October 21, 2025 11:15
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>
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 #2 from the 7-agent review: dashpay_payments had no
wallet_id column and no filters in the load query — all payments
from all wallets were loaded into every wallet's changeset.

Changes:
- dashpay_payments schema: add `wallet_id BLOB` column
- write_identity_dashpay_subset: now takes wallet_id parameter,
  writes it in the INSERT
- load() payments section: filters by `WHERE wallet_id = ?1`

No migration needed — v40 already deleted all rows from this table.
The fresh-install schema now includes the column.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
shumkov added a commit that referenced this pull request Apr 13, 2026
Add wallet_id BLOB column to dashpay_profiles, dashpay_contacts,
and dashpay_contact_requests schemas. Write paths store wallet_id.
Load paths filter by wallet_id.

Same pattern as the dashpay_payments fix (Critical #2). No migration
needed — v40 already nuked all data in these tables. The fresh-
install schema now includes the column.

This prevents cross-wallet data contamination: loading wallet A no
longer sees profiles, contacts, or contact requests from wallet B.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants