feat: add multi-address type support for on-chain wallet

[ben-kaufman]

Multi-Address Type Support for On-Chain Wallet

Summary

Adds comprehensive multi-address type support, allowing a single LDK Node to manage multiple Bitcoin address types (Legacy, NestedSegwit, NativeSegwit, Taproot) simultaneously:

• New AddressType enum with Legacy, NestedSegwit, NativeSegwit, and Taproot variants
• Configurable primary address type via Config.address_type for new addresses and change outputs
• Monitor additional address types via Config.address_types_to_monitor for migration scenarios
• Per-type balance queries with Node::get_balance_for_address_type()
• Address generation for any monitored type with OnchainPayment::new_address_for_type()
• Unified UTXO selection across all address types for transactions
• Cross-type transaction signing using appropriate signature schemes per input
• Parallel wallet sync for all monitored types on Esplora, Electrum, and Bitcoind backends
• Per-wallet balance events emitted during sync for each address type
• Transaction persistence per address type with automatic key derivation
• RBF and CPFP work seamlessly for transactions spanning multiple address types

Motivation

Many Bitcoin wallets need to support multiple address types simultaneously:

• Migration scenarios: Users upgrading from Legacy to SegWit addresses still need to receive funds sent to old addresses
• Compatibility: Some services only support specific address types
• Lightning requirements: Channel funding requires SegWit addresses, but users may have a Legacy primary wallet

API Changes

Configuration

New configuration options in Config:

• address_type: AddressType - Primary address type for new addresses and change outputs (default: NativeSegwit)
• address_types_to_monitor: Vec<AddressType> - Additional address types to monitor (default: empty)

let mut config = default_config();
config.address_type = AddressType::NativeSegwit;  // Primary
config.address_types_to_monitor = vec![AddressType::Legacy, AddressType::Taproot];

Builder Methods

• Builder::set_address_type(AddressType) - Set the primary address type
• Builder::set_address_types_to_monitor(Vec<AddressType>) - Set additional types to monitor

Node Methods

• Node::get_balance_for_address_type(AddressType) - Get balance for a specific address type
• Node::list_monitored_address_types() - List all monitored address types (primary + monitored)

OnchainPayment Methods

• OnchainPayment::new_address_for_type(AddressType) - Get address for specific type

New Types

• AddressType enum: Legacy, NestedSegwit, NativeSegwit, Taproot
• AddressTypeBalance / OnchainBalanceForType - Balance info for a specific address type

Implementation Details

Multi-Wallet Architecture

Internally manages multiple BDK wallet instances (one per address type) with unified coordination:

• Aggregated balance reporting across all wallets
• Cross-wallet UTXO selection for transactions
• Multi-wallet PSBT signing with correct signature schemes per input type
• Parallel sync across all wallets

┌─────────────────────────────────────────┐
│              LDK Node Wallet            │
├─────────────────────────────────────────┤
│  ┌─────────┐ ┌─────────┐ ┌─────────┐   │
│  │ Legacy  │ │SegWit   │ │Taproot  │   │
│  │ BDK     │ │ BDK     │ │ BDK     │   │
│  │ Wallet  │ │ Wallet  │ │ Wallet  │   │
│  └─────────┘ └─────────┘ └─────────┘   │
│         │         │           │         │
│         └─────────┼───────────┘         │
│                   ▼                     │
│        Unified UTXO Selection           │
│        Cross-Wallet Signing             │
│        Aggregated Balances              │
└─────────────────────────────────────────┘

Chain Source Updates

All chain backends (Esplora, Electrum, Bitcoind) updated to:

• Sync all monitored wallets in parallel using async task sets
• Emit balance change events per wallet type
• Support transaction lookup across all wallets
• Handle wallet-specific sync errors gracefully

Wallet Persistence

• Each address type uses separate storage keys with type-specific prefixes
• Automatic BIP-44/49/84/86 derivation path selection based on address type
• Existing single-wallet data remains compatible

Test Plan

