From 1791fe5f2760cb2d2d42ffe707623ff0709ab00f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:10:30 +0300 Subject: [PATCH 1/5] fix(platform-wallet): accept legacy dashj key purposes on inbound contact requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Contacts established through the legacy Android/dashj client could never be paid from iOS. `send_payment` failed with "No DashpayExternalAccount found for contact ... — call register_external_contact_account first" on every attempt, while contacts created on iOS worked fine. Those contacts are only ever built by the deferred path: the signerless sweep enqueues a `RegisterExternal` op and the signer-backed drain completes it. `validate_contact_request` rejected every legacy document there, because the `recipientKeyIndex` on an inbound dashj request points at the recipient's AUTHENTICATION (key ids 0-2) or TRANSFER (key id 3) key rather than ENCRYPTION/DECRYPTION. The drain classified that as a purpose-only mismatch — correctly refusing to mark the channel broken — so it retried forever and never succeeded, and `send_payment` kept finding no external account. Mainnet device logs show 27 of one wallet's 29 contacts in this state, the 2 survivors being the ones established on iOS. Purpose is not a security boundary for this ECDH: it is defined over the secp256k1 keypair, and DIP-9 indexes the identity-key tree by key type and id, never by purpose, so the same derivation reaches all of them. The gates that do carry weight — the ECDSA key-type gate and the disabled-key check — are untouched. The previous policy was calibrated on a 368-document testnet census that contains no dashj-era cohort. Split the policy in two rather than widening the existing predicate: `recipient_key_purpose_is_valid` still governs the requests we mint (and so `select_recipient_key_index` still practices key separation), while the new `*_key_purpose_is_acceptable_on_receive` govern what we accept from immutable history. A `contactRequest` cannot be re-minted to fit a rule we invent later, so rejecting one is a permanent sentence on a relationship the user has no way to appeal. The node-operational purposes (SYSTEM, VOTING, OWNER) stay rejected, and stay a non-permanent purpose mismatch, so a later evidence-driven widening can still recover those contacts instead of finding their channels broken. --- .../src/wallet/identity/crypto/validation.rs | 208 ++++++++++++------ .../identity/network/contact_requests.rs | 15 +- .../src/platform/dashpay/contact_request.rs | 125 ++++++++++- packages/rs-sdk/src/platform/dashpay/mod.rs | 5 +- 4 files changed, 270 insertions(+), 83 deletions(-) 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..ea1bd45139c 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,13 +977,18 @@ 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 +/// 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 stays on the /// [`dash_sdk::platform::dashpay::recipient_key_purpose_is_valid`] membership -/// policy; only the preference ORDER below (DECRYPTION first, ENCRYPTION +/// policy, and only the preference ORDER below (DECRYPTION first, ENCRYPTION /// second) is local to the selector. fn select_recipient_key_index(recipient_identity: &Identity) -> Result { // Skip disabled (revoked) keys: encrypting the DIP-15 compact xpub to a 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; From 289b8fbc0c0de6ba2aef64674d7576df21256bc8 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:57:48 +0300 Subject: [PATCH 2/5] refactor(platform-wallet): source the recipient-key cohort from the shared mint predicate select_recipient_key_index documented that it defers to recipient_key_purpose_is_valid but repeated the DECRYPTION/ENCRYPTION list inline, so a mint-policy change would silently desync the SDK's request-creation gate from the wallet's key selection. Filter through the predicate and keep only the preference order (DECRYPTION first, then lowest key id) local to the selector. Raised by CodeRabbit on #4372. --- .../identity/network/contact_requests.rs | 53 ++++++++++--------- 1 file changed, 29 insertions(+), 24 deletions(-) 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 ea1bd45139c..efe53f23c68 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 @@ -986,35 +986,40 @@ fn external_account_needs_rebuild(contact: &EstablishedContact, has_external: bo /// 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 stays on the -/// [`dash_sdk::platform::dashpay::recipient_key_purpose_is_valid`] membership -/// policy, and only the preference ORDER below (DECRYPTION first, ENCRYPTION -/// second) is local to the selector. +/// 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` From afec36d01c44ba7f633d3d5dff9bd6d37ed9af82 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 18:29:33 +0300 Subject: [PATCH 3/5] fix(platform-wallet): don't charge a legacy-cohort decrypt failure to the document MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #4372 pointed out that the widening lets legacy requests reach the `RegisterExternal` path — derivation at the legacy key id, ECDH, AES decrypt, compact-xpub parse — and that a failure there is classified permanent, so the drain marks `payment_channel_broken`. If our ECDH/AES conventions turn out to differ from dashj's, that would 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. Decrypt and compact-xpub parse are the only gates on the plaintext, so a convention gap is indistinguishable from a corrupt document at that point. When a request was accepted only by the widened receive-side policy (it names a purpose we would never mint), a permanent register fault now leaves the entry queued instead of breaking the channel. The cost is a retry; the alternative costs the user a relationship they cannot repair. Adds `legacy_key_id_and_purpose_survive_the_whole_external_build`: key id 3 (the TRANSFER slot dashj references) through the production provider's ECDH at the real DIP-9 auth path, with the sender's side derived independently from our public key at that same path, then encrypt → decrypt → parse → register. It pins that nothing downstream of the predicate is purpose- or id-sensitive. It deliberately does not claim to prove dashj byte compatibility — that needs a dashj-generated known answer this repo has no fixture for, which is exactly why the classification change above is the safety net rather than the test. --- .../identity/network/contact_requests.rs | 34 ++++++ .../src/wallet/identity/network/payments.rs | 115 ++++++++++++++++++ 2 files changed, 149 insertions(+) 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 efe53f23c68..ccf9a943f28 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 @@ -2105,6 +2105,21 @@ impl DashPayView<'_, B> { ); continue; }; + // Did only the widened receive-side policy let this + // request through — i.e. does it name a key 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 treated as the document's fault. + let accepted_by_legacy_widening = 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); let validation = crate::wallet::identity::crypto::validation::validate_contact_request( &contact_identity, @@ -2237,6 +2252,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, 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..2de7e59ee47 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,121 @@ 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 `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 From 38e6c918a2e4e8f1c2937ac5ed896bc5158db321 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:29:54 +0300 Subject: [PATCH 4/5] fix(platform-wallet): classify sender-side widening as legacy too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that `accepted_by_legacy_widening` inspected only the recipient key, while this PR widens the sender rule as well (ENCRYPTION-only to ENCRYPTION-or-AUTHENTICATION). An AUTHENTICATION sender paired with a mint-valid DECRYPTION/ENCRYPTION recipient therefore reached the decrypt purely because of the receive-side policy, yet the flag stayed false — so a decrypt or compact-xpub failure took the ordinary permanent arm and destroyed the channel, which is exactly what the classification exists to prevent for payloads whose dashj byte compatibility is unverified. The flag is now the OR of both referenced keys against their respective mint-side rules. `sender_only_legacy_shape_is_not_charged_for_a_decrypt_failure` pins the shape the reviewer named: AUTHENTICATION sender, DECRYPTION recipient, undecryptable ciphertext. Verified it catches the reported defect — with the sender term removed it fails with drained 1 vs 0 and the channel marked broken. --- .../identity/network/contact_requests.rs | 45 ++++-- .../src/wallet/identity/network/payments.rs | 147 ++++++++++++++++++ 2 files changed, 178 insertions(+), 14 deletions(-) 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 ccf9a943f28..9d0604e80e1 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 @@ -2106,20 +2106,37 @@ impl DashPayView<'_, B> { continue; }; // Did only the widened receive-side policy let this - // request through — i.e. does it name a key 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 treated as the document's fault. - let accepted_by_legacy_widening = 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); + // 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, 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 2de7e59ee47..88505879b2d 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -5141,6 +5141,153 @@ mod tests { ); } + /// 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 From ee984ad465609923028cc383213e5872e1a59e5f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 21:39:07 +0300 Subject: [PATCH 5/5] feat(platform-wallet): make a failed legacy external build self-diagnosing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the fix does not work on a real mainnet wallet, the current logs say a contact failed but not enough to say why — which is how this bug went undiagnosed in the first place. The open question (whether dashj-produced ciphertext decrypts under our ECDH/AES conventions) can only be answered from an exported log, so the log has to carry the answer. Three additions, all public metadata — never the shared secret, never the decrypted xpub, which is the contact's payment key and would leak into a log the user hands over: - Our identity's key inventory (id:purpose/type, disabled marker), once per drain that has external builds queued. The whole bug is a statement about this layout: an identity minted before DashPay encryption keys existed carries only AUTHENTICATION/TRANSFER slots, and nothing downstream reads correctly without it. - Per-attempt context before anything can fail: both key ids with their purposes and types, the HD identity index, the ECDH path, the ciphertext length, and whether the widened receive policy is what admitted the request. - A one-line pass verdict (entries / drained / still_queued), so "did the legacy contacts build?" is answerable without counting lines in a multi-megabyte export. The two failure messages now state what they imply, because the distinction is the whole diagnosis and is not obvious from the error text alone: - decrypt failure ⇒ the shared secret did not match (AES-CBC under a wrong key is pseudorandom and PKCS7 rejects it ~99.6% of the time), i.e. a key-derivation or ECDH-convention gap; - decrypt success + parse failure ⇒ the secret was right and only the plaintext layout differs. The decrypted length now leads that message, since it is the discriminator. --- .../identity/network/contact_requests.rs | 92 +++++++++++++++++++ .../src/wallet/identity/network/contacts.rs | 29 +++++- 2 files changed, 117 insertions(+), 4 deletions(-) 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 9d0604e80e1..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 @@ -1933,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 @@ -2207,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 @@ -2361,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() ))) })? }