Skip to content

chore: impl ContextProvider#1

Merged
QuantumExplorer merged 3 commits into
masterfrom
fix/sdk-deadlock
Oct 9, 2024
Merged

chore: impl ContextProvider#1
QuantumExplorer merged 3 commits into
masterfrom
fix/sdk-deadlock

Conversation

@lklimek
Copy link
Copy Markdown
Contributor

@lklimek lklimek commented Oct 9, 2024

Implemented ContextProvider as required by Sdk.

Now it reads contracts from local db instead of querying Platform each time.

Copy link
Copy Markdown
Member

@QuantumExplorer QuantumExplorer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First PR!

@QuantumExplorer QuantumExplorer merged commit 81aa4f3 into master Oct 9, 2024
@QuantumExplorer QuantumExplorer deleted the fix/sdk-deadlock branch October 9, 2024 13:56
PastaPastaPasta added a commit that referenced this pull request Feb 18, 2026
Add Phase 5 (identity creation) and Phase 6 (DPNS name registration) to
the E2E test suite, covering the #1 critical coverage gap. Identity
creation uses wallet balance funding with retry logic (3 attempts, 300s
timeout). DPNS registration generates unique timestamp-based names to
avoid collisions and contested name fees.

Source changes: make select fields pub on AddNewIdentityScreen and
RegisterDpnsNameScreen to work around AccessKit ComboBox limitations,
and expose identities module and AppContext.db for test access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
PastaPastaPasta added a commit that referenced this pull request Feb 18, 2026
Add Phase 5 (identity creation) and Phase 6 (DPNS name registration) to
the E2E test suite, covering the #1 critical coverage gap. Identity
creation uses wallet balance funding with retry logic (3 attempts, 300s
timeout). DPNS registration generates unique timestamp-based names to
avoid collisions and contested name fees.

Source changes: make select fields pub on AddNewIdentityScreen and
RegisterDpnsNameScreen to work around AccessKit ComboBox limitations,
and expose identities module and AppContext.db for test access.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
shumkov added a commit that referenced this pull request Apr 11, 2026
Phase 9b-4b shipped a deterministic deadlock that would trigger on
the first received DashPay payment for any wallet in the map.

**Root cause.** `context::transaction_processing::received_transaction_finality`
holds a `wallet_arc.write()` guard on a specific wallet while
scanning its outputs. Inside the loop it called
`match_transaction_to_contact(self, &address)`, which iterated
every wallet in `app_context.wallets` and did `wallet_arc.read()`
on each one — including the wallet whose write guard is currently
held on the same thread. `std::sync::RwLock` does not support
read-while-write on the same thread; this is a deterministic
deadlock, not a race. The cache helpers
`cache_contact_highest_receive_index_blocking` and
`cache_payment_via_platform_wallet_blocking` went through
`resolve_platform_wallet_by_owner` → `platform_wallet_for_identity` →
`require_platform_wallet` → `get_platform_wallet`, which also
calls `w.read()` on the same wallet — a second deadlock.

**Fix.** The outer loop already holds the wallet guard and already
has `wallet.platform_wallet.as_ref()` in hand. We don't need to
re-iterate or re-lock — resolve the DashPay contact using the
platform wallet on the already-held guard, and pass that
`&PlatformWallet` directly to the persistence helpers.

- `transaction_processing.rs` — call
  `pw.dashpay().match_incoming_dashpay_address_blocking(&address)`
  directly on the already-resolved platform wallet. No calls into
  `app_context.wallets`, no re-locking.
- `platform_wallet_cache.rs` — replace
  `cache_contact_highest_receive_index_blocking` and
  `cache_payment_via_platform_wallet_blocking` with
  `cache_contact_highest_receive_index_with_pw_blocking` and
  `cache_payment_with_pw_blocking` that take `&PlatformWallet`
  directly. Document the deadlock rationale in the module header.
- `incoming_payments.rs` — delete the now-unused
  `match_transaction_to_contact` helper, the dead `async
  process_incoming_payment` (zero call sites), the
  `IncomingPaymentInfo` struct it returned, and the now-unused
  `cache_contact_highest_receive_index` async helper.

Net: fewer LOC, one mutation path, and no more deadlock.

Review finding: rust-quality-engineer MUST FIX #1.

Co-Authored-By: Claude Opus 4.6 (1M context) <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
Update evo-tool's persister and backend tasks for the new
SentContactRequestKey / ReceivedContactRequestKey struct types.

Fixes the Critical #1 bug from the 7-agent review: the established
contact reconstruction in load() used reversed incoming_requests
key `(contact_id, owner_id)` instead of `(owner_id, contact_id)`.
With the new struct types, this reversal is impossible — the
compiler enforces correct field assignment:

  ReceivedContactRequestKey { owner_id, sender_id: contact_id }

Updated: sqlite.rs (write + load + round-trip test),
contact_requests.rs (send_contact_request changeset construction).

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