-
Notifications
You must be signed in to change notification settings - Fork 16
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.
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
bump rust-bitcoin 0.32 #97
Conversation
df9fb78
to
e1e9aca
Compare
This also required to bump the ledger client to 0.5.0. I reverted some of the temp changes introduced when bumping the bitbox client, which were only required while bitcoin 0.31 was used elsewhere.
e1e9aca
to
b454228
Compare
I was able to test this using Liana GUI with a Ledger to create a wallet (both wsh and tr), register descriptor, confirm address and spend a coin. However, I think we need the following change since there is no longer "m/" in the string derivation path (this change still covers the case that there is "m/" present in some future version): diff --git a/src/ledger.rs b/src/ledger.rs
index 00a6f2b..26b14be 100644
--- a/src/ledger.rs
+++ b/src/ledger.rs
@@ -106,9 +115,9 @@ impl<T: Transport + Sync + Send> HWI for Ledger<T> {
let fg = self.get_master_fingerprint().await?;
let xpub = self.get_extended_pubkey(&path).await?;
let policy = format!(
- "tr([{}{}]{}/**)",
+ "tr([{}/{}]{}/**)",
fg,
- path.to_string().trim_start_matches('m'),
+ path.to_string().trim_start_matches("m/"),
xpub
);
let (descriptor_template, keys) = This can be confirmed by the following test, which fails without this change: diff --git a/src/ledger.rs b/src/ledger.rs
index 00a6f2b..26b14be 100644
--- a/src/ledger.rs
+++ b/src/ledger.rs
@@ -75,6 +75,15 @@ impl<T: 'static + Transport + Sync + Send> From<Ledger<T>> for Box<dyn HWI + Sen
}
}
+fn key_string_from_parts(fg: Fingerprint, path: DerivationPath, xpub: Xpub) -> String {
+ format!(
+ "[{}/{}]{}",
+ fg,
+ path.to_string().trim_start_matches("m/"),
+ xpub
+ )
+}
+
#[async_trait]
impl<T: Transport + Sync + Send> HWI for Ledger<T> {
fn device_kind(&self) -> DeviceKind {
@@ -313,3 +322,17 @@ impl<T: core::fmt::Debug> From<BitcoinClientError<T>> for HWIError {
HWIError::Device(format!("{:#?}", e))
}
}
+
+mod test {
+ use std::str::FromStr;
+
+ use super::*;
+
+ #[test]
+ fn test_key_string_from_parts() {
+ let path = DerivationPath::from_str("m/48'/1'/0'/2'").unwrap();
+ let fg = Fingerprint::from_hex("aabbccdd").unwrap();
+ let xpub = Xpub::from_str("tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N").unwrap();
+ assert_eq!(key_string_from_parts(fg, path, xpub), "[aabbccdd/48'/1'/0'/2']tpubDExA3EC3iAsPxPhFn4j6gMiVup6V2eH3qKyk69RcTc9TTNRfFYVPad8bJD5FCHVQxyBT4izKsvr7Btd2R4xmQ1hZkvsqGBaeE82J71uTK4N");
+ }
+}
|
humm, i'm wondering we can maybe overlook smth here, instead blindly adding a |
I'm not sure we need to check that explicitly as the |
4b284be
to
1fbc5be
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1fbc5be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK f59f447.
No description provided.