perf(platform-wallet): keep the contact fetch off the drain's repeating-rejection path

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;

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,

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);
@@ -165,12 +193,28 @@ 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.
+///
+/// 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 {
     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.
@@ -205,17 +249,52 @@ pub fn validate_contact_request(
             }
         }
         None => {
-            validation.add_error(format!(
+            validation.add_absent_key_error(format!(
                 "Sender key index {} not found on identity {}",
                 sender_key_index,
                 sender_identity.id(),
             ));
         }
     }
 
-    // -----------------------------------------------------------------------
-    // 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.
+///
+/// # 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 {
+    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.
@@ -268,7 +347,7 @@ pub fn validate_contact_request(
             }
         }
         None => {
-            validation.add_error(format!(
+            validation.add_absent_key_error(format!(
                 "Recipient key index {} not found on identity {}",
                 recipient_key_index,
                 recipient_identity.id(),
@@ -576,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