diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index 0a56e4029..69e7d1d45 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -647,7 +647,7 @@ impl FFIDerivedAddress { pool_type: FFIDerivedAddressPoolType::from(d.pool_type), derivation_index: d.derivation_index, address: c_address.into_raw(), - public_key: d.public_key, + public_key: d.public_key.inner.serialize(), } }) .collect() diff --git a/key-wallet-manager/src/event_tests.rs b/key-wallet-manager/src/event_tests.rs index c8c33fca6..6bd2f3d9e 100644 --- a/key-wallet-manager/src/event_tests.rs +++ b/key-wallet-manager/src/event_tests.rs @@ -12,10 +12,11 @@ use dashcore::hash_types::CycleHash; use dashcore::hashes::Hash; use dashcore::opcodes; use dashcore::{ - BlockHash, CompactTarget, OutPoint, ScriptBuf, TxIn, TxMerkleNode, TxOut, Txid, Witness, + BlockHash, CompactTarget, OutPoint, PublicKey, ScriptBuf, TxIn, TxMerkleNode, TxOut, Txid, + Witness, }; use key_wallet::account::StandardAccountType; -use key_wallet::managed_account::address_pool::AddressPoolType; +use key_wallet::managed_account::address_pool::{AddressPoolType, PublicKeyType}; use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::managed_account::managed_account_type::ManagedAccountType; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; @@ -820,17 +821,13 @@ async fn test_mempool_tx_to_highest_external_carries_addresses_derived() { derived.derivation_index ); let stored_pubkey = match stored.public_key.as_ref().expect("ECDSA pool stores pubkey") { - key_wallet::managed_account::address_pool::PublicKeyType::ECDSA(b) => b, + PublicKeyType::ECDSA(b) => b, other => panic!("BIP44 external pool produced non-ECDSA key: {:?}", other), }; + let expected = PublicKey::from_slice(stored_pubkey) + .expect("BIP44 external pool must store a valid compressed ECDSA key"); assert_eq!( - stored_pubkey.len(), - 33, - "BIP44 external pool must store 33-byte compressed keys" - ); - assert_eq!( - &derived.public_key[..], - stored_pubkey.as_slice(), + derived.public_key, expected, "public key mismatch at index {}", derived.derivation_index ); diff --git a/key-wallet-manager/src/events.rs b/key-wallet-manager/src/events.rs index 92a3f88f6..0080b3979 100644 --- a/key-wallet-manager/src/events.rs +++ b/key-wallet-manager/src/events.rs @@ -10,7 +10,7 @@ use std::fmt; use dashcore::ephemerealdata::chain_lock::ChainLock; use dashcore::ephemerealdata::instant_lock::InstantLock; use dashcore::prelude::CoreBlockHeight; -use dashcore::Txid; +use dashcore::{PublicKey, Txid}; use key_wallet::account::AccountType; use key_wallet::managed_account::address_pool::{AddressPoolType, PublicKeyType}; use key_wallet::managed_account::transaction_record::TransactionRecord; @@ -45,20 +45,20 @@ pub struct DerivedAddress { pub derivation_index: u32, /// The derived address. pub address: dashcore::Address, - /// Compressed ECDSA public key (33 bytes). Non-ECDSA pools + /// ECDSA public key for the derived address. Non-ECDSA pools /// (BLS / EdDSA) are skipped during projection. - pub public_key: [u8; 33], + pub public_key: PublicKey, } impl DerivedAddress { /// Project a [`DerivedAddressInfo`] from key-wallet into a /// `DerivedAddress` event payload. Returns `None` for non-ECDSA pools - /// (BLS / EdDSA) since the event field carries a 33-byte compressed - /// key — those pools don't trigger gap-limit extension on Core - /// transactions in practice, but skip rather than panic if they do. - /// Drops are logged so a future change that wires gap-limit extension - /// into a BLS / EdDSA pool surfaces in traces rather than silently - /// orphaning UTXOs at the persister. + /// (BLS / EdDSA) since the event field carries an ECDSA key. Those + /// pools don't trigger gap-limit extension on Core transactions in + /// practice, but skip rather than panic if they do. Drops are logged + /// so a future change that wires gap-limit extension into a BLS / + /// EdDSA pool surfaces in traces rather than silently orphaning UTXOs + /// at the persister. pub(crate) fn from_info(derived: DerivedAddressInfo) -> Option { let account_type = derived.account_type; let pool_type = derived.pool_type; @@ -73,31 +73,29 @@ impl DerivedAddress { ); return None; } - Some(PublicKeyType::ECDSA(bytes)) => { - if bytes.len() != 33 { + Some(PublicKeyType::ECDSA(bytes)) => match PublicKey::from_slice(bytes) { + Ok(pk) => pk, + Err(err) => { // Producer (`generate_address_at_index`) always stores - // 33-byte compressed keys, so a length mismatch is a - // bug, not an expected drop. + // valid compressed keys, so a parse failure is a bug, + // not an expected drop. tracing::warn!( ?account_type, ?pool_type, index, - len = bytes.len(), - "dropping derived address: ECDSA public key is not 33 bytes" + %err, + "dropping derived address: ECDSA public key failed to parse" ); return None; } - let mut arr = [0u8; 33]; - arr.copy_from_slice(bytes); - arr - } + }, Some(PublicKeyType::BLS(_)) | Some(PublicKeyType::EdDSA(_)) => { tracing::debug!( ?account_type, ?pool_type, index, "dropping non-ECDSA derived address from event projection \ - (event field is 33-byte compressed ECDSA only)" + (event field is ECDSA only)" ); return None; } @@ -425,30 +423,34 @@ mod project_derived_addresses_tests { use key_wallet::managed_account::address_pool::AddressInfo; use std::collections::BTreeMap; - /// Compressed encoding of the secp256k1 generator point — a known - /// on-curve 33-byte value `dashcore::PublicKey::from_slice` accepts. - /// Tests don't care which key it is; they care that projection - /// preserves the bytes round-trip. + /// Compressed encoding of the secp256k1 generator point (G). const TEST_PUBKEY_G: [u8; 33] = [ 0x02, 0x79, 0xbe, 0x66, 0x7e, 0xf9, 0xdc, 0xbb, 0xac, 0x55, 0xa0, 0x62, 0x95, 0xce, 0x87, 0x0b, 0x07, 0x02, 0x9b, 0xfc, 0xdb, 0x2d, 0xce, 0x28, 0xd9, 0x59, 0xf2, 0x81, 0x5b, 0x16, 0xf8, 0x17, 0x98, ]; + /// Compressed encoding of 2G. A second well-known on-curve point, + /// used to distinguish two `DerivedAddressInfo` entries when testing + /// dedup behavior. The point must be on-curve so the projection's + /// `PublicKey::from_slice` parse succeeds. + const TEST_PUBKEY_2G: [u8; 33] = [ + 0x02, 0xc6, 0x04, 0x7f, 0x94, 0x41, 0xed, 0x7d, 0x6d, 0x30, 0x45, 0x40, 0x6e, 0x95, 0xc0, + 0x7c, 0xd8, 0x5c, 0x77, 0x8e, 0x4b, 0x8c, 0xef, 0x3c, 0xa7, 0xab, 0xac, 0x09, 0xb9, 0x5c, + 0x70, 0x9e, 0xe5, + ]; + /// Build a stub `DerivedAddressInfo` for unit-testing the projection - /// without spinning up a wallet. `tag` differentiates the - /// `public_key` so "first-seen wins" can be observed across - /// duplicates; pass `None` to use the default valid key. + /// without spinning up a wallet. `alt_key` toggles between G and 2G + /// so "first-seen wins" can be observed across duplicates. fn make_derived( account_type: AccountType, pool_type: AddressPoolType, index: u32, - tag: Option, + alt_key: bool, ) -> DerivedAddressInfo { - // Take a real compressed pubkey and tweak only the `public_key` - // bytes via `tag` (which the projection passes through verbatim). - // We don't actually re-derive the address from the tagged bytes — - // tests don't depend on address↔pubkey consistency, only on + // Always derive the address from `G` regardless of `alt_key`. + // Tests don't depend on address↔pubkey consistency, only on // dedup keys and pubkey round-trip. let pubkey = dashcore::PublicKey::from_slice(&TEST_PUBKEY_G).expect("generator point is valid"); @@ -458,12 +460,11 @@ mod project_derived_addresses_tests { ChildNumber::from_normal_idx(0).unwrap(), ChildNumber::from_normal_idx(index).unwrap(), ]); - let mut pubkey_bytes = TEST_PUBKEY_G.to_vec(); - if let Some(t) = tag { - // Tag a non-prefix byte so `[u8; 33]` round-trip is observable - // without affecting the leading `0x02` compressed marker. - pubkey_bytes[32] = t; - } + let pubkey_bytes = if alt_key { + TEST_PUBKEY_2G.to_vec() + } else { + TEST_PUBKEY_G.to_vec() + }; DerivedAddressInfo { account_type, pool_type, @@ -501,21 +502,16 @@ mod project_derived_addresses_tests { #[test] fn project_dedups_overlapping_keys() { let acct = standard_account_0(); - let first = make_derived(acct, AddressPoolType::External, 5, Some(0xaa)); - let first_bytes = match first.info.public_key.as_ref().expect("ECDSA pubkey") { - PublicKeyType::ECDSA(b) => b.clone(), - _ => unreachable!(), - }; - let second = make_derived(acct, AddressPoolType::External, 5, Some(0xbb)); + let first = make_derived(acct, AddressPoolType::External, 5, false); + let second = make_derived(acct, AddressPoolType::External, 5, true); let projected = project_derived_addresses(vec![first, second]); assert_eq!(projected.len(), 1, "duplicate (account, pool, index) must dedup to one entry"); assert_eq!(projected[0].account_type, acct); assert_eq!(projected[0].pool_type, AddressPoolType::External); assert_eq!(projected[0].derivation_index, 5); - // First-seen wins: surviving pubkey matches `first`, not `second`. - assert_eq!(&projected[0].public_key[..], first_bytes.as_slice()); - assert_eq!(projected[0].public_key[32], 0xaa); + let expected = PublicKey::from_slice(&TEST_PUBKEY_G).expect("G is valid"); + assert_eq!(projected[0].public_key, expected); } /// Different indices on the same pool must NOT dedup. @@ -523,9 +519,9 @@ mod project_derived_addresses_tests { fn project_keeps_distinct_indices_on_same_pool() { let acct = standard_account_0(); let infos = vec![ - make_derived(acct, AddressPoolType::External, 5, None), - make_derived(acct, AddressPoolType::External, 6, None), - make_derived(acct, AddressPoolType::External, 7, None), + make_derived(acct, AddressPoolType::External, 5, false), + make_derived(acct, AddressPoolType::External, 6, false), + make_derived(acct, AddressPoolType::External, 7, false), ]; let projected = project_derived_addresses(infos); assert_eq!(projected.len(), 3); @@ -540,8 +536,8 @@ mod project_derived_addresses_tests { fn project_keeps_same_index_across_different_pools() { let acct = standard_account_0(); let projected = project_derived_addresses(vec![ - make_derived(acct, AddressPoolType::External, 5, None), - make_derived(acct, AddressPoolType::Internal, 5, None), + make_derived(acct, AddressPoolType::External, 5, false), + make_derived(acct, AddressPoolType::Internal, 5, false), ]); assert_eq!(projected.len(), 2); let pools: std::collections::BTreeSet = @@ -555,9 +551,19 @@ mod project_derived_addresses_tests { /// length 0 is the contract — no panic. #[test] fn project_drops_entry_with_missing_pubkey() { - let mut info = make_derived(standard_account_0(), AddressPoolType::External, 5, None); + let mut info = make_derived(standard_account_0(), AddressPoolType::External, 5, false); info.info.public_key = None; let projected = project_derived_addresses(vec![info]); assert!(projected.is_empty()); } + + /// 33 bytes that don't parse as a valid compressed secp256k1 point + /// must drop rather than panic. + #[test] + fn project_drops_entry_with_invalid_curve_point() { + let mut info = make_derived(standard_account_0(), AddressPoolType::External, 5, false); + info.info.public_key = Some(PublicKeyType::ECDSA(vec![0xff; 33])); + let projected = project_derived_addresses(vec![info]); + assert!(projected.is_empty()); + } }