• test_multi_wallet_setup - Basic multi-wallet configuration
• test_multi_wallet_balance_aggregation - Balance aggregation across types
• test_multi_wallet_get_address_balance - Per-type balance queries
• test_cross_wallet_spending - Spending from multiple wallet types
• test_cross_wallet_spending_three_types - Three wallet types in one tx
• test_multi_wallet_send_operation - Send with multi-wallet config
• test_multi_wallet_send_all - Send all with aggregated UTXOs
• test_send_all_drains_all_wallets - Verify all wallets are drained
• test_multi_wallet_utxo_selection - UTXO selection across types
• test_rbf_single_wallet_input_with_multi_wallet_config - Single-wallet RBF
• test_rbf_cross_wallet_transaction - Cross-wallet RBF
• test_rbf_additional_inputs_fee_rate_correctness - Fee rate with added inputs
• test_cpfp_for_cross_wallet_transaction - Cross-wallet CPFP
• test_multi_wallet_persistence_across_restart - Persistence verification
• test_new_address_for_type - Address generation per type
• test_new_address_for_unmonitored_type - Error for unmonitored types
• test_all_address_types_as_primary - Each type as primary
• test_sync_updates_all_wallet_balances - Sync updates all types
• 31+ total multi-wallet tests

Breaking Changes

None. The default configuration (address_types_to_monitor = []) maintains existing single-wallet behavior.

Files Changed

File	Changes
src/config.rs	New AddressType enum and config options
src/builder.rs	Multi-wallet initialization, descriptor generation per type
src/wallet/mod.rs	Multi-wallet coordination, unified UTXO selection, cross-wallet signing
src/wallet/persist.rs	Per-type storage keys
src/chain/mod.rs	Multi-wallet sync coordination
src/chain/esplora.rs	Parallel sync for multiple wallets
src/chain/electrum.rs	Parallel sync for multiple wallets
src/chain/bitcoind.rs	Multi-wallet support
src/payment/onchain.rs	new_address_for_type()
src/lib.rs	New public APIs
src/balance.rs	AddressTypeBalance struct
src/io/utils.rs	Per-type changeset serialization
bindings/ldk_node.udl	FFI interface updates
tests/multi_wallet_tests.rs	Comprehensive test suite (1800+ lines)

[ovitrif]

acked the concept & description

discussed with Spiral and with Ben in private

next step: Ben is exploring a more "fork-friendly" alternative

[ovitrif]

Caution

Sharing here some interesting concepts that I didn't double-check but if they're not slop we might have to address this in the new implementation.

---

Review Summary

PR adds multi-address type support (Legacy/NestedSegwit/NativeSegwit/Taproot) via multiple BDK wallet instances per node. +6296/-975 lines across 32 files. The architecture is fundamentally sound but has several critical issues that must be addressed before merge.

---

CRITICAL ISSUES (Must Fix)

1. Backwards-Incompatible Persistence Namespace Change (Data Loss Risk)

Files: src/io/utils.rs:430-493

The persistence macro now appends address type suffixes to secondary namespaces:

• Old: KVStoreSync::read(store, primary_ns, secondary_ns, key)
• New: KVStoreSync::read(store, primary_ns, "secondary_ns/native_segwit", key)

Impact: Existing users upgrading to this version will have their wallet data stored under the OLD namespace (without suffix). The new code looks in the NEW namespace (with /native_segwit suffix). BDK will not find the existing descriptor, triggering return Ok(None) in read_bdk_wallet_change_set, which causes create_wallet_for_address_type to create a brand new empty wallet. All existing on-chain transaction history and address derivation indices are silently lost.

Required fix: Add migration logic that:

1. First tries to read from the new namespace (with suffix)
2. Falls back to reading from the old namespace (without suffix)
3. On successful fallback read, writes to the new namespace and optionally deletes old data

2. Incorrect UTXO Weight Calculation in Coin Selection

File: src/wallet/mod.rs:2249-2314

select_utxos_with_algorithm calculates satisfaction weight using the primary wallet's descriptor for ALL UTXOs:

let descriptor = primary_wallet.public_descriptor(utxo.keychain);
let satisfaction_weight = descriptor.max_weight_to_satisfy()...

For non-primary UTXOs (e.g., Legacy UTXOs when primary is NativeSegwit), utxo.keychain (External/Internal) maps to the wrong descriptor. A NativeSegwit descriptor returns 272 WU weight, but a Legacy P2PKH input actually weighs 588 WU. This underestimates fees by ~2x for Legacy inputs, potentially creating transactions that fail relay or confirmation.

