Merge #487: Check hot signer usage using descriptor

307d12d980b618a47f5a929046d0643e688cd1ff Check hot signer usage using descriptor (edouard)

Pull request description:

  It is a refont of the installer flow
  to use less optional components in the installer context.

  Hot signer is stored a the install step if its fingerprint is present in the descriptor string

  close #397

ACKs for top commit:
  edouardparis:
    Self-ACK 307d12d980b618a47f5a929046d0643e688cd1ff

Tree-SHA512: 018ddb17ccc780f808b804b3ded273d81032d0ea3275ce5fd4cc7418c996c53f47be842895a25583b404675a041e0f1bb7b5e861a2570f66f1c14d03f966f266
This commit is contained in:
edouard 2023-05-09 11:55:56 +02:00
commit 7d4ac9d56a
No known key found for this signature in database
GPG Key ID: E65F7A089C20DC8F
6 changed files with 152 additions and 107 deletions

View File

@ -30,8 +30,10 @@ pub struct Context {
Option<[u8; 32]>,
)>,
pub data_dir: PathBuf,
pub signer: Option<Arc<Signer>>,
pub hw_is_used: bool,
// In case a user entered a mnemonic,
// we dont want to override the generated signer with it.
pub recovered_signer: Option<Arc<Signer>>,
}
impl Context {
@ -46,8 +48,8 @@ impl Context {
bitcoind_config: None,
descriptor: None,
data_dir,
signer: None,
hw_is_used: false,
recovered_signer: None,
}
}

View File

