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 01/11] 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 982f1078ef05185c2cbae7525d6ea75b777bf785 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 17:35:25 +0300 Subject: [PATCH 02/11] perf(platform-wallet): keep the contact fetch off the drain's repeating-rejection path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `RegisterExternal` entry the key-purpose policy turns away stays queued on purpose — the `contactRequest` is immutable, so what might change is our acceptance policy, and breaking the channel instead would need a superseding request from the contact to heal. The consequence is that the sweep re-offers the entry every pass, forever. The drain fetched the contact identity from Platform *before* it could validate, so each of those passes spent a round trip to reach a verdict it already had locally: the dominant rejection is on the `recipientKeyIndex` key, which is our own and always resident. Mainnet logs from one wallet show 27 contacts and 396 fetch-then-reject cycles in a single session, with a second wallet-lifetime of them still to come. Split `validate_contact_request` into `validate_sender_key` (needs the counterparty) and `validate_recipient_key` (needs only us); the composed function is unchanged, so every other caller keeps the identical contract. The drain now runs the recipient half first and only fetches when that passes. Both halves route their failure through one `apply_drain_validation_failure` helper so they classify identically — purpose-only leaves the entry queued, anything else marks the channel broken. Per-entry purpose rejections drop to DEBUG and are aggregated into a single end-of-drain WARN carrying the distinct reasons and their counts. That signal is how the legacy dashj cohort was found in an exported log, so it has to stay visible — just not 27 times a pass. `drain_decides_our_own_key_fault_without_fetching_the_contact` pins the ordering: a wallet-owned but keyless owner must have its channel marked broken with no contact fetch configured on the mock. Under the old ordering the fetch failed first and the verdict was never reached. --- .../src/wallet/identity/crypto/validation.rs | 43 ++++- .../identity/network/contact_requests.rs | 163 +++++++++++++----- .../src/wallet/identity/network/payments.rs | 86 +++++++++ 3 files changed, 242 insertions(+), 50 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 4a01e667424..f263de5816b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs @@ -165,12 +165,26 @@ pub fn validate_contact_request( sender_key_index: u32, recipient_identity: &Identity, recipient_key_index: u32, +) -> ContactRequestValidation { + let mut validation = validate_sender_key(sender_identity, sender_key_index); + validation.merge(validate_recipient_key( + recipient_identity, + recipient_key_index, + )); + validation +} + +/// The sender half of [`validate_contact_request`] — the checks that need the +/// **counterparty's** identity. +/// +/// Split out so the deferred-crypto drain can run the recipient half first. +/// See [`validate_recipient_key`] for why that ordering matters. +pub fn validate_sender_key( + sender_identity: &Identity, + sender_key_index: u32, ) -> ContactRequestValidation { let mut validation = ContactRequestValidation::new(); - // ----------------------------------------------------------------------- - // Sender key validation - // ----------------------------------------------------------------------- match sender_identity.get_public_key_by_id(sender_key_index) { Some(key) => { // Must be ECDSA_SECP256K1 for ECDH. @@ -213,9 +227,26 @@ pub fn validate_contact_request( } } - // ----------------------------------------------------------------------- - // Recipient key validation - // ----------------------------------------------------------------------- + validation +} + +/// The recipient half of [`validate_contact_request`] — the checks that need +/// only **our own** identity, which is always already resident. +/// +/// Split out because the deferred-crypto drain would otherwise pay a Platform +/// round trip (`Identity::fetch` of the contact) before it could discover that +/// the request is unusable for a reason it could have known locally. A +/// purpose-rejected entry stays queued by design — the policy, not the +/// immutable document, is what might change — so that fetch was repeating on +/// every sweep, forever. Mainnet logs from one wallet show 27 contacts and 396 +/// such fetch-then-reject cycles in a single session. Running this half first +/// costs nothing and removes the network entirely from that loop. +pub fn validate_recipient_key( + recipient_identity: &Identity, + recipient_key_index: u32, +) -> ContactRequestValidation { + let mut validation = ContactRequestValidation::new(); + match recipient_identity.get_public_key_by_id(recipient_key_index) { Some(key) => { // Must be an ECDSA variant for ECDH compatibility. 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..aeaed85e355 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 @@ -1929,6 +1929,15 @@ impl DashPayView<'_, B> { } let mut cleared: Vec = Vec::new(); + // Distinct key-purpose rejections seen this drain, and how many entries + // each blocked — summarised once at the end instead of one WARN per + // entry. A purpose-rejected entry stays queued by design (the policy, + // not the immutable document, is what might change), so per-entry + // WARNing repeats every sweep for the life of the wallet: mainnet logs + // from one wallet show 396 such lines for 27 contacts in a single + // session. Every reason still reaches the log, once, with its count. + let mut policy_blocked: std::collections::BTreeMap = + std::collections::BTreeMap::new(); // How much of `cleared` is already dequeued + persisted, and the running // total actually removed. Bookkeeping lands per entry, so at most one // entry's worth can ever be in flight. @@ -2044,6 +2053,48 @@ impl DashPayView<'_, B> { } }; + // Validate OUR key first — it needs nothing but the + // resident identity, so a request that can never be used is + // rejected before spending a Platform round trip on the + // contact. This is the dominant rejection in practice + // (legacy documents reference our AUTHENTICATION/TRANSFER + // key), and because such an entry stays queued the fetch + // below would otherwise repeat on every sweep, forever. + let our_identity = { + let wm = self.wallet_manager.read().await; + wm.get_wallet_info(&self.wallet_id) + .and_then(|info| { + info.identity_manager + .managed_identity(&entry.owner_identity_id) + }) + .map(|m| m.identity.clone()) + }; + let Some(our_identity) = our_identity else { + tracing::warn!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + "drain: our identity vanished mid-drain; leaving queued" + ); + continue; + }; + let recipient_validation = + crate::wallet::identity::crypto::validation::validate_recipient_key( + &our_identity, + *our_decryption_key_index, + ); + if !recipient_validation.is_valid { + if self + .apply_drain_validation_failure( + entry, + &recipient_validation, + &mut policy_blocked, + ) + .await + { + cleared.push(entry.key()); + } + continue; + } + // Fetch the contact identity (transient on failure → leave). // Bounded: a Platform round trip, and nothing in this entry // has committed yet. @@ -2077,56 +2128,24 @@ impl DashPayView<'_, B> { } }; - // Validate key indices (purpose + type) BEFORE ECDH — the - // same gate the resident sweep path applies, so the deferred - // path enforces the identical contract. A purpose-only - // mismatch (e.g. a legacy doc referencing an AUTH key) is left - // queued for a future acceptance-policy change; a hard failure - // (key type / missing / disabled) marks the channel broken and - // clears the entry. - let our_identity = { - let wm = self.wallet_manager.read().await; - wm.get_wallet_info(&self.wallet_id) - .and_then(|info| { - info.identity_manager - .managed_identity(&entry.owner_identity_id) - }) - .map(|m| m.identity.clone()) - }; - let Some(our_identity) = our_identity else { - tracing::warn!( - owner = %entry.owner_identity_id, contact = %entry.contact_id, - "drain: our identity vanished mid-drain; leaving queued" - ); - continue; - }; + // The sender half — the checks that need the identity we + // just fetched. Together with the recipient half above this + // is exactly `validate_contact_request`, so the deferred + // path still enforces the identical contract as the + // resident sweep path; only the ORDER differs, to keep the + // fetch off the repeating-rejection path. let validation = - crate::wallet::identity::crypto::validation::validate_contact_request( + crate::wallet::identity::crypto::validation::validate_sender_key( &contact_identity, *contact_encryption_key_index, - &our_identity, - *our_decryption_key_index, ); if !validation.is_valid { - if validation.is_purpose_only() { - tracing::warn!( - owner = %entry.owner_identity_id, contact = %entry.contact_id, - errors = ?validation.errors, - "drain: contact request key-purpose mismatch; leaving queued (not marking broken)" - ); - continue; + if self + .apply_drain_validation_failure(entry, &validation, &mut policy_blocked) + .await + { + cleared.push(entry.key()); } - tracing::warn!( - owner = %entry.owner_identity_id, contact = %entry.contact_id, - errors = ?validation.errors, - "drain: contact request failed key-index validation; marking channel broken" - ); - self.mark_contact_channel_broken( - &entry.owner_identity_id, - &entry.contact_id, - ) - .await; - cleared.push(entry.key()); continue; } @@ -2305,9 +2324,65 @@ impl DashPayView<'_, B> { .flush_drained_contact_crypto(&entries, &cleared[flushed..]) .await; + // One line for every entry the key-purpose policy turned away, instead + // of one per entry per sweep. Kept at WARN and carrying the distinct + // reasons: this is the signal that a live on-chain cohort is failing + // our acceptance policy, which is exactly how the legacy dashj cohort + // was found — it must stay visible in an exported log, just not 27 + // times a pass. + if !policy_blocked.is_empty() { + let blocked: usize = policy_blocked.values().sum(); + tracing::warn!( + entries = blocked, + reasons = ?policy_blocked, + "drain: contact requests left queued by the key-purpose policy \ + (not marking broken; they retry when the policy changes)" + ); + } + drained_total } + /// The drain's validation-failure policy, shared by the recipient-half and + /// sender-half checks so both halves classify identically. + /// + /// - A **purpose-only** failure is counted into `policy_blocked` for the + /// drain's end-of-run summary and left queued: the request is immutable, + /// so what might change is our acceptance policy, and a channel marked + /// broken here would need a superseding request from the contact to heal + /// — an appeal the user cannot file. + /// - Anything else (missing key, wrong key type, disabled key) is a real + /// permanent fault: mark the channel broken so the sweep stops + /// collecting it. + /// + /// Returns `true` when the caller should clear the entry from the queue. + async fn apply_drain_validation_failure( + &self, + entry: &crate::changeset::PendingContactCrypto, + validation: &crate::wallet::identity::crypto::validation::ContactRequestValidation, + policy_blocked: &mut std::collections::BTreeMap, + ) -> bool { + if validation.is_purpose_only() { + for reason in &validation.errors { + *policy_blocked.entry(reason.clone()).or_default() += 1; + } + tracing::debug!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + errors = ?validation.errors, + "drain: contact request key-purpose mismatch; leaving queued (not marking broken)" + ); + return false; + } + tracing::warn!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + errors = ?validation.errors, + "drain: contact request failed key-index validation; marking channel broken" + ); + self.mark_contact_channel_broken(&entry.owner_identity_id, &entry.contact_id) + .await; + true + } + /// Apply the dequeue for entries a drain just completed: remove them from /// their owners' in-memory queues and persist the removal. Returns how many /// were actually removed. 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..b10cb28e536 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,92 @@ mod tests { ); } + /// A `RegisterExternal` entry whose fault lies in OUR OWN key is decided + /// without a Platform round trip. + /// + /// The owner here is wallet-owned (so the drain gets past the HD-index + /// bail) but carries no keys at all, so `recipientKeyIndex` 0 resolves to + /// nothing — a hard, permanent fault that must break the channel. The mock + /// SDK has NO contact-identity fetch configured, so this can only pass if + /// the recipient half of the validation ran *before* the fetch: the old + /// ordering fetched first, failed transiently, and left the channel intact. + /// + /// That ordering is what keeps a purpose-rejected entry — which stays + /// queued by design, and so is retried on every sweep forever — from + /// spending a network round trip each time. + #[tokio::test] + async fn drain_decides_our_own_key_fault_without_fetching_the_contact() { + use crate::changeset::{PendingContactCrypto, PendingContactCryptoOp}; + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + use crate::wallet::identity::{ContactRequest, EstablishedContact}; + + 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 = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + 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"); + // Wallet-owned (HD index 0) but keyless: `recipientKeyIndex` 0 + // cannot resolve, which is a hard fault, not a purpose mismatch. + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 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 { + 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( + Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""), + Network::Testnet, + ); + let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + + assert_eq!( + drained, 1, + "a hard validation fault must clear the entry rather than retry it forever" + ); + 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, + "the channel must be marked broken from our own key alone — reaching this \ + verdict proves the recipient half ran before the (unconfigured) contact fetch" + ); + } + /// 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 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 03/11] 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 04/11] 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 608dac581ee7450588d22aac4ee66ee591041a10 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 19:08:39 +0300 Subject: [PATCH 05/11] fix(platform-wallet): make the split validation's mixed-failure policy explicit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #4373 caught that deciding on the recipient half alone silently changed the mixed-failure policy, contradicting this PR's own claim that only the ordering changed. It did: when our key is purpose-rejected AND the contact's key carries a hard fault, the composed validator merged both, saw `hard_error`, and marked the channel permanently broken; stopping at the recipient half classifies it purpose-only and leaves it queued. Keeping that behaviour, but as a stated decision rather than an accident. The `hard_error` precedence exists to stop a permanent fault from becoming a retry-forever loop — and that loop was expensive precisely because each retry fetched. With the fetch gone it costs a map lookup per sweep, while marking the channel broken is unappealable by the user: only a fresh request from the contact clears it. A sender-side hard fault still breaks the channel as soon as our own key stops being the blocker. Documented on `validate_recipient_key` and at the drain call site. `unaccepted_recipient_purpose_never_fetches_and_stays_recoverable` pins both halves: our key carries an unaccepted purpose, the contact identity IS configured on the mock with a hard sender-side fault, and the drain runs twice. Under the old ordering the fetch succeeds, the merge escalates to broken, and the entry is cleared (verified: the test fails with drained 1 vs 0 against the parent branch's file). Passing therefore proves no fetch was spent — a short-circuit that handled only hard recipient faults would still fail it. Also makes `validate_sender_key` / `validate_recipient_key` `pub(crate)`: they are drain-internal decomposition, and both the `crypto` and `validation` modules are public, so `pub` would have committed them as external API. --- .../src/wallet/identity/crypto/validation.rs | 28 +++- .../identity/network/contact_requests.rs | 8 + .../src/wallet/identity/network/payments.rs | 146 ++++++++++++++++++ 3 files changed, 178 insertions(+), 4 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 f263de5816b..ecbb03ca8c5 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs @@ -177,9 +177,11 @@ pub fn validate_contact_request( /// The sender half of [`validate_contact_request`] — the checks that need the /// **counterparty's** identity. /// -/// Split out so the deferred-crypto drain can run the recipient half first. -/// See [`validate_recipient_key`] for why that ordering matters. -pub fn validate_sender_key( +/// Crate-private: external callers go through the complete +/// [`validate_contact_request`] contract. Split out so the deferred-crypto +/// drain can run the recipient half first — see [`validate_recipient_key`] for +/// why that ordering matters, and what it changes for a mixed failure. +pub(crate) fn validate_sender_key( sender_identity: &Identity, sender_key_index: u32, ) -> ContactRequestValidation { @@ -241,7 +243,25 @@ pub fn validate_sender_key( /// every sweep, forever. Mainnet logs from one wallet show 27 contacts and 396 /// such fetch-then-reject cycles in a single session. Running this half first /// costs nothing and removes the network entirely from that loop. -pub fn validate_recipient_key( +/// +/// # What this changes for a MIXED failure +/// +/// Deciding on this half alone is a real policy change, not just a reordering. +/// When our key is purpose-rejected AND the sender's key carries a hard fault +/// (missing / disabled / wrong type), the composed [`validate_contact_request`] +/// would merge both, see `hard_error`, and mark the channel permanently +/// broken. Stopping here classifies it purpose-only and leaves it queued. +/// +/// That is the intended outcome. The `hard_error` precedence exists to stop a +/// genuinely permanent fault from becoming a retry-forever loop — but the +/// forever-loop it guards against was expensive precisely because each retry +/// fetched. With the fetch gone, a purpose-rejected entry costs a map lookup +/// per sweep, while marking the channel broken is unappealable by the user: +/// only a fresh request from the CONTACT clears it. Deferring the broken mark +/// until the fault is one we can see locally trades a cheap retry for an +/// irreversible one. A sender-side hard fault still marks the channel broken +/// the moment our own key stops being the blocker. +pub(crate) fn validate_recipient_key( recipient_identity: &Identity, recipient_key_index: u32, ) -> ContactRequestValidation { 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 75ed9290088..8f384c0eb8a 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 @@ -2065,6 +2065,14 @@ impl DashPayView<'_, B> { // (legacy documents reference our AUTHENTICATION/TRANSFER // key), and because such an entry stays queued the fetch // below would otherwise repeat on every sweep, forever. + // + // Deciding here means a MIXED failure — our key + // purpose-rejected and the contact's key hard-faulted — + // now leaves the entry queued where the composed validator + // would have marked the channel broken. Deliberate: see + // `validate_recipient_key`. Marking broken is unappealable + // by the user, and the retry it avoids no longer costs a + // fetch. let our_identity = { let wm = self.wallet_manager.read().await; wm.get_wallet_info(&self.wallet_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 5e41e75fddd..fcd80ee5242 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,152 @@ mod tests { ); } + /// An unaccepted recipient PURPOSE — the actual repeating case — is decided + /// locally, and a co-occurring sender-side hard fault does not change that. + /// + /// This is the discriminating test for both halves of the change: + /// + /// * The contact identity IS configured on the mock, and its key at the + /// sender index is missing — a hard fault. Under the old composed + /// validation the drain would fetch, merge both halves, see `hard_error`, + /// mark the channel broken and clear the entry. Asserting the entry is + /// still queued and the channel still intact therefore proves the fetch + /// never happened; a "hard faults only" short-circuit that still fetched + /// for purpose mismatches would fail here. + /// * It pins the deliberate mixed-failure policy change: purpose-rejected + /// on our side wins, and the entry stays recoverable. + /// + /// Drained twice, because the cost this PR removes is per sweep, not once. + #[tokio::test] + async fn unaccepted_recipient_purpose_never_fetches_and_stays_recoverable() { + 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]); + + // Our key at the referenced index: valid ECDSA, but a purpose the + // receive-side policy does not accept — a purpose-only rejection. + let our_key = IdentityPublicKey::V0(IdentityPublicKeyV0 { + id: 0, + purpose: Purpose::VOTING, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type: KeyType::ECDSA_SECP256K1, + read_only: false, + data: dashcore::secp256k1::PublicKey::from_secret_key( + &dashcore::secp256k1::Secp256k1::new(), + &dashcore::secp256k1::SecretKey::from_slice(&[0x24u8; 32]).expect("secret"), + ) + .serialize() + .to_vec() + .into(), + disabled_at: None, + }); + let our_identity = Identity::V0(IdentityV0 { + id: owner, + public_keys: [(0u32, our_key)].into_iter().collect(), + balance: 0, + revision: 0, + }); + + // The contact identity the drain WOULD fetch: keyless, so the sender + // index is a hard fault. Configured on the mock so that a fetch, if it + // happened, would succeed and escalate the verdict to "broken". + let mut sdk = dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk"); + sdk.mock() + .expect_fetch::(contact, Some(bare_identity([0xBB; 32]))) + .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 { + 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); + for pass in 1..=2 { + let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + assert_eq!( + drained, 0, + "pass {pass}: a purpose-rejected entry must stay queued, not be cleared" + ); + } + + 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_eq!( + managed.dashpay().pending_contact_crypto.len(), + 1, + "the entry must survive repeated drains so a policy change can still pick it up" + ); + assert!( + !managed + .dashpay() + .established_contacts() + .get(&contact) + .expect("contact resident") + .payment_channel_broken, + "the channel must stay intact — reaching this verdict without the configured \ + fetch being consumed is what proves no Platform round trip was spent" + ); + } + /// A `RegisterExternal` entry whose fault lies in OUR OWN key is decided /// without a Platform round trip. /// 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 06/11] 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 07/11] 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() ))) })? } From abe80c6f3872de3a773f1b96e672ff7c31a3358f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 23:06:20 +0300 Subject: [PATCH 08/11] perf(platform-wallet): move purpose-mismatch reasons into the drain summary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review on #4373: the helper borrowed the validation result and cloned every reason before the map lookup. These entries stay queued by design and revisit the path on every sweep, so that is an allocation per contact per pass for the life of the wallet — 27 per pass on the mainnet wallet that motivated this work. Takes the validation by value and moves the strings instead. Neither caller uses the result afterwards, so nothing is lost, and the common case (a reason already counted) now allocates nothing at all. --- .../identity/network/contact_requests.rs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 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 660754dc528..33feda02445 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 @@ -2166,7 +2166,7 @@ impl DashPayView<'_, B> { if self .apply_drain_validation_failure( entry, - &recipient_validation, + recipient_validation, &mut policy_blocked, ) .await @@ -2229,7 +2229,7 @@ impl DashPayView<'_, B> { ); if !validation.is_valid { if self - .apply_drain_validation_failure(entry, &validation, &mut policy_blocked) + .apply_drain_validation_failure(entry, validation, &mut policy_blocked) .await { cleared.push(entry.key()); @@ -2504,21 +2504,27 @@ impl DashPayView<'_, B> { /// collecting it. /// /// Returns `true` when the caller should clear the entry from the queue. + /// + /// Takes `validation` by value: a purpose-rejected entry stays queued by + /// design and comes back through here on every sweep, so cloning its + /// reasons into the summary would allocate once per contact per pass for + /// the life of the wallet. Moving them costs nothing — neither caller uses + /// the result afterwards. async fn apply_drain_validation_failure( &self, entry: &crate::changeset::PendingContactCrypto, - validation: &crate::wallet::identity::crypto::validation::ContactRequestValidation, + validation: crate::wallet::identity::crypto::validation::ContactRequestValidation, policy_blocked: &mut std::collections::BTreeMap, ) -> bool { if validation.is_purpose_only() { - for reason in &validation.errors { - *policy_blocked.entry(reason.clone()).or_default() += 1; - } tracing::debug!( owner = %entry.owner_identity_id, contact = %entry.contact_id, errors = ?validation.errors, "drain: contact request key-purpose mismatch; leaving queued (not marking broken)" ); + for reason in validation.errors { + *policy_blocked.entry(reason).or_default() += 1; + } return false; } tracing::warn!( From d9afc996c06291784f5a797815718bf6698eef6f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:51:04 +0300 Subject: [PATCH 09/11] fix(platform-wallet): state the decrypt diagnostics as likelihoods, not verdicts Review on #4373, and the sharper of the two findings. The messages added for the legacy-interop investigation asserted their conclusions outright: a PKCS7 rejection "means the ECDH shared secret did not match", and a clean unpad "SUCCEEDED, so the shared secret matched". Neither follows. `decrypt_extended_public_key` is unauthenticated AES-CBC with padding as its only check, so a rejection is equally consistent with a corrupted ciphertext, and a wrong key clears PKCS7 roughly 1 in 256 times and lands in the length branch instead. A diagnostic whose whole purpose is to direct an investigation is the last place to overstate certainty, so both now name the likely cause and the alternative that produces the same symptom. Also gates the identity key-inventory block on `tracing::enabled!(INFO)`. It allocates a set, a string per key and a join, and takes the wallet-manager read lock; purpose-rejected entries revisit this path every sweep, so leaving that unconditional added recurring cost to the path this PR exists to make cheap. --- .../identity/network/contact_requests.rs | 7 ++- .../src/wallet/identity/network/contacts.rs | 44 ++++++++++--------- 2 files changed, 30 insertions(+), 21 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 33feda02445..33627e9c170 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 @@ -1940,7 +1940,12 @@ impl DashPayView<'_, B> { // 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. - { + // Gated on the level: the block allocates a set, a string per key and a + // join, and takes the wallet-manager read lock. Purpose-rejected + // entries stay queued and revisit this path every sweep, so leaving + // that work unconditional would add recurring cost to the very path + // this change exists to make cheap. + if tracing::enabled!(tracing::Level::INFO) { use dpp::identity::accessors::IdentityGettersV0; use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; let owners: std::collections::BTreeSet = entries 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 7d2ef339a96..821bfe9e4d9 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs @@ -450,22 +450,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. + // This failing is the most diagnostic event on the whole path, so the + // message names what it points at — as a likelihood, not a verdict. + // AES-CBC is unauthenticated here and PKCS7 is the only check on the + // plaintext, so a padding rejection is consistent with a mismatched + // ECDH secret AND with a corrupted or malformed ciphertext; a wrong key + // also clears padding roughly 1 in 256 times and lands at step 3 + // instead. Neither outcome proves which, and stating otherwise would + // misdirect exactly the legacy-interop investigation these messages + // exist to serve. The ciphertext length is included because a + // non-96-byte blob points 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}); ciphertext {} bytes — the ECDH \ - shared secret did not match the sender's (PKCS7 rejected the plaintext)", + "Failed to decrypt contact xpub ({e}); ciphertext {} bytes. PKCS7 rejected \ + the plaintext — most likely the ECDH shared secret did not match the \ + sender's, though a corrupted ciphertext produces the same symptom", contact_encrypted_xpub.len() ))) })?; @@ -491,17 +493,19 @@ 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 + // PKCS7 unpadded cleanly but the plaintext is not a shape we + // know. That is consistent with a correct secret over an + // unexpected LAYOUT, and also with a wrong key whose garbage + // happened to carry valid padding (~1 in 256) — the length + // is the best discriminator available, so it leads the + // message, but it is not proof either way. 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 {} 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", + form nor a 78/107-byte BIP32/DIP-14 serialization ({e}). PKCS7 accepted \ + the plaintext, which suggests the shared secret matched and the layout \ + differs, but unauthenticated CBC also lets a wrong key land here", decrypted_xpub_bytes.len() ))) })? From fea6e95e30e036452cc3c86358b8da8d2e13c50f Mon Sep 17 00:00:00 2001 From: romchornyi Date: Wed, 12 Aug 2026 01:05:01 +0300 Subject: [PATCH 10/11] fix(platform-wallet): pool the same funding sources for a contact payment as for a plain send (#4378) Co-authored-by: Roman <51091564+jeanpierreroma@users.noreply.github.com> --- .../rs-platform-wallet/src/wallet/core/mod.rs | 1 + .../src/wallet/core/transaction.rs | 2 +- .../src/wallet/identity/network/payments.rs | 300 +++++++++++++++--- 3 files changed, 265 insertions(+), 38 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/core/mod.rs b/packages/rs-platform-wallet/src/wallet/core/mod.rs index dbc42285aa2..36fe92318d6 100644 --- a/packages/rs-platform-wallet/src/wallet/core/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/core/mod.rs @@ -10,5 +10,6 @@ pub mod wallet; pub use balance::WalletBalance; pub use balance_handler::BalanceUpdateHandler; pub use generation::WalletGeneration; +pub(crate) use transaction::resolve_source_accounts; pub use transaction::{SignedCoreTransaction, ASSET_LOCK_FUNDING_SOURCES, SEND_FUNDING_SOURCES}; pub use wallet::CoreWallet; diff --git a/packages/rs-platform-wallet/src/wallet/core/transaction.rs b/packages/rs-platform-wallet/src/wallet/core/transaction.rs index 3ee44e049f6..d7f883cf1e0 100644 --- a/packages/rs-platform-wallet/src/wallet/core/transaction.rs +++ b/packages/rs-platform-wallet/src/wallet/core/transaction.rs @@ -266,7 +266,7 @@ pub const ASSET_LOCK_FUNDING_SOURCES: [AccountTypePreference; 3] = SEND_FUNDING_ /// DashPay source. A set selector matching nothing resolves to an empty list, /// not an error — a wallet with no contacts still sends from its standard /// accounts. -fn resolve_source_accounts( +pub(crate) fn resolve_source_accounts( accounts: &key_wallet::account::ManagedAccountCollection, preference: AccountTypePreference, source_index: u32, 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 63ae99350dd..22653dcff1f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -1098,7 +1098,7 @@ impl DashPayView<'_, B> { // re-acquires that (non-reentrant) lock internally. self.drain_pending_contact_crypto(provider).await; - let (payment_address, used_flip_changeset, tx, fee) = { + let (payment_address, used_flip_changeset, tx, fee, funding_accounts) = { let mut wm = self.wallet_manager.write().await; // Resolve the external account's xpub so we can derive addresses. @@ -1199,32 +1199,72 @@ impl DashPayView<'_, B> { let current_height = info.core_wallet.synced_height(); - let managed_account = info - .core_wallet - .accounts - .standard_bip44_accounts - .get_mut(&0) - .ok_or_else(|| { - PlatformWalletError::TransactionBuild( - "BIP-44 managed account 0 not found".to_string(), - ) - })?; - let account = wallet - .accounts - .standard_bip44_accounts - .get(&0) - .ok_or_else(|| { - PlatformWalletError::TransactionBuild( - "BIP-44 account 0 not found in wallet".to_string(), - ) - })?; - - let builder = TransactionBuilder::new() + // Pool the same funding set as a plain send (#4329): BIP44 + + // BIP32 + every DashPay receiving account. Pinning this path to + // BIP44 alone was the reason a wallet whose balance had moved into + // contact-receiving accounts hit "Insufficient funds" on a screen + // showing plenty — the exact symptom #4329 fixed for the core send + // path, which this path never picked up (it only took that PR's + // `set_funding` → `add_funding` rename). + // + // Order is load-bearing: BIP44 is offered first, and the builder + // takes the change address from the first funding source, so + // change keeps returning to BIP44 as before. CoinJoin stays out by + // construction — spending mixed outputs alongside transparent ones + // links them and undoes the mixing — and so do the contact + // *external* accounts, which hold the counterparty's xpub and no + // key this wallet can sign with. + let mut builder = TransactionBuilder::new() .set_current_height(current_height) .set_selection_strategy(SelectionStrategy::LargestFirst) - .add_funding(managed_account, account) .add_output(&payment_address, amount_duffs); + // Derivation paths for every offered UTXO, since the signer closure + // below can no longer resolve them from one account. + let mut funding_paths: std::collections::HashMap< + dashcore::Address, + key_wallet::bip32::DerivationPath, + > = std::collections::HashMap::new(); + // Accounts whose UTXOs were OFFERED to selection. A superset of the + // contributors — releasing a reservation on an account that + // supplied nothing is a no-op, and the superset is what keeps the + // rejection path from stranding inputs in an account we forgot. + let mut offered_accounts: Vec = Vec::new(); + + for &preference in crate::SEND_FUNDING_SOURCES.iter() { + for at in crate::wallet::core::resolve_source_accounts( + &info.core_wallet.accounts, + preference, + account_index, + ) { + if offered_accounts.contains(&at) { + continue; + } + // A source the wallet simply does not have contributes + // nothing rather than failing the send — a wallet with no + // BIP32 account, or no contacts, still pays from BIP44. + let (Some(account), Some(managed)) = ( + wallet.accounts.account_of_type(at), + info.core_wallet.accounts.funds_account_mut(&at), + ) else { + continue; + }; + for utxo in managed.utxos.values() { + if let Some(path) = managed.address_derivation_path(&utxo.address) { + funding_paths.insert(utxo.address.clone(), path); + } + } + builder = builder.add_funding(managed, account); + offered_accounts.push(at); + } + } + if offered_accounts.is_empty() { + return Err(PlatformWalletError::TransactionBuild( + "no spendable funding account (BIP44/BIP32/DashPay receiving) found" + .to_string(), + )); + } + // Sign through the injected signer (blanket // `impl TransactionSigner for S`) rather than the // resident `wallet`, so funding-input signatures are produced @@ -1235,9 +1275,7 @@ impl DashPayView<'_, B> { // rust-dashcore#872 (pinned above). No caller-side // recomputation needed. let (tx, fee) = match builder - .build_signed(signer, |addr| { - managed_account.address_derivation_path(&addr) - }) + .build_signed(signer, |addr| funding_paths.get(&addr).cloned()) .await { Ok(built) => built, @@ -1269,7 +1307,13 @@ impl DashPayView<'_, B> { } }; - (payment_address, used_flip_changeset, tx, fee) + ( + payment_address, + used_flip_changeset, + tx, + fee, + offered_accounts, + ) }; // Persist the payment-address used flip now that the wallet-manager @@ -1291,16 +1335,28 @@ impl DashPayView<'_, B> { // --- 3. Broadcast the transaction, releasing the build's UTXO // reservation if the broadcast is definitively rejected pre-send. --- - let txid = match crate::wallet::reservations::broadcast_releasing_on_rejection( - self.broadcaster.as_ref(), - &self.wallet_manager, - &self.wallet_id, - key_wallet::account::account_type::StandardAccountType::BIP44Account, - 0, - &tx, - ) - .await - { + // Release across EVERY account that offered inputs, not just BIP44: + // now that the build pools funding, a rejected broadcast whose inputs + // came from a BIP32 or contact-receiving account would otherwise leave + // those reserved until the TTL backstop, and an immediate retry would + // fail with a spurious insufficient-funds. + let broadcast_result = match self.broadcaster.broadcast(&tx).await { + Err(e) if matches!(e, crate::broadcaster::BroadcastError::Rejected { .. }) => { + crate::wallet::reservations::release_reservation_after_rejected_broadcast( + &self.wallet_manager, + &self.wallet_id, + &funding_accounts, + &tx, + // This path does not thread the build's reservation token + // either; keep the historical unconditional release. + None, + ) + .await; + Err(e) + } + other => other, + }; + let txid = match broadcast_result { Ok(txid) => txid, Err(e) => { // A definitive rejection means the transaction never reached @@ -5949,6 +6005,110 @@ mod tests { } } + /// A contact payment funds from a DashPay **receiving** account when BIP44 + /// alone cannot cover it — the pooled funding set a plain send has used + /// since #4329. + /// + /// This path kept its BIP44-only pin through that PR (it took only the + /// `set_funding` → `add_funding` rename), so a wallet whose balance had + /// moved into contact-receiving accounts saw the funds in its total and got + /// `Insufficient funds` trying to pay a contact. Reported from mainnet + /// after 8 successful contact payments drained BIP44: `available 41505, + /// required 100000`, on a screen showing plenty. + /// + /// BIP44 is left empty here, so reaching the signer at all proves the + /// receiving account was offered to selection. + #[tokio::test] + async fn contact_payment_funds_from_a_dashpay_receiving_account() { + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + + 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([0x11; 32]); + let contact_id = Identifier::from([0x22; 32]); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity( + bare_identity([0x11; 32]), + 0, + wallet_id, + &WalletPersister::new(wallet_id, Arc::clone(&persister) as _), + ) + .expect("add owner"); + } + + // The receiving side: register the account, then give it the wallet's + // only money. BIP44 stays empty. + iw.dashpay() + .register_contact_account( + &owner_id, + &contact_id, + 0, + test_receiving_xpub(&owner_id, &contact_id), + ) + .await + .expect("register receiving account"); + plant_receival_utxo(&manager, wallet_id, owner_id, contact_id, 0x21, 1_000_000).await; + + // The sending side, so the external-account lookup passes. + let shared_key = [0x55u8; 32]; + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("mnemonic") + .to_seed(""); + 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, + &owner_id, + &contact_id, + ) + .expect("derive a valid compact xpub") + .compact + .to_bytes() + }; + let encrypted = + platform_encryption::encrypt_extended_public_key(&shared_key, &[0x11u8; 16], &compact); + iw.dashpay() + .register_external_contact_account( + &owner_id, + &bare_identity([0x22; 32]), + &encrypted, + zeroize::Zeroizing::new(shared_key), + ) + .await + .expect("register external account"); + + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + let signer = SeedSigner::new(seed, Network::Testnet); + let result = iw + .dashpay() + .send_payment(&owner_id, &contact_id, 100_000, None, &signer, &provider) + .await; + + // Whatever happens later (this wallet has no live broadcaster), the one + // outcome the fix rules out is coin selection refusing for lack of + // funds while a funded receiving account sits right there. + if let Err(e) = &result { + let msg = e.to_string(); + assert!( + !msg.contains("Insufficient funds") && !msg.contains("No UTXOs available"), + "the contact-receiving account's 1_000_000 duffs must be offered to \ + selection — BIP44-only funding is the bug this pins, got: {msg}" + ); + } + } + /// A failed `build_signed` must return the consumed payment address to /// the pool. Without the rollback every failed build (insufficient /// funds, a refusing signer) permanently advances the next index by one: @@ -6059,6 +6219,72 @@ mod tests { ); } + /// A rejected broadcast releases the UTXO reservation on EVERY account + /// that funded the payment, not just BIP44. + /// + /// Pooling made this reachable: before it, one account funded the send and + /// releasing that one was complete. Now inputs can come from a BIP32 or + /// contact-receiving account too, and a release that still named only + /// BIP44 would leave those reserved until the TTL backstop — so the + /// immediate retry a user makes after "payment rejected" would fail with a + /// spurious insufficient-funds on money that is demonstrably theirs. + /// + /// Neither account can cover the payment alone here, so a successful retry + /// is only possible if BOTH were released. + #[tokio::test] + async fn rejected_broadcast_releases_every_pooled_funding_account() { + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + + let (manager, _persister, wallet_id, owner_id, contact_id) = + register_sender_and_external_account().await; + let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet_arc.identity(); + + // 60_000 + 60_000, for a 100_000 payment: neither side alone is + // enough, so selection must take from both and a retry must find both + // free again. + fund_bip44_account_0(&manager, wallet_id, 0xC1, 60_000).await; + iw.dashpay() + .register_contact_account( + &owner_id, + &contact_id, + 0, + test_receiving_xpub(&owner_id, &contact_id), + ) + .await + .expect("register receiving account"); + plant_receival_utxo(&manager, wallet_id, owner_id, contact_id, 0xC2, 60_000).await; + + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""); + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + let signer = SeedSigner::new(seed, Network::Testnet); + + let rejecting = with_rejecting_broadcaster(iw); + let err = rejecting + .dashpay() + .send_payment(&owner_id, &contact_id, 100_000, None, &signer, &provider) + .await + .expect_err("the rejecting broadcaster must fail the send"); + assert!( + matches!(err, PlatformWalletError::TransactionBroadcast(_)), + "the send must reach the broadcast (so inputs were reserved), got: {err:?}" + ); + + // The retry is the assertion: it needs inputs from both accounts, so + // it can only succeed if the rejection released both reservations. + let accepting = with_accepting_broadcaster(iw); + accepting + .dashpay() + .send_payment(&owner_id, &contact_id, 100_000, None, &signer, &provider) + .await + .expect( + "an immediate retry must reselect every pooled input — a reservation left \ + on the contact-receiving account strands funds until the TTL backstop", + ); + } + /// A definitively rejected broadcast must return the consumed payment /// address to the pool AND persist the revert — unlike a failed build, /// the used flip was already persisted before the broadcast attempt, so From 0eac01e41cf29d1d267554433c40073bd12d9100 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 12 Aug 2026 01:15:37 +0300 Subject: [PATCH 11/11] fix(platform-wallet): only an immutable fault may permanently break a contact channel MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review pushed back that a contact request should never be rejected locally, since it cleared consensus. Not absolute — ECDH over a BLS or hash-only key is impossible whatever consensus says — but the principle lands on the part that matters: `payment_channel_broken` is permanent, heals only when the CONTACT sends a fresh request, and the user cannot appeal it. The decision now turns on whether the fault can ever resolve, not on how badly validation failed: - An **absent key id** becomes retryable. Consensus checks nothing about the keys a `contactRequest` names, and identities gain keys — that is what the DashPay enablement flow does, and what dashwallet-ios#981 exists to notice when it happened on another device. A document referencing a key we do not have *yet* was ending the relationship over a gap that may close on its own. Same on the contact's side: the drain's "contact encryption key missing" arm now leaves the entry queued instead of breaking the channel. - **Key type** and **disabled key** stay permanent: a key's type is fixed for its lifetime, and a key we revoked is one we will never use again. Both are facts about immutable or deliberate state, not about today's snapshot. `is_permanent()` replaces `is_purpose_only()` as the drain's predicate, so the question it answers is the one being asked. The co-occurrence guard survives unchanged — a purpose mismatch alongside a genuinely permanent fault is still permanent. This is also the failure mode this stack already lived through: the key-purpose policy was wrong for two years, and only its softer classification kept 27 mainnet contacts recoverable. Had it been "hard", they would have been permanently broken with no path back. Two ordering tests were pinned on an absent key being permanent, so they no longer discriminated; both re-armed on a BLS key at the referenced index, which is permanent for a reason that cannot change. `an_absent_key_is_not_a_permanent_fault` and `a_non_ecdh_key_type_is_a_permanent_fault` pin the new boundary — verified the first fails when the absent key is put back on the permanent classification. --- .../src/wallet/identity/crypto/validation.rs | 79 +++++++++++++++++- .../identity/network/contact_requests.rs | 34 ++++---- .../src/wallet/identity/network/payments.rs | 81 ++++++++++++++++--- 3 files changed, 163 insertions(+), 31 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 ecbb03ca8c5..0110458e026 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/validation.rs @@ -71,6 +71,14 @@ impl ContactRequestValidation { self.hard_error = true; } + /// Add an ABSENT-KEY error: the referenced key id does not exist on the + /// identity *today*. Sets `is_valid = false` but NOT `hard_error`, because + /// identities gain keys — see [`is_permanent`](Self::is_permanent). + pub fn add_absent_key_error(&mut self, error: String) { + self.errors.push(error); + self.is_valid = false; + } + /// Add a key-PURPOSE error: sets `is_valid = false` AND flags /// `purpose_mismatch` so callers can downgrade a *purpose-only* failure /// to a non-permanent skip rather than a permanent broken-channel mark. @@ -89,11 +97,31 @@ impl ContactRequestValidation { /// Whether the *sole* cause of invalidity is a key-purpose mismatch — /// the only case that may be downgraded to a non-permanent skip. /// A purpose mismatch that co-occurs with a hard error (disabled / - /// missing / wrong-type key) is NOT purpose-only and must stay permanent. + /// wrong-type key) is NOT purpose-only and must stay permanent. pub fn is_purpose_only(&self) -> bool { self.purpose_mismatch && !self.hard_error } + /// Whether this failure can never resolve on its own — the only kind that + /// may permanently break a contact's payment channel. + /// + /// The distinction is not "did validation fail" but "can the world change + /// such that it stops failing". A `contactRequest` clears consensus without + /// consensus checking anything about the keys it names, so a document can + /// reference a key id our identity does not have *yet*: identities gain + /// keys (that is what the DashPay enablement flow does, and what + /// dashwallet-ios#981 exists to notice when it happened on another device). + /// Recording that as permanent turns a temporary gap into a relationship + /// the user cannot repair — only a fresh request from the CONTACT clears + /// the flag. + /// + /// So an absent key is retryable, alongside a purpose mismatch. What stays + /// permanent is what immutable facts make impossible: a key whose *type* + /// cannot do ECDH, and a key we have deliberately disabled. + pub fn is_permanent(&self) -> bool { + self.hard_error + } + /// Merge another validation result into this one. pub fn merge(&mut self, other: ContactRequestValidation) { self.errors.extend(other.errors); @@ -221,7 +249,7 @@ pub(crate) fn validate_sender_key( } } None => { - validation.add_error(format!( + validation.add_absent_key_error(format!( "Sender key index {} not found on identity {}", sender_key_index, sender_identity.id(), @@ -319,7 +347,7 @@ pub(crate) fn validate_recipient_key( } } None => { - validation.add_error(format!( + validation.add_absent_key_error(format!( "Recipient key index {} not found on identity {}", recipient_key_index, recipient_identity.id(), @@ -627,6 +655,51 @@ mod tests { assert!(!result.purpose_mismatch); } + /// A key id the identity does not have **yet** must not be permanent. + /// + /// Identities gain keys — that is what the DashPay enablement flow does, + /// and dashwallet-ios#981 exists to notice it happening on another device. + /// A `contactRequest` clears consensus without consensus checking anything + /// about the keys it names, and it can never be re-minted, so recording + /// "we have no key 5 today" as a permanent verdict ends a relationship over + /// a gap that may close on its own — and only the CONTACT can clear the + /// flag, so the user cannot appeal it. + #[test] + fn an_absent_key_is_not_a_permanent_fault() { + let sender = make_identity(vec![make_key( + 0, + KeyType::ECDSA_SECP256K1, + Purpose::ENCRYPTION, + )]); + let recipient = make_identity(vec![]); + + let result = validate_contact_request(&sender, 0, &recipient, 0); + assert!(!result.is_valid, "an absent key still fails validation"); + assert!( + !result.is_permanent(), + "but it must be retryable: the identity can gain the key later" + ); + } + + /// A key type that cannot do ECDH is permanent — a key's type is fixed for + /// its lifetime, so no future state makes this request usable. + #[test] + fn a_non_ecdh_key_type_is_a_permanent_fault() { + let sender = make_identity(vec![make_key( + 0, + KeyType::ECDSA_SECP256K1, + Purpose::ENCRYPTION, + )]); + let recipient = make_identity(vec![make_key(0, KeyType::BLS12_381, Purpose::ENCRYPTION)]); + + let result = validate_contact_request(&sender, 0, &recipient, 0); + assert!(!result.is_valid); + assert!( + result.is_permanent(), + "a BLS key can never do secp256k1 ECDH, so this one may break the channel" + ); + } + /// 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 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 33627e9c170..c99018792cf 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 @@ -2268,16 +2268,17 @@ impl DashPayView<'_, B> { } } None => { + // Left queued, not broken: the contact's identity + // can gain this key later, exactly as ours can, and + // the document that names it cleared consensus and + // cannot be re-minted. Breaking here would end the + // relationship over a gap that may close by itself. tracing::warn!( owner = %entry.owner_identity_id, contact = %entry.contact_id, - "drain: contact encryption key missing; marking channel broken" + key_index = *contact_encryption_key_index, + "drain: contact has no key at the referenced index yet; \ + leaving queued (not marking broken)" ); - self.mark_contact_channel_broken( - &entry.owner_identity_id, - &entry.contact_id, - ) - .await; - cleared.push(entry.key()); continue; } }; @@ -2499,14 +2500,15 @@ impl DashPayView<'_, B> { /// The drain's validation-failure policy, shared by the recipient-half and /// sender-half checks so both halves classify identically. /// - /// - A **purpose-only** failure is counted into `policy_blocked` for the - /// drain's end-of-run summary and left queued: the request is immutable, - /// so what might change is our acceptance policy, and a channel marked - /// broken here would need a superseding request from the contact to heal - /// — an appeal the user cannot file. - /// - Anything else (missing key, wrong key type, disabled key) is a real - /// permanent fault: mark the channel broken so the sweep stops - /// collecting it. + /// - A failure that can still resolve — a purpose mismatch (our acceptance + /// policy might change) or an absent key id (identities gain keys) — is + /// counted into `policy_blocked` and left queued. The `contactRequest` + /// cleared consensus and is immutable; a channel marked broken here needs + /// a superseding request from the CONTACT to heal, an appeal the user + /// cannot file. + /// - Only a fault that immutable facts make permanent — a key type that + /// cannot do ECDH, a key we disabled — breaks the channel, so the sweep + /// stops collecting it. /// /// Returns `true` when the caller should clear the entry from the queue. /// @@ -2521,7 +2523,7 @@ impl DashPayView<'_, B> { validation: crate::wallet::identity::crypto::validation::ContactRequestValidation, policy_blocked: &mut std::collections::BTreeMap, ) -> bool { - if validation.is_purpose_only() { + if !validation.is_permanent() { tracing::debug!( owner = %entry.owner_identity_id, contact = %entry.contact_id, errors = ?validation.errors, 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 63ae99350dd..c62c67d2aec 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -1795,6 +1795,44 @@ mod tests { .xpub } + /// An identity carrying exactly one key, for the validation paths that + /// turn on a key's type or purpose rather than its presence. + fn identity_with_key( + id_bytes: [u8; 32], + key_id: u32, + key_type: dpp::identity::KeyType, + purpose: dpp::identity::Purpose, + ) -> Identity { + use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dpp::identity::{IdentityPublicKey, SecurityLevel}; + let data = dashcore::secp256k1::PublicKey::from_secret_key( + &dashcore::secp256k1::Secp256k1::new(), + &dashcore::secp256k1::SecretKey::from_slice(&[0x37u8; 32]).expect("secret"), + ) + .serialize() + .to_vec(); + Identity::V0(IdentityV0 { + id: Identifier::from(id_bytes), + public_keys: [( + key_id, + IdentityPublicKey::V0(IdentityPublicKeyV0 { + id: key_id, + purpose, + security_level: SecurityLevel::HIGH, + contract_bounds: None, + key_type, + read_only: false, + data: data.into(), + disabled_at: None, + }), + )] + .into_iter() + .collect(), + balance: 0, + revision: 0, + }) + } + fn bare_identity(id_bytes: [u8; 32]) -> Identity { Identity::V0(IdentityV0 { id: Identifier::from(id_bytes), @@ -5078,12 +5116,22 @@ mod tests { revision: 0, }); - // The contact identity the drain WOULD fetch: keyless, so the sender - // index is a hard fault. Configured on the mock so that a fetch, if it - // happened, would succeed and escalate the verdict to "broken". + // The contact identity the drain WOULD fetch: its key at the sender + // index is BLS, a permanent fault. Configured on the mock so that a + // fetch, if it happened, would succeed and escalate the verdict to + // "broken". (A keyless contact would not work as the discriminator — + // an absent key is retryable by design.) let mut sdk = dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk"); sdk.mock() - .expect_fetch::(contact, Some(bare_identity([0xBB; 32]))) + .expect_fetch::( + contact, + Some(identity_with_key( + [0xBB; 32], + 0, + KeyType::BLS12_381, + Purpose::ENCRYPTION, + )), + ) .await .expect("set the contact-identity fetch expectation"); let sdk = Arc::new(sdk); @@ -5176,11 +5224,14 @@ mod tests { /// without a Platform round trip. /// /// The owner here is wallet-owned (so the drain gets past the HD-index - /// bail) but carries no keys at all, so `recipientKeyIndex` 0 resolves to - /// nothing — a hard, permanent fault that must break the channel. The mock - /// SDK has NO contact-identity fetch configured, so this can only pass if - /// the recipient half of the validation ran *before* the fetch: the old - /// ordering fetched first, failed transiently, and left the channel intact. + /// bail) and its key at `recipientKeyIndex` 0 is BLS — a type that can + /// never do ECDH, so this is one of the few genuinely permanent faults and + /// must break the channel. (An *absent* key would not do: identities gain + /// keys, so that is deliberately retryable.) The mock SDK has NO + /// contact-identity fetch configured, so this can only pass if the + /// recipient half of the validation ran *before* the fetch: the old + /// ordering fetched first, failed transiently, and left the channel + /// intact. /// /// That ordering is what keeps a purpose-rejected entry — which stays /// queued by design, and so is retried on every sweep forever — from @@ -5190,6 +5241,7 @@ mod tests { use crate::changeset::{PendingContactCrypto, PendingContactCryptoOp}; use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; use crate::wallet::identity::{ContactRequest, EstablishedContact}; + use dpp::identity::{KeyType, Purpose}; let (manager, persister, wallet_id) = make_wallet().await; let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); @@ -5201,10 +5253,15 @@ mod tests { { let mut wm = iw.wallet_manager.write().await; let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); - // Wallet-owned (HD index 0) but keyless: `recipientKeyIndex` 0 - // cannot resolve, which is a hard fault, not a purpose mismatch. + // Wallet-owned (HD index 0) with a BLS key at index 0: the + // referenced key exists but its type rules out ECDH permanently. info.identity_manager - .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .add_identity( + identity_with_key([0xAA; 32], 0, KeyType::BLS12_381, Purpose::ENCRYPTION), + 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);