Skip to content

Let callers specify birth_height on wallet add/import paths #688

@xdustinface

Description

@xdustinface

In key-wallet-manager, four wallet-creation/import entry points silently fall back to self.last_processed_height when setting the new wallet's birth height:

  • WalletManager::create_wallet_with_random_mnemonickey-wallet-manager/src/lib.rs:307
  • WalletManager::import_wallet_from_extended_priv_keykey-wallet-manager/src/lib.rs:348
  • WalletManager::import_wallet_from_xpubkey-wallet-manager/src/lib.rs:396
  • WalletManager::import_wallet_from_byteskey-wallet-manager/src/lib.rs:441

last_processed_height is the wrong default in every one of these cases:

  • Random mnemonic — a freshly generated mnemonic cannot have any prior on-chain history, so the correct birth height is the current chain tip, not the last processed height (which can lag behind the tip while the manager is catching up). Scanning from last_processed_height to tip is wasted work.
  • xprv / xpub import — the manager has no way to know when the key was created. Using last_processed_height silently drops any history that exists before that height, which is a correctness issue for users importing an older wallet.
  • Bytes import — the current bincode::encode_to_vec(&final_wallet, ...) call in create_wallet_from_mnemonic_return_serialized_bytes (lib.rs:265) only serializes Wallet, not ManagedWalletInfo. On the round-trip the birth_height is lost entirely and the code falls back to last_processed_height, which again silently truncates history.

By contrast, create_wallet_from_mnemonic and create_wallet_from_mnemonic_return_serialized_bytes already accept an explicit birth_height parameter, which is the right shape.

Proposed change

1. Add birth_height: CoreBlockHeight to the three "we can't know" entry points

  • create_wallet_with_random_mnemonic(birth_height, ...) — caller (typically an SPV client) passes the current chain tip.
  • import_wallet_from_extended_priv_key(xprv, birth_height, ...) — caller passes the known creation height, or 0 to rescan from genesis.
  • import_wallet_from_xpub(xpub, birth_height, ...) — same as above.

This matches the existing create_wallet_from_mnemonic signature and makes the intent explicit at the call site.

2. Preserve birth_height through the bytes round-trip

Rather than adding birth_height as a parameter to import_wallet_from_bytes, fix the serialization so it round-trips correctly. The Wallet blob currently carries only immutable identity. The mutable state that matters for resuming scanning (birth_height, first_loaded_at, metadata) lives on ManagedWalletInfo and is silently discarded.

Option A (preferred): serialize a bundle.

  • Change create_wallet_from_mnemonic_return_serialized_bytes to serialize (Wallet, ManagedWalletInfo) (or a dedicated SerializableWallet struct that owns both).
  • Change import_wallet_from_bytes to decode the same shape and install the managed info as-is, without calling set_birth_height/set_first_loaded_at.
  • This also preserves any other managed state the user expects to survive export/import (address pool cursors, transaction history, etc.) which today is silently reset to defaults.

Option B (narrower): keep serializing only Wallet but additionally embed birth_height and first_loaded_at in the serialized payload.

Option A is the right long-term shape because the bytes API clearly wants "export a wallet I can restore later"; today it exports a wallet shell with a lot of state reset to defaults. But it's a bigger change, so Option B is a reasonable intermediate step if we want to ship in stages.

Affected FFI

Changing the Rust signatures ripples into the FFI layer:

  • key-wallet-ffi/src/wallet_manager.rswallet_manager_import_wallet_from_bytes (lib.rs:311) and any corresponding wallet_manager_create_random / wallet_manager_import_* entry points need the new parameter (or, for the bytes case, the updated payload format).
  • FFI tests under key-wallet-ffi/tests/ and key-wallet-ffi/src/wallet_manager_*_tests.rs need updating.
  • Downstream Swift/dash-spv-ffi consumers need to pass the new argument.

Out of scope

  • Changing create_wallet_from_mnemonic / create_wallet_from_mnemonic_return_serialized_bytes — they already take birth_height.
  • Repurposing last_processed_height or the wallet manager's tip tracking — this issue is strictly about who owns the birth-height decision at wallet creation.

Acceptance criteria

  • create_wallet_with_random_mnemonic, import_wallet_from_extended_priv_key, and import_wallet_from_xpub accept an explicit birth_height.
  • No remaining set_birth_height(self.last_processed_height) calls in key-wallet-manager/src/lib.rs.
  • import_wallet_from_bytes preserves birth_height (and ideally the full ManagedWalletInfo) from the serialized payload, with no fallback to last_processed_height.
  • FFI signatures updated and FFI tests green.
  • Existing Rust tests updated to pass the new argument.

Metadata

Metadata

Assignees

No one assigned

    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