fix(wallet): isolate wallet databases from SPV cache + stop cross-wallet top-up corruption - #954
Conversation
…able SPV cache dir platform-wallet.sqlite and platform-wallet-shielded.sqlite (identities, keys, UTXOs, contacts, Orchard state) lived inside <data_dir>/spv/<network>/, the same directory as the disposable, resyncable dash-spv chain cache (headers, filters, blocks, masternode state, peers). A user or a future "clear disposable cache" feature deleting that directory would take irreplaceable wallet state down with it. Each network's wallet database now lives directly under <data_dir>/, as a sibling of det-app.sqlite: det-<network>.sqlite and det-<network>-shielded.sqlite. The spv/<network>/ directory now holds only genuinely disposable chain-cache data. No migration: this only changes where new/future opens look. An existing install's spv/<network>/platform-wallet.sqlite is not moved automatically. Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…t ownership
top_up_identity force-registered the target identity into the paying
wallet's upstream IdentityManager whenever it wasn't already managed there
-- true for every identity actually owned by a different wallet. The
identities table write is correctly refused by its own cross-wallet
ownership guard, but the accompanying identity_keys rows land under the
payer's wallet_id anyway. On the next load the true owning wallet's
wallet-scoped reader finds those keys' owner absent from its managed set
and fails the entire wallet load fatally ("Saved wallet data appears
damaged and cannot be loaded"). Separately, IdentityManager::add_identity
overwrites whatever the payer's own wallet already had at that HD index,
so a later top-up of the payer's own identity at the same index could
silently submit for the wrong identity -- funds misdirection, not just a
load failure.
Funding an identity you don't own, from a different wallet's balance, is a
deliberate feature (top_up_identity_screen, send_screen) and must keep
working. Fixed by routing on the identity's real recorded owner
(StoredQualifiedIdentity.wallet_hash, not the associated_wallets field --
confirmed to be populated with every currently-loaded wallet regardless of
actual ownership, so unusable as a guard): an owned identity still takes
the existing orchestrated path; a foreign identity now funds through a
manager-free path (WalletBackend::create_asset_lock_proof +
TopUpIdentity::top_up_identity_with_private_key) that never touches the
paying wallet's identity manager.
Needs live-network verification before shipping (tracked separately,
memcan dash-evo-tool TODO 7e1a5fa8): offline tests cover routing,
provisioning, and the corruption regression, but a real foreign top-up
end-to-end, and the UseAssetLock resume variant, are untested on-network.
Two upstream functions needed for full IS-to-CL fallback and asset-lock
cleanup on the new path are pub(crate) (tracked as platform TODO
e39c9d4a) -- until exposed, a foreign top-up has no IS-to-CL retry and a
spent lock used for one can linger in the resumable-funding list.
Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
📝 WalkthroughWalkthroughThe PR moves durable wallet and shielded databases outside disposable SPV caches. It adds ownership-aware identity top-up routing, index-less funding accounts, asset-lock resumption, error handling, and regression coverage for cross-wallet funding isolation. ChangesIdentity storage and cross-wallet top-ups
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
⛔ Blockers found — Sonnet deferred (commit 27231c2) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/backend_task/identity/top_up_identity.rs (1)
196-236: 🗄️ Data Integrity & Integration | 🔵 TrivialConfirm the TODO's tracked-consumption gap is acceptable for this release.
The TODO at lines 207-211 states that a spent lock used through this path keeps its pre-consumption status in the funding list, because
consume_asset_lockstayspub(crate)upstream. A user (or the UI's "recover unfinished funding" list) could see the same already-used lock as still eligible, and attempt to reuse it. Platform's own asset-lock consumption checks would likely reject a genuine reuse, but the local funding list still misrepresents the lock's state until reconciliation catches up.Since this is already tracked as an upstream contribution, do you want me to open a tracking issue for restoring
consume_asset_lock/upgrade_to_chain_lock_proofcalls once upstream exposes them?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/backend_task/identity/top_up_identity.rs` around lines 196 - 236, Track the missing asset-lock consumption and recovery behavior in the existing upstream TODO around top_up_foreign_identity, including restoring consume_asset_lock and upgrade_to_chain_lock_proof once upstream exposes them. For this release, leave the current implementation unchanged and ensure the tracked issue covers removing used locks from resumable funding and retrying rejected InstantSend proofs.src/context/wallet_lifecycle/tests.rs (1)
4519-4560: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the dispatch reaches funding before checking anti-displacement.
cross_wallet_top_up_never_displaces_the_paying_wallets_own_identitydiscards the dispatch result as_errorand asserts only thatownstill resolves to itself afterward. If the ownership-aware routing rejects the call early (before it can reach the write that would displaceown), this assertion passes trivially regardless of whether the anti-displacement fix is present.Its sibling test,
cross_wallet_top_up_leaves_the_paying_wallets_identity_state_intact, adds an explicit check that the error is notIdentityNotWalletOwned/IdentityIndexMismatchso the test proves the call reached funding. Add the same check here so a regression in the anti-displacement logic cannot be masked by an unrelated early rejection.♻️ Suggested addition
let _error = dispatch_wallet_funded_top_up(&ctx, &sender, &foreign, &payer_arc, 0).await; + assert!( + !matches!( + _error, + TaskError::IdentityNotWalletOwned { .. } | TaskError::IdentityIndexMismatch { .. } + ), + "the foreign top-up must reach funding, not be rejected as unowned, so this test \ + actually exercises the anti-displacement path: {_error:?}" + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/context/wallet_lifecycle/tests.rs` around lines 4519 - 4560, Update cross_wallet_top_up_never_displaces_the_paying_wallets_own_identity to retain and inspect the result from dispatch_wallet_funded_top_up, asserting it does not fail with IdentityNotWalletOwned or IdentityIndexMismatch. Keep the existing resolution assertion, ensuring the test verifies funding was reached before validating that the payer’s own identity remained intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/kv-keys.md`:
- Line 166: Update the key name in the documentation table row to escape the
pipe within the `<decline|cancel>` placeholder as `\|`, keeping the row’s
five-column Markdown structure intact.
In `@src/wallet_backend/payments.rs`:
- Around line 893-932: In resume_unbound_topup_asset_lock, provision the
seed-derived identity top-up funding account inside the with_secret_session
closure before calling wallet.asset_locks().resume_asset_lock. Reuse the same
idempotent provisioning operation and arguments used by create_asset_lock_proof,
then preserve the existing resume and key-derivation flow.
---
Nitpick comments:
In `@src/backend_task/identity/top_up_identity.rs`:
- Around line 196-236: Track the missing asset-lock consumption and recovery
behavior in the existing upstream TODO around top_up_foreign_identity, including
restoring consume_asset_lock and upgrade_to_chain_lock_proof once upstream
exposes them. For this release, leave the current implementation unchanged and
ensure the tracked issue covers removing used locks from resumable funding and
retrying rejected InstantSend proofs.
In `@src/context/wallet_lifecycle/tests.rs`:
- Around line 4519-4560: Update
cross_wallet_top_up_never_displaces_the_paying_wallets_own_identity to retain
and inspect the result from dispatch_wallet_funded_top_up, asserting it does not
fail with IdentityNotWalletOwned or IdentityIndexMismatch. Keep the existing
resolution assertion, ensuring the test verifies funding was reached before
validating that the payer’s own identity remained intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c200f746-5ba7-4819-ba31-8f7879a5a273
📒 Files selected for processing (17)
CHANGELOG.mddocs/kv-keys.mddocs/user-stories.mdsrc/backend_task/error.rssrc/backend_task/identity/mod.rssrc/backend_task/identity/top_up_identity.rssrc/backend_task/migration/finish_unwire.rssrc/context/identity_db.rssrc/context/wallet_lifecycle/mod.rssrc/context/wallet_lifecycle/registration.rssrc/context/wallet_lifecycle/spv.rssrc/context/wallet_lifecycle/tests.rssrc/database/initialization.rssrc/wallet_backend/identity_ops.rssrc/wallet_backend/mod.rssrc/wallet_backend/payments.rstests/backend-e2e/wallet_reregistration.rs
| | `det:dashpay:blocked:<base58_contact_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `()` | Presence-only flag: contact is blocked | | ||
| | `det:dashpay:declined:<base58_counterparty_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `()` | Presence-only flag: incoming contact request declined | | ||
| | `det:dashpay:withdrawn:<base58_counterparty_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `()` | Presence-only flag: outgoing contact request withdrawn | | ||
| | `det:dashpay:request_action:<decline|cancel>:<base58_request_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `ContactRequestActionPhase` | Durable recovery phase for a paid hide/corrective-unhide followed by a local marker write | |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Escape the | inside the key name to fix the broken table row.
The unescaped | in <decline|cancel> splits this row into 6 cells instead of 5, as flagged by markdownlint. Escape it as \| so the table renders correctly.
🐛 Proposed fix
-| `det:dashpay:request_action:<decline|cancel>:<base58_request_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `ContactRequestActionPhase` | Durable recovery phase for a paid hide/corrective-unhide followed by a local marker write |
+| `det:dashpay:request_action:<decline\|cancel>:<base58_request_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `ContactRequestActionPhase` | Durable recovery phase for a paid hide/corrective-unhide followed by a local marker write |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | `det:dashpay:request_action:<decline|cancel>:<base58_request_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `ContactRequestActionPhase` | Durable recovery phase for a paid hide/corrective-unhide followed by a local marker write | | |
| | `det:dashpay:request_action:<decline\|cancel>:<base58_request_id>` | `DetScope::Identity(&owner)` | `det-<net>.sqlite` | `ContactRequestActionPhase` | Durable recovery phase for a paid hide/corrective-unhide followed by a local marker write | |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 166-166: Table column count
Expected: 5; Actual: 6; Too many cells, extra data will be missing
(MD056, table-column-count)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/kv-keys.md` at line 166, Update the key name in the documentation table
row to escape the pipe within the `<decline|cancel>` placeholder as `\|`,
keeping the row’s five-column Markdown structure intact.
Source: Linters/SAST tools
| pub(crate) async fn resume_unbound_topup_asset_lock( | ||
| &self, | ||
| seed_hash: &WalletSeedHash, | ||
| out_point: dash_sdk::dpp::dashcore::OutPoint, | ||
| ) -> Result< | ||
| ( | ||
| dash_sdk::dpp::prelude::AssetLockProof, | ||
| dash_sdk::dpp::dashcore::PrivateKey, | ||
| ), | ||
| TaskError, | ||
| > { | ||
| let tracked = self | ||
| .list_tracked_asset_locks(seed_hash) | ||
| .await? | ||
| .into_iter() | ||
| .find(|lock| lock.out_point == out_point); | ||
| unbound_topup_lock_eligible( | ||
| tracked | ||
| .as_ref() | ||
| .map(|lock| (lock.funding_type, &lock.status)), | ||
| )?; | ||
|
|
||
| let scope = Self::hd_scope(seed_hash); | ||
| self.inner | ||
| .secret_access | ||
| .with_secret_session(&scope, async |session| { | ||
| let wallet = self.resolve_wallet(seed_hash).await?; | ||
| let (proof, credit_output_path) = wallet | ||
| .asset_locks() | ||
| .resume_asset_lock(&out_point, None) | ||
| .await | ||
| .map_err(|e| TaskError::WalletBackend { | ||
| source: Arc::new(e), | ||
| })?; | ||
| let private_key = | ||
| self.derive_private_key_from_held(session.plaintext(), &credit_output_path)?; | ||
| Ok((proof, private_key)) | ||
| }) | ||
| .await | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Files matching payments/identity/seam:"
fd -a 'payments\.rs|identity_ops\.rs|secret_seam\.rs|Cargo\.toml|Cargo\.lock' . | sed 's#^\./##'
echo
echo "Relevant symbols in src/wallet_backend:"
rg -n "resume_unbound_topup_asset_lock|ensure_unbound_topup_funding_account|unbound_topup_lock_eligible|create_asset_lock_proof|top_up_identity|provision_identity_funding_account|IdentityTopUpNotBound|asset_locks" src/wallet_backend -S
echo
echo "payments.rs outline:"
ast-grep outline src/wallet_backend/payments.rs --match resume_unbound_topup_asset_lock --view expanded || true
ast-grep outline src/wallet_backend/payments.rs --match create_asset_lock_proof --view expanded || true
echo
echo "payments.rs relevant lines 800-970:"
sed -n '800,970p' src/wallet_backend/payments.rs | nl -ba -v800
echo
echo "identity_ops.rs relevant lines around funding/account resolution:"
rg -n "ensure_identity_funding_accounts|FromExistingAssetLock|profile_topup|provision_identity_funding_account" src/wallet_backend -S -C 8Repository: dashpay/dash-evo-tool
Length of output: 4807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Cargo dependencies mentioning platform_wallet / dash_sdk:"
rg -n "platform_wallet|dash_sdk|KeyWallet|AssetLockManager|resume_asset_lock|create_asset_lock_proof|asset_locks" Cargo.toml Cargo.lock -S
echo
echo "Cargo.lock platform_wallet versions:"
sed -n '/name = "platform_wallet"/,/^name =/p' Cargo.lock | head -80 || trueRepository: dashpay/dash-evo-tool
Length of output: 217
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "payments.rs section 800-970"
sed -n '800,970p' src/wallet_backend/payments.rs | cat -n | sed 's/^\s*//'
echo
echo "identity_ops.rs provisioning helpers section"
sed -n '320,490p' src/wallet_backend/identity_ops.rs | cat -n | sed 's/^\s*//'
echo
echo "mod.rs provisioning references"
sed -n '630,680p' src/wallet_backend/mod.rs | cat -n | sed 's/^\s*//'
sed -n '2370,2445p' src/wallet_backend/mod.rs | cat -n | sed 's/^\s*//'
sed -n '2530,2565p' src/wallet_backend/mod.rs | cat -n | sed -s 's/^\s*//'
echo
echo "Cargo files"
sed -n '1,140p' Cargo.toml
sed -n '1,80p' Cargo.lock | cat -nRepository: dashpay/dash-evo-tool
Length of output: 31736
🌐 Web query:
Dash Platform SDK platform_wallet AssetLockManager resume_asset_lock source
💡 Result:
In the Dash Platform SDK, the AssetLockManager is a component within the platform-wallet package responsible for managing the lifecycle and orchestration of asset locks [1][2]. The resume_asset_lock operation is a functional component of the AssetLockManager designed to resume the processing or funding workflow associated with an existing asset lock [1][2][3]. Key technical aspects include: 1. Purpose: It is used in workflows like identity top-ups or shielded funding, allowing a wallet to resume an interrupted or pending asset lock process rather than starting a new one [2][3]. 2. Implementation: It is implemented within the rs-platform-wallet crate (e.g., in modules like orchestration.rs or manager.rs) [2]. The functionality is also exposed via Foreign Function Interface (FFI) to support cross-language usage in SDKs like the Swift SDK (e.g., as platform_wallet_manager_resume_fund_from_asset_lock) [2][3]. 3. Current Status and Considerations: Recent development updates (as of mid-2026) have focused on completing the AssetLockManager FFI, adding tracking accessors, and integrating SPV event subscription [1]. Technical discussions have highlighted the need for robust Platform-side re-validation when using resume operations to ensure the integrity of the asset lock state [4]. Developers should refer to the rs-platform-wallet package within the dashpay/platform repository for the specific source implementation, as it is part of the core wallet orchestration logic rather than the high-level Evo SDK facade [1][2][5].
Citations:
- 1: https://github.com/dashpay/platform/releases/tag/v3.1.0-dev.2
- 2: feat: platform-address funding from asset-lock proofs platform#3671
- 3: feat: shielded funding from asset-lock proofs platform#3753
- 4: Asset-lock resume never re-validates Platform-side consumption — stuck locks survive crash/resync and re-trigger deterministic rejection forever platform#4227
- 5: https://dashpay.github.io/platform/evo-sdk/overview.html
🌐 Web query:
github dashpay/platform AssetLockManager resume_asset_lock KeyWallet add_account identity_topup_not_bound
💡 Result:
In the Dash Platform ecosystem, the components you referenced relate to wallet management, asset lock orchestration, and identity funding. AssetLockManager is a core component within the platform wallet architecture responsible for tracking, orchestrating, and resuming asset lock operations [1][2]. It manages the lifecycle of asset locks, including subscribing to SPV (Simplified Payment Verification) events, handling changesets for tracked asset locks, and coordinating retry logic for state transitions that require L1 (Core) locking transactions [1][2][3]. The resume_asset_lock operation allows the wallet to continue a previously interrupted top-up or funding process, often by resolving funding through ChainLock (CL) or Instant Send (IS) fallback mechanisms [2][3]. KeyWallet refers to a component in the rust-dashcore library that manages wallet accounts, including the recent addition of PlatformPayment account support [4][5]. The add_account function is used to insert new managed accounts into the wallet's collection [6]. The mention of identity_topup_not_bound appears in the context of the wallet's state management (specifically within clear or cleanup methods), where it represents an unbound identity top-up record that hasn't been associated with a specific identity or successfully processed [4]. In summary: - AssetLockManager handles the technical orchestration of L1-to-Platform funding [2][3]. - resume_asset_lock enables recovery of these funding transactions [2]. - KeyWallet manages the account structures, including those for platform payments [4][5]. - identity_topup_not_bound is a state tracking field for top-up operations within the wallet that are not yet bound to an identity [4].
Citations:
- 1: https://github.com/dashpay/platform/releases/tag/v3.1.0-dev.2
- 2: feat: platform-address funding from asset-lock proofs platform#3671
- 3: feat: shielded funding from asset-lock proofs platform#3753
- 4: feat(key-wallet): Add DIP-17 Platform Payment account support rust-dashcore#229
- 5: feat: new managed platform account to track platform balances rust-dashcore#368
- 6: feat: improve key wallet rust-dashcore#112
🌐 Web query:
site:github.com/dashpay/platform AssetLockManager resume_asset_lock "funding_type"
💡 Result:
The AssetLockManager provides functionality for managing the lifecycle of asset locks within the Dash Platform, including mechanisms for resuming interrupted funding operations [1]. The specific operation resume_asset_lock (often exposed via Foreign Function Interface as platform_address_wallet_resume_top_up_with_existing_asset_lock_signer or similar variants like platform_wallet_manager_shielded_resume_fund_from_asset_lock) is designed to continue a funding process that was previously initiated but not completed [2][3]. Regarding funding_type (often represented by the AssetLockFundingType enum), it is a critical parameter used to determine how the asset lock is derived and processed [3][4]. When resuming a funding operation, it is essential that the funding_type matches the type used during the initial creation of the asset lock [3]. Key points regarding this process include: - Consistency: If the funding_type is incorrectly specified during a resume operation, the wallet may attempt to derive addresses from the wrong account, leading to failed state transitions or rejections [3]. - Implementation details: Funding types distinguish between different sources and methodologies, such as AssetLockAddressTopUp (Type 14) and AssetLockShieldedAddressTopUp (Type 18) [3]. - Error Handling: Resuming operations have historically faced challenges, such as failures to properly re-validate Platform-side consumption or silent failure handling, which developers should monitor for in current implementations [5][6]. For technical implementation, developers typically interact with these managers via language-specific SDKs (such as the Swift SDK) which marshal arguments to the underlying Rust AssetLockManager FFI entry points [2][3].
Citations:
- 1: https://github.com/dashpay/platform/releases/tag/v3.1.0-dev.2
- 2: feat: platform-address funding from asset-lock proofs platform#3671
- 3: feat: shielded funding from asset-lock proofs platform#3753
- 4: feat(platform-wallet): serde support platform#3637
- 5: Asset-lock resume never re-validates Platform-side consumption — stuck locks survive crash/resync and re-trigger deterministic rejection forever platform#4227
- 6: Found-013:
recover_asset_lock_blockingswallows every failure — persist errors leave a memory-only tracked lock, missing wallet returns silently platform#4028
🌐 Web query:
site:github.com/dashpay/platform resume_asset_lock "pub async"
💡 Result:
The term "resume_asset_lock" refers to an asynchronous function used within the Dash Platform codebase (specifically in the rs-platform-wallet library) to resume wallet-related operations involving asset locks [1][2][3]. In the context of the Dash Platform, asset locks are mechanisms used to lock Dash output on-chain to mint credits or fund identities [4]. Because these processes can span multiple states (e.g., Built, Broadcast, InstantSendLocked, ChainLocked), the platform provides functionality to resume interrupted processes [1][5]. Key details regarding "resume_asset_lock" and its related operations include: - Functionality: It is an async function that allows the wallet to resume funding or top-up operations using existing asset locks [1][2][3]. It operates by tracking the status of asset locks and retrying or continuing the process from the locally-tracked state [6][5]. - Rust Implementation: The operation is implemented in Rust, often utilizing the async/await pattern to handle network and consensus-dependent operations without blocking execution [1]. - FFI and SDK Usage: The functionality is exposed to higher-level languages (like Swift) via Foreign Function Interface (FFI) wrappers [2][3]. For example, there are specific FFI functions such as platform_address_wallet_resume_top_up_with_existing_asset_lock_signer and platform_wallet_manager_shielded_resume_fund_from_asset_lock, which are then exposed as async methods in the Dash Swift SDK [2][3]. - Recent Updates: As of May 2026, the Dash Platform v3.1.0-dev.2 release included significant updates to these systems, including the addition of resumable asset lock operations and improvements to the underlying async architecture [1]. It is important to note that the resume functionality is generally designed to proceed based on locally tracked status and may not always re-validate the current state against the platform in real-time, which is a known area of technical discussion regarding the consistency of these operations [5].
Citations:
- 1: https://github.com/dashpay/platform/releases/tag/v3.1.0-dev.2
- 2: feat: platform-address funding from asset-lock proofs platform#3671
- 3: feat: shielded funding from asset-lock proofs platform#3753
- 4: https://github.com/dashpay/platform/blob/v2.0.1/packages/rs-dpp/src/identity/state_transition/asset_lock_proof/instant/instant_asset_lock_proof.rs
- 5: Asset-lock resume never re-validates Platform-side consumption — stuck locks survive crash/resync and re-trigger deterministic rejection forever platform#4227
- 6: Asset-lock ChainLock finality wait has no per-record promotion and no cancellation — stuck locks unrecoverable, app shutdown hangs platform#4238
🌐 Web query:
site:github.com/dashpay/platform "resume_asset_lock"
💡 Result:
In the context of the Dash Platform ecosystem, resume_asset_lock refers to a functional mechanism within the platform-wallet package designed to continue or recover the lifecycle of an asset lock transaction that has been previously initiated but did not complete successfully [1][2][3]. Key technical aspects include: Purpose and Functionality: It enables the wallet to resume the funding process for an identity registration or top-up using an existing, previously broadcasted asset lock transaction, rather than creating and broadcasting a new one [1][2][4]. This is particularly useful in scenarios where a transaction times out waiting for consensus, such as InstantSend (IS) or ChainLock (CL) finality [3][4]. Implementation: - It is part of the platform-wallet orchestration logic [1][3]. - It is exposed through Foreign Function Interface (FFI) bindings (e.g., as part of platform-wallet-ffi) and wrapped for higher-level languages like Swift (e.g., resumeTopUpFromAssetLock or shieldedResumeFundFromAssetLock) [2][3]. - It leverages the TrackedAssetLock mechanism to identify and manage the state of these locks [1][4]. Contextual Considerations: - Error Handling and Retries: While it is a critical component for robust wallet operation, developers have noted that certain implementations, such as the finality timeout retry path, have historically required careful wiring to ensure the original lock is resumed rather than ignored in favor of new locks [4]. - Security and Validation: Ongoing development and issue tracking have highlighted the importance of platform-side re-validation when resuming asset locks to ensure integrity [5][6]. This functionality is central to ensuring that user funds are not unnecessarily locked or orphaned due to transient network or consensus timing issues [4].
Citations:
- 1: https://github.com/dashpay/platform/releases/tag/v3.1.0-dev.2
- 2: feat: platform-address funding from asset-lock proofs platform#3671
- 3: feat: shielded funding from asset-lock proofs platform#3753
- 4: feat(swift-sdk): seed shielded pool notes from the example app platform#3858
- 5: Asset-lock resume never re-validates Platform-side consumption — stuck locks survive crash/resync and re-trigger deterministic rejection forever platform#4227
- 6: Asset-lock ChainLock finality wait has no per-record promotion and no cancellation — stuck locks unrecoverable, app shutdown hangs platform#4238
Provision the unbound top-up funding account before resuming.
resume_asset_lock(&out_point, None) needs the same seed-derived funding account that create_asset_lock_proof registers. A cold reload leaves identity_topup_not_bound absent, because identity-funding re-provision is deferred there.
Add the idempotent provisioning call inside the held-seed session, matching create_asset_lock_proof.
🐛 Proposed fix
let scope = Self::hd_scope(seed_hash);
self.inner
.secret_access
.with_secret_session(&scope, async |session| {
+ let plaintext = session.plaintext();
+ let seed = plaintext
+ .expose_hd_seed()
+ .ok_or(TaskError::WalletStateInconsistent)?;
+ self.ensure_unbound_topup_funding_account(seed_hash, seed)
+ .await?;
let wallet = self.resolve_wallet(seed_hash).await?;
let (proof, credit_output_path) = wallet
.asset_locks()
.resume_asset_lock(&out_point, None)
.await
.map_err(|e| TaskError::WalletBackend {
source: Arc::new(e),
})?;
let private_key =
self.derive_private_key_from_held(session.plaintext(), &credit_output_path)?;
Ok((proof, private_key))
})
.await📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub(crate) async fn resume_unbound_topup_asset_lock( | |
| &self, | |
| seed_hash: &WalletSeedHash, | |
| out_point: dash_sdk::dpp::dashcore::OutPoint, | |
| ) -> Result< | |
| ( | |
| dash_sdk::dpp::prelude::AssetLockProof, | |
| dash_sdk::dpp::dashcore::PrivateKey, | |
| ), | |
| TaskError, | |
| > { | |
| let tracked = self | |
| .list_tracked_asset_locks(seed_hash) | |
| .await? | |
| .into_iter() | |
| .find(|lock| lock.out_point == out_point); | |
| unbound_topup_lock_eligible( | |
| tracked | |
| .as_ref() | |
| .map(|lock| (lock.funding_type, &lock.status)), | |
| )?; | |
| let scope = Self::hd_scope(seed_hash); | |
| self.inner | |
| .secret_access | |
| .with_secret_session(&scope, async |session| { | |
| let wallet = self.resolve_wallet(seed_hash).await?; | |
| let (proof, credit_output_path) = wallet | |
| .asset_locks() | |
| .resume_asset_lock(&out_point, None) | |
| .await | |
| .map_err(|e| TaskError::WalletBackend { | |
| source: Arc::new(e), | |
| })?; | |
| let private_key = | |
| self.derive_private_key_from_held(session.plaintext(), &credit_output_path)?; | |
| Ok((proof, private_key)) | |
| }) | |
| .await | |
| } | |
| pub(crate) async fn resume_unbound_topup_asset_lock( | |
| &self, | |
| seed_hash: &WalletSeedHash, | |
| out_point: dash_sdk::dpp::dashcore::OutPoint, | |
| ) -> Result< | |
| ( | |
| dash_sdk::dpp::prelude::AssetLockProof, | |
| dash_sdk::dpp::dashcore::PrivateKey, | |
| ), | |
| TaskError, | |
| > { | |
| let tracked = self | |
| .list_tracked_asset_locks(seed_hash) | |
| .await? | |
| .into_iter() | |
| .find(|lock| lock.out_point == out_point); | |
| unbound_topup_lock_eligible( | |
| tracked | |
| .as_ref() | |
| .map(|lock| (lock.funding_type, &lock.status)), | |
| )?; | |
| let scope = Self::hd_scope(seed_hash); | |
| self.inner | |
| .secret_access | |
| .with_secret_session(&scope, async |session| { | |
| let plaintext = session.plaintext(); | |
| let seed = plaintext | |
| .expose_hd_seed() | |
| .ok_or(TaskError::WalletStateInconsistent)?; | |
| self.ensure_unbound_topup_funding_account(seed_hash, seed) | |
| .await?; | |
| let wallet = self.resolve_wallet(seed_hash).await?; | |
| let (proof, credit_output_path) = wallet | |
| .asset_locks() | |
| .resume_asset_lock(&out_point, None) | |
| .await | |
| .map_err(|e| TaskError::WalletBackend { | |
| source: Arc::new(e), | |
| })?; | |
| let private_key = | |
| self.derive_private_key_from_held(session.plaintext(), &credit_output_path)?; | |
| Ok((proof, private_key)) | |
| }) | |
| .await | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/wallet_backend/payments.rs` around lines 893 - 932, In
resume_unbound_topup_asset_lock, provision the seed-derived identity top-up
funding account inside the with_secret_session closure before calling
wallet.asset_locks().resume_asset_lock. Reuse the same idempotent provisioning
operation and arguments used by create_asset_lock_proof, then preserve the
existing resume and key-derivation flow.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The cross-wallet routing fix prevents the targeted identity-manager corruption, but the database relocation is not upgrade-safe. Released builds store durable wallet and shielded state at the old SPV-directory paths, while this head opens fresh replacement databases without migrating that state; one new provisioning test also never exercises the branch it claims to cover.
Source: reviewers codex/general=gpt-5.6-sol(completed); codex/rust-quality=gpt-5.6-sol(completed); verifier=codex/verifier=gpt-5.6-sol(completed); coordinator=openclaw-agent/cliproxy/gpt-5.6-sol(orchestration-only).
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/wallet_backend/mod.rs`:
- [BLOCKING] src/wallet_backend/mod.rs:481-524: Migrate the released databases before opening the new paths
The released `v1.0.0-weekly.20260731` code opens `<data_dir>/spv/<network>/platform-wallet.sqlite` and configures `<data_dir>/spv/<network>/platform-wallet-shielded.sqlite`. This head instead opens `<data_dir>/det-<network>.sqlite` and `<data_dir>/det-<network>-shielded.sqlite`, but no code checks or migrates the released locations. `SqlitePersister::open` creates the missing destination, so an ordinary upgrade loads an empty persister and makes the old wallets, identities, keys, address state, UTXOs, tracked asset locks, contacts, scheduled votes, contracts, tokens, and other per-network `DetKv` data inaccessible. The shielded coordinator similarly loses access to its previous state. Migrate both databases before either destination is opened, make the operation idempotent and crash-safe, preserve SQLite WAL/SHM state, define safe handling when both paths exist, and add an upgrade test beginning with populated databases in the released layout.
In `src/context/wallet_lifecycle/tests.rs`:
- [SUGGESTION] src/context/wallet_lifecycle/tests.rs:4345-4355: Exercise the missing-account provisioning branch
`register_test_wallet` registers the upstream wallet with `WalletAccountCreationOptions::Default`. In the pinned key-wallet revision, the default special-purpose accounts include `IdentityTopUpNotBoundToIdentity`, and `ManagedWalletInfo::from_wallet` copies that account into the managed collection. Both membership probes are therefore already true before the first `ensure_unbound_topup_funding_account` call, so both calls return through the early no-op path. The test still passes if the new derivation and insertion branches are broken. Build a watch-only fixture whose account manifest deliberately omits this account, then verify that provisioning restores it in both the key-wallet and managed collections.
| @@ -495,12 +516,12 @@ impl WalletBackend { | |||
|
|
|||
| // Wire the upstream shielded coordinator into the manager. | |||
| // | |||
| // Uses a dedicated SQLite file (`platform-wallet-shielded.sqlite`) owned | |||
| // Uses a dedicated SQLite file (`det-<network>-shielded.sqlite`) owned | |||
| // entirely by the upstream coordinator — the single source of truth for | |||
| // all Orchard state. The coordinator starts empty — no wallets are bound | |||
| // until `ensure_shielded_bound` runs (on wallet unlock). Subsequent | |||
| // calls with the same path are idempotent (upstream no-ops). | |||
| pwm.configure_shielded(spv_storage_dir.join("platform-wallet-shielded.sqlite")) | |||
| pwm.configure_shielded(shielded_database_path(ctx.data_dir(), network)) | |||
There was a problem hiding this comment.
🔴 Blocking: Migrate the released databases before opening the new paths
The released v1.0.0-weekly.20260731 code opens <data_dir>/spv/<network>/platform-wallet.sqlite and configures <data_dir>/spv/<network>/platform-wallet-shielded.sqlite. This head instead opens <data_dir>/det-<network>.sqlite and <data_dir>/det-<network>-shielded.sqlite, but no code checks or migrates the released locations. SqlitePersister::open creates the missing destination, so an ordinary upgrade loads an empty persister and makes the old wallets, identities, keys, address state, UTXOs, tracked asset locks, contacts, scheduled votes, contracts, tokens, and other per-network DetKv data inaccessible. The shielded coordinator similarly loses access to its previous state. Migrate both databases before either destination is opened, make the operation idempotent and crash-safe, preserve SQLite WAL/SHM state, define safe handling when both paths exist, and add an upgrade test beginning with populated databases in the released layout.
source: ['codex']
| let seed = [0xAEu8; 64]; | ||
| let (seed_hash, _wallet_arc) = register_test_wallet(&ctx, &backend, seed, "payer").await; | ||
|
|
||
| backend | ||
| .ensure_unbound_topup_funding_account(&seed_hash, &seed) | ||
| .await | ||
| .expect("the index-less top-up account must provision on a watch-only wallet"); | ||
| backend | ||
| .ensure_unbound_topup_funding_account(&seed_hash, &seed) | ||
| .await | ||
| .expect("the second call must be a no-op, proving both collections hold the account"); |
There was a problem hiding this comment.
🟡 Suggestion: Exercise the missing-account provisioning branch
register_test_wallet registers the upstream wallet with WalletAccountCreationOptions::Default. In the pinned key-wallet revision, the default special-purpose accounts include IdentityTopUpNotBoundToIdentity, and ManagedWalletInfo::from_wallet copies that account into the managed collection. Both membership probes are therefore already true before the first ensure_unbound_topup_funding_account call, so both calls return through the early no-op path. The test still passes if the new derivation and insertion branches are broken. Build a watch-only fixture whose account manifest deliberately omits this account, then verify that provisioning restores it in both the key-wallet and managed collections.
source: ['codex']
3 participants
TL;DR
Two independent wallet-reliability fixes, split out of PR #901 into their own PR:
User story
As a Dash Evo Tool user with more than one wallet or identity on this device, I want my wallet, identity, and key data to stay safe from cache-clearing and from paying for someone else's identity, so a routine action never bricks a wallet or loses track of my funds.
Scenario
Actual behavior
Expected behavior
Detailed discussion
Commit 1 — wallet-database isolation
platform-wallet.sqlite/platform-wallet-shielded.sqlitemoved from<data_dir>/spv/<network>/(shared with the disposabledash-spvchain cache) to<data_dir>/det-<network>.sqlite/det-<network>-shielded.sqlite, siblings ofdet-app.sqlite. Thespv/<network>/directory now holds only genuinely disposable chain-cache data. No migration — this changes where new/future opens look; an existing install's old-location file is not moved automatically.Commit 2 — cross-wallet top-up corruption + funds-misdirection fix
Root cause:
WalletBackend::top_up_identityunconditionally force-registered the target identity into the paying wallet's upstreamIdentityManagerbefore funding. For an identity actually owned by a different wallet, theidentitiestable write is correctly refused by its own cross-wallet ownership guard, but the accompanyingidentity_keysrows land under the payer'swallet_idanyway — on the next load, the true owner's wallet-scoped reader finds those keys orphaned and fails the entire wallet load fatally. Separately (found during this investigation, not the original report):IdentityManager::add_identityoverwrites whatever the payer's own wallet already had at that HD index, so a later top-up of the payer's own identity at the same index could silently submit for the wrong identity.Funding an identity you don't own is a deliberate, existing feature (
top_up_identity_screen,send_screen) and had to keep working. Fix routes on the identity's real recorded owner (StoredQualifiedIdentity.wallet_hash) — notQualifiedIdentity.associated_wallets, which turned out to be populated with every currently-loaded wallet regardless of actual ownership, so it can't be used as a guard. An owned identity still takes the existing orchestrated path; a foreign identity now funds through a manager-free path that never touches the paying wallet's identity manager.Known gaps, tracked separately, not blocking this PR's correctness for the owned-identity path:
dash-evo-toolTODO7e1a5fa8): a real foreign top-up end-to-end and theUseAssetLockresume variant are untested on-network; offline coverage stops at UTXO selection.pub(crate)(memcanplatformTODOe39c9d4a) — until exposed, a foreign top-up has no IS→CL retry, and a spent lock used for one can linger in the resumable-funding list.Testing
cargo fmt --all— clean.cargo clippy --all-features --all-targets -- -D warnings— clean, independently re-verified on this branch after the rebase offv1.0-dev.wallet_databases_live_outside_the_disposable_spv_cache,cross_wallet_top_up_leaves_the_paying_wallets_identity_state_intact,cross_wallet_top_up_never_displaces_the_paying_wallets_own_identity,resolve_top_up_route_*(×3),reconcile_managed_identities_skips_identities_linked_to_another_wallet,unbound_topup_funding_account_provisions_on_the_watch_only_wallet,only_an_index_less_lock_can_fund_a_foreign_identity_top_up,an_already_spent_lock_is_refused_before_submission, plus updated fixtures acrosswallet_lifecycle::tests— all independently re-run and confirmed passing by name, not just aggregate count.Breaking changes
None for the owned-identity path (unchanged). The cross-wallet top-up path now succeeds via a different internal route; behavior from the user's perspective (identity gets funded) is unchanged pending live-network confirmation.
Prior work
Split out of #901 (DPNS voting redesign) — unrelated in scope, bundled there only because both were being worked in the same session. Originates from investigating a real user-reported "Saved wallet data appears damaged" incident; see memcan
dash-evo-toolTODOs17420d0e(this PR, done) and7e1a5fa8(live-network follow-up, open), andplatformTODOs4fbe235c,50cdd209,e39c9d4afor related upstream items.🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
New Features
Bug Fixes
Documentation