Required fix: Look up the correct wallet for each UTXO (by finding which wallet owns the outpoint) and use that wallet's descriptor for weight calculation.

3. Fee Calculation Bug for Cross-Wallet Transactions in Payment Store

File: src/wallet/mod.rs:1734-1735

let total_fee = primary_wallet.calculate_fee(&tx).unwrap_or(Amount::ZERO).to_sat();

BDK's calculate_fee computes sum(inputs) - sum(outputs), but it can only know the values of inputs it owns. For cross-wallet transactions, the primary wallet doesn't own inputs from other wallets, so calculate_fee returns an error or incorrect value, which falls back to Amount::ZERO. This means cross-wallet transactions get recorded with zero fee in the payment store.

Required fix: Calculate fees by iterating all wallets and summing input values, similar to how calculate_fee_from_psbt works with the PSBT inputs.

4. sign_owned_inputs Silently Accepts Missing Input Values

File: src/wallet/mod.rs:3315-3318

Err(bitcoin::psbt::ExtractTxError::MissingInputValue { tx }) => Ok(tx),

This swallows the MissingInputValue error and returns the transaction anyway. If input values are missing, fee verification cannot be performed, and the transaction could be underpaying fees or even be invalid. This is used in cross-wallet RBF signing paths.

Required fix: Either ensure all input values are populated before extraction, or return an error here.

---

HIGH SEVERITY ISSUES

5. Lock Drop & Re-acquire Pattern in CPFP Creates Race Window

File: src/wallet/mod.rs:1354-1380

The accelerate_by_cpfp method drops both wallets and persisters locks to get a change address from the primary wallet, then re-acquires them:

drop(wallets);
drop(persisters);
let mut wallets_primary = self.wallets.lock().unwrap();
// ... get address ...
drop(wallets_primary);
drop(persisters_primary);
wallets = self.wallets.lock().unwrap();
persisters = self.persisters.lock().unwrap();

