Skip to content

refactor(wallet): unify all asset lock paths and add transaction recovery #799

@lklimek

Description

@lklimek

Summary

All asset lock creation flows should use a single shared code path with deterministic HD-derived keys and built-in recovery for broken/rejected transactions. Currently there are multiple independent implementations with inconsistent error handling and key management.

Background

PR #790 (fix/deterministic-asset-lock-keys) introduced deterministic HD key derivation for generic asset lock transactions, replacing OsRng-generated keys that caused permanent fund loss when Platform rejected a state transition after Core had already broadcast the asset lock. It also introduced broadcast_and_commit_asset_lock() as a shared helper for the broadcast → track → UTXO cleanup sequence.

However, several asset lock flows still have their own inline implementations, and recovery from broken transactions is incomplete.

Problem

1. Multiple divergent asset lock paths

The codebase has at least 4 separate asset lock creation flows:

Flow File Key derivation Broadcast path Recovery
Identity registration create_asset_lock.rs registration_asset_lock_transaction() — HD-derived ✅ broadcast_and_commit_asset_lock() Partial (Search for Asset Locks)
Identity top-up create_asset_lock.rs top_up_asset_lock_transaction() — HD-derived ✅ broadcast_and_commit_asset_lock() Partial
Fund platform address fund_platform_address_from_wallet_utxos.rs generic_asset_lock_transaction() — HD-derived ✅ (PR #790) Inline broadcast + manual UTXO cleanup ❌ None
Shield from asset lock shielded/bundle.rs (shield_from_asset_lock) generic_asset_lock_transaction() — HD-derived ✅ (PR #790) Inline broadcast + manual UTXO cleanup ❌ None

2. Inconsistent broadcast + commit sequence

broadcast_and_commit_asset_lock() (introduced in PR #790 for identity flows) handles:

  • Register tx in transactions_waiting_for_finality
  • Broadcast raw transaction
  • Remove used UTXOs from wallet
  • Persist UTXO removal to DB
  • Recalculate affected address balances
  • Clean up finality tracking on failure

The platform address funding and shielded flows do this sequence inline with subtle differences:

  • fund_platform_address_from_wallet_utxos.rs does UTXO cleanup but doesn't register in transactions_waiting_for_finality before broadcast
  • shield_from_asset_lock in bundle.rs does register in finality tracking but has its own error recovery

3. No transaction recovery mechanism

When a transaction fails after broadcast (Platform rejects, timeout, nonce mismatch):

  • The Core transaction is already confirmed on L1
  • The funds are locked in the asset lock output
  • With PR fix(wallet): use deterministic HD key for generic asset lock transactions #790's deterministic keys, the funds are theoretically recoverable (key can be re-derived)
  • But there's no UI or backend flow to actually recover — the user must manually "Search for Asset Locks" and hope the system finds them

Proposed Solution

Phase 1: Unify all asset lock paths through broadcast_and_commit_asset_lock()

Refactor fund_platform_address_from_wallet_utxos.rs and shielded/bundle.rs to use broadcast_and_commit_asset_lock() instead of inline broadcast + UTXO cleanup. This ensures:

  • Consistent transactions_waiting_for_finality tracking
  • Consistent UTXO cleanup and balance recalculation
  • Consistent error handling and cleanup on broadcast failure

Phase 2: Add asset lock recovery flow

When a Platform state transition fails after the asset lock is broadcast:

  1. Persist the failed asset lock — save (txid, derivation_path, amount, intended_purpose) to a failed_asset_locks DB table
  2. Show recovery UI — on the wallet screen, show a banner or section listing failed asset locks with "Retry" and "Recover to wallet" actions
  3. Retry: re-derive the key from the wallet seed, wait for the asset lock proof (if not already available), and retry the original state transition
  4. Recover to wallet: create a transaction spending the asset lock output back to the wallet using the re-derived key

Phase 3: Unify key derivation paths (DIP-9 compliance)

PR #790 noted TODO items about DIP-9 compliance:

  • Identity registration currently uses m/9'/ct'/5'/0'/index' but DIP-9 assigns sub-feature 1'
  • Identity top-up currently uses m/9'/ct'/5'/identity_index'/top_up_index' but DIP-9 assigns sub-feature 2'
  • Changing these paths would break existing wallets that already derived keys on the old paths

This needs a migration strategy:

  • Scan both old and new paths during recovery
  • Gradually migrate new wallets to the DIP-9 compliant paths
  • Keep old path scanning for backward compatibility

Acceptance Criteria

  • All 4 asset lock flows use broadcast_and_commit_asset_lock() (or equivalent shared helper)
  • Failed asset locks are persisted to DB with enough info to retry or recover
  • UI shows failed/pending asset locks with retry/recover actions
  • Recovery flow re-derives keys from wallet seed and retries the operation
  • "Recover to wallet" creates a spend-back transaction for unrecoverable asset locks
  • All asset lock keys are deterministically derived (no OsRng) — already done in PR fix(wallet): use deterministic HD key for generic asset lock transactions #790
  • DIP-9 path compliance plan documented (migration strategy for existing wallets)

Related

🤖 Co-authored by Claudius the Magnificent AI Agent

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions