Skip to content

feat: support all address types#431

Open
ben-kaufman wants to merge 34 commits intomasterfrom
feat/multiple-addresses-types
Open

feat: support all address types#431
ben-kaufman wants to merge 34 commits intomasterfrom
feat/multiple-addresses-types

Conversation

@ben-kaufman
Copy link
Contributor

This PR adds support for multiple Bitcoin address types in the iOS app, allowing users to select and monitor different address formats. Users can select their preferred address type from: Legacy (P2PKH), Nested SegWit (P2SH-P2WPKH), Native SegWit (P2WPKH), and Taproot (P2TR).

This requires extensive testing across all functionalities, send, receive, channel open, channel close (funds should always go to native segwit address except when selected address when channel opened was Taproot, in which case it should be a Taproot address), cpfp, rbf. All should be checked for each address type, and for multiple address types combined (ie. send with utxos of different address types, open channel with inputs of different address types, boost txs with inputs of different address types, etc.)

@ben-kaufman ben-kaufman requested a review from ovitrif February 5, 2026 11:13
@claude

This comment has been minimized.

@ben-kaufman ben-kaufman force-pushed the feat/multiple-addresses-types branch from 919e450 to 8c8e243 Compare February 11, 2026 04:38
@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@claude

This comment has been minimized.

@jvsena42
Copy link
Member

@claude create a checklist of manual test flows

@claude

This comment has been minimized.

@ben-kaufman ben-kaufman force-pushed the feat/multiple-addresses-types branch from 21fca73 to dc1ab2a Compare February 18, 2026 05:44
@ben-kaufman

This comment was marked as outdated.

@ovitrif ovitrif self-requested a review February 18, 2026 07:49
Co-authored-by: João Victor Sena <jvsena42@users.noreply.github.com>
@jvsena42
Copy link
Member

I didn't know that Claude would automatically commit 😶‍🌫️

@jvsena42

This comment was marked as outdated.

@ben-kaufman ben-kaufman force-pushed the feat/multiple-addresses-types branch from d436239 to 8956313 Compare February 19, 2026 10:10
@jvsena42

This comment was marked as outdated.

@jvsena42

This comment was marked as outdated.

@jvsena42

This comment was marked as outdated.

jvsena42

This comment was marked as outdated.

…ase LDK retries (#450)

* Fix activity not created on send: create immediately from txid, increase LDK retries

* Update CoreService.swift

* Update CoreService.swift

* Improve address search

* Update ldk-node version
* Replace node restart with dynamic address type APIs

* fix tests

* update ldk-node version
Comment on lines +84 to +89
let selectedAddressType = LDKNode.AddressType.fromStorage(UserDefaults.standard.string(forKey: "selectedAddressType"))
config.addressType = selectedAddressType

let monitoredTypesString = UserDefaults.standard.string(forKey: "addressTypesToMonitor") ?? "nativeSegwit"
let monitoredTypes = LDKNode.AddressType.parseCommaSeparated(monitoredTypesString)
config.addressTypesToMonitor = monitoredTypes.filter { $0 != selectedAddressType }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can reuse addressTypeStateFromUserDefaults helper to avoid duplication.

Suggested change
let selectedAddressType = LDKNode.AddressType.fromStorage(UserDefaults.standard.string(forKey: "selectedAddressType"))
config.addressType = selectedAddressType
let monitoredTypesString = UserDefaults.standard.string(forKey: "addressTypesToMonitor") ?? "nativeSegwit"
let monitoredTypes = LDKNode.AddressType.parseCommaSeparated(monitoredTypesString)
config.addressTypesToMonitor = monitoredTypes.filter { $0 != selectedAddressType }
let (selectedAddressType, monitoredTypes) = Self.addressTypeStateFromUserDefaults()
config.addressType = selectedAddressType
config.addressTypesToMonitor = monitoredTypes.filter { $0 != selectedAddressType }

Comment on lines +82 to +90
/// Short label for compact UI (e.g. "Native").
var shortLabel: String {
switch self {
case .legacy: return "Legacy"
case .nestedSegwit: return "Nested"
case .nativeSegwit: return "Native"
case .taproot: return "Taproot"
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Prefer to only use the full name for the different address types. Never heard them being abbreviated like this. Doesn't seem to break the UI in the address viewer as far as I can see.

}
}

if showDevSettings {
Copy link
Contributor

@pwltr pwltr Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume we don't watch all address types by default for performance reasons? It is a bit problematic because for example on the address viewer all addresses are exposed and the user might send to an address that is not monitored, wondering why they didn't receive anything. Maybe we should monitor all by default, and show this section always with a note to disable address types for faster syncs?

Copy link
Contributor

@pwltr pwltr Feb 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or alternatively don't show address types in the address viewer that have not yet been used. But then users may send to unmonitored address in another wallet using same seed that has these addresses exposed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think monitoring all by default will just be unnecessary performance downgrade for most users. Maybe better if you either hide in address viewer the address types which are not monitored, or we don't hide this section in dev mode

amountViewModel.handleNumberPadInput(key, currency: currency)
}

CustomButton(title: t("common__continue"), isDisabled: amountSats == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not related to this PR but this button should be disabled if amount exceeds available balance.

@jvsena42
Copy link
Member

⚠️ Couldn't boost a transaction with all Types, maybe the remaining balance was too low

Simulator.Screen.Recording.-.iPhone.16.-.2026-02-20.at.10.41.25.mp4

bitkit_foreground_2026-02-20_12-20-21.log
bitkit_foreground_2026-02-20_11-13-30.log

@jvsena42
Copy link
Member

⚠️ I cannot CPFP legacy transactions at all, probably because the new transaction fee would be to high

Simulator.Screen.Recording.-.iPhone.16.-.2026-02-20.at.10.53.34.mp4

bitkit_foreground_2026-02-20_12-20-21.log

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments