fix!: reject WIF keys without 0x01 flag, reject sighash types >0xff, maintain symmetry in {sign,recover} compact signatures, drop unused divergent segments

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## Unreleased
 
+### Added
+
+- Make `PublicKey::with_serialized` public
+
 ### Changed
 
 - **Breaking:** the `bincode` feature and binary serialization dependencies now use
@@ -47,6 +51,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - **Breaking:** `dashcore::base58` is `base58ck` 0.5.0, so `Base58CkString`
   replaces the `encode_*` functions and the `Base58` error variants carry
   `base58::DecodeCheckError`
+- **Breaking:** `ExtendedPrivKey::new_master` rejects seeds outside 16 to 64
+  bytes with `bip32::Error::InvalidSeedLength`, which
+  `RootExtendedPrivKey::new_master` returns instead of `InvalidParameter`
+- **Breaking:** `MessageSignature::from_slice` rejects headers above 34
 - Relicense `dash-network`, `dash-network-seeds`, `dash-spv`, `dash-spv-ffi`,
   `dash-spv-bench`, `git-state` and `masternode-seeds-fetcher` to CC0-1.0
 - All workspace crates declare a MSRV of 1.89
@@ -58,6 +66,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - `transaction_sighash` in `key-wallet-ffi` hashes non-standard flags as given
   and rejects flags above `0xff`
 - Reject 256-bit child numbers in `key-wallet` with an odd number of hex digits
+- Reject WIF keys whose compression flag is not `0x01`, with
+  `key::Error::InvalidWifCompressionFlag`
+- Accept an IL of zero in `key-wallet` private child derivation
+- `transaction_sign_input` in `key-wallet-ffi` rejects flags above `0xff`
 
 ### Removed
 
@@ -74,6 +86,14 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 - **Breaking:** remove the `hex` module from `dashcore-private`, and the
   `btreemap_byte_values`, `btreemap_as_seq`, `btreemap_as_seq_byte_values` and
   `hex_bytes` serde helpers from `dashcore::serde_utils`
+- **Breaking:** remove ordering from `PublicKey`, `ExtendedPubKey` and
+  `PubKeyOrAddress`, and `PublicKey::to_sort_key` and `SortKey` with it
+- **Breaking:** remove `PublicKey::read_from` and
+  `Witness::push_bitcoin_signature`
+- **Breaking:** remove `key::Error::NotSupported`,
+  `bip32::Error::NotSupported` and `dashcore_rpc::Error::Secp256k1`
+- **Breaking:** remove `signer::CompactSignature`. Use `MessageSignature`
+  instead
 
 ## 0.44.0 - 2026-07-01
 

dash/src/blockdata/script/builder.rs

@@ -73,11 +73,9 @@ impl Builder {
 
     /// Adds instructions to push a public key onto the stack.
     pub fn push_key(self, key: &PublicKey) -> Builder {
-        if key.compressed {
-            self.push_slice(key.inner.serialize())
-        } else {
-            self.push_slice(key.inner.serialize_uncompressed())
-        }
+        key.with_serialized(|bytes| {
+            self.push_slice(<&PushBytes>::try_from(bytes).expect("a key is 33 or 65 bytes"))
+        })
     }
 
     /// Adds instructions to push an XOnly public key onto the stack.

dash/src/blockdata/witness.rs

@@ -10,13 +10,11 @@ use core::ops::Index;
 
 #[cfg(feature = "bincode")]
 use bincode::{Decode, Encode};
-use secp256k1::ecdsa;
 
 use crate::consensus::encode::{Error, MAX_VEC_SIZE};
 use crate::consensus::{Decodable, Encodable, WriteExt};
 use crate::io::{self, Read, Write};
 use crate::prelude::*;
-use crate::sighash::EcdsaSighashType;
 use crate::taproot::TAPROOT_ANNEX_PREFIX;
 use crate::{Script, VarInt};
 
@@ -275,20 +273,6 @@ impl Witness {
         self.content[end_varint..end_varint + new_element.len()].copy_from_slice(new_element);
     }
 
-    /// Pushes a DER-encoded ECDSA signature with a signature hash type as a new element on the
-    /// witness, requires an allocation.
-    pub fn push_bitcoin_signature(
-        &mut self,
-        signature: &ecdsa::SerializedSignature,
-        hash_type: EcdsaSighashType,
-    ) {
-        // Note that a maximal length ECDSA signature is 72 bytes, plus the sighash type makes 73
-        let mut sig = [0; 73];
-        sig[..signature.len()].copy_from_slice(signature);
-        sig[signature.len()] = hash_type.to_consensus_u8();
-        self.push(&sig[..signature.len() + 1]);
-    }
-
     fn element_at(&self, index: usize) -> Option<&[u8]> {
         let varint = VarInt::consensus_decode(&mut &self.content[index..]).ok()?;
         let start = index + varint.len();
@@ -496,8 +480,6 @@ impl From<Vec<&[u8]>> for Witness {
 
 #[cfg(test)]
 mod test {
-    use secp256k1::ecdsa;
-
     use super::*;
     use crate::Transaction;
     use crate::consensus::{deserialize, serialize};
@@ -580,21 +562,6 @@ mod test {
         }
     }
 
-    #[test]
-    fn test_push_ecdsa_sig() {
-        // The very first signature in block 734,958
-        let sig_bytes = hex!(
-            "304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b541"
-        );
-        let sig = ecdsa::Signature::from_der(&sig_bytes).unwrap();
-        let mut witness = Witness::default();
-        witness.push_bitcoin_signature(&sig.serialize_der(), EcdsaSighashType::All);
-        let expected_witness = vec![hex!(
-            "304402207c800d698f4b0298c5aac830b822f011bb02df41eb114ade9a6702f364d5e39c0220366900d2a60cab903e77ef7dd415d46509b1f78ac78906e3296f495aa1b1b54101"
-        )];
-        assert_eq!(witness.to_vec(), expected_witness);
-    }
-
     #[test]
     fn test_witness() {
         let w0 = hex!("03d2e15674941bad4a996372cb87e1856d3652606d98562fe39c5e9e7e413f2105");

dash/src/crypto/key.rs

@@ -17,7 +17,20 @@ mod tests {
     use crate::Network::{Mainnet, Testnet};
     use crate::address::Address;
     use crate::crypto::key::{PrivateKey, PublicKey};
-    use crate::io;
+
+    /// Core's `DecodeSecret` takes a 34-byte payload only when it ends in
+    /// the 0x01 compression flag.
+    #[test_case::test_matrix([0x00, 0x02, 0xff])]
+    fn wif_compression_flag_must_be_one(flag: u8) {
+        let mut data =
+            crate::base58::decode_check("cVt4o7BGAig1UXywgGSmARhxMdzP5qvQsxKkSsc1XEkw3tDTQFpy")
+                .unwrap();
+        data[33] = flag;
+        assert_eq!(
+            PrivateKey::from_wif(crate::base58::Base58CkString::encode_unbounded(&data).as_str()),
+            Err(crate::crypto::key::Error::InvalidWifCompressionFlag(flag))
+        );
+    }
 
     #[test]
     fn test_key_derivation() {
@@ -139,55 +152,6 @@ mod tests {
         assert_tokens(&pk_u.readable(), &[Token::BorrowedStr(PK_STR_U)]);
     }
 
-    fn random_key(mut seed: u8) -> PublicKey {
-        loop {
-            let mut data = [0; 65];
-            for byte in &mut data[..] {
-                *byte = seed;
-                // totally a rng
-                seed = seed.wrapping_mul(41).wrapping_add(43);
-            }
-            if data[0] % 2 == 0 {
-                data[0] = 4;
-                if let Ok(key) = PublicKey::from_slice(&data[..]) {
-                    return key;
-                }
-            } else {
-                data[0] = 2 + (data[0] >> 7);
-                if let Ok(key) = PublicKey::from_slice(&data[..33]) {
-                    return key;
-                }
-            }
-        }
-    }
-
-    #[test]
-    fn pubkey_read_write() {
-        const N_KEYS: usize = 20;
-        let keys: Vec<_> = (0..N_KEYS).map(|i| random_key(i as u8)).collect();
-
-        let mut v = vec![];
-        for k in &keys {
-            k.write_into(&mut v).expect("writing into vec");
-        }
-
-        let mut dec_keys = vec![];
-        let mut cursor = io::Cursor::new(&v);
-        for _ in 0..N_KEYS {
-            dec_keys.push(PublicKey::read_from(&mut cursor).expect("reading from vec"));
-        }
-
-        assert_eq!(keys, dec_keys);
-
-        // sanity checks
-        assert!(PublicKey::read_from(&mut cursor).is_err());
-        assert!(PublicKey::read_from(io::Cursor::new(&[])).is_err());
-        assert!(PublicKey::read_from(io::Cursor::new(&[0; 33][..])).is_err());
-        assert!(PublicKey::read_from(io::Cursor::new(&[2; 32][..])).is_err());
-        assert!(PublicKey::read_from(io::Cursor::new(&[0; 65][..])).is_err());
-        assert!(PublicKey::read_from(io::Cursor::new(&[4; 64][..])).is_err());
-    }
-
     #[test]
     #[cfg(feature = "rand-std")]
     fn public_key_constructors() {

dash/src/sign_message.rs

@@ -135,8 +135,9 @@ mod message_signing {
             if bytes.len() != 65 {
                 return Err(MessageSignatureError::InvalidLength);
             }
-            // We just check this here so we can safely subtract further.
-            if bytes[0] < 27 {
+            // Only headers 27..=34 encode a recovery id and a compression
+            // flag.
+            if !(27..=34).contains(&bytes[0]) {
                 return Err(MessageSignatureError::InvalidEncoding(
                     secp256k1::Error::InvalidRecoveryId,
                 ));
@@ -283,6 +284,17 @@ mod tests {
         );
     }
 
+    #[test_case::test_matrix([26, 35])]
+    #[cfg(feature = "secp-recovery")]
+    fn test_message_signature_header_out_of_range(header: u8) {
+        let mut bytes = [0u8; 65];
+        bytes[0] = header;
+        assert_eq!(
+            MessageSignature::from_slice(&bytes),
+            Err(MessageSignatureError::InvalidEncoding(secp256k1::Error::InvalidRecoveryId))
+        );
+    }
+
     #[test]
     #[cfg(all(feature = "secp-recovery", feature = "base64"))]
     fn test_incorrect_message_signature() {

dash/src/signer.rs

@@ -3,12 +3,13 @@
 use core::convert::TryInto;
 
 use anyhow::{anyhow, bail};
-use hashes::{Hash, ripemd160, sha256, sha256d};
+use hashes::{Hash, hash160, sha256d};
 
 use crate::PublicKey as ECDSAPublicKey;
 use crate::prelude::Vec;
-use crate::secp256k1::ecdsa::{RecoverableSignature, RecoveryId};
+use crate::secp256k1::ecdsa::RecoverableSignature;
 use crate::secp256k1::{Message, SecretKey};
+use crate::sign_message::MessageSignature;
 
 /// verifies the ECDSA signature
 /// The provided signature must be recoverable. Which means: it must contain the recovery byte as a prefix
@@ -21,7 +22,7 @@ pub fn verify_data_signature(
 
     let msg =
         Message::from_digest(data_hash.try_into().map_err(|_| anyhow!("Invalid hash length"))?);
-    let sig: RecoverableSignature = RecoverableSignature::from_compact_signature(signature)?;
+    let sig = MessageSignature::from_slice(signature)?.signature;
 
     let pub_key = ECDSAPublicKey::from_slice(public_key).map_err(anyhow::Error::msg)?;
 
@@ -36,16 +37,14 @@ pub fn verify_hash_signature(
     data_signature: &[u8],
     public_key_id: &[u8],
 ) -> Result<(), anyhow::Error> {
-    let signature: RecoverableSignature =
-        RecoverableSignature::from_compact_signature(data_signature)?;
+    let signature = MessageSignature::from_slice(data_signature)?.signature;
 
     let msg =
         Message::from_digest(data_hash.try_into().map_err(|_| anyhow!("Invalid hash length"))?);
     let recovered_public_key = signature.recover(msg).map_err(anyhow::Error::msg)?;
 
-    let recovered_compressed_public_key = recovered_public_key.serialize();
-    let hash_recovered_key = ripemd160_sha256(&recovered_compressed_public_key);
-    let are_equal = public_key_id == hash_recovered_key;
+    let hash_recovered_key = ECDSAPublicKey::new(recovered_public_key).pubkey_hash();
+    let are_equal = public_key_id == hash_recovered_key.as_byte_array();
 
     if are_equal {
         Ok(())
@@ -71,58 +70,9 @@ pub fn sign_hash(data_hash: &[u8], private_key: &[u8]) -> Result<[u8; 65], anyho
     let msg =
         Message::from_digest(data_hash.try_into().map_err(|_| anyhow!("Invalid hash length"))?);
 
-    let signature = RecoverableSignature::sign_ecdsa_recoverable(msg, &pk)
-        // TODO the compression flag should be obtained from the private key type
-        .to_compact_signature(true);
-    Ok(signature)
-}
-
-/// converts the signature from/to compact format. Compact format is when the signature
-/// is prefixed by the recovery byte
-pub trait CompactSignature
-where
-    Self: Sized,
-{
-    /// Converts the Signature with Recovery byte to the compact format where
-    /// the first byte of signature is occupied by the recovery byte
-    fn to_compact_signature(&self, is_compressed: bool) -> [u8; 65];
-    /// Creates the Self from compacted version of signature
-    fn from_compact_signature(signature: impl AsRef<[u8]>) -> Result<Self, anyhow::Error>;
-}
-
-impl CompactSignature for RecoverableSignature {
-    fn from_compact_signature(signature: impl AsRef<[u8]>) -> Result<Self, anyhow::Error> {
-        if signature.as_ref().len() != 65 {
-            bail!("the signature must be 65 bytes long")
-        }
-
-        let recovery_byte = signature.as_ref()[0];
-        let number = u8::from_be(recovery_byte) as i32;
-        let mut i = number - 27 - 4;
-        if i < 0 {
-            i += 4;
-        }
-        if !((i == 0) || (i == 1) || (i == 2) || (i == 3)) {
-            bail!("the recovery number must be between 0..4, got: '{}'", i);
-        }
-
-        RecoverableSignature::from_compact(
-            &signature.as_ref()[1..],
-            RecoveryId::try_from(i).unwrap(),
-        )
-        .map_err(anyhow::Error::msg)
-    }
-
-    fn to_compact_signature(&self, is_compressed: bool) -> [u8; 65] {
-        let (recovery_byte, signature) = self.serialize_compact();
-        let mut val = i32::from(recovery_byte.to_u8()) + 27 + 4;
-        if !is_compressed {
-            val -= 4;
-        }
-        let prefix = val.to_le_bytes()[0];
-        let compact_signature = [&[prefix], signature.as_slice()].concat();
-        compact_signature.try_into().unwrap()
-    }
+    let signature = RecoverableSignature::sign_ecdsa_recoverable(msg, &pk);
+    // TODO the compression flag should be obtained from the private key type
+    Ok(MessageSignature::new(signature, true).serialize())
 }
 
 /// calculates double sha256 on data
@@ -132,8 +82,7 @@ pub fn double_sha(payload: impl AsRef<[u8]>) -> Vec<u8> {
 
 /// calculates the RIPEMD169(SHA256(data))
 pub fn ripemd160_sha256(data: &[u8]) -> Vec<u8> {
-    let hash = sha256::Hash::hash(data).to_byte_array();
-    ripemd160::Hash::hash(&hash).to_byte_array().to_vec()
+    hash160::Hash::hash(data).to_byte_array().to_vec()
 }
 
 #[cfg(test)]
@@ -276,6 +225,6 @@ mod test {
         let validation_result =
             verify_data_signature(&data, &unrecoverable_signature_bytes, &k.public_key_compressed);
 
-        assert_error_contains!(validation_result, "the signature must be 65 bytes long")
+        assert_error_contains!(validation_result, "length not 65 bytes")
     }
 }

key-wallet-ffi/Cargo.toml

@@ -47,3 +47,4 @@ cbindgen = "0.29"
 dashcore = { path = "../dash", features = ["test-utils"] }
 key-wallet = { path = "../key-wallet", features = ["test-utils"] }
 key-wallet-manager = { path = "../key-wallet-manager", features = ["test-utils"] }
+test-case = "3.3"

key-wallet-ffi/FFI_API.md

@@ -3802,7 +3802,7 @@ transaction_sign_input(tx: *mut FFITransaction, input_index: u32, private_key: *
 ```
 
 **Description:**
-Sign a transaction input  # Safety - `tx` must be a valid pointer to an FFITransaction - `private_key` must be a valid pointer to a 32-byte private key - `script_pubkey` must be a valid pointer to the script pubkey  # Returns - 0 on success - -1 on error
+Sign a transaction input  # Safety - `tx` must be a valid pointer to an FFITransaction - `private_key` must be a valid pointer to a 32-byte private key - `script_pubkey` must be a valid pointer to the script pubkey  # Returns - 0 on success - -1 on error, including a `sighash_type` above 0xff
 
 **Safety:**
 - `tx` must be a valid pointer to an FFITransaction - `private_key` must be a valid pointer to a 32-byte private key - `script_pubkey` must be a valid pointer to the script pubkey

key-wallet-ffi/src/account_derivation.rs

@@ -262,11 +262,7 @@ pub unsafe extern "C" fn account_derive_private_key_as_wif_at(
         account.inner().derive_from_master_xpriv_extended_xpriv_at(master_xpriv.inner(), index),
         error
     );
-    let dash_priv = dashcore::PrivateKey {
-        compressed: true,
-        network: account.inner().network(),
-        inner: derived.private_key,
-    };
+    let dash_priv = dashcore::PrivateKey::new(derived.private_key, account.inner().network());
     unwrap_or_return!(CString::new(dash_priv.to_wif()), error).into_raw()
 }