-
Notifications
You must be signed in to change notification settings - Fork 9
refactor: split key wallet #74
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
Changes from 4 commits
0467808
3dd8f79
2348b2c
d8f05ec
93d1bfa
18b98c0
ba7e02e
edc92b1
dca15ac
c1a3c53
5b43816
ce1787b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,7 @@ use core::{cmp, fmt}; | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use std::collections::{HashMap, HashSet}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::Amount; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::Network; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey, KeySource}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::blockdata::script::ScriptBuf; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::blockdata::transaction::Transaction; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -513,7 +514,16 @@ impl GetKey for ExtendedPrivKey { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| KeyRequest::Bip32((fingerprint, path)) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let key = if self.fingerprint(secp) == fingerprint { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let k = self.derive_priv(secp, &path)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Some(k.to_priv()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Some(PrivateKey { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| compressed: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| network: match k.network { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Dash => Network::Dash, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Testnet => Network::Testnet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Regtest => Network::Regtest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Devnet => Network::Devnet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inner: k.private_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Network conversion logic is duplicated and brittle – introduce a Three Refactor once: +impl From<key_wallet::Network> for Network {
+ fn from(n: key_wallet::Network) -> Self {
+ match n {
+ key_wallet::Network::Dash => Network::Dash,
+ key_wallet::Network::Testnet => Network::Testnet,
+ key_wallet::Network::Regtest => Network::Regtest,
+ key_wallet::Network::Devnet => Network::Devnet,
+ }
+ }
+}and then - network: match k.network { /* … */ },
+ network: k.network.into(),reducing maintenance effort and eliminating copy-paste errors. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -547,7 +557,16 @@ impl GetKey for $set<ExtendedPrivKey> { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| for xpriv in self.iter() { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if xpriv.parent_fingerprint == fingerprint { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let k = xpriv.derive_priv(secp, &path)?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Ok(Some(k.to_priv())); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return Ok(Some(PrivateKey { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| compressed: true, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| network: match k.network { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Dash => Network::Dash, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Testnet => Network::Testnet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Regtest => Network::Regtest, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| key_wallet::Network::Devnet => Network::Devnet, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| inner: k.private_key, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Ok(None) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -582,7 +601,7 @@ impl_get_key_for_map!(BTreeMap); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| impl_get_key_for_map!(HashMap); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// Errors when getting a key. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[derive(Clone, PartialEq, Eq, Debug)] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| #[non_exhaustive] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| pub enum GetKeyError { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| /// A bip32 error. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -829,7 +848,6 @@ mod tests { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use secp256k1::{All, SecretKey}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use super::*; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::Network::Dash; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::bip32::{ChildNumber, ExtendedPrivKey, ExtendedPubKey, KeySource}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::blockdata::script::ScriptBuf; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use crate::blockdata::transaction::Transaction; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -879,7 +897,8 @@ mod tests { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut hd_keypaths: BTreeMap<secp256k1::PublicKey, KeySource> = Default::default(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut sk: ExtendedPrivKey = ExtendedPrivKey::new_master(Dash, &seed).unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let mut sk: ExtendedPrivKey = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExtendedPrivKey::new_master(key_wallet::Network::Dash, &seed).unwrap(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let fprint = sk.fingerprint(secp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -131,7 +131,9 @@ fn build_extended_private_key() -> ExtendedPrivKey { | |
| let xpriv = ExtendedPrivKey::from_str(extended_private_key).unwrap(); | ||
|
|
||
| let sk = PrivateKey::from_wif(seed).unwrap(); | ||
| let seeded = ExtendedPrivKey::new_master(NETWORK, &sk.inner.secret_bytes()).unwrap(); | ||
| let seeded = | ||
| ExtendedPrivKey::new_master(key_wallet::Network::Testnet, &sk.inner.secret_bytes()) | ||
| .unwrap(); | ||
| assert_eq!(xpriv, seeded); | ||
|
|
||
| xpriv | ||
|
|
@@ -326,8 +328,17 @@ fn parse_and_verify_keys( | |
|
|
||
| let path = | ||
| derivation_path.into_derivation_path().expect("failed to convert derivation path"); | ||
| let derived_priv = | ||
| ext_priv.derive_priv(secp, &path).expect("failed to derive ext priv key").to_priv(); | ||
| let ext_derived = ext_priv.derive_priv(secp, &path).expect("failed to derive ext priv key"); | ||
| let derived_priv = PrivateKey { | ||
| compressed: true, | ||
| network: match ext_derived.network { | ||
| key_wallet::Network::Dash => Network::Dash, | ||
| key_wallet::Network::Testnet => Network::Testnet, | ||
| key_wallet::Network::Regtest => Network::Regtest, | ||
| key_wallet::Network::Devnet => Network::Devnet, | ||
| }, | ||
| inner: ext_derived.private_key, | ||
| }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid hand-rolled network mapping – implement Pattern-matching every time a conversion is needed is brittle and error-prone. Add a simple -let derived_priv = PrivateKey {
- compressed: true,
- network: match ext_derived.network {
- key_wallet::Network::Dash => Network::Dash,
- key_wallet::Network::Testnet => Network::Testnet,
- key_wallet::Network::Regtest => Network::Regtest,
- key_wallet::Network::Devnet => Network::Devnet,
- },
- inner: ext_derived.private_key,
-};
+let derived_priv = PrivateKey {
+ compressed: true,
+ network: ext_derived.network.into(),
+ inner: ext_derived.private_key,
+};This keeps the test lean and centralises future maintenance to a single location.
🤖 Prompt for AI Agents |
||
| assert_eq!(wif_priv, derived_priv); | ||
| let derived_pub = derived_priv.public_key(secp); | ||
| key_map.insert(derived_pub, derived_priv); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,29 @@ | ||||||||
| [package] | ||||||||
| name = "key-wallet-ffi" | ||||||||
| version = "0.39.6" | ||||||||
| authors = ["The Dash Core Developers"] | ||||||||
| edition = "2021" | ||||||||
| description = "FFI bindings for key-wallet library" | ||||||||
| keywords = ["dash", "wallet", "ffi", "bindings"] | ||||||||
| readme = "README.md" | ||||||||
| license = "CC0-1.0" | ||||||||
|
|
||||||||
| [lib] | ||||||||
| name = "key_wallet_ffi" | ||||||||
| crate-type = ["cdylib", "staticlib", "lib"] | ||||||||
|
|
||||||||
| [features] | ||||||||
| default = [] | ||||||||
|
|
||||||||
| [dependencies] | ||||||||
| key-wallet = { path = "../key-wallet", default-features = false, features = ["std"] } | ||||||||
| bitcoin_hashes = "0.14.0" | ||||||||
| secp256k1 = { version = "0.30.0", features = ["global-context"] } | ||||||||
| uniffi = { version = "0.27", features = ["cli"] } | ||||||||
| thiserror = "1.0" | ||||||||
|
|
||||||||
| [build-dependencies] | ||||||||
| uniffi = { version = "0.27", features = ["build"] } | ||||||||
|
|
||||||||
| [dev-dependencies] | ||||||||
| uniffi = { version = "0.27", features = ["bindgen-tests"] } | ||||||||
|
||||||||
| uniffi = { version = "0.27", features = ["bindgen-tests"] } | |
| uniffi = { version = "0.27", features = ["bindgen-tests"] } | |
Uh oh!
There was an error while loading. Please reload this page.