Skip to content

Installer: let user choose derivation path #1705

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
edouardparis merged 2 commits into from
May 21, 2025
Merged

Installer: let user choose derivation path #1705

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8efc55b
ui: add a widget to let user select custom derivation path of an xpub
pythcoiner May 14, 2025
ce8e1f3
installer: let user choose account (derivation path) before fetching …
pythcoiner May 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion liana-gui/src/installer/descriptor.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use async_hwi::{DeviceKind, Version};
use liana::miniscript::{bitcoin::bip32::Fingerprint, descriptor::DescriptorPublicKey};
use liana::miniscript::{
bitcoin::bip32::{ChildNumber, Fingerprint},
descriptor::DescriptorPublicKey,
};

use crate::{
app::settings::ProviderKey, hw::is_compatible_with_tapminiscript, services::keys::api::KeyKind,
Expand Down Expand Up @@ -107,6 +110,7 @@ pub struct Key {
pub name: String,
pub fingerprint: Fingerprint,
pub key: DescriptorPublicKey,
pub account: Option<ChildNumber>,
}

pub struct Path {
Expand Down
12 changes: 11 additions & 1 deletion liana-gui/src/installer/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
use liana::miniscript::{
bitcoin::{bip32::Fingerprint, Network},
bitcoin::{
bip32::{ChildNumber, Fingerprint},
Network,
},
DescriptorPublicKey,
};
use std::collections::HashMap;
Expand Down Expand Up @@ -67,6 +70,7 @@ pub enum Message {
ImportBackup,
WalletFromBackup((HashMap<Fingerprint, settings::KeySetting>, Backup)),
WalletAliasEdited(String),
SelectAccount(Fingerprint, ChildNumber),
}

impl Close for Message {
Expand All @@ -81,6 +85,12 @@ impl From<ImportExportMessage> for Message {
}
}

impl From<(Fingerprint, ChildNumber)> for Message {
fn from(value: (Fingerprint, ChildNumber)) -> Self {
Self::SelectAccount(value.0, value.1)
}
}

#[derive(Debug, Clone)]
pub enum SelectBackend {
// view messages
Expand Down
79 changes: 73 additions & 6 deletions liana-gui/src/installer/step/descriptor/editor/key.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::str::FromStr;
use std::sync::{Arc, Mutex};

use iced::{Subscription, Task};
use liana::miniscript::bitcoin::bip32::Xpub;
use liana::miniscript::bitcoin::bip32::{ChildNumber, Xpub};
use liana::miniscript::{
bitcoin::{
bip32::{DerivationPath, Fingerprint},
Expand Down Expand Up @@ -89,6 +89,7 @@ pub struct EditXpubModal {
hot_signer_fingerprint: Fingerprint,
chosen_signer: Option<Key>,
modal: Option<ExportModal>,
accounts: HashMap<Fingerprint, ChildNumber>,
}

impl EditXpubModal {
Expand All @@ -104,6 +105,10 @@ impl EditXpubModal {
hot_signer_fingerprint: Fingerprint,
keys: Vec<Key>,
) -> Self {
let accounts = keys
.iter()
.filter_map(|k| k.account.map(|acc| (k.fingerprint, acc)))
.collect();
Self {
device_must_support_tapminiscript,
path_kind,
Expand Down Expand Up @@ -139,6 +144,7 @@ impl EditXpubModal {
hot_signer,
duplicate_master_fg: false,
modal: None,
accounts,
}
}

Expand All @@ -158,6 +164,10 @@ impl super::DescriptorEditModal for EditXpubModal {
self.duplicate_master_fg = false;
self.error = None;
match message {
Message::SelectAccount(fg, index) => {
self.accounts.insert(fg, index);
return Task::none();
}
Message::Select(i) => {
if let Some(HardwareWallet::Supported {
device,
Expand All @@ -170,6 +180,11 @@ impl super::DescriptorEditModal for EditXpubModal {
self.processing = true;
self.form_key_source_kind = None;
let device_version = version.clone();
let account = self
.accounts
.get(fingerprint)
.copied()
.unwrap_or(ChildNumber::from_hardened_idx(0).expect("hardcoded"));
let fingerprint = *fingerprint;
let device_kind = *kind;
let device_cloned = device.clone();
Expand All @@ -181,10 +196,11 @@ impl super::DescriptorEditModal for EditXpubModal {
device_kind,
fingerprint,
network,
get_extended_pubkey(device_cloned, fingerprint, network).await,
get_extended_pubkey(device_cloned, fingerprint, network, account)
.await,
)
},
|(device_version, device_kind, fingerprint, network, res)| {
move |(device_version, device_kind, fingerprint, network, res)| {
Message::DefineDescriptor(message::DefineDescriptor::KeyModal(
message::ImportKeyModal::FetchedKey(match res {
Err(e) => Err(e),
Expand All @@ -198,6 +214,7 @@ impl super::DescriptorEditModal for EditXpubModal {
fingerprint,
name: "".to_string(),
key,
account: Some(account),
})
} else {
Err(Error::Unexpected(
Expand Down Expand Up @@ -233,6 +250,7 @@ impl super::DescriptorEditModal for EditXpubModal {
fingerprint,
name: "".to_string(),
key: DescriptorPublicKey::from_str(&key_str).unwrap(),
account: None,
});
self.form_name.value = self
.keys
Expand Down Expand Up @@ -273,7 +291,7 @@ impl super::DescriptorEditModal for EditXpubModal {
message::ImportKeyModal::FetchedKey(res) => {
self.processing = false;
match res {
Ok(key) => {
Ok(mut key) => {
// If it is a provider key that has just been fetched, do some additional sanity checks.
if let Some(key_kind) = key.source.provider_key_kind() {
// We don't need to check key's status as redeemed keys are not returned.
Expand Down Expand Up @@ -302,6 +320,7 @@ impl super::DescriptorEditModal for EditXpubModal {
};
self.form_token.valid = self.form_token_warning.is_none();
}
key.account = self.accounts.get(&key.fingerprint).copied();
// User can set name for key if it is not a provider key or is a valid provider key.
if key.source.provider_key().is_none() || self.form_token.valid {
self.form_name.valid = key.name.is_empty()
Expand Down Expand Up @@ -374,6 +393,7 @@ impl super::DescriptorEditModal for EditXpubModal {
fingerprint,
name: "".to_string(),
key: DescriptorPublicKey::XPub(key),
account: None,
});
self.form_name.value = "".to_string();
self.form_name.valid = true;
Expand Down Expand Up @@ -429,6 +449,7 @@ impl super::DescriptorEditModal for EditXpubModal {
key.kind
),
key: key.xpub.clone(),
account: None,
}),
}),
))
Expand Down Expand Up @@ -503,6 +524,8 @@ impl super::DescriptorEditModal for EditXpubModal {
hw.fingerprint() == chosen_signer,
None,
self.device_must_support_tapminiscript,
Some(&self.accounts),
true,
))
}
})
Expand All @@ -528,6 +551,7 @@ impl super::DescriptorEditModal for EditXpubModal {
key.source.device_version(),
Some(key.fingerprint) == chosen_signer,
self.device_must_support_tapminiscript,
&self.accounts,
))
}
})
Expand Down Expand Up @@ -576,14 +600,31 @@ pub fn default_derivation_path(network: Network) -> DerivationPath {
.unwrap()
}

pub fn derivation_path(network: Network, account: ChildNumber) -> DerivationPath {
assert!(account.is_hardened());
Copy link
Copy Markdown
Collaborator Author

@pythcoiner pythcoiner May 16, 2025

Choose a reason for hiding this comment

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

Note: added this assert to catch if an unhardened derivation path is used

Copy link
Copy Markdown
Collaborator

@jp1ac4 jp1ac4 May 20, 2025

Choose a reason for hiding this comment

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

nit: I feel it might be slightly better to return an Option or Result here and let the caller unwrap it.

let network = if network == Network::Bitcoin {
ChildNumber::Hardened { index: 0 }
} else {
ChildNumber::Hardened { index: 1 }
};
vec![
ChildNumber::Hardened { index: 48 },
network,
account,
ChildNumber::Hardened { index: 2 },
]
.into()
}

/// LIANA_STANDARD_PATH: m/48'/0'/0'/2';
/// LIANA_TESTNET_STANDARD_PATH: m/48'/1'/0'/2';
pub async fn get_extended_pubkey(
hw: std::sync::Arc<dyn async_hwi::HWI + Send + Sync>,
fingerprint: Fingerprint,
network: Network,
account: ChildNumber,
) -> Result<DescriptorPublicKey, Error> {
let derivation_path = default_derivation_path(network);
let derivation_path = derivation_path(network, account);
let xkey = hw
.get_extended_pubkey(&derivation_path)
.await
Expand Down Expand Up @@ -619,4 +660,30 @@ mod tests {
"48'/1'/0'/2'"
);
}

#[test]
fn test_derivation_path() {
assert_eq!(
derivation_path(Network::Bitcoin, ChildNumber::Hardened { index: 0 }).to_string(),
"48'/0'/0'/2'"
);
assert_eq!(
derivation_path(Network::Regtest, ChildNumber::Hardened { index: 0 }).to_string(),
"48'/1'/0'/2'"
);
assert_eq!(
derivation_path(Network::Bitcoin, ChildNumber::Hardened { index: 1 }).to_string(),
"48'/0'/1'/2'"
);
assert_eq!(
derivation_path(Network::Regtest, ChildNumber::Hardened { index: 1 }).to_string(),
"48'/1'/1'/2'"
);
}

#[test]
#[should_panic]
fn unhardened_derivation_path() {
derivation_path(Network::Bitcoin, ChildNumber::Normal { index: 0 }).to_string();
}
}
1 change: 1 addition & 0 deletions liana-gui/src/installer/step/descriptor/editor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -764,6 +764,7 @@ mod tests {
fingerprint: key.master_fingerprint(),
key,
source: KeySource::Device(async_hwi::DeviceKind::Specter, None),
account: None,
};

// Use Specter device for primary key
Expand Down
21 changes: 18 additions & 3 deletions liana-gui/src/installer/step/share_xpubs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::sync::{Arc, Mutex};
use std::{
collections::HashMap,
sync::{Arc, Mutex},
};

use iced::{Subscription, Task};
use liana::miniscript::bitcoin::{
Expand Down Expand Up @@ -73,6 +76,7 @@ pub struct ShareXpubs {
hw_xpubs: Vec<HardwareWalletXpubs>,
xpubs_signer: SignerXpubs,
modal: Option<ExportModal>,
accounts: HashMap<Fingerprint, ChildNumber>,
}

impl ShareXpubs {
Expand All @@ -82,6 +86,7 @@ impl ShareXpubs {
hw_xpubs: Vec::new(),
xpubs_signer: SignerXpubs::new(signer),
modal: None,
accounts: Default::default(),
}
}
}
Expand All @@ -91,6 +96,10 @@ impl Step for ShareXpubs {
// Verification of the values is happening when the user click on Next button.
fn update(&mut self, hws: &mut HardwareWallets, message: Message) -> Task<Message> {
match message {
Message::SelectAccount(fg, index) => {
self.accounts.insert(fg, index);
return Task::none();
}
Message::ImportXpub(fg, res) => {
if let Some(hw_xpubs) = self.hw_xpubs.iter_mut().find(|x| x.fingerprint == fg) {
hw_xpubs.processing = false;
Expand Down Expand Up @@ -138,6 +147,11 @@ impl Step for ShareXpubs {
}) = hws.list.get(i)
{
let device = device.clone();
let account = self
.accounts
.get(fingerprint)
.copied()
.unwrap_or(ChildNumber::from_hardened_idx(0).expect("hardcoded"));
let fingerprint = *fingerprint;
let network = self.network;
if let Some(hw_xpubs) = self
Expand All @@ -159,7 +173,7 @@ impl Step for ShareXpubs {
async move {
(
fingerprint,
get_extended_pubkey(device, fingerprint, network).await,
get_extended_pubkey(device, fingerprint, network, account).await,
)
},
|(fingerprint, res)| Message::ImportXpub(fingerprint, res),
Expand Down Expand Up @@ -212,9 +226,10 @@ impl Step for ShareXpubs {
Some(&hw_xpubs.xpubs),
hw_xpubs.processing,
hw_xpubs.error.as_ref(),
&self.accounts,
)
} else {
view::hardware_wallet_xpubs(i, hw, None, false, None)
view::hardware_wallet_xpubs(i, hw, None, false, None, &self.accounts)
}
})
.collect(),
Expand Down
16 changes: 14 additions & 2 deletions liana-gui/src/installer/view/editor/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

pub mod template;

use iced::widget::{container, pick_list, scrollable, slider, Button, Space};
use iced::widget::{self, container, pick_list, scrollable, slider, Button, Space};
use iced::{Alignment, Length};

use liana::miniscript::bitcoin::Network;
Expand Down Expand Up @@ -353,6 +353,18 @@ pub fn edit_key_modal<'a>(
duplicate_master_fg: bool,
) -> Element<'a, Message> {
let xpub_valid = form_xpub.valid && !form_xpub.value.is_empty();
let info = Column::new()
.push(Space::with_height(5))
.push(widget::tooltip::Tooltip::new(
icon::tooltip_icon(),
"Switch account if you already use the same hardware in other configurations",
widget::tooltip::Position::Bottom,
));
let source = Row::new()
.push(p1_regular("Select the source of your key").bold())
.push(Space::with_width(10))
.push(info)
Copy link
Copy Markdown
Collaborator

@jp1ac4 jp1ac4 May 20, 2025

Choose a reason for hiding this comment

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

nit: I feel this tooltip would be more visible if it were next to the pick list within the HW card. Same applies for "Share Xpubs" below.

Copy link
Copy Markdown
Collaborator Author

@pythcoiner pythcoiner May 20, 2025

Choose a reason for hiding this comment

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

this have been request by @edouardparis previously and ACK'd by @nondiremanuel iirc

.push(Space::with_width(Length::Fill));
let content = Column::new()
.padding(25)
.push_maybe(error.map(|e| card::error("Failed to import xpub", e.to_string())))
Expand All @@ -367,7 +379,7 @@ pub fn edit_key_modal<'a>(
)
.push(
Column::new()
.push(p1_regular("Select the source of your key"))
.push(source)
.spacing(10)
.push(Column::with_children(hws).spacing(10))
.push(Column::with_children(keys).spacing(10))
Expand Down
Loading
Loading