fix: wallet UTXO filter, SPV IS lock relay, asset lock DB store#823
Draft
fix: wallet UTXO filter, SPV IS lock relay, asset lock DB store#823
Conversation
WAL mode allows concurrent readers during writes and reduces lock contention. This is especially important when multiple async tasks access the database simultaneously. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add `unconfirmed_outpoints` tracking to Wallet — populated during SPV reconciliation from UTXO confirmation flags. `select_unspent_utxos_for()` skips UTXOs that are neither confirmed nor IS-locked, preventing failures in asset-lock transactions that require IS-locked inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… confirmed When SPV balance snapshot shows confirmed_balance >= total_balance > 0, the per-UTXO unconfirmed_outpoints set may be stale (updated by reconciliation independently of the balance snapshot). Bypass the per-UTXO filter in this case so IS-locked UTXOs aren't incorrectly rejected. Includes regression tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
After broadcasting a transaction, the SPV node misses IS lock INVs because peers are connected with relay:false. The MempoolManager's bloom filter rebuild races with IS lock creation by the quorum (~1-2s). Add re_request_is_locks_after_broadcast() that waits 2s then re-sends filterload + mempool to all peers, causing them to dump current IS lock INVs including the one for our broadcast tx. Workaround for dashpay/rust-dashcore#487. Migration path documented in TODO comments referencing rust-dashcore PR #626. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Bump dash-sdk rev to 51346ccac7 which includes the fix for on_address_found not being called during incremental-only sync (platform PR #3468). Also adapts to API changes: - process_mempool_transaction: bool -> Option<InstantLock> - process_instant_send_lock: Txid -> InstantLock - TransactionRecord fields replaced with methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
CreateRegistrationAssetLock and CreateTopUpAssetLock did not store the asset lock transaction in the DB before broadcasting. When the IS lock arrived via SPV, the finality listener failed to look up the transaction, preventing unused_asset_locks from being populated. Store the tx in the DB before broadcast (matching the pattern used by broadcast_and_commit_asset_lock) and clean it up on broadcast failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Update rev to b56bbc3ee which includes the merge of v3.1-dev into the fix/address-sync-incremental-discovery branch, ensuring on_address_found is called during incremental-only sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pin to latest v3.1-dev (9d799d33) instead of PR #3468 branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
7 tasks
lklimek
added a commit
that referenced
this pull request
Apr 10, 2026
* refactor(app): lazy network contexts, unified network switch, MCP network tools Rebased PR #803 onto current v1.0-dev by diffing against the squash-merged PR #767 base. Single commit replacing 57 granular commits that had interleaved merges from squash-merged branches. Key changes: - Defer non-active network context creation until switch - Simplify network switch to single BackendTask::SwitchNetwork - Add MCP tools: network_switch, network_refresh_endpoints - Unify context storage for MCP network operations - Force SPV backend in headless mode - Add user-friendly token validation error messages - Various SPV and shielded wallet fixes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(app): use FeatureGate::Shielded instead of naive supports_shielded() check Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): wave 1 — doc comments, stale config, error format - PROJ-001: use unwrap_or_default() in DapiNodesDiscovered handler so addresses are saved even when the network has no prior config entry - PROJ-002: fix SwitchNetwork doc comment — it IS dispatched to run_backend_task, not intercepted by AppState - PROJ-003: update CLAUDE.md MCP context provider names to match current code (ContextHolder::Shared / ContextHolder::Standalone) - PROJ-005: correct LOCAL_core_rpc_port in .env.example from 20302 to 19898 - CODE-006: use Display format ({network}) instead of Debug ({network:?}) in NetworkContextCreationFailed error message - CODE-008: remove duplicate update_settings() call from SwitchNetwork backend task handler; finalize_network_switch() already persists it Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): wave 2 — banner lifecycle, async dispatch, macro completeness, dialog consistency - Move network-switch progress banner from per-frame allocation to one-shot creation at switch initiation; clear via take_and_clear() on completion or error (CODE-001) - Replace synchronous reinit_core_client_and_sdk call in display_task_result with a deferred flag dispatched as BackendTask from the next ui() frame (PROJ-004) - Make set_ctx! macro exhaustive by adding a skip list for explicitly-handled variants; compiler now catches new Screen additions (CODE-003) - Wrap blocking AppContext::new() in tokio::task::block_in_place() inside the async SwitchNetwork handler (CODE-002) - Replace raw egui::Window fetch confirmation with ConfirmationDialog, matching SPV-clear and DB-clear dialogs on the same screen (CODE-009) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(context): use create_core_rpc_client() in reinit to preserve cookie auth Replace the direct Client::new(Auth::UserPass(...)) call in reinit_core_client_and_sdk() with Self::create_core_rpc_client(), which tries cookie authentication first and falls back to user/pass. Fixes setups that rely on .cookie auth being silently bypassed on reinit. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(review): wave A — network fallback, switch guard, init safety, path sanitization - Use chosen_network (not saved_network) for NetworkChooserScreen so the UI reflects the actual fallback network after init failure - Block ALL overlapping network switches, not just duplicates to the same network, preventing state corruption from out-of-order completion - Use OnceCell::const_new() in new_shared() — the pre-filled guard was misleading since Shared mode never enters the init path - Move core_backend_mode store/persist after provider bind succeeds so a failed bind does not leave the mode and provider out of sync - Catch and sanitize init_app_context() errors in MCP ctx() to avoid leaking filesystem paths to MCP callers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): wave B — token name escape, address logging, error source, SPV status - Escape control characters in InvalidTokenNameCharacter display to prevent unreadable banners from tab/newline-injected token names - Log warning when PlatformAddress re-encoding fails instead of silently dropping entries from the balances map - Add diagnostic detail field to NetworkContextCreationFailed for Debug output (user-facing message unchanged) - Check actual SPV status via ConnectionStatus on no-op network switch instead of hardcoding spv_started: true Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(review): wave C — FeatureGate consistency, wallet state cleanup, stale screen handling, address network - Replace direct is_developer_mode() calls with FeatureGate::DeveloperMode pattern in wallets_screen for UI consistency - Add reset_transient_state() to WalletsBalancesScreen to clear pending operations on network switch (platform balance refresh, unlock flags, asset lock search, core wallet dialog) - Clear wallet references in WalletSendScreen, SingleKeyWalletSendScreen, and CreateAssetLockScreen on network switch to prevent stale wallet Arcs from the previous context - Add network field to PlatformAddressBalances result so the display handler can verify the result matches the current network, discarding stale results Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): update token name test to expect escaped output Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * chore: remove unneeded generated docs * docs: add backend E2E test coverage requirements and test specs 83 test case specifications across 8 BackendTask groups: CoreTask (11), WalletTask (8), IdentityTask (11), DashPayTask (14), TokenTask (21), BroadcastStateTransition (2), MnListTask (6), ShieldedTask (10). Includes shared fixture design, error tests, and conditional skip guards. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * docs: add backend E2E development plan 9 tasks: 1 sequential (framework helpers + fixtures) + 8 parallel (one per BackendTask group). 5 new framework helper modules with production-code staleness annotations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): add framework helpers, fixtures, and test module stubs Add shared OnceCell-based fixtures (SharedIdentity, SharedToken, SharedDashPayPair) and domain-specific helper modules (dashpay_helpers, token_helpers, mnlist_helpers, shielded_helpers) for backend E2E tests. Create 8 empty test stub files (core_tasks, wallet_tasks, identity_tasks, dashpay_tasks, token_tasks, broadcast_st_tasks, mnlist_tasks, shielded_tasks) with module declarations in main.rs so parallel implementation agents can work independently. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): implement MnListTask tests (TC-068 to TC-073) Six read-only P2P masternode list query tests: - TC-068: FetchEndDmlDiff between tip-100 and tip - TC-069: FetchEndQrInfo with genesis as known block - TC-070: FetchEndQrInfoWithDmls (same flow, different variant) - TC-071: FetchDiffsChain over two consecutive 100-block windows - TC-072: FetchChainLocks (conditional on E2E_CORE_RPC_URL) - TC-073: FetchEndDmlDiff with all-zeros hash must return Err Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(e2e): implement WalletTask tests (TC-012 to TC-019) Adds 8 backend E2E tests for WalletTask variants. TC-014 through TC-017 form a sequential fund→verify→transfer→withdraw flow using a shared OnceCell. TC-018 exercises FundPlatformAddressFromAssetLock via a live asset lock built from CreateRegistrationAssetLock. TC-019 confirms typed WalletNotFound error on unknown seed hash. Also fixes pre-existing compilation errors in identity_tasks.rs (private sdk field access, unused imports, clone-on-copy) introduced by the Task 3 merge. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(e2e): implement CoreTask tests (TC-001 to TC-011) Replaces the stub in tests/backend-e2e/core_tasks.rs with 11 test functions covering all CoreTask variants: refresh wallet (core-only and with platform), refresh single-key wallet, create registration and top-up asset locks, recover asset locks, chain lock queries (single and multi-network), send single-key wallet payment, list core wallets (conditional on E2E_CORE_RPC_URL), and error path for invalid address. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(e2e): implement DashPayTask tests (TC-031 to TC-044) 14 test cases covering the full DashPay contact lifecycle: - TC-031..TC-036: Profile, contacts, and contact request queries - TC-037..TC-042: Sequential contact flow (send/accept/register/update) - TC-043: Reject contact request (with third identity) - TC-044: Error path — nonexistent username Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): implement TokenTask tests (TC-045 to TC-065) Implement 21 backend E2E tests covering the full token lifecycle: - Registration (TC-045), querying (TC-046..TC-052), minting (TC-053) - Burn (TC-054), transfer (TC-055), freeze/unfreeze (TC-056/057) - Destroy frozen funds (TC-058), pause/resume (TC-059/060) - Set price and purchase (TC-061/062), config update (TC-063) - Perpetual rewards estimation (TC-064), unauthorized mint error (TC-065) Uses shared fixtures (SharedIdentity, SharedToken) and a module-level SecondIdentity OnceCell for tests needing a recipient/target identity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * test(e2e): implement BroadcastStateTransition tests (TC-066 to TC-067) TC-066: build a valid IdentityUpdateTransition with a fresh key, fetch the current identity nonce from Platform via a dedicated test SDK, sign the transition with the master key, and broadcast via BackendTask::BroadcastStateTransition. Asserts BroadcastedStateTransition and re-fetches the identity to confirm the new key is visible on Platform. TC-067: build a signed IdentityUpdateTransition with an intentionally invalid nonce (u64::MAX) and assert that BackendTask::BroadcastStateTransition returns Err(TaskError::...) from Platform rejection. Follows up with RefreshIdentity to confirm Platform state is intact. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * test(e2e): implement ShieldedTask tests (TC-074 to TC-083) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): QA fixes — wrong result variant, runtime config, Core RPC guards - TC-004/TC-005: assert Message(...) not InstantLockedTransaction (QA-001) - wallet_tasks.rs: add missing multi_thread + worker_threads = 12 (QA-002) - mnlist_tasks.rs: add require_core_rpc() guard on TC-068..TC-071 (QA-003) - fixtures.rs: make extract_authentication_key pub for reuse (QA-004) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): remove Core RPC-specific tests from SPV-only E2E suite Removed TC-007 (GetBestChainLock), TC-008 (GetBestChainLocks), TC-010 (ListCoreWallets), TC-068..TC-072 (MnList queries) — all require Core RPC which is not available in SPV mode. TC-003 (RefreshSingleKeyWalletInfo), TC-006 (RecoverAssetLocks), TC-009 (SendSingleKeyWalletPayment) are kept — they expose production code that incorrectly requires Core RPC in SPV mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): restore MnList P2P tests with SPV-based block hash retrieval Rewrite mnlist_helpers to use DAPI (GetBlockchainStatus) for chain tip and Network::known_genesis_block_hash() for genesis — no Core RPC needed. Restore TC-068 through TC-071 using genesis+tip instead of arbitrary height lookups. TC-071 uses a single-segment chain (DAPI only provides tip hash, not hashes at arbitrary heights). TC-072 stays removed as it genuinely requires Core RPC. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): handle encrypted keys in extract_authentication_key, document stack size - Skip encrypted private keys instead of panicking (matches dashpay_helpers pattern) - Document RUST_MIN_STACK=16777216 requirement for SDK's deep call stacks Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): derive signing keys during registration, add P2P node guard The wallet encrypts private keys after identity registration, making post-registration extraction from QualifiedIdentity impossible (all keys are PrivateKeyData::Encrypted). Capture raw master key bytes from build_identity_registration before they become encrypted. Also add require_local_core_p2p() guard to MnList P2P tests (TC-068 to TC-073) that need a local Dash Core node on 127.0.0.1:19999. Tests skip gracefully when no node is available. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): token key level, funding amounts, shielded skip, broadcast simplification - fixtures: prefer HIGH over MASTER key in find_authentication_public_key to avoid InvalidSignaturePublicKeySecurityLevelError on token operations - identity_tasks: reduce top-up amounts from 50M/5M to 500K duffs to match the 2M duffs wallet funding budget - shielded_tasks: gracefully skip tests when platform returns "not implemented" or "not supported" instead of panicking - broadcast_st_tasks: replace TestContextProvider with proof-less SDK (with_proofs(false)) to avoid quorum key lookup failures during nonce fetch Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase identity funding for token contract registration - Asset lock: 1M → 5M duffs (~50B credits, enough for 40B token registration) - Wallet funding: 2M → 10M duffs (covers asset lock + transaction fees) - Remove test keywords from token contract (each keyword costs 10B credits) 1 duff ≈ 1000 Platform credits. Token contract registration costs ~20B credits (base 10B + token 10B) without keywords. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): DPNS propagation wait, funding strategy, assertion fixes, shielded skip - Add DPNS name propagation polling (up to 60s) in SharedDashPayPair fixture after registering names, preventing tc_033/tc_037/tc_043 from failing due to names not yet queryable on Platform. - tc_033: retry search assertion up to 30s instead of asserting immediately. - tc_042: add ECDSA_SECP256K1 AUTHENTICATION key to identity B before calling UpdateContactInfo (which requires this key type). - tc_043: add DPNS propagation wait after registering identity C's name before sending contact request. - tc_021: reduce FundPlatformAddressFromWalletUtxos amount from 500K to 200K duffs to avoid depleting SharedIdentity wallet. - tc_023: relax fee > 0 assertion — actual_fee may be 0 for credit transfers where fees are deducted from the transferred amount. - tc_013: remove "must be empty" assertion for platform address balances — the workdir is persistent so addresses may exist. - tc_017: fund a fresh platform address before withdrawal since tc_016 drains the original one. - tc_018: increase IS lock proof timeout from 120s to 240s. - tc_066/harness: increase SPV sync timeout from 300s to 600s. - tc_067: refresh identity from Platform before building state transition to get accurate key IDs after other tests add keys. - Expand is_platform_shielded_unsupported() to match CoreRpc connection errors and unsupported variant types (15-19). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase all test wallet funding to match 5M duffs asset lock All create_funded_test_wallet calls now use 10M duffs (was 2-3M) to ensure enough for the 5M duffs asset lock + transaction fees. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase asset lock to 25M duffs, wallet funding to 30M Token contract registration requires ~20B credits (base 10B + token 10B). At ~1000 credits/duff, need 25M duffs in asset lock. Wallet needs 30M to cover asset lock + transaction fees. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): CRITICAL key for minting, IS lock timeout, tc_048 result variant - Reorder key priority to CRITICAL-first in both find_authentication_public_key (fixtures.rs) and find_auth_public_key (token_tasks.rs) — minting requires CRITICAL and CRITICAL can do everything HIGH can. - Make IS lock wait lenient in create_funded_test_wallet: if spendable balance times out but total balance is sufficient, warn and continue instead of panicking. Increases timeout from 120s to 180s for large funding amounts. - Fix tc_048 assertion to expect FetchedContract (not FetchedContractWithTokenPosition) — FetchTokenByContractId returns a plain contract without position. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): set recipient_id for token minting (self-mint) Platform requires DestinationIdentityForTokenMinting to be set. Pass the sending identity's ID as recipient for self-minting. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): DPNS timeouts, auth key, address lookup, cleanup distribution - Increase DPNS propagation timeouts: shared_dashpay_pair 60s->120s, tc_033 search 30s->90s, tc_043 propagation 60s->120s - Add retry loops for UsernameResolutionFailed in tc_037 and tc_043 SendContactRequest (up to 60s backoff) - Fix tc_042 UpdateContactInfo: reload identity from local DB after RefreshIdentity to get the updated key set (RefreshIdentity returns stale input QI) - Fix tc_067 BroadcastInvalidST: reload identity from local DB after RefreshIdentity to get current key state, use refreshed QI for signing - Fix tc_016/tc_017: fetch platform address balances first and use the discovered funded address instead of relying on stale FUNDED_PLATFORM - Fix tc_021: add retry loop polling FetchPlatformAddressBalances until credits appear (up to 30s) - Fix tc_017: add retry loop for FetchPlatformAddressBalances after funding (up to 30s) - Cleanup: derive a fresh receive address per sweep to distribute UTXOs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): broadcast ordering, DPNS delays, wallet address sync, timeouts Fixes 9 consistently failing backend E2E tests: - tc_066/tc_067: Rename broadcast_st_tasks.rs to z_broadcast_st_tasks.rs so broadcast tests no longer run first alphabetically, avoiding SPV initialization timeout poisoning the OnceCell for all subsequent tests. - tc_033/tc_037/tc_043: Add initial sleep delays (10-15s) after DPNS registration and increase retry timeouts from 60-90s to 120s to allow Platform propagation of DPNS names before username resolution. - tc_016/tc_017: Derive platform addresses via platform_receive_address() before fetching balances, ensuring addresses are in watched_addresses. Prefer the derived address when selecting source/withdrawal address to avoid "Platform address not found in wallet" errors from stale DB state. - tc_021: Increase platform credits poll timeout from 30s to 90s for asset lock broadcast + proof confirmation on testnet. - tc_018: Increase IS lock proof timeout from 240s to 360s. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): make wallet tests self-contained with ensure_funded_platform helper Replace direct OnceCell `.get().expect()` calls in tc_015/tc_016/tc_017 with a lazy `ensure_funded_platform()` helper using `get_or_init`. Any test can now run independently — the first caller funds the platform address, subsequent callers reuse the cached state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): use >= 20 char DPNS names to avoid contest voting period Contested DPNS names (< 20 chars) enter a masternode voting period and don't appear as regular domain documents. This broke SearchProfiles and username resolution in DashPay tests. - e2epair-a/b: 18 → 26 chars (8 hex bytes instead of 4) - e2erej-c: 17 → 25 chars - register_dpns: 11-19 → 24 chars Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): deterministic workdir with file-lock based slot selection Replace git-hash-keyed workdir with a fixed path (/tmp/dash-evo-e2e-testnet). If locked by another process, falls back to -1, -2, etc. (up to 10 slots). Benefits: - Database, wallets, and SPV data persist across commits - Concurrent test runs get separate workdirs automatically - No more stale workdirs accumulating per git revision Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): handle normalizedLabel in SearchProfiles comparison SearchProfiles returns normalizedLabel (with homograph conversion, e.g. i→1) instead of the original label. Compare against both forms. Production bug: search_profiles reads normalizedLabel at line 490 instead of label — should be fixed in production code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): add context provider to SDK builder, fix key lookup in broadcast tests TC-066: SdkBuilder requires a context provider even with proofs disabled. Add NoopContextProvider (matching mnlist_helpers pattern) to build_test_sdk(). TC-067: can_sign_with_master_key() searches private_keys which may reference key IDs not present in the refreshed identity's public_keys() (e.g., keys added by tc_066). Look up the master AUTHENTICATION key directly from identity.public_keys() instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * feat(test): add nonce retry, funding mutex, and WAL mode for parallel E2E tests - run_task_with_nonce_retry(): retries up to 3x (2s delay) on IdentityNonceOverflow/NotFound - FUNDING_MUTEX: narrows UTXO critical section in create_funded_test_wallet() to broadcast only - WAL journal mode: enables concurrent reads during writes in Database::new() Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): merge DashPay dependency chain into single lifecycle test Collapses TC-037 → TC-038 → TC-039 → TC-040 → TC-042 into one tc_037_dashpay_contact_lifecycle() function with private step helpers, removing the INCOMING_REQUEST_ID OnceCell and the five individual tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): merge token lifecycle dependency chain into single test Collapse tc_053..tc_063 (mint → burn → transfer → freeze → unfreeze → destroy_frozen → pause → resume → set_price → purchase → update_config) into a single `tc_053_token_lifecycle()` test with private step functions. Removes the `MINTED` OnceCell and `ensure_minted()` helper (mint is now step 1 of the lifecycle test). Keeps `SECOND_IDENTITY` / `ensure_second_identity()` which are still required by the independent tc_065 error-path test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): merge wallet platform dependency chain into single test Collapse the TC-014 → TC-015 → TC-016 → TC-017 sequence into one `tc_014_wallet_platform_lifecycle()` test backed by four private step functions. Removes the `FUNDED_PLATFORM` OnceCell, `FundedPlatformState` struct, and `ensure_funded_platform()` helper. All other tests (TC-012, TC-013, TC-018, TC-019) are unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): merge shielded lifecycle dependency chain into single test Collapse tc_074..tc_082 into tc_074_shielded_lifecycle() with private step_* helpers. Remove ensure_shielded_balance() — shielding is now step_shield_from_asset_lock(). Keep tc_079 and tc_083 unchanged. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor(test): merge identity and broadcast dependency chains into lifecycle tests Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): fix clippy needless_borrow warnings in merged lifecycle tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(test): use nonce retry for state transitions in lifecycle tests Replace run_task() with run_task_with_nonce_retry() in all step functions of merged lifecycle tests (tc_037, tc_053, tc_014, tc_074, tc_020, tc_066) for state-transition operations. Read-only operations (fetch, search, refresh) keep plain run_task(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(wallet): filter unconfirmed UTXOs from coin selection select_unspent_utxos_for() previously selected UTXOs without checking confirmation status, causing downstream failures for asset-lock transactions that require IS-locked inputs. Add unconfirmed_outpoints tracking to the Wallet model, populated by reconcile_spv_wallets() from upstream per-UTXO confirmation flags. UTXO selection now skips outpoints that are neither confirmed nor IS-locked, while still including them in balance display. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): require confirmed funds in create_funded_test_wallet Previously, when IS lock timed out, the harness continued if total balance was sufficient. This is wrong — unconfirmed UTXOs cannot be used for Platform operations (asset locks). Replace graceful degradation with block confirmation fallback: when IS lock times out (180s), wait up to 300s more for block confirmation. Only panic if both IS lock and block confirmation fail. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(spv): re-request IS locks after broadcast to work around relay:false After broadcasting a transaction, the SPV node misses IS lock INVs because peers are connected with relay:false. The MempoolManager's bloom filter rebuild (triggered by notify_wallet_after_broadcast) races with IS lock creation by the quorum (~1-2s). Add re_request_is_locks_after_broadcast() that waits 2s then re-sends filterload + mempool to all peers, causing them to dump current IS lock INVs including the one for our broadcast tx. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(deps): update platform SDK to fix incremental address sync Update dash-sdk and rs-sdk-trusted-context-provider rev to 51346ccac7a955d1ea48f061ad2e12a42d3c8c37 which fixes the incremental address sync bug where on_address_found is not called for seeded balances (dashpay/platform PR #3468). Adapt to upstream API changes in rust-dashcore: - process_mempool_transaction second param: bool -> Option<InstantLock> - process_instant_send_lock param: Txid -> InstantLock - TransactionRecord fields (height, timestamp, block_hash, is_ours) replaced with methods and TransactionContext enum Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): filter SharedToken contract by owner identity The SharedToken fixture scanned the DB for any contract with tokens, which could pick up a stale contract from a previous run with a different wallet seed. Filter by owner_id to ensure we use the contract owned by the current SharedIdentity. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(core): store asset lock in DB before broadcast CreateRegistrationAssetLock and CreateTopUpAssetLock did not store the asset lock transaction in the DB before broadcasting. When the IS lock arrived via SPV, the finality listener failed to look up the transaction, preventing unused_asset_locks from being populated. Store the tx in the DB before broadcast (matching the pattern used by broadcast_and_commit_asset_lock) and clean it up on broadcast failure. Fixes tc_018 timeout waiting for asset lock IS proof. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): use app SDK instead of standalone for tc_066 The standalone SDK with NoopContextProvider and proofs disabled panics with "queries without proofs are not supported yet". Replace with the app context's SDK which has a proper context provider. Add a public sdk() accessor to AppContext to enable test access. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): look up MASTER key from identity public keys in tc_066 The shared identity's QualifiedIdentity.private_keys may contain stale key IDs from prior test runs (persistent workdir). Looking up the MASTER AUTHENTICATION key from the identity's actual public_keys() ensures the key ID always matches, consistent with step_broadcast_invalid. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): retry profile load after update in tc_032 Platform may take a few seconds to propagate the profile update across nodes. The immediate LoadProfile query could hit a node that hasn't synced yet, returning None. Add a retry loop (3s intervals, 30s total). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase poll timeout and reset sync state in tc_020/tc_014 Platform needs time to process funding transactions in blocks (~2.5 min on testnet). Increase poll timeout from 90s to 180s and reset the platform sync checkpoint before polling so incremental sync doesn't skip newly funded addresses. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): capture initial balance before sending in tx_is_ours Reading B's balance after A sends (during wait_for_spendable_balance reconciliation) may include the send amount, inflating the wait target to an unreachable value. Move initial_b capture before the send. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): add 120s timeout to tc_046 to prevent indefinite hang QueryMyTokenBalances makes SDK network calls that can hang if a Platform node is unresponsive. Wrap in tokio::time::timeout so the test fails cleanly instead of blocking the entire test run. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(wallet): skip stale unconfirmed filter when SPV reports all funds confirmed When `spv_balance_known` is true and `confirmed_balance >= total_balance > 0`, the aggregate SPV snapshot is authoritative — the per-UTXO `unconfirmed_outpoints` set may be stale (updated by reconciliation, which runs independently of the balance snapshot). In that case, bypass the per-UTXO filter so UTXOs that are IS-locked at the aggregate level are not incorrectly rejected. Adds two regression tests: one verifying the fast-path activates when fully confirmed, one verifying it stays inactive when partially unconfirmed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): clean stale wallets from persistent DB on harness init Compute the framework wallet hash from E2E_WALLET_MNEMONIC before SPV starts, then purge all other wallets from the DB and AppContext. SPV builds a bloom filter for every loaded wallet address — accumulated test wallets from previous runs inflate that filter and push sync time past the 600 s timeout. Also deduplicate the mnemonic/seed derivation that was previously split across two points in init(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * fix(test): fetch fresh identity and register new key private in tc_066 The signer implementation looks up private keys from the qualified identity's key storage. The test was passing a stale fixture identity that lacked both the current Platform public keys and the new key's private key, causing "Key 6 (AUTHENTICATION) not found" errors. Now mirrors the production pattern (add_key_to_identity.rs): - Fetch current identity from Platform for accurate keys + revision - Register new key's private key in signer before building transition - Bump revision to match Platform expectations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): filter asset lock by amount in tc_018, retry on timeout in tc_014 tc_018: The test picked up a smaller asset lock created by a concurrent test on the same framework wallet. Now filters unused_asset_locks by amount (>= 90M credits) to find the correct one. tc_014: FundPlatformAddressFromWalletUtxos times out when the asset lock proof does not arrive within 300s on testnet. Switch from nonce-only retry to run_task_with_retry which also retries ConfirmationTimeout. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(test): add run_task_with_retry helper for transient errors Retries on ConfirmationTimeout, IdentityNonceOverflow, and IdentityNonceNotFound with exponential back-off (5s base). Useful for testnet operations where asset lock proofs can be slow to arrive. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase tc_046 QueryMyTokenBalances timeout to 300s The SDK TokenAmount::fetch_many call to DAPI can take over 120s on a loaded testnet. Increase timeout to 300s to match other network-dependent operations. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): increase tc_020 platform address balance poll timeout to 360s On testnet, blocks are ~2.5 min apart. If the funding tx lands right after a block, the next block (carrying the balance) may not arrive within 180s. Increase to 360s (two full block intervals plus margin). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(db): enable WAL journal mode for concurrent read/write access WAL mode allows concurrent readers during writes and reduces lock contention. This is especially important when multiple async tasks access the database simultaneously. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): filter unconfirmed UTXOs from coin selection Add `unconfirmed_outpoints` tracking to Wallet — populated during SPV reconciliation from UTXO confirmation flags. `select_unspent_utxos_for()` skips UTXOs that are neither confirmed nor IS-locked, preventing failures in asset-lock transactions that require IS-locked inputs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(wallet): skip stale unconfirmed filter when SPV reports all funds confirmed When SPV balance snapshot shows confirmed_balance >= total_balance > 0, the per-UTXO unconfirmed_outpoints set may be stale (updated by reconciliation independently of the balance snapshot). Bypass the per-UTXO filter in this case so IS-locked UTXOs aren't incorrectly rejected. Includes regression tests. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(spv): re-request IS locks after broadcast to work around relay:false After broadcasting a transaction, the SPV node misses IS lock INVs because peers are connected with relay:false. The MempoolManager's bloom filter rebuild races with IS lock creation by the quorum (~1-2s). Add re_request_is_locks_after_broadcast() that waits 2s then re-sends filterload + mempool to all peers, causing them to dump current IS lock INVs including the one for our broadcast tx. Workaround for dashpay/rust-dashcore#487. Migration path documented in TODO comments referencing rust-dashcore PR #626. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): update platform SDK to fix incremental address sync Bump dash-sdk rev to 51346ccac7 which includes the fix for on_address_found not being called during incremental-only sync (platform PR #3468). Also adapts to API changes: - process_mempool_transaction: bool -> Option<InstantLock> - process_instant_send_lock: Txid -> InstantLock - TransactionRecord fields replaced with methods Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(core): store asset lock in DB before broadcast CreateRegistrationAssetLock and CreateTopUpAssetLock did not store the asset lock transaction in the DB before broadcasting. When the IS lock arrived via SPV, the finality listener failed to look up the transaction, preventing unused_asset_locks from being populated. Store the tx in the DB before broadcast (matching the pattern used by broadcast_and_commit_asset_lock) and clean it up on broadcast failure. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): pin platform SDK to PR #3468 with address sync fix Update rev to b56bbc3ee which includes the merge of v3.1-dev into the fix/address-sync-incremental-discovery branch, ensuring on_address_found is called during incremental-only sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): pin platform SDK to PR #3468 with address sync fix Update rev to b56bbc3ee which includes the merge of v3.1-dev into the fix/address-sync-incremental-discovery branch, ensuring on_address_found is called during incremental-only sync. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): use DIP-17 platform payment addresses in tc_020 step_top_up_from_platform_addresses and step_transfer_to_addresses used BIP44 receive addresses (m/44'/1'/0'/0/index) which are not scanned by sync_address_balances. Switch to platform_receive_address() which derives DIP-17 Platform payment addresses (m/9'/1'/17'/...) that WalletAddressProvider includes in its scan set. Also add two-phase poll: direct AddressInfo query detects when Platform has the balance, then gives sync 30s grace to catch up — cutting feedback time from 360s to ~35s on SDK sync bugs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * test(e2e): add tc_031 incremental address sync test, fix tc_020 address type - tc_020: use platform_receive_address() (DIP-17 m/9'/1'/17'/...) instead of receive_address() (BIP44 m/44'/1'/0'/...). sync_address_balances only scans DIP-17 addresses via WalletAddressProvider. - tc_020: add two-phase poll with direct AddressInfo::fetch fallback for faster SDK sync bug detection (30s vs 360s). - tc_031: new test verifying full→incremental sync preserves seeded balances via on_address_found callback (Platform SDK PR #3468 regression test). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): pin platform SDK to v3.1-dev PR #3468 (on_address_found fix) doesn't add value — the incremental sync path already handles seeded balances correctly on v3.1-dev. Pin to latest v3.1-dev (9d799d33) instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(deps): pin platform SDK to v3.1-dev Pin to latest v3.1-dev (9d799d33) instead of PR #3468 branch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(test): add TODO for tc_018 asset lock known_addresses bug (#799) CreateRegistrationAssetLock's one-time key address is not registered in known_addresses, so received_asset_lock_finality skips the wallet when the IS lock arrives. Root cause tracked in issue #799. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(test): add TODO comments for known test failures - tc_003, tc_006, tc_009: Core RPC-only tests, fail in SPV mode - tc_014 step_withdraw: sync_address_balances returns balance that Platform rejects — proof/processor disagreement (upstream bug) - tc_018: asset lock one-time key not in known_addresses (#799) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): retry identity fetch after broadcast for DAPI propagation delay The broadcast is confirmed by one DAPI node but the immediate re-fetch may hit a different node that hasn't processed the block yet. Add a 30s retry loop with 3s intervals for the verification fetch. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor(test): remove workarounds from backend E2E tests Replace retry loops, hardcoded sleeps, and fallback queries with single calls and TODO comments that document the underlying bugs. Tests should expose issues clearly, not hide them behind workarounds. Changes: - Remove `run_task_with_retry` (ConfirmationTimeout retry is a workaround for the IS lock relay bug) - tc_020: remove two-phase poll with direct AddressInfo::fetch fallback, use simple FetchPlatformAddressBalances poll loop - tc_032: remove profile load retry loop (DAPI propagation) - tc_033: remove 10s sleep and search retry loop (DAPI propagation) - tc_037/step_send_contact_request: remove 10s sleep and UsernameResolutionFailed retry (DAPI propagation) - tc_043: remove 15s sleep and UsernameResolutionFailed retry - tc_066: remove DAPI propagation retry loop on re-fetch after broadcast, fetch once and log warning if stale - register_dpns: remove 3-attempt retry with 30s sleep for identity propagation delay - wallet_tasks step_fund: replace run_task_with_retry with run_task_with_nonce_retry - Fix pre-existing clippy clone_on_copy warnings in tc_031 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * refactor(test): add MAX_TEST_TIMEOUT constant, replace hardcoded timeouts Define MAX_TEST_TIMEOUT (360s) in harness and reference it from all test files instead of hardcoded Duration values. Only SPV init (600s) is exempt. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * refactor: remove production fixes that belong to PR #823 Remove behavioral fixes already in fix/wallet-spv-fixes (PR #823): - WAL mode (src/database/mod.rs) - UTXO unconfirmed filter (wallet/utxos.rs, wallet/mod.rs) - Unconfirmed outpoints tracking (wallet_lifecycle.rs, database/wallet.rs) - SPV IS lock re-request workaround (spv/manager.rs) - Asset lock DB store before broadcast (create_asset_lock.rs) Keep only SDK API adaptations needed for compilation with v3.1-dev: - process_mempool_transaction(bool -> None) - process_instant_send_lock(Txid -> InstantLock) - TransactionRecord field -> method access Tests will fail at runtime until PR #823 is merged into v1.0-dev. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * style(test): fix formatting after MAX_TEST_TIMEOUT refactor Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): apply triage fixes from PR #818 comment review - Replace Debug-string parsing in shielded_helpers with typed TaskError matching + FeatureGate proactive check for shielded support - Make step_sync_notes return bool to halt lifecycle when unsupported - TC-083: assert specific WalletNotFound variant instead of is_err() - TC-065: assert PlatformRejected variant for unauthorized mint - TC-064: expect error or zero-amount (no perpetual distribution) - Filter DashPay incoming requests by sender identity (tc_037, tc_043) - Use run_task_with_nonce_retry for DashPay state transitions (tc_043) - Assert specific funded address in wallet balance checks (tc_014) - Document why platform address funding is safe outside FUNDING_MUTEX - Remove substring-based asset lock success assertions (tc_004, tc_005) - Add explicit skip with warning for tc_009 SPV-mode limitation - Fix log messages: "10M duffs" -> "30M duffs" (fixtures, token_tasks) - Fix docstring: MASTER key is included as fallback, not skipped - Add comment explaining owner_id filter mitigates stale contract - Add comment explaining empty contract_keywords (cost) - Remove duplicate find_auth_public_key from token_tasks (use fixtures) - Move #[allow(dead_code)] above doc comment in task_runner - Drop wallets read-lock before get_receive_address in cleanup - Log wallet balance before startup purge in harness - Remove duplicate doc comment line in harness - Show actual elapsed time instead of hardcoded "480s" in error - Add INTENTIONAL(CMT-038) comment for non-Unix try_lock_exclusive - Eliminate false-PASS patterns: change tracing::warn to panic/assert for DashPayProfile(None), username not found, missing contact requests, and key not found after broadcast - Parallelize DashPay pair fixture setup with tokio::join! - Extract create_dashpay_member and register_dpns_name helpers Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(test): tc_065 accept SdkError variant, tc_066 add 1s DAPI propagation delay - tc_065: accept both PlatformRejected and SdkError (wrapping DestinationIdentityForTokenMintingNotSetError) as valid rejections - tc_066: add 1s sleep before re-fetch to allow DAPI node propagation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): tc_065 typed match for consensus rejection, tc_066 keep TODO tc_065: match PlatformRejected or SdkError wrapping ConsensusError (DestinationIdentityForTokenMintingNotSetError). Added TODO for dedicated TaskError variant for token authorization errors. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix(test): rename tc_065 to reflect actual behavior (missing destination, not auth) The token fixture has new_tokens_destination_identity: None, so the mint fails with DestinationIdentityForTokenMintingNotSetError before authorization is checked. Renamed from tc_065_mint_unauthorized to tc_065_mint_without_destination with TODO to add a proper authorization test once the fixture sets a default mint destination. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * Revert "fix(test): rename tc_065 to reflect actual behavior (missing destination, not auth)" This reverts commit 3450e6d. * fix(test): set token mint destination to owner, tc_065 tests real authorization Token fixture now sets new_tokens_destination_identity to the owner's identity. This ensures tc_065 tests actual authorization rejection (owner-only minting rules) instead of hitting DestinationIdentityFor TokenMintingNotSetError before auth is checked. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <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
1 participant
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.
Summary
Production fixes discovered during backend E2E test development (PR #818). Cherry-picked to ship independently — no test code included.
select_unspent_utxos_for()skips UTXOs that are neither confirmed nor IS-locked, preventing asset-lock failures from spending unconfirmed inputsunconfirmed_outpointscheck (which may be stale relative to the balance snapshot). Includes regression tests.relay:falsein SPV handshake: after broadcasting, wait 2s then re-sendfilterload+mempoolto trigger peers to dump IS lock INVs (tracks SPV broadcast_transaction does not notify local wallet manager — balance stale until block rust-dashcore#487, migration path documented)9d799d33) — adapts to API changes:process_mempool_transaction(bool → Option<InstantLock>),process_instant_send_lock(Txid → InstantLock),TransactionRecordfields → methodsCreateRegistrationAssetLockandCreateTopUpAssetLocknow store the tx in DB before broadcasting, so the SPV finality listener can look it up when the IS lock arrives. Cleans up on broadcast failure.Test plan
cargo build --all-featurespassescargo clippy --all-features --all-targets -- -D warningspassescargo +nightly fmt --all --checkpasses🤖 Co-authored by Claudius the Magnificent AI Agent