diff --git a/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs b/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs index 3c3a18f0e97..4a01e667424 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs @@ -3,10 +3,12 @@ //! Validates that the sender and recipient identities have the correct key //! types and purposes before a contact request is submitted to the platform. -use dash_sdk::platform::dashpay::recipient_key_purpose_is_valid; +use dash_sdk::platform::dashpay::{ + recipient_key_purpose_is_acceptable_on_receive, sender_key_purpose_is_acceptable_on_receive, +}; use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; -use dpp::identity::{Identity, KeyType, Purpose}; +use dpp::identity::{Identity, KeyType}; /// Result of validating a contact request before it is sent. #[derive(Debug, Clone)] @@ -110,32 +112,51 @@ impl ContactRequestValidation { /// Validate a contact request against the verified on-chain envelope. /// -/// The empirical testnet census (368 docs) shows two live -/// honest cohorts: the dominant mobile population references an **unbound -/// ENCRYPTION key for BOTH indices** (mobile identities carry no DECRYPTION -/// key), and the newest cohort uses bound **ENCRYPTION(sender) / -/// DECRYPTION(recipient)** — our original convention. Consensus enforces -/// neither purpose nor boundedness on these integer fields. This validator is -/// therefore *liberal on receive*: it accepts the purposes mobile actually -/// uses while keeping the ECDSA key-*type* gate (every observed key is -/// ECDSA_SECP256K1) and the disabled-key check. +/// Consensus enforces neither purpose nor boundedness on `senderKeyIndex` / +/// `recipientKeyIndex`, and a `contactRequest` document is immutable — so this +/// validator is *liberal on receive* by necessity. Rejecting a document is not +/// a retry, it is a permanent sentence on a contact relationship the user +/// cannot renegotiate. What it keeps strict is what actually carries weight: +/// the ECDSA key-*type* gate (ECDH needs the full secp256k1 key) and the +/// disabled-key check. +/// +/// Three live cohorts are known: +/// - **Newest** — bound `ENCRYPTION(sender)` / `DECRYPTION(recipient)`, our +/// original convention. +/// - **Mobile** — an unbound `ENCRYPTION` key for BOTH indices; these +/// identities carry no DECRYPTION key at all. (Both of the above come from +/// a 368-document *testnet* census.) +/// - **Legacy Android/dashj** — references the recipient's `AUTHENTICATION` +/// (key ids 0-2) or `TRANSFER` (key id 3) key, sometimes pairing it with an +/// `AUTHENTICATION` sender key. Absent from the testnet census and +/// discovered only from **mainnet** device logs (2026-08): 27 of one +/// wallet's 29 contacts, every one of them established before the iOS +/// client existed. Under the previous, testnet-calibrated policy all 27 +/// were permanently unpayable. /// /// # Checks performed /// /// **Sender key:** /// - Key at `sender_key_index` exists on the sender identity. /// - Key type is `ECDSA_SECP256K1` (required for ECDH). -/// - Key purpose is `ENCRYPTION` (bound or unbound) — a non-ENCRYPTION -/// purpose is flagged as a `purpose_mismatch` (non-permanent). +/// - Key purpose is `ENCRYPTION` or `AUTHENTICATION` (bound or unbound) — +/// anything else is flagged as a `purpose_mismatch` (non-permanent). /// - Key is not disabled. /// /// **Recipient key (our key):** /// - Key at `recipient_key_index` exists on the recipient identity. /// - Key type is compatible (`ECDSA_SECP256K1` or `ECDSA_HASH160`). -/// - Key purpose is `ENCRYPTION` **or** `DECRYPTION` — anything else -/// (AUTHENTICATION/MASTER/TRANSFER) is flagged as a `purpose_mismatch`. +/// - Key purpose is `DECRYPTION`, `ENCRYPTION`, `AUTHENTICATION` or +/// `TRANSFER` — the node-operational purposes (`SYSTEM`, `VOTING`, +/// `OWNER`) are flagged as a `purpose_mismatch`. /// - Key is not disabled. /// +/// The accepted sets live in `dash_sdk::platform::dashpay` as +/// `*_key_purpose_is_acceptable_on_receive`, deliberately separate from the +/// stricter `recipient_key_purpose_is_valid` that governs the requests we +/// *mint*: accepting history is not the same decision as choosing a key for a +/// new document, and only the latter can still practice key separation. +/// /// A failure whose *only* cause is a purpose mismatch sets /// [`ContactRequestValidation::purpose_mismatch`], signalling callers to skip /// (and retry) rather than permanently break the channel. @@ -161,12 +182,15 @@ pub fn validate_contact_request( )); } - // Must have ENCRYPTION purpose (bound or unbound — both live - // cohorts use ENCRYPTION for the sender). A non-ENCRYPTION - // purpose is a non-permanent purpose mismatch. - if key.purpose() != Purpose::ENCRYPTION { + // ENCRYPTION is the modern convention; legacy dashj documents + // reference an AUTHENTICATION key. Both are accepted on receive — + // the document is immutable, so rejecting it is a permanent + // sentence on a contact the user cannot appeal. Anything else is + // still a non-permanent purpose mismatch (skip and retry). + if !sender_key_purpose_is_acceptable_on_receive(key.purpose()) { validation.add_purpose_error(format!( - "Sender key {} has purpose {:?}, but ENCRYPTION is required for contact requests", + "Sender key {} has purpose {:?}, but ENCRYPTION or AUTHENTICATION is \ + required for contact requests", sender_key_index, key.purpose(), )); @@ -214,18 +238,22 @@ pub fn validate_contact_request( } } - // Purpose must be ENCRYPTION or DECRYPTION: the mobile - // cohort's recipientKeyIndex points at an ENCRYPTION key, the - // newest cohort's at a DECRYPTION key — both honest. Anything - // else (AUTHENTICATION/MASTER/TRANSFER) is a non-permanent purpose - // mismatch: legacy 2024 docs reference AUTHENTICATION keys, so we - // skip-and-retry rather than permanently break the channel. The - // accepted cohort is owned by the shared SDK predicate so this - // validator and the recipient-key selector cannot disagree. - if !recipient_key_purpose_is_valid(key.purpose()) { + // Four honest cohorts reach this point: the newest references our + // DECRYPTION key, the mobile population our ENCRYPTION key, and + // the legacy Android/dashj population our AUTHENTICATION (key ids + // 0-2) or TRANSFER (key id 3) key. Purpose is not a security + // boundary here — ECDH is defined over the secp256k1 keypair and + // DIP-9 indexes the identity-key tree by type and id, never by + // purpose — so the type and disabled-key gates around this block + // are what actually protect the derivation. The node-operational + // purposes (SYSTEM/VOTING/OWNER) stay out — nothing on chain + // references them for DashPay — and remain a non-permanent purpose + // mismatch: skip and retry, never break the channel, so a later + // evidence-driven widening can still pick those contacts up. + if !recipient_key_purpose_is_acceptable_on_receive(key.purpose()) { validation.add_purpose_error(format!( - "Recipient key {} has purpose {:?}, but ENCRYPTION or DECRYPTION is \ - required for contact requests", + "Recipient key {} has purpose {:?}, which is not accepted for contact \ + requests", recipient_key_index, key.purpose(), )); @@ -373,11 +401,11 @@ mod tests { #[test] fn test_sender_wrong_purpose() { - let sender = make_identity(vec![make_key( - 0, - KeyType::ECDSA_SECP256K1, - Purpose::AUTHENTICATION, - )]); + // VOTING, not AUTHENTICATION: the legacy dashj cohort pairs an + // AUTHENTICATION sender key with an AUTHENTICATION recipient key and + // is now accepted on receive, so AUTHENTICATION no longer exercises + // the sender-side rejection this test is about. + let sender = make_identity(vec![make_key(0, KeyType::ECDSA_SECP256K1, Purpose::VOTING)]); let recipient = make_identity(vec![make_key( 0, KeyType::ECDSA_SECP256K1, @@ -460,9 +488,13 @@ mod tests { // references an UNBOUND ENCRYPTION key for BOTH senderKeyIndex and // recipientKeyIndex (mobile identities carry no DECRYPTION key); the // newest cohort uses bound ENCRYPTION(sender)/DECRYPTION(recipient). - // Consensus enforces neither purpose nor boundedness. So the validator - // must accept ENCRYPTION for the sender and ENCRYPTION-or-DECRYPTION for - // the recipient, keep the ECDSA type gate, and reject AUTHENTICATION. + // Consensus enforces neither purpose nor boundedness, and mainnet adds a + // third, legacy Android/dashj cohort referencing AUTHENTICATION/TRANSFER + // keys. So the validator accepts ENCRYPTION-or-AUTHENTICATION for the + // sender and everything but the node-operational purposes for the + // recipient, while keeping the + // ECDSA type gate and the disabled-key check — those are the checks that + // actually protect the ECDH. // ----------------------------------------------------------------------- /// Mobile-cohort shape: sender references an ENCRYPTION key, recipient @@ -493,35 +525,81 @@ mod tests { assert!(!result.purpose_mismatch); } - /// A recipient key of purpose AUTHENTICATION must FAIL validation (legacy - /// 2024 cohort / test-noise shape). Without the recipient-purpose gate an - /// AUTHENTICATION recipient key is silently accepted and a wrong shared - /// secret could be derived. + /// Legacy Android/dashj shape: the inbound request references OUR + /// AUTHENTICATION or TRANSFER key. Both must validate. + /// + /// This is the regression guard for the mainnet bug where 27 of a + /// wallet's 29 contacts — every one established before the iOS client + /// existed — were permanently unpayable. The deferred account build kept + /// failing `key-purpose mismatch`, so `send_payment` found no + /// `DashpayExternalAccount` and the user saw "call + /// register_external_contact_account first" forever. The documents are + /// immutable: no user action could have fixed it. + #[test] + fn legacy_dashj_recipient_key_purposes_are_accepted() { + for purpose in [Purpose::AUTHENTICATION, Purpose::TRANSFER] { + let sender = make_identity(vec![make_key( + 0, + KeyType::ECDSA_SECP256K1, + Purpose::ENCRYPTION, + )]); + let recipient = make_identity(vec![make_key(0, KeyType::ECDSA_SECP256K1, purpose)]); + + let result = validate_contact_request(&sender, 0, &recipient, 0); + assert!( + result.is_valid, + "a {purpose:?} recipient key must be accepted from an immutable on-chain \ + document, errors: {:?}", + result.errors + ); + assert!(!result.purpose_mismatch); + } + } + + /// The whole legacy pair — AUTHENTICATION sender against AUTHENTICATION + /// recipient — is the exact shape 15 of the logged mainnet failures took. #[test] - fn recipient_authentication_key_is_rejected_as_purpose_mismatch() { + fn legacy_dashj_authentication_pair_is_accepted() { let sender = make_identity(vec![make_key( - 0, + 1, KeyType::ECDSA_SECP256K1, - Purpose::ENCRYPTION, + Purpose::AUTHENTICATION, )]); let recipient = make_identity(vec![make_key( - 0, + 1, KeyType::ECDSA_SECP256K1, Purpose::AUTHENTICATION, )]); - let result = validate_contact_request(&sender, 0, &recipient, 0); - assert!( - !result.is_valid, - "an AUTHENTICATION recipient key must be rejected" - ); - assert!( - result.purpose_mismatch, - "an AUTHENTICATION recipient is a PURPOSE mismatch (non-permanent skip), not a hard/permanent failure" - ); - assert!(result.errors.iter().any(|e| e.contains("ENCRYPTION") - || e.contains("DECRYPTION") - || e.contains("purpose"))); + let result = validate_contact_request(&sender, 1, &recipient, 1); + assert!(result.is_valid, "errors: {:?}", result.errors); + assert!(!result.purpose_mismatch); + } + + /// The node-operational purposes are the ones still refused for a + /// recipient key — and they must stay a non-permanent purpose mismatch, so + /// a future evidence-driven widening can still pick those contacts up + /// instead of finding them broken. + #[test] + fn recipient_node_operational_key_is_rejected_as_purpose_mismatch() { + let sender = make_identity(vec![make_key( + 0, + KeyType::ECDSA_SECP256K1, + Purpose::ENCRYPTION, + )]); + for purpose in [Purpose::SYSTEM, Purpose::VOTING, Purpose::OWNER] { + let recipient = make_identity(vec![make_key(0, KeyType::ECDSA_SECP256K1, purpose)]); + + let result = validate_contact_request(&sender, 0, &recipient, 0); + assert!( + !result.is_valid, + "a {purpose:?} recipient key must be rejected" + ); + assert!( + result.purpose_mismatch && !result.hard_error, + "{purpose:?} must be a PURPOSE mismatch (non-permanent skip), not a hard failure" + ); + } } /// Sender ENCRYPTION + recipient DECRYPTION (our existing convention, @@ -544,15 +622,17 @@ mod tests { assert!(!result.purpose_mismatch); } - /// A sender key of purpose AUTHENTICATION is a purpose mismatch (the - /// classification flag must be set so the sweep/accept paths skip rather - /// than permanently break the channel). + /// A sender purpose outside the accepted set stays a purpose mismatch — + /// the classification flag must be set so the sweep/accept paths skip + /// rather than permanently break the channel. TRANSFER stands in for + /// AUTHENTICATION here: the latter is now an accepted legacy shape, but + /// no observed document puts TRANSFER on the sender side. #[test] - fn sender_authentication_key_is_a_purpose_mismatch() { + fn unaccepted_sender_purpose_is_a_purpose_mismatch() { let sender = make_identity(vec![make_key( 0, KeyType::ECDSA_SECP256K1, - Purpose::AUTHENTICATION, + Purpose::TRANSFER, )]); let recipient = make_identity(vec![make_key( 0, diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs index ec6eda0072d..b197cc01e7b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs @@ -977,39 +977,49 @@ fn external_account_needs_rebuild(contact: &EstablishedContact, has_external: bo /// 2. Fall back to the recipient's first `ECDSA_SECP256K1` **ENCRYPTION** key. /// 3. Error only if the recipient has neither. /// -/// No AUTHENTICATION fallback: no live client population needs it, and reusing -/// signing keys for ECDH is poor key separation. `ECDSA_SECP256K1` is required -/// either way (every observed key is that type, and ECDH needs the full key). +/// No AUTHENTICATION or TRANSFER fallback: reusing a signing or +/// fund-authorizing key for ECDH is poor key separation, and nothing forces us +/// to when we are the one choosing. `ECDSA_SECP256K1` is required either way +/// (every observed key is that type, and ECDH needs the full key). /// -/// The accepted cohort (DECRYPTION or ENCRYPTION) is the shared -/// [`dash_sdk::platform::dashpay::recipient_key_purpose_is_valid`] membership -/// policy; only the preference ORDER below (DECRYPTION first, ENCRYPTION -/// second) is local to the selector. +/// That mainnet's legacy Android/dashj population *does* reference +/// AUTHENTICATION/TRANSFER keys is a fact about immutable history, handled by +/// the wider receive-side policy +/// ([`dash_sdk::platform::dashpay::recipient_key_purpose_is_acceptable_on_receive`]). +/// It must not relax what we mint: this selector calls +/// [`dash_sdk::platform::dashpay::recipient_key_purpose_is_valid`] for +/// membership, so the cohort cannot drift from the SDK's request-creation +/// gate. Only the preference ORDER below (DECRYPTION first, ENCRYPTION second) +/// is local to the selector. fn select_recipient_key_index(recipient_identity: &Identity) -> Result { + // Membership comes from the shared mint predicate, never from a purpose + // list repeated here — a local copy is exactly how the SDK's + // request-creation gate and this selector would drift apart on the next + // policy change. + // // Skip disabled (revoked) keys: encrypting the DIP-15 compact xpub to a // key whose private half may be compromised would hand the contact's // payment xpub to whoever holds the revoked key. `disabled_at().is_none()` // mirrors the validator's disabled-key gate. - // Prefer a DECRYPTION key. - if let Some((id, _)) = recipient_identity.public_keys().iter().find(|(_, k)| { - k.purpose() == Purpose::DECRYPTION - && k.key_type() == KeyType::ECDSA_SECP256K1 - && k.disabled_at().is_none() - }) { - return Ok(*id); - } - // Fall back to an ENCRYPTION key (mobile cohort). - if let Some((id, _)) = recipient_identity.public_keys().iter().find(|(_, k)| { - k.purpose() == Purpose::ENCRYPTION - && k.key_type() == KeyType::ECDSA_SECP256K1 - && k.disabled_at().is_none() - }) { - return Ok(*id); - } - Err(PlatformWalletError::InvalidIdentityData( - "Recipient identity has no enabled ECDSA_SECP256K1 DECRYPTION or ENCRYPTION key" - .to_string(), - )) + let mut eligible: Vec<(&u32, &dpp::identity::IdentityPublicKey)> = recipient_identity + .public_keys() + .iter() + .filter(|(_, k)| { + dash_sdk::platform::dashpay::recipient_key_purpose_is_valid(k.purpose()) + && k.key_type() == KeyType::ECDSA_SECP256K1 + && k.disabled_at().is_none() + }) + .collect(); + // The only policy local to this selector: DECRYPTION before ENCRYPTION, + // then lowest key id (which `public_keys()`'s BTreeMap order already + // gives, and the stable sort preserves). + eligible.sort_by_key(|(_, k)| k.purpose() != Purpose::DECRYPTION); + eligible.first().map(|(id, _)| **id).ok_or_else(|| { + PlatformWalletError::InvalidIdentityData( + "Recipient identity has no enabled ECDSA_SECP256K1 DECRYPTION or ENCRYPTION key" + .to_string(), + ) + }) } /// Select our OWN ECDH root key: the first **enabled** `ECDSA_SECP256K1` @@ -1923,6 +1933,56 @@ impl DashPayView<'_, B> { return 0; } + // Our identity's key inventory, once per drain that has external + // builds queued. The whole legacy-cohort bug is a statement about this + // layout — an identity minted before DashPay encryption keys existed + // carries only AUTHENTICATION/TRANSFER slots, so inbound requests + // reference those ids and nothing downstream makes sense without + // knowing that. Reading it back off an exported log beats asking the + // user to query Platform. On-chain public metadata only; no key data. + { + use dpp::identity::accessors::IdentityGettersV0; + use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + let owners: std::collections::BTreeSet = entries + .iter() + .filter(|e| matches!(e.op, PendingContactCryptoOp::RegisterExternal { .. })) + .map(|e| e.owner_identity_id) + .collect(); + if !owners.is_empty() { + let wm = self.wallet_manager.read().await; + if let Some(info) = wm.get_wallet_info(&self.wallet_id) { + for owner in owners { + let Some(managed) = info.identity_manager.managed_identity(&owner) else { + continue; + }; + let keys: Vec = managed + .identity + .public_keys() + .iter() + .map(|(id, k)| { + format!( + "{id}:{:?}/{:?}{}", + k.purpose(), + k.key_type(), + if k.disabled_at().is_some() { + "/DISABLED" + } else { + "" + } + ) + }) + .collect(); + tracing::info!( + owner = %owner, + identity_index = ?managed.identity_index, + keys = %keys.join(" "), + "drain: our identity key inventory (id:purpose/type)" + ); + } + } + } + } + let mut cleared: Vec = Vec::new(); // How much of `cleared` is already dequeued + persisted, and the running // total actually removed. Bookkeeping lands per entry, so at most one @@ -2095,6 +2155,38 @@ impl DashPayView<'_, B> { ); continue; }; + // Did only the widened receive-side policy let this + // request through — i.e. does EITHER referenced key name a + // purpose we would never mint ourselves? That marks it as + // the legacy dashj cohort, whose ECDH/AES byte + // compatibility with our implementation has not been + // cross-validated against a dashj-produced payload. Used + // below to keep a decrypt failure from being charged to the + // document. + // + // BOTH sides matter: the widening moved the sender policy + // from ENCRYPTION-only to ENCRYPTION-or-AUTHENTICATION too, + // so an AUTHENTICATION sender paired with a mint-valid + // recipient is just as much an unverified legacy payload as + // the recipient-side case, and equally must not have a + // convention gap charged to it. + let accepted_by_legacy_widening = { + let recipient_widened = our_identity + .get_public_key_by_id(*our_decryption_key_index) + .map(|k| { + !dash_sdk::platform::dashpay::recipient_key_purpose_is_valid( + k.purpose(), + ) + }) + .unwrap_or(false); + // The mint-side sender rule is ENCRYPTION-only; anything + // else reaching here was admitted by the widening. + let sender_widened = contact_identity + .get_public_key_by_id(*contact_encryption_key_index) + .map(|k| k.purpose() != Purpose::ENCRYPTION) + .unwrap_or(false); + recipient_widened || sender_widened + }; let validation = crate::wallet::identity::crypto::validation::validate_contact_request( &contact_identity, @@ -2165,6 +2257,38 @@ impl DashPayView<'_, B> { } }; + // Everything the external build is about to depend on, in + // one line, BEFORE it can fail. Recorded at INFO because + // the legacy cohort's viability is an open question that + // only real mainnet wallets can answer, and an exported log + // is the only channel we get: without this, a failure below + // says what broke but not what it was working from. + // + // Public metadata only — key ids, purposes, types and + // lengths. Never the shared secret, and never the + // decrypted xpub. + { + use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + let our_key = our_identity.get_public_key_by_id(*our_decryption_key_index); + let their_key = + contact_identity.get_public_key_by_id(*contact_encryption_key_index); + tracing::info!( + owner = %entry.owner_identity_id, + contact = %entry.contact_id, + identity_index, + our_key_id = *our_decryption_key_index, + our_key_purpose = ?our_key.map(|k| k.purpose()), + our_key_type = ?our_key.map(|k| k.key_type()), + their_key_id = *contact_encryption_key_index, + their_key_purpose = ?their_key.map(|k| k.purpose()), + their_key_type = ?their_key.map(|k| k.key_type()), + ciphertext_len = encrypted_public_key.len(), + legacy_widened = accepted_by_legacy_widening, + ecdh_path = %path, + "drain: building external account" + ); + } + // ECDH via the Keychain-backed provider (scalar stays in the // signer; we only get the shared secret). // Bounded: the last step before the external-account @@ -2227,6 +2351,25 @@ impl DashPayView<'_, B> { .await; cleared.push(entry.key()); } + // A permanent fault on a legacy-cohort request is NOT + // charged to the document. Decrypt and compact-xpub + // parse are the only gates on the plaintext, so an + // ECDH/AES convention gap between us and dashj would + // surface here as a "permanent" fault and break every + // legacy channel at once — and a broken channel only + // heals when the CONTACT sends a fresh request, an + // appeal the user cannot file. Leaving it queued keeps + // a later convention fix able to recover it, and costs + // only a retry. + Err(e) if e.is_permanent() && accepted_by_legacy_widening => { + tracing::warn!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + error = %e.into_inner(), + "drain: legacy-cohort external register failed; leaving queued \ + (not marking broken — may be our own convention gap)" + ); + continue; + } Err(e) if e.is_permanent() => { tracing::warn!( owner = %entry.owner_identity_id, contact = %entry.contact_id, @@ -2300,6 +2443,16 @@ impl DashPayView<'_, B> { .flush_drained_contact_crypto(&entries, &cleared[flushed..]) .await; + // One-line verdict for the pass. "Did the legacy contacts build?" is + // answerable from this alone, without counting per-entry lines across a + // multi-megabyte export. + tracing::info!( + entries = entries.len(), + drained = drained_total, + still_queued = entries.len().saturating_sub(drained_total), + "drain: pass complete" + ); + drained_total } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs index 2cfdd8320f2..7d2ef339a96 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs @@ -449,12 +449,24 @@ impl DashPayView<'_, B> { } // --- 2. Decrypt the contact's xpub with the signer-derived secret. --- + // + // This failing is the single most diagnostic event on the whole path, + // so the message carries what tells the two hypotheses apart. AES-CBC + // with a WRONG key yields pseudorandom bytes, and PKCS7 then rejects + // them ~99.6% of the time — so a failure here means the ECDH shared + // secret did not match the sender's, i.e. a key-derivation or + // ECDH-convention gap, NOT a corrupt document. (A failure at step 3 + // below means the opposite: the secret was right and the plaintext + // layout is what differs.) The ciphertext length is included because a + // non-96-byte blob would instead point at a malformed document, which + // the contract's minItems/maxItems: 96 should already have prevented. let decrypted_xpub_bytes = platform_encryption::decrypt_extended_public_key(&shared_key, contact_encrypted_xpub) .map_err(|e| { Permanent(PlatformWalletError::InvalidIdentityData(format!( - "Failed to decrypt contact xpub: {}", - e + "Failed to decrypt contact xpub ({e}); ciphertext {} bytes — the ECDH \ + shared secret did not match the sender's (PKCS7 rejected the plaintext)", + contact_encrypted_xpub.len() ))) })?; @@ -479,9 +491,18 @@ impl DashPayView<'_, B> { .map_err(Permanent)?, Err(_) => { key_wallet::bip32::ExtendedPubKey::decode(&decrypted_xpub_bytes).map_err(|e| { + // Reaching here means the DECRYPT succeeded — PKCS7 unpadded + // cleanly, so the shared secret was almost certainly right — + // and only the plaintext LAYOUT is unexpected. The decrypted + // length is the discriminator, so it leads the message. The + // bytes themselves are never logged: they are the contact's + // payment xpub, and this text reaches an exported log. Permanent(PlatformWalletError::InvalidIdentityData(format!( - "Decrypted contact xpub is neither a 69-byte DIP-15 compact form \ - nor a 78/107-byte BIP32/DIP-14 serialization: {e}" + "Decrypted contact xpub is {} bytes — neither a 69-byte DIP-15 compact \ + form nor a 78/107-byte BIP32/DIP-14 serialization ({e}). The decrypt \ + itself SUCCEEDED, so the shared secret matched and it is the plaintext \ + layout that differs", + decrypted_xpub_bytes.len() ))) })? } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index d447f282222..88505879b2d 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -5026,6 +5026,268 @@ mod tests { ); } + /// The whole external-account build works with a **legacy key id and + /// purpose** — derivation at that id, ECDH, AES decrypt, compact-xpub + /// parse, registration — not just the purpose predicate. + /// + /// Key id 3 is the TRANSFER slot the legacy dashj cohort references, and + /// the widened receive-side policy is what now lets it reach this code at + /// all. The two sides are derived independently — our side through the + /// production `ContactCryptoProvider::ecdh_shared_secret` at the real + /// DIP-9 auth path, the sender's side by hand from our public key at that + /// same path — so the asserted symmetry is real and not one value handed + /// to both halves. + /// + /// What this does NOT prove: that a payload produced by **dashj** decrypts + /// under our ECDH/AES conventions. That needs a dashj-generated known + /// answer, which no fixture in this repo has. It is why the drain treats a + /// permanent register fault on a legacy-cohort request as "leave queued" + /// rather than "break the channel". + #[tokio::test] + async fn legacy_key_id_and_purpose_survive_the_whole_external_build() { + use crate::wallet::identity::network::contact_requests::{ + ContactCryptoProvider, SeedCryptoProvider, + }; + use crate::wallet::identity::IdentityWallet; + use key_wallet::bip32::KeyDerivationType; + + let (manager, _persister, wallet_id) = make_wallet().await; + let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet_arc.identity(); + let owner_id = Identifier::from([0xAA; 32]); + let contact_id = Identifier::from([0xBB; 32]); + + // The legacy slot: key id 3, the one dashj documents put in + // `recipientKeyIndex` and that the mint-side policy would refuse. + const LEGACY_KEY_ID: u32 = 3; + let path = + IdentityWallet::::identity_auth_derivation_path( + Network::Testnet, + KeyDerivationType::ECDSA, + 0, + LEGACY_KEY_ID, + ) + .expect("auth path at the legacy key id"); + + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""); + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + + // The contact's encryption keypair (the "sender" of the request). + let secp = dashcore::secp256k1::Secp256k1::new(); + let contact_secret = dashcore::secp256k1::SecretKey::from_slice(&[0x42u8; 32]) + .expect("valid contact secret"); + let contact_public = + dashcore::secp256k1::PublicKey::from_secret_key(&secp, &contact_secret); + + // Our side, through the production provider. + let ours = provider + .ecdh_shared_secret(&path, &contact_public) + .await + .expect("ECDH at the legacy key id"); + + // The sender's side, derived independently from our PUBLIC key at the + // same path — the direction dashj would compute. + let our_public = provider + .receiving_xpub(&path) + .await + .expect("our xpub at the legacy key id") + .public_key; + let theirs = platform_encryption::derive_shared_key_ecdh(&contact_secret, &our_public); + assert_eq!( + ours.as_slice(), + theirs.as_slice(), + "both sides must derive the same secret at a TRANSFER-purpose key id" + ); + + // The sender encrypts a real compact xpub to that secret. + let compact = { + let w = key_wallet::wallet::Wallet::from_seed_bytes( + seed, + Network::Testnet, + WalletAccountCreationOptions::None, + ) + .expect("seed wallet"); + crate::wallet::identity::crypto::dip14::derive_contact_xpub( + &w, + Network::Testnet, + 0, + &contact_id, + &owner_id, + ) + .expect("derive a valid compact xpub") + .compact + .to_bytes() + }; + let encrypted = + platform_encryption::encrypt_extended_public_key(&theirs, &[0x11u8; 16], &compact); + + // The production registration path: decrypt + parse + register. + let registration = iw + .dashpay() + .register_external_contact_account( + &owner_id, + &bare_identity([0xBB; 32]), + &encrypted, + ours, + ) + .await + .expect("a legacy-key-id payload must build the external account"); + assert_eq!( + registration, + crate::wallet::identity::network::contacts::ExternalAccountRegistration::Built, + "the account must be built from this payload, not found pre-existing" + ); + } + + /// A **sender-only** legacy shape — AUTHENTICATION sender against a + /// mint-valid DECRYPTION recipient — is also shielded from the + /// broken-channel mark when the payload fails to decrypt. + /// + /// The widening moved the sender rule from ENCRYPTION-only to + /// ENCRYPTION-or-AUTHENTICATION as well, so this request reaches the + /// decrypt purely because of the receive-side policy, exactly like the + /// recipient-side case. A flag that inspected only the recipient key would + /// classify it as an ordinary permanent fault and destroy the channel. + /// + /// The ciphertext here is deliberate garbage — standing in for the + /// convention gap we cannot rule out without a dashj-produced fixture. + #[tokio::test] + async fn sender_only_legacy_shape_is_not_charged_for_a_decrypt_failure() { + use crate::changeset::{PendingContactCrypto, PendingContactCryptoOp}; + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + use crate::wallet::identity::{ContactRequest, EstablishedContact}; + use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dpp::identity::{IdentityPublicKey, IdentityV0, KeyType, Purpose, SecurityLevel}; + + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + let secp = dashcore::secp256k1::Secp256k1::new(); + let key_at = |id: u32, purpose: Purpose, byte: u8| { + IdentityPublicKey::V0(IdentityPublicKeyV0 { + id, + purpose, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: dashcore::secp256k1::PublicKey::from_secret_key( + &secp, + &dashcore::secp256k1::SecretKey::from_slice(&[byte; 32]).expect("secret"), + ) + .serialize() + .to_vec() + .into(), + disabled_at: None, + }) + }; + + // Our key is DECRYPTION — mint-valid, so the RECIPIENT side needed no + // widening at all. Only the sender side does. + let our_identity = Identity::V0(IdentityV0 { + id: owner, + public_keys: [(0u32, key_at(0, Purpose::DECRYPTION, 0x24))] + .into_iter() + .collect(), + balance: 0, + revision: 0, + }); + let contact_identity = Identity::V0(IdentityV0 { + id: contact, + public_keys: [(0u32, key_at(0, Purpose::AUTHENTICATION, 0x42))] + .into_iter() + .collect(), + balance: 0, + revision: 0, + }); + + let mut sdk = dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk"); + sdk.mock() + .expect_fetch::(contact, Some(contact_identity)) + .await + .expect("set the contact-identity fetch expectation"); + let sdk = Arc::new(sdk); + + let persister = Arc::new(RecordingPersister::default()); + let handler: Arc = Arc::new(NoopEventHandler); + let manager = Arc::new(PlatformWalletManager::new( + sdk, + Arc::clone(&persister), + handler, + )); + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""); + let wallet_id = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed, + WalletAccountCreationOptions::Default, + Some(0), + ) + .await + .expect("wallet creation") + .wallet_id(); + let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet_arc.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(our_identity, 0, wallet_id, &p) + .expect("add owner"); + let outgoing = ContactRequest::new(owner, contact, 0, 0, 0, vec![0u8; 96], 0, 0); + let incoming = ContactRequest::new(contact, owner, 0, 0, 0, vec![0u8; 96], 0, 0); + let managed = info + .identity_manager + .managed_identity_mut(&owner) + .expect("owner resident"); + managed.apply_established_contact(EstablishedContact::new(contact, outgoing, incoming)); + managed + .dashpay_pending_contact_crypto_mut() + .push(PendingContactCrypto { + owner_identity_id: owner, + contact_id: contact, + op: PendingContactCryptoOp::RegisterExternal { + // Undecryptable under any shared secret — the stand-in + // for a dashj/us convention mismatch. + encrypted_public_key: vec![7u8; 96], + our_decryption_key_index: 0, + contact_encryption_key_index: 0, + }, + enqueued_at_ms: 0, + }); + } + + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + assert_eq!( + drained, 0, + "a legacy-cohort decrypt failure must leave the entry queued, not clear it" + ); + + let wm = iw.wallet_manager.read().await; + let managed = wm + .get_wallet_info(&wallet_id) + .expect("info") + .identity_manager + .managed_identity(&owner) + .expect("owner resident"); + assert!( + !managed + .dashpay() + .established_contacts() + .get(&contact) + .expect("contact resident") + .payment_channel_broken, + "a sender-only legacy shape must not have a possible convention gap charged \ + to it — the channel stays recoverable" + ); + } + /// A `RegisterExternal` entry the drain cannot complete (here: the owner /// isn't wallet-owned, so no HD index → it bails before any network fetch) /// must be **left queued**, never dropped or crashed — so a later drain can diff --git a/packages/rs-sdk/src/platform/dashpay/contact_request.rs b/packages/rs-sdk/src/platform/dashpay/contact_request.rs index d1f59bc9cab..d595faaaed7 100644 --- a/packages/rs-sdk/src/platform/dashpay/contact_request.rs +++ b/packages/rs-sdk/src/platform/dashpay/contact_request.rs @@ -142,25 +142,79 @@ pub struct SendContactRequestResult { } /// Whether `purpose` is acceptable for the `senderKeyIndex` key of a contact -/// request. The sender always references its own ENCRYPTION key. +/// request **we are about to mint**. The sender always references its own +/// ENCRYPTION key. +/// +/// Mint-side only — see [`sender_key_purpose_is_acceptable_on_receive`] for +/// what we accept from documents already on chain. fn sender_key_purpose_is_valid(purpose: Purpose) -> bool { purpose == Purpose::ENCRYPTION } /// Whether `purpose` is acceptable for the `recipientKeyIndex` key of a -/// contact request. The newest cohort references the recipient's -/// DECRYPTION key (our original convention); the dominant mobile cohort has no -/// DECRYPTION key and references its ENCRYPTION key. Accept either; reject -/// AUTHENTICATION/MASTER/TRANSFER. +/// contact request **we are about to mint**. The newest cohort references the +/// recipient's DECRYPTION key (our original convention); the dominant mobile +/// cohort has no DECRYPTION key and references its ENCRYPTION key. Accept +/// either; reject every other purpose. +/// +/// This is the single source of truth for what we are willing to *create*. +/// The recipient-key selector (`select_recipient_key_index`) defers to it so +/// the minted cohort cannot drift between the SDK and wallet layers. It +/// deliberately stays strict: reusing a signing or fund-authorizing key for +/// ECDH is poor key separation, and no new document needs to. /// -/// This is the single source of truth for the recipient-key cohort membership -/// policy. The pre-send validator (`rs-platform-wallet` `validate_contact_request`) -/// and the recipient-key selector (`select_recipient_key_index`) both defer to -/// it so the accepted cohort cannot drift between the SDK and wallet layers. +/// It is NOT the acceptance policy for inbound documents — a `contactRequest` +/// is immutable, so history cannot be re-minted to fit this rule. See +/// [`recipient_key_purpose_is_acceptable_on_receive`]. pub fn recipient_key_purpose_is_valid(purpose: Purpose) -> bool { matches!(purpose, Purpose::DECRYPTION | Purpose::ENCRYPTION) } +/// Whether `purpose` on the `recipientKeyIndex` key of an **inbound, already +/// on-chain** contact request is acceptable for the ECDH that unwraps the +/// sender's `encryptedPublicKey`. +/// +/// Strictly wider than [`recipient_key_purpose_is_valid`], and deliberately +/// so. `contactRequest` documents are immutable and consensus enforces no +/// purpose constraint on these integer fields, so the acceptance policy is the +/// *only* thing standing between a user and their own payment history. +/// Mainnet device logs (2026-08, a 29-contact wallet whose contacts were +/// established through the legacy Android/dashj client) show 27 of 29 inbound +/// requests referencing the recipient's AUTHENTICATION (key ids 0-2) or +/// TRANSFER (key id 3) key — under the mint-side rule every one of those +/// contacts is unpayable forever, with no action the user can take. +/// +/// Purpose carries no cryptographic weight here: ECDH is defined over the +/// secp256k1 keypair, and DIP-9's identity-key tree is indexed by key *type* +/// and id, never by purpose, so the same derivation reaches all of them. The +/// gates that do carry weight — `ECDSA_SECP256K1` key type and the +/// disabled-key check — are enforced separately by the caller and are +/// unaffected by this predicate. +/// +/// The node-operational purposes (SYSTEM, VOTING, OWNER) stay rejected: +/// nothing on chain references them for DashPay, and they have no business in +/// a payment-channel handshake. +pub fn recipient_key_purpose_is_acceptable_on_receive(purpose: Purpose) -> bool { + matches!( + purpose, + Purpose::DECRYPTION | Purpose::ENCRYPTION | Purpose::AUTHENTICATION | Purpose::TRANSFER + ) +} + +/// Receive-side counterpart of [`sender_key_purpose_is_valid`]: whether +/// `purpose` on the `senderKeyIndex` key of an **inbound, already on-chain** +/// contact request is acceptable for ECDH. +/// +/// Same reasoning as [`recipient_key_purpose_is_acceptable_on_receive`]. The +/// legacy cohort is narrower on this side — the observed mainnet documents +/// pair an AUTHENTICATION sender key with an AUTHENTICATION recipient key — so +/// only AUTHENTICATION is added. A sender referencing any other purpose has +/// not been seen and stays a purpose mismatch (skip-and-retry, never a +/// permanently broken channel), leaving room to widen again on evidence. +pub fn sender_key_purpose_is_acceptable_on_receive(purpose: Purpose) -> bool { + matches!(purpose, Purpose::ENCRYPTION | Purpose::AUTHENTICATION) +} + impl Sdk { /// Create a contact request document /// @@ -642,9 +696,11 @@ mod tests { } #[test] - fn recipient_key_purpose_rejects_authentication() { - // No AUTHENTICATION fallback — reusing signing keys for ECDH is poor - // key separation and no live population needs it. + fn mint_side_still_refuses_authentication_and_transfer() { + // What we CREATE stays strict: reusing a signing or fund-authorizing + // key for ECDH is poor key separation, and no new document needs to. + // Widening the receive-side acceptance below must never leak into the + // key we pick for our own outgoing requests. assert!(!recipient_key_purpose_is_valid(Purpose::AUTHENTICATION)); assert!(!recipient_key_purpose_is_valid(Purpose::TRANSFER)); } @@ -658,6 +714,51 @@ mod tests { assert!(!sender_key_purpose_is_valid(Purpose::AUTHENTICATION)); } + #[test] + fn receive_side_accepts_the_legacy_dashj_cohort() { + // Regression guard for the mainnet legacy cohort: inbound requests + // minted by the Android/dashj client reference the recipient's + // AUTHENTICATION (key ids 0-2) or TRANSFER (key id 3) key. Rejecting + // them made every pre-iOS contact permanently unpayable — the document + // is immutable, so no user action could ever fix it. + for purpose in [ + Purpose::DECRYPTION, + Purpose::ENCRYPTION, + Purpose::AUTHENTICATION, + Purpose::TRANSFER, + ] { + assert!( + recipient_key_purpose_is_acceptable_on_receive(purpose), + "{purpose:?} recipient key must be accepted from an on-chain document" + ); + } + // The sender side of the same legacy documents pairs AUTHENTICATION + // with AUTHENTICATION; ENCRYPTION remains the modern convention. + assert!(sender_key_purpose_is_acceptable_on_receive( + Purpose::ENCRYPTION + )); + assert!(sender_key_purpose_is_acceptable_on_receive( + Purpose::AUTHENTICATION + )); + } + + #[test] + fn receive_side_still_refuses_node_operational_purposes() { + // Not observed on chain for DashPay — widening is evidence-driven, so + // these stay out until something real needs them. A rejection here is + // a skip-and-retry purpose mismatch, never a permanently broken + // channel, so a later widening can still recover those contacts. + for purpose in [Purpose::SYSTEM, Purpose::VOTING, Purpose::OWNER] { + assert!(!recipient_key_purpose_is_acceptable_on_receive(purpose)); + assert!(!sender_key_purpose_is_acceptable_on_receive(purpose)); + } + // TRANSFER is accepted for the recipient (legacy key id 3) but has + // never been seen on the sender side. + assert!(!sender_key_purpose_is_acceptable_on_receive( + Purpose::TRANSFER + )); + } + #[test] fn test_ecdh_shared_secret_symmetry() { // Test that both parties derive the same shared secret diff --git a/packages/rs-sdk/src/platform/dashpay/mod.rs b/packages/rs-sdk/src/platform/dashpay/mod.rs index ce482872996..182edd8854b 100644 --- a/packages/rs-sdk/src/platform/dashpay/mod.rs +++ b/packages/rs-sdk/src/platform/dashpay/mod.rs @@ -7,8 +7,9 @@ mod contact_request; mod contact_request_queries; pub use contact_request::{ - recipient_key_purpose_is_valid, ContactRequestInput, ContactRequestResult, EcdhProvider, - RecipientIdentity, SendContactRequestInput, SendContactRequestResult, + recipient_key_purpose_is_acceptable_on_receive, recipient_key_purpose_is_valid, + sender_key_purpose_is_acceptable_on_receive, ContactRequestInput, ContactRequestResult, + EcdhProvider, RecipientIdentity, SendContactRequestInput, SendContactRequestResult, }; pub use contact_request_queries::ContactRequestDocuments;