@ -10,10 +10,15 @@ use liana_ui::widget::Element;
use tracing::{error, info, warn};
use context::Context;
use std::io::Write;
use std::path::PathBuf;
use std::sync::{Arc, Mutex};
use crate::app::{config as gui_config, settings as gui_settings};
use crate::{
app::{config as gui_config, settings as gui_settings},
signer::Signer,
};
pub use message::Message;
use step::{
@ -24,6 +29,7 @@ use step::{
pub struct Installer {
current: usize,
steps: Vec<Box<dyn Step>>,
signer: Arc<Mutex<Signer>>,
/// Context is data passed through each step.
context: Context,
@ -55,6 +61,7 @@ impl Installer {
current: 0,
steps: vec![Welcome::default().into()],
context: Context::new(network, destination_path),
signer: Arc::new(Mutex::new(Signer::generate(network).unwrap())),
},
Command::none(),
)
@ -98,29 +105,30 @@ impl Installer {
}
pub fn update(&mut self, message: Message) -> Command<Message> {
let hot_signer_fingerprint = self.signer.lock().unwrap().fingerprint();
match message {
Message::CreateWallet => {
self.steps = vec![
Welcome::default().into(),
DefineDescriptor::new().into(),
BackupMnemonic::default().into(),
DefineDescriptor::new(self.signer.clone()).into(),
BackupMnemonic::new(self.signer.clone()).into(),
BackupDescriptor::default().into(),
RegisterDescriptor::default().into(),
DefineBitcoind::new().into(),
Final::new().into(),
Final::new(hot_signer_fingerprint).into(),
];
self.next()
}
Message::ParticipateWallet => {
self.steps = vec![
Welcome::default().into(),
ParticipateXpub::new().into(),
ParticipateXpub::new(self.signer.clone()).into(),
ImportDescriptor::new(false).into(),
BackupMnemonic::default().into(),
BackupMnemonic::new(self.signer.clone()).into(),
BackupDescriptor::default().into(),
RegisterDescriptor::default().into(),
DefineBitcoind::new().into(),
Final::new().into(),
Final::new(hot_signer_fingerprint).into(),
];
self.next()
}
@ -131,7 +139,7 @@ impl Installer {
RecoverMnemonic::default().into(),
RegisterDescriptor::default().into(),
DefineBitcoind::new().into(),
Final::new().into(),
Final::new(hot_signer_fingerprint).into(),
];
self.next()
}
@ -146,7 +154,10 @@ impl Installer {
.get_mut(self.current)
.expect("There is always a step")
.update(message);
Command::perform(install(self.context.clone()), Message::Installed)
Command::perform(
install(self.context.clone(), self.signer.clone()),
Message::Installed,
)
}
Message::Installed(Err(e)) => {
let mut data_dir = self.context.data_dir.clone();
@ -219,7 +230,7 @@ pub fn daemon_check(cfg: liana::config::Config) -> Result<(), Error> {
}
}
pub async fn install(ctx: Context) -> Result<PathBuf, Error> {
pub async fn install(ctx: Context, signer: Arc<Mutex<Signer>>) -> Result<PathBuf, Error> {
let mut cfg: liana::config::Config = ctx.extract_daemon_config();
let data_dir = cfg.data_dir.unwrap();
@ -248,8 +259,14 @@ pub async fn install(ctx: Context) -> Result<PathBuf, Error> {
info!("Daemon configuration file created");
if let Some(signer) = &ctx.signer {
if cfg
.main_descriptor
.to_string()
.contains(&signer.lock().unwrap().fingerprint().to_string())
{
signer
.lock()
.unwrap()
.store(
&cfg.data_dir().expect("Already checked"),
cfg.bitcoin_config.network,
@ -259,6 +276,17 @@ pub async fn install(ctx: Context) -> Result<PathBuf, Error> {
info!("Hot signer mnemonic stored");
}
if let Some(signer) = &ctx.recovered_signer {
signer
.store(
&cfg.data_dir().expect("Already checked"),
cfg.bitcoin_config.network,
)
.map_err(|e| Error::Unexpected(format!("Failed to store mnemonic: {}", e)))?;
info!("Recovered signer mnemonic stored");
}
// create liana GUI configuration file
let gui_config_path = create_and_write_file(
network_datadir_path.clone(),

View File

@ -1,7 +1,7 @@
use std::collections::{BTreeMap, HashMap, HashSet};
use std::path::PathBuf;
use std::str::FromStr;
use std::sync::Arc;
use std::sync::{Arc, Mutex};
use iced::Command;
use liana::{
@ -97,13 +97,13 @@ pub struct DefineDescriptor {
recovery_paths: Vec<RecoveryPath>,
modal: Option<Box<dyn DescriptorEditModal>>,
signer: Arc<Signer>,
signer: Arc<Mutex<Signer>>,
error: Option<String>,
}
impl DefineDescriptor {
pub fn new() -> Self {
pub fn new(signer: Arc<Mutex<Signer>>) -> Self {
Self {
network: Network::Bitcoin,
data_dir: None,
@ -112,7 +112,7 @@ impl DefineDescriptor {
spending_threshold: 1,
recovery_paths: vec![RecoveryPath::new()],
modal: None,
signer: Arc::new(Signer::generate(Network::Bitcoin).unwrap()),
signer,
error: None,
}
}
@ -125,9 +125,7 @@ impl DefineDescriptor {
fn set_network(&mut self, network: Network) {
self.network = network;
if let Some(signer) = Arc::get_mut(&mut self.signer) {
signer.set_network(network);
}
self.signer.lock().unwrap().set_network(network);
if let Some(mut network_datadir) = self.data_dir.clone() {
network_datadir.push(self.network.to_string());
self.network_valid = !network_datadir.exists();
@ -448,7 +446,6 @@ impl Step for DefineDescriptor {
ctx.bitcoin_config.network = self.network;
ctx.keys = Vec::new();
let mut hw_is_used = false;
let mut signer_is_used = false;
let mut spending_keys: Vec<DescriptorPublicKey> = Vec::new();
for spending_key in self.spending_keys.iter().clone() {
if let Some(DescriptorPublicKey::XPub(xpub)) = spending_key.key.as_ref() {
@ -457,9 +454,6 @@ impl Step for DefineDescriptor {
master_fingerprint,
name: spending_key.name.clone(),
});
if master_fingerprint == self.signer.fingerprint() {
signer_is_used = true;
}
if spending_key.device_kind.is_some() {
hw_is_used = true;
}
@ -489,9 +483,6 @@ impl Step for DefineDescriptor {
master_fingerprint,
name: recovery_key.name.clone(),
});
if master_fingerprint == self.signer.fingerprint() {
signer_is_used = true;
}
if recovery_key.device_kind.is_some() {
hw_is_used = true;
}
@ -539,9 +530,6 @@ impl Step for DefineDescriptor {
ctx.descriptor = Some(LianaDescriptor::new(policy));
ctx.hw_is_used = hw_is_used;
if signer_is_used {
ctx.signer = Some(self.signer.clone());
}
true
}
@ -650,12 +638,6 @@ fn check_key_network(key: &DescriptorPublicKey, network: Network) -> bool {
}
}
impl Default for DefineDescriptor {
fn default() -> Self {
Self::new()
}
}
impl From<DefineDescriptor> for Box<dyn Step> {
fn from(s: DefineDescriptor) -> Box<dyn Step> {
Box::new(s)
@ -738,7 +720,8 @@ pub struct EditXpubModal {
edit_name: bool,
hws: Vec<HardwareWallet>,
hot_signer: Arc<Signer>,
hot_signer: Arc<Mutex<Signer>>,
hot_signer_fingerprint: Fingerprint,
chosen_signer: Option<(Fingerprint, Option<DeviceKind>)>,
}
@ -752,8 +735,9 @@ impl EditXpubModal {
network: Network,
account_indexes: HashMap<Fingerprint, ChildNumber>,
keys_aliases: HashMap<Fingerprint, String>,
hot_signer: Arc<Signer>,
hot_signer: Arc<Mutex<Signer>>,
) -> Self {
let hot_signer_fingerprint = hot_signer.lock().unwrap().fingerprint();
Self {
form_name: form::Value {
valid: true,
@ -773,6 +757,7 @@ impl EditXpubModal {
network,
edit_name: false,
chosen_signer: key.map(|k| (k.master_fingerprint(), None)),
hot_signer_fingerprint,
hot_signer,
}
}
@ -838,7 +823,7 @@ impl DescriptorEditModal for EditXpubModal {
return self.load();
}
Message::UseHotSigner => {
let fingerprint = self.hot_signer.fingerprint();
let fingerprint = self.hot_signer.lock().unwrap().fingerprint();
self.chosen_signer = Some((fingerprint, None));
self.form_xpub.valid = true;
if let Some(alias) = self.keys_aliases.get(&fingerprint) {
@ -859,7 +844,10 @@ impl DescriptorEditModal for EditXpubModal {
"[{}{}]{}",
fingerprint,
derivation_path.to_string().trim_start_matches('m'),
self.hot_signer.get_extended_pubkey(&derivation_path)
self.hot_signer
.lock()
.unwrap()
.get_extended_pubkey(&derivation_path)
);
}
Message::DefineDescriptor(message::DefineDescriptor::KeyModal(msg)) => match msg {
@ -957,8 +945,8 @@ impl DescriptorEditModal for EditXpubModal {
self.error.as_ref(),
self.processing,
self.chosen_signer.map(|s| s.0),
&self.hot_signer,
self.keys_aliases.get(&self.hot_signer.fingerprint),
&self.hot_signer_fingerprint,
self.keys_aliases.get(&self.hot_signer_fingerprint),
&self.form_xpub,
&self.form_name,
self.edit_name,
@ -1073,13 +1061,13 @@ impl HardwareWalletXpubs {
}
pub struct SignerXpubs {
signer: Arc<Signer>,
signer: Arc<Mutex<Signer>>,
xpubs: Vec<String>,
next_account: ChildNumber,
}
impl SignerXpubs {
fn new(signer: Arc<Signer>) -> Self {
fn new(signer: Arc<Mutex<Signer>>) -> Self {
Self {
signer,
xpubs: Vec::new(),
@ -1095,11 +1083,12 @@ impl SignerXpubs {
fn select(&mut self, network: Network) {
let derivation_path = generate_derivation_path(network, self.next_account);
self.next_account = self.next_account.increment().unwrap();
let signer = self.signer.lock().unwrap();
self.xpubs.push(format!(
"[{}{}]{}",
self.signer.fingerprint(),
signer.fingerprint(),
derivation_path.to_string().trim_start_matches('m'),
self.signer.get_extended_pubkey(&derivation_path)
signer.get_extended_pubkey(&derivation_path)
));
}
@ -1120,14 +1109,14 @@ pub struct ParticipateXpub {
}
impl ParticipateXpub {
pub fn new() -> Self {
pub fn new(signer: Arc<Mutex<Signer>>) -> Self {
Self {
network: Network::Bitcoin,
network_valid: true,
data_dir: None,
xpubs_hw: Vec::new(),
shared: false,
xpubs_signer: SignerXpubs::new(Arc::new(Signer::generate(Network::Bitcoin).unwrap())),
xpubs_signer: SignerXpubs::new(signer),
}
}
@ -1137,9 +1126,11 @@ impl ParticipateXpub {
self.xpubs_signer.reset();
}
self.network = network;
if let Some(signer) = Arc::get_mut(&mut self.xpubs_signer.signer) {
signer.set_network(network);
}
self.xpubs_signer
.signer
.lock()
.unwrap()
.set_network(network);
if let Some(mut network_datadir) = self.data_dir.clone() {
network_datadir.push(self.network.to_string());
self.network_valid = !network_datadir.exists();
@ -1147,12 +1138,6 @@ impl ParticipateXpub {
}
}
impl Default for ParticipateXpub {
fn default() -> Self {
Self::new()
}
}
impl Step for ParticipateXpub {
// form value is set as valid each time it is edited.
// Verification of the values is happening when the user click on Next button.
@ -1211,11 +1196,6 @@ impl Step for ParticipateXpub {
ctx.bitcoin_config.network = self.network;
// Drop connections to hardware wallets.
self.xpubs_hw = Vec::new();
if !self.xpubs_signer.xpubs.is_empty() {
ctx.signer = Some(self.xpubs_signer.signer.clone());
} else {
ctx.signer = None;
}
true
}
@ -1515,7 +1495,9 @@ mod tests {
#[tokio::test]
async fn test_define_descriptor_use_hotkey() {
let mut ctx = Context::new(Network::Signet, PathBuf::from_str("/").unwrap());
let sandbox: Sandbox<DefineDescriptor> = Sandbox::new(DefineDescriptor::new());
let sandbox: Sandbox<DefineDescriptor> = Sandbox::new(DefineDescriptor::new(Arc::new(
Mutex::new(Signer::generate(Network::Bitcoin).unwrap()),
)));
// Edit primary key
sandbox
@ -1582,14 +1564,21 @@ mod tests {
sandbox.check(|step| {
assert!(step.modal.is_none());
assert!((step).apply(&mut ctx));
assert!(ctx.signer.is_some());
assert!(ctx
.descriptor
.as_ref()
.unwrap()
.to_string()
.contains(&step.signer.lock().unwrap().fingerprint().to_string()));
});
}
#[tokio::test]
async fn test_define_descriptor_stores_if_hw_is_used() {
let mut ctx = Context::new(Network::Signet, PathBuf::from_str("/").unwrap());
let sandbox: Sandbox<DefineDescriptor> = Sandbox::new(DefineDescriptor::new());
let sandbox: Sandbox<DefineDescriptor> = Sandbox::new(DefineDescriptor::new(Arc::new(
Mutex::new(Signer::generate(Network::Bitcoin).unwrap()),
)));
let specter_key = message::DefinePath::Key(
0,

View File

@ -1,5 +1,5 @@
use std::collections::HashSet;
use std::sync::Arc;
use std::sync::{Arc, Mutex};
use iced::Command;
use liana::{bip39, signer::HotSigner};
@ -11,10 +11,21 @@ use crate::{
signer::Signer,
};
#[derive(Default)]
pub struct BackupMnemonic {
words: [&'static str; 12],
done: bool,
signer: Arc<Mutex<Signer>>,
}
impl BackupMnemonic {
pub fn new(signer: Arc<Mutex<Signer>>) -> Self {
let words = signer.lock().unwrap().mnemonic();
Self {
done: false,
words,
signer,
}
}
}
impl From<BackupMnemonic> for Box<dyn Step> {
@ -24,11 +35,6 @@ impl From<BackupMnemonic> for Box<dyn Step> {
}
impl Step for BackupMnemonic {
fn load_context(&mut self, ctx: &Context) {
if let Some(signer) = &ctx.signer {
self.words = signer.mnemonic();
}
}
fn update(&mut self, message: Message) -> Command<Message> {
if let Message::UserActionDone(done) = message {
self.done = done;
@ -36,7 +42,13 @@ impl Step for BackupMnemonic {
Command::none()
}
fn skip(&self, ctx: &Context) -> bool {
ctx.signer.is_none()
if let Some(descriptor) = &ctx.descriptor {
!descriptor
.to_string()
.contains(&self.signer.lock().unwrap().fingerprint().to_string())
} else {
false
}
}
fn view(&self, progress: (usize, usize)) -> Element<Message> {
view::backup_mnemonic(progress, &self.words, self.done)
@ -109,7 +121,6 @@ impl Step for RecoverMnemonic {
if self.skip {
// If the user click previous, we dont want the skip to be set to true.
self.skip = false;
ctx.signer = None;
return true;
}
@ -148,8 +159,7 @@ impl Step for RecoverMnemonic {
}
}
ctx.signer = Some(Arc::new(signer));
ctx.recovered_signer = Some(Arc::new(signer));
true
}
fn view(&self, progress: (usize, usize)) -> Element<Message> {

View File

@ -11,7 +11,10 @@ use std::path::PathBuf;
use std::str::FromStr;
use iced::Command;
use liana::{config::BitcoindConfig, miniscript::bitcoin};
use liana::{
config::BitcoindConfig,
miniscript::bitcoin::{util::bip32::Fingerprint, Network},
};
use jsonrpc::{client::Client, simple_http::SimpleHttpTransport};
@ -61,7 +64,7 @@ pub struct DefineBitcoind {
is_running: Option<Result<(), Error>>,
}
fn bitcoind_default_cookie_path(network: &bitcoin::Network) -> Option<String> {
fn bitcoind_default_cookie_path(network: &Network) -> Option<String> {
#[cfg(target_os = "linux")]
let configs_dir = dirs::home_dir();
@ -76,16 +79,16 @@ fn bitcoind_default_cookie_path(network: &bitcoin::Network) -> Option<String> {
path.push("Bitcoin");
match network {
bitcoin::Network::Bitcoin => {
Network::Bitcoin => {
path.push(".cookie");
}
bitcoin::Network::Testnet => {
Network::Testnet => {
path.push("testnet3/.cookie");
}
bitcoin::Network::Regtest => {
Network::Regtest => {
path.push("regtest/.cookie");
}
bitcoin::Network::Signet => {
Network::Signet => {
path.push("signet/.cookie");
}
}
@ -95,12 +98,12 @@ fn bitcoind_default_cookie_path(network: &bitcoin::Network) -> Option<String> {
None
}
fn bitcoind_default_address(network: &bitcoin::Network) -> String {
fn bitcoind_default_address(network: &Network) -> String {
match network {
bitcoin::Network::Bitcoin => "127.0.0.1:8332".to_string(),
bitcoin::Network::Testnet => "127.0.0.1:18332".to_string(),
bitcoin::Network::Regtest => "127.0.0.1:18443".to_string(),
bitcoin::Network::Signet => "127.0.0.1:38332".to_string(),
Network::Bitcoin => "127.0.0.1:8332".to_string(),
Network::Testnet => "127.0.0.1:18332".to_string(),
Network::Regtest => "127.0.0.1:18443".to_string(),
Network::Signet => "127.0.0.1:38332".to_string(),
}
}
@ -227,15 +230,19 @@ pub struct Final {
context: Option<Context>,
warning: Option<String>,
config_path: Option<PathBuf>,
hot_signer_fingerprint: Fingerprint,
hot_signer_is_not_used: bool,
}
impl Final {
pub fn new() -> Self {
pub fn new(hot_signer_fingerprint: Fingerprint) -> Self {
Self {
context: None,
generating: false,
warning: None,
config_path: None,
hot_signer_fingerprint,
hot_signer_is_not_used: false,
}
}
}
@ -243,6 +250,20 @@ impl Final {
impl Step for Final {
fn load_context(&mut self, ctx: &Context) {
self.context = Some(ctx.clone());
if let Some(signer) = &ctx.recovered_signer {
self.hot_signer_fingerprint = signer.fingerprint();
self.hot_signer_is_not_used = false;
} else if ctx
.descriptor
.as_ref()
.unwrap()
.to_string()
.contains(&self.hot_signer_fingerprint.to_string())
{
self.hot_signer_is_not_used = false;
} else {
self.hot_signer_is_not_used = true;
}
}
fn update(&mut self, message: Message) -> Command<Message> {
match message {
@ -276,16 +297,15 @@ impl Step for Final {
self.generating,
self.config_path.as_ref(),
self.warning.as_ref(),
if self.hot_signer_is_not_used {
None
} else {
Some(self.hot_signer_fingerprint)
},
)
}
}
impl Default for Final {
fn default() -> Self {
Self::new()
}
}
impl From<Final> for Box<dyn Step> {
fn from(s: Final) -> Box<dyn Step> {
Box::new(s)

View File

@ -25,7 +25,6 @@ use crate::{
message::{self, Message},
prompt, Error,
},
signer::Signer,
};
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
@ -888,6 +887,7 @@ pub fn install<'a>(
generating: bool,
config_path: Option<&std::path::PathBuf>,
warning: Option<&'a String>,
signer: Option<Fingerprint>,
) -> Element<'a, Message> {
layout(
progress,
@ -911,7 +911,7 @@ pub fn install<'a>(
)
.width(Length::Fill),
)
.push_maybe(if context.hws.is_empty() && context.signer.is_none() {
.push_maybe(if context.hws.is_empty() && signer.is_none() {
None
} else {
Some(
@ -950,21 +950,17 @@ pub fn install<'a>(
},
))
})
.push_maybe(context.signer.as_ref().map(|signer| {
.push_maybe(signer.as_ref().map(|fingerprint| {
Row::new()
.spacing(5)
.push_maybe(context.keys.iter().find_map(|k| {
if k.master_fingerprint == signer.fingerprint()
{
if k.master_fingerprint == *fingerprint {
Some(text(k.name.clone()).small().bold())
} else {
None
}
}))
.push(
text(format!("#{}", signer.fingerprint()))
.small(),
)
.push(text(format!("#{}", fingerprint)).small())
.push(text("This computer").small())
})),
)
@ -1245,7 +1241,7 @@ pub fn edit_key_modal<'a>(
error: Option<&Error>,
processing: bool,
chosen_signer: Option<Fingerprint>,
signer: &Signer,
hot_signer_fingerprint: &Fingerprint,
signer_alias: Option<&'a String>,
form_xpub: &form::Value<String>,
form_name: &'a form::Value<String>,
@ -1288,10 +1284,10 @@ pub fn edit_key_modal<'a>(
},
))
.push(
Button::new(if Some(signer.fingerprint) == chosen_signer {
hw::selected_hot_signer(signer.fingerprint, signer_alias)
Button::new(if Some(*hot_signer_fingerprint) == chosen_signer {
hw::selected_hot_signer(hot_signer_fingerprint, signer_alias)
} else {
hw::unselected_hot_signer(signer.fingerprint, signer_alias)
hw::unselected_hot_signer(hot_signer_fingerprint, signer_alias)
})
.width(Length::Fill)
.on_press(Message::UseHotSigner)