Between drops and re-acquisition, another thread could: sync the wallet (changing balances), spend UTXOs (invalidating the parent tx's spendable outputs), or modify wallet state in ways that make the subsequent CPFP transaction invalid.

Recommended fix: Restructure to avoid dropping and re-acquiring, or use a single transaction-building method that doesn't require lock juggling.

6. Blocking Async Call Inside Wallet Lock Scope

File: src/wallet/mod.rs:384-415

prepare_utxos_for_psbt is called while the caller holds the wallets Mutex lock, and it calls runtime.block_on(chain_source.get_transaction(...)) to fetch transactions from the chain source. This blocks the current thread while holding the wallet lock, preventing any other thread from accessing any wallet until the network call completes.

Recommended fix: Pre-fetch required transactions before acquiring the wallet lock, or restructure to release the lock before the network call.

7. No Build-Time Validation for Lightning Witness Requirement

File: src/wallet/mod.rs:1963-2012

When address_type is Legacy and address_types_to_monitor is empty, there's no witness wallet available. Lightning operations (channel funding, shutdown scripts) require witness addresses. This will fail at runtime with a generic WalletOperationFailed error.

Recommended fix: Validate at Builder::build() time that at least one witness-capable wallet (NativeSegwit or Taproot) is available, either as primary or monitored. Return a clear BuildError if not.

8. Change Address Reuse in Cross-Wallet RBF

File: src/wallet/mod.rs:1034-1035, 1197-1198

Cross-wallet RBF uses peek_address(KeychainKind::Internal, 0) which always returns the same address without advancing the index. While this is correct for RBF replacement (same change destination), it creates an address reuse pattern that may be observable across RBF attempts. Also, if peek_address(Internal, 0) was already used in a different confirmed transaction, the change output will be linked to that address.

---

MEDIUM SEVERITY ISSUES

9. Separate Mutexes for wallets and persisters Invite Deadlocks

File: src/wallet/mod.rs:104-105

wallets: Mutex<HashMap<AddressType, PersistedWallet<KVStoreWalletPersister>>>,
persisters: Mutex<HashMap<AddressType, KVStoreWalletPersister>>,

Throughout the code, both locks are always acquired together (wallets first, then persisters). If any code path ever acquires them in the opposite order, deadlock occurs. This is a maintenance hazard. Consider combining into a single Mutex<(HashMap<...>, HashMap<...>)> or a single struct.

10. Greedy Coin Selection in Cross-Wallet RBF Doesn't Sort UTXOs

File: src/wallet/mod.rs:1128-1146

When cross-wallet RBF needs additional inputs, it iterates additional_utxos in whatever order list_unspent() returns them. There's no sorting by value (largest first would minimize the number of additional inputs and thus minimize weight increase). This could select many small UTXOs when one large one would suffice.

11. fmt::Display Not Implemented for AddressType

File: src/config.rs:106-120

AddressType has Debug but no Display impl. Throughout the codebase, it's logged with {:?} (debug format). For user-facing FFI/API, a proper Display impl would be cleaner.

12. AllRetainingReserve Path Creates and Immediately Cancels a Transaction

File: src/wallet/mod.rs:2571-2733

The AllRetainingReserve with anchor reserve path builds a temporary transaction to estimate fees, then cancels it and builds the real one. This is wasteful and fragile - the "cancel" of the temp tx might have side effects on wallet state (BDK's cancel_tx mutates the wallet).

13. No Deduplication of address_types_to_monitor Beyond Primary

File: src/config.rs:161-168

additional_address_types() only filters out the primary address type. If a user passes [Legacy, Legacy, Legacy], three duplicate Legacy wallets would be created. The additional_wallets loop in builder.rs:1567 skips the primary type but doesn't deduplicate within the list.

---

LOW SEVERITY / CODE QUALITY ISSUES

14. Excessive Logging at INFO Level

Multiple files contain log_info! calls for routine operations (fee calculations, UTXO counts, sync progress). In production with multiple address types, this could be very noisy. Consider using log_debug! or log_trace! for per-sync, per-UTXO, and per-input logging.

15. _persisters Unused in build_cross_wallet_rbf

File: src/wallet/mod.rs:943

fn build_cross_wallet_rbf(
    &self, wallets: ...,
    _persisters: &mut HashMap<AddressType, KVStoreWalletPersister>, // unused

The persisters parameter is accepted but unused (prefixed with _). If wallet state needs to be persisted after RBF signing, this is a bug. If not, the parameter should be removed.

16. Hardcoded Dust Limit

File: src/wallet/mod.rs:67

const DUST_LIMIT_SATS: u64 = 546;

546 sats is the dust limit for P2PKH/P2SH outputs. For P2WPKH it's 294 sats, and for P2TR it's 330 sats. Using 546 for all types means NativeSegwit and Taproot change outputs are unnecessarily rejected as dust when they're between 294-545 sats.

17. Test Coverage Gaps

While there are 31+ multi-wallet tests, notable gaps include:

• No test for upgrading from single-wallet (old version) to multi-wallet (backwards compat)
• No concurrent access tests (multiple threads sending/syncing simultaneously)
• No test for Legacy-only primary without any witness wallet (Lightning failure path)
• No test for very large number of UTXOs across wallets (performance)
• No test for cross-wallet RBF when change is exactly dust limit
• No fuzzing targets for cross-wallet PSBT construction

---

ARCHITECTURE OBSERVATIONS

What's Done Well

• Clean separation between primary and monitored wallets
• Unified UTXO selection across all wallets with proper foreign UTXO handling
• Per-wallet persistence with independent changesets (correct BDK pattern)
• Parallel sync for monitored wallets using JoinSet
• Proper BIP 44/49/84/86 derivation from same master key
• Change outputs correctly routed to primary wallet
• Comprehensive test suite covering most happy paths

Design Concerns

• The Wallet struct is now ~3200 lines and handles wallet management, transaction building, coin selection, RBF, CPFP, signing, and payment store updates. Consider splitting into smaller modules.
• The build_transaction_psbt method is ~470 lines with deeply nested match arms and multiple lock acquire/release cycles. This is hard to audit and maintain.

---

Verification Steps

After addressing the critical issues:

1. cargo build - Verify clean build
2. cargo test - All existing tests pass
3. cargo test --test multi_wallet_tests - All new tests pass
4. Manual test: Start a node with existing single-wallet data, verify wallet loads correctly (migration)
5. cargo clippy - No new warnings
6. cargo fmt --check - Clean formatting

[ovitrif]

@ben-kaufman I drafted the PR until the next iteration.