Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[workspace]
members = ["dash", "hashes", "internals", "fuzz", "rpc-client", "rpc-json", "rpc-integration-test"]
members = ["dash", "hashes", "internals", "fuzz", "rpc-client", "rpc-json", "rpc-integration-test", "key-wallet", "key-wallet-ffi"]
resolver = "2"

[workspace.package]
Expand Down
11 changes: 5 additions & 6 deletions dash/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ default = [ "std", "secp-recovery", "bincode" ]
base64 = [ "base64-compat" ]
rand-std = ["secp256k1/rand"]
rand = ["secp256k1/rand"]
serde = ["actual-serde", "dashcore_hashes/serde", "secp256k1/serde"]
serde = ["actual-serde", "dashcore_hashes/serde", "secp256k1/serde", "key-wallet/serde"]
secp-lowmemory = ["secp256k1/lowmemory"]
secp-recovery = ["secp256k1/recovery"]
signer = ["secp-recovery", "rand", "base64"]
Expand All @@ -32,14 +32,14 @@ bls = ["blsful"]
eddsa = ["ed25519-dalek"]
quorum_validation = ["bls", "bls-signatures"]
message_verification = ["bls"]
bincode = [ "dep:bincode", "dashcore_hashes/bincode" ]
bincode = [ "dep:bincode", "dep:bincode_derive", "dashcore_hashes/bincode" ]

# At least one of std, no-std must be enabled.
#
# The no-std feature doesn't disable std - you need to turn off the std feature for that by disabling default.
# Instead no-std enables additional features required for this crate to be usable without std.
# As a result, both can be enabled without conflict.
std = ["secp256k1/std", "dashcore_hashes/std", "bech32/std", "internals/std"]
std = ["secp256k1/std", "dashcore_hashes/std", "bech32/std", "internals/std", "key-wallet/std"]
no-std = ["core2", "dashcore_hashes/alloc", "dashcore_hashes/core2", "secp256k1/alloc"]

[package.metadata.docs.rs]
Expand All @@ -51,6 +51,7 @@ internals = { path = "../internals", package = "dashcore-private" }
bech32 = { version = "0.9.1", default-features = false }
dashcore_hashes = { path = "../hashes", default-features = false }
secp256k1 = { default-features = false, features = ["hashes"], version= "0.30.0" }
key-wallet = { path = "../key-wallet", default-features = false, features = ["std"] }
core2 = { version = "0.4.0", optional = true, features = ["alloc"], default-features = false }
rustversion = { version="1.0.20"}
# Do NOT use this as a feature! Use the `serde` feature instead.
Expand All @@ -62,6 +63,7 @@ hex_lit = "0.1.1"
anyhow = { version= "1.0" }
hex = { version= "0.4" }
bincode = { version= "=2.0.0-rc.3", optional = true }
bincode_derive = { version= "=2.0.0-rc.3", optional = true }
bitflags = "2.9.0"
blsful = { version = "3.0.0-pre8", optional = true }
ed25519-dalek = { version = "2.1", features = ["rand_core"], optional = true }
Expand All @@ -79,9 +81,6 @@ bincode = { version= "=2.0.0-rc.3" }
assert_matches = "1.5.0"
dashcore = { path = ".", features = ["core-block-hash-use-x11", "message_verification", "quorum_validation", "signer"] }

[[example]]
name = "bip32"

[[example]]
name = "handshake"
required-features = ["std"]
Expand Down
58 changes: 0 additions & 58 deletions dash/examples/bip32.rs

This file was deleted.

6 changes: 4 additions & 2 deletions dash/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,13 +103,15 @@ pub mod amount;
pub mod base58;
pub mod bip152;
pub mod bip158;
pub mod bip32;
// Re-export bip32 from key-wallet
pub use key_wallet::bip32;
pub mod blockdata;
pub mod consensus;
// Private until we either make this a crate or flatten it - still to be decided.
pub mod bls_sig_utils;
pub(crate) mod crypto;
mod dip9;
// Re-export dip9 from key-wallet
use key_wallet::dip9;
pub mod ephemerealdata;
pub mod error;
pub mod hash_types;
Expand Down
2 changes: 1 addition & 1 deletion dash/src/psbt/map/global.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ impl Map for PartiallySignedTransaction {
},
value: {
let mut ret = Vec::with_capacity(4 + derivation.len() * 4);
ret.extend(fingerprint.as_bytes());
ret.extend(fingerprint.to_bytes());
derivation.into_iter().for_each(|n| ret.extend(&u32::from(*n).to_le_bytes()));
ret
},
Expand Down
28 changes: 23 additions & 5 deletions dash/src/psbt/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use std::collections::{HashMap, HashSet};

use crate::Amount;
use crate::bip32::{self, ExtendedPrivKey, ExtendedPubKey, KeySource};
use crate::Network;
use crate::blockdata::script::ScriptBuf;
use crate::blockdata::transaction::Transaction;
use crate::blockdata::transaction::txout::TxOut;
Expand Down Expand Up @@ -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,
})
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Network conversion logic is duplicated and brittle – introduce a From impl

Three match blocks convert key_wallet::Networkcrate::Network.
Every new network variant will require editing multiple sites and rebuilding the GetKey impls.

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.
Do the same in the BTreeSet impl below.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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,
})
// Add this once (e.g. at the top of dash/src/psbt/mod.rs)
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,
}
}
}
// …later, in the existing PSBT mapping…
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,
- },
+ network: k.network.into(),
inner: k.private_key,
})
🤖 Prompt for AI Agents
In dash/src/psbt/mod.rs around lines 517 to 526, the network conversion from
key_wallet::Network to crate::Network is duplicated and fragile due to repeated
match statements. To fix this, implement the From trait for converting
key_wallet::Network into crate::Network once, then replace all match blocks with
calls to this From implementation. Apply the same refactoring to the BTreeSet
implementation to centralize and simplify network conversions, reducing
maintenance and preventing errors.

} else {
None
};
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -879,7 +897,7 @@ 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);

Expand Down
15 changes: 12 additions & 3 deletions dash/tests/psbt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ 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
Expand Down Expand Up @@ -326,8 +326,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,
};
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid hand-rolled network mapping – implement From once.

Pattern-matching every time a conversion is needed is brittle and error-prone. Add a simple From impl (or TryFrom if ever fallible) inside dashcore and use ext_derived.network.into() here.

-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.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In dash/tests/psbt.rs around lines 330 to 339, the network conversion is done
via manual pattern matching, which is error-prone. To fix this, implement a From
trait for converting key_wallet::Network to Network inside the dashcore module,
then replace the match expression with a simple call to
ext_derived.network.into() to centralize and simplify the conversion logic.

assert_eq!(wif_priv, derived_priv);
let derived_pub = derived_priv.public_key(secp);
key_map.insert(derived_pub, derived_priv);
Expand Down
29 changes: 29 additions & 0 deletions key-wallet-ffi/Cargo.toml
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"]

[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"] }
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uniffi = { version = "0.27", features = ["bindgen-tests"] }
uniffi = { version = "0.27", features = ["bindgen-tests"] }

Loading
Loading