diff --git a/packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs b/packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs index be6087ead65..e4ba065bf3c 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/schema/contacts.rs @@ -16,7 +16,10 @@ use platform_wallet::wallet::platform_wallet::WalletId; use crate::sqlite::error::WalletStorageError; use crate::sqlite::schema::blob; -#[cfg(feature = "__test-helpers")] +// `any(test, …)`, not `__test-helpers`-only: `load_ignored_senders` (and its +// `decode_pair_key` helper) are gated the same way — they're reached by +// `identities::load_state`, whose gate includes plain `test`. +#[cfg(any(test, feature = "__test-helpers"))] use dpp::prelude::Identifier; #[cfg(feature = "__test-helpers")] use platform_wallet::changeset::{ @@ -24,9 +27,9 @@ use platform_wallet::changeset::{ }; #[cfg(feature = "__test-helpers")] use platform_wallet::wallet::identity::{ContactRequest, EstablishedContact}; -#[cfg(feature = "__test-helpers")] +#[cfg(any(test, feature = "__test-helpers"))] use rusqlite::Connection; -#[cfg(feature = "__test-helpers")] +#[cfg(any(test, feature = "__test-helpers"))] use std::collections::BTreeMap; /// Single source of truth for the `contacts.state` TEXT-column domain. @@ -140,14 +143,24 @@ pub fn apply( } } if !cs.removed_sent.is_empty() { + // Pending-sent tombstone. State-filtered: the contacts table is one + // row per pair, so an unfiltered DELETE would also destroy an + // `established` row — both request blobs plus the user's + // alias/note/hidden/accepted-accounts — if an emitter ever produced + // a pending tombstone for an established pair. The changeset + // contract says `removed_sent` means "the pending sent entry was + // removed", so the writer only ever deletes a pending-sent row. + let sent = contact_state_db_label(ContactState::Sent); let mut stmt = tx.prepare_cached( - "DELETE FROM contacts WHERE wallet_id = ?1 AND owner_id = ?2 AND contact_id = ?3", + "DELETE FROM contacts \ + WHERE wallet_id = ?1 AND owner_id = ?2 AND contact_id = ?3 AND state = ?4", )?; for key in &cs.removed_sent { stmt.execute(params![ wallet_id.as_slice(), key.owner_id.as_slice(), key.recipient_id.as_slice(), + sent, ])?; } } @@ -180,14 +193,20 @@ pub fn apply( } } if !cs.removed_incoming.is_empty() { + // Pending-received tombstone. State-filtered for the same reason + // as `removed_sent` above: never let a pending tombstone destroy + // an `established` pair-row. + let received = contact_state_db_label(ContactState::Received); let mut stmt = tx.prepare_cached( - "DELETE FROM contacts WHERE wallet_id = ?1 AND owner_id = ?2 AND contact_id = ?3", + "DELETE FROM contacts \ + WHERE wallet_id = ?1 AND owner_id = ?2 AND contact_id = ?3 AND state = ?4", )?; for key in &cs.removed_incoming { stmt.execute(params![ wallet_id.as_slice(), key.owner_id.as_slice(), key.sender_id.as_slice(), + received, ])?; } } @@ -377,7 +396,9 @@ fn decode_request( } } -#[cfg(feature = "__test-helpers")] +// Widened to `any(test, …)` alongside `load_ignored_senders`, its only +// `test`-arm caller (the `__test-helpers`-gated readers use it too). +#[cfg(any(test, feature = "__test-helpers"))] fn decode_pair_key(a: &[u8], b: &[u8]) -> Result<(Identifier, Identifier), WalletStorageError> { let a32 = <[u8; 32]>::try_from(a) .map_err(|_| WalletStorageError::blob_decode("contacts.id column is not 32 bytes"))?; @@ -397,6 +418,36 @@ pub fn load_state_for_test( load_state(conn, wallet_id) } +/// Read the wallet's `ignored_senders` rows, grouped per owner identity. +/// +/// This table — not the `ignored_senders` snapshot inside the identity +/// `entry_blob` — is the AUTHORITATIVE ignore record at load time: every +/// ignore INSERTs a row and every un-ignore DELETEs it transactionally, +/// while the blob is only as fresh as the last identity-entry flush. +/// An un-ignore never re-flushes the identity entry (it persists only the +/// `ContactChangeSet`), and `IdentityChangeSet::merge` UNIONs the blob's +/// ignored set across buffered snapshots — so a blob-based restore would +/// resurrect un-ignored senders, stickily (the next snapshot re-persists +/// the resurrected entry). The identity loader therefore restores the +/// ignored set from this reader and disregards the blob field. +#[cfg(any(test, feature = "__test-helpers"))] +pub(crate) fn load_ignored_senders( + conn: &Connection, + wallet_id: &WalletId, +) -> Result>, WalletStorageError> { + let mut stmt = + conn.prepare("SELECT owner_id, sender_id FROM ignored_senders WHERE wallet_id = ?1")?; + let mut map: BTreeMap> = BTreeMap::new(); + let mut rows = stmt.query(params![wallet_id.as_slice()])?; + while let Some(row) = rows.next()? { + let owner: Vec = row.get(0)?; + let sender: Vec = row.get(1)?; + let (owner, sender) = decode_pair_key(&owner, &sender)?; + map.entry(owner).or_default().insert(sender); + } + Ok(map) +} + #[cfg(test)] mod tests { use super::*; diff --git a/packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs b/packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs index 224a77bbe9a..72b22435f27 100644 --- a/packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs +++ b/packages/rs-platform-wallet-storage/src/sqlite/schema/identities.rs @@ -158,6 +158,11 @@ pub fn load_state( let mut stmt = conn.prepare( "SELECT identity_id, entry_blob, tombstoned FROM identities WHERE wallet_id = ?1", )?; + // The ignored-senders TABLE is the authoritative ignore record (every + // ignore/un-ignore maintains it transactionally); the `entry_blob`'s + // snapshot copy can be stale — see `contacts::load_ignored_senders`. + let mut ignored_by_owner = + crate::sqlite::schema::contacts::load_ignored_senders(conn, wallet_id)?; let mut state = IdentityManagerStartState::default(); let mut rows = stmt.query(params![wallet_id.as_slice()])?; while let Some(row) = rows.next()? { @@ -168,7 +173,8 @@ pub fn load_state( continue; } let entry: IdentityEntry = blob::decode(&payload)?; - let managed = managed_identity_from_entry(&entry, wallet_id); + let ignored = ignored_by_owner.remove(&entry.id).unwrap_or_default(); + let managed = managed_identity_from_entry(&entry, wallet_id, ignored); match entry.identity_index { Some(idx) => { state @@ -193,6 +199,7 @@ pub fn load_state( fn managed_identity_from_entry( entry: &IdentityEntry, wallet_id: &WalletId, + ignored_senders: std::collections::BTreeSet, ) -> platform_wallet::wallet::identity::ManagedIdentity { use dpp::identity::v0::IdentityV0; use dpp::identity::Identity; @@ -214,17 +221,25 @@ fn managed_identity_from_entry( managed.contested_dpns_names = entry.contested_dpns_names.clone(); managed.wallet_id = entry.wallet_id.or(Some(*wallet_id)); // Scalar-snapshot collections ride the identity `entry_blob` - // (payments / profile / contact_profiles / ignored_senders), so they - // restore from `entry`. The relational request collections are loaded - // separately from the `contacts` table and stay defaulted here. + // (payments / profile / contact_profiles), so they restore from + // `entry`. The relational request collections are loaded separately + // from the `contacts` table and stay defaulted here. // High-water sync cursors, the per-session rescan guard, the // verify-failed auto-accept markers, and the deferred contact-crypto // queue (not persisted; a signerless sweep re-enqueues its ops on // load) are in-memory by design: a cold restore starts them at their - // defaults so the next sweep re-fetches / re-evaluates safely. The - // constructor starts a fresh empty ignored set, so per-element apply - // reproduces the persisted set exactly. - for sender in &entry.ignored_senders { + // defaults so the next sweep re-fetches / re-evaluates safely. + // + // Ignored senders restore from the `ignored_senders` TABLE (passed in + // by the loader), NOT from `entry.ignored_senders`: an un-ignore + // deletes only the table row (no fresh identity-entry flush), and the + // changeset merge UNIONs the blob's set across buffered snapshots — + // so the blob copy can resurrect an un-ignored sender. The table is + // maintained transactionally by both the ignore and un-ignore writers + // and is therefore authoritative. The constructor starts a fresh + // empty ignored set, so per-element apply reproduces the table's set + // exactly. + for sender in &ignored_senders { managed.apply_ignored_sender(*sender); } *managed.dashpay_profile_mut() = entry.dashpay_profile.clone(); diff --git a/packages/rs-platform-wallet-storage/tests/sqlite_load_reconstruction.rs b/packages/rs-platform-wallet-storage/tests/sqlite_load_reconstruction.rs index 9c97a96be1d..97cfaf974a7 100644 --- a/packages/rs-platform-wallet-storage/tests/sqlite_load_reconstruction.rs +++ b/packages/rs-platform-wallet-storage/tests/sqlite_load_reconstruction.rs @@ -1402,3 +1402,243 @@ fn tc_p4_010_empty_db_default_state() { assert!(logs_contain("wallets_seen=0")); assert!(logs_contain("wallets_pending_rehydration=0")); } + +/// A pending tombstone must never destroy an ESTABLISHED pair-row. The +/// contacts table is one row per `(owner, contact)` pair, so an unfiltered +/// `removed_incoming` / `removed_sent` DELETE aimed at a pending entry +/// would take the established row — both request blobs plus the user's +/// alias/note/hidden/accepted-accounts — with it (the ignore-an- +/// established-contact shape). The writer's state filter must scope the +/// DELETE to the pending state the tombstone describes. Was red against +/// the unfiltered DELETE. +#[test] +fn pending_tombstones_do_not_destroy_established_rows() { + let (persister, _tmp, path) = fresh_persister(); + let w = wid(0xA7); + ensure_wallet_meta(&persister, &w); + + let owner = Identifier::from([0x51; 32]); + let contact = Identifier::from([0x52; 32]); + let est_key = SentContactRequestKey { + owner_id: owner, + recipient_id: contact, + }; + + // Flush 1: the pair is established, with full user metadata. + let mut established = std::collections::BTreeMap::new(); + established.insert(est_key, established_contact(0x51, 0x52)); + persister + .store( + w, + PlatformWalletChangeSet { + contacts: Some(ContactChangeSet { + established, + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + + // Flush 2: pending tombstones for the SAME pair (both directions — + // e.g. an `ignore_sender` emitted against an established contact by a + // pre-guard caller). Neither may touch the established row. + let mut cs = ContactChangeSet::default(); + cs.removed_incoming.insert(ReceivedContactRequestKey { + owner_id: owner, + sender_id: contact, + }); + cs.removed_sent.insert(SentContactRequestKey { + owner_id: owner, + recipient_id: contact, + }); + persister + .store( + w, + PlatformWalletChangeSet { + contacts: Some(cs), + ..Default::default() + }, + ) + .unwrap(); + drop(persister); + + let p2 = reopen(&path); + let conn = p2.lock_conn_for_test(); + let state = platform_wallet_storage::sqlite::schema::contacts::load_state_for_test(&conn, &w) + .expect("contacts load_state"); + let survived = state + .established + .get(&est_key) + .expect("established row must survive pending tombstones"); + assert_eq!(survived.alias, Some("best friend".to_string())); + assert_eq!(survived.note, Some("met at conf".to_string())); + assert_eq!(survived.accepted_accounts, vec![1, 7, 42]); + + // Control: the same tombstones DO delete genuinely-pending rows. + let (persister, _tmp2, path2) = fresh_persister(); + let w2 = wid(0xA8); + ensure_wallet_meta(&persister, &w2); + let mut incoming = std::collections::BTreeMap::new(); + incoming.insert( + ReceivedContactRequestKey { + owner_id: owner, + sender_id: contact, + }, + contact_request_entry(0x52, 0x51), + ); + persister + .store( + w2, + PlatformWalletChangeSet { + contacts: Some(ContactChangeSet { + incoming_requests: incoming, + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + let mut cs = ContactChangeSet::default(); + cs.removed_incoming.insert(ReceivedContactRequestKey { + owner_id: owner, + sender_id: contact, + }); + persister + .store( + w2, + PlatformWalletChangeSet { + contacts: Some(cs), + ..Default::default() + }, + ) + .unwrap(); + drop(persister); + let p3 = reopen(&path2); + let conn = p3.lock_conn_for_test(); + let state = platform_wallet_storage::sqlite::schema::contacts::load_state_for_test(&conn, &w2) + .expect("contacts load_state"); + assert!( + state.incoming_requests.is_empty(), + "a received-state row is still deleted by its tombstone" + ); +} + +/// Un-ignore must be durable across a restart: the `ignored_senders` +/// TABLE (maintained transactionally by both ignore and un-ignore) is the +/// authoritative restore source — NOT the `entry_blob`'s snapshot copy, +/// which goes stale on un-ignore (nothing re-flushes the identity entry) +/// and is UNION-merged across buffered snapshots. Blob-based restore +/// resurrected the ignore, stickily. Was red against the blob-based +/// loader. +#[test] +fn tc_p4_020_unignore_survives_restart_table_is_authoritative() { + use platform_wallet_storage::sqlite::schema::identities; + use std::collections::BTreeMap; + use std::collections::BTreeSet; + + let (persister, _tmp, path) = fresh_persister(); + let w = wid(0xC1); + ensure_wallet_meta(&persister, &w); + + let owner = Identifier::from([0x61; 32]); + let sender = Identifier::from([0x62; 32]); + + // Flush 1: the identity entry snapshot carries the sender as ignored + // (as a live sweep would have persisted it), and the contacts + // changeset records the ignore in the table. + let mut entry = identity_entry(0x61, Some(0)); + entry.ignored_senders = BTreeSet::from([sender]); + let mut identities = BTreeMap::new(); + identities.insert(entry.id, entry); + persister + .store( + w, + PlatformWalletChangeSet { + identities: Some(IdentityChangeSet { + identities, + removed: Default::default(), + }), + contacts: Some(ContactChangeSet { + ignored: BTreeSet::from([(owner, sender)]), + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + + // Flush 2: the user un-ignores. Production persists ONLY the contact + // changeset (no fresh identity snapshot) — the blob still lists the + // sender as ignored. + persister + .store( + w, + PlatformWalletChangeSet { + contacts: Some(ContactChangeSet { + unignored: BTreeSet::from([(owner, sender)]), + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + drop(persister); + + // Restart: the restored identity must NOT have the sender ignored — + // the table row was deleted; the stale blob copy must not resurrect it. + let p2 = reopen(&path); + let conn = p2.lock_conn_for_test(); + let state = identities::load_state(&conn, &w).expect("load_state"); + drop(conn); + let managed = state + .wallet_identities + .get(&w) + .and_then(|bucket| bucket.get(&0)) + .expect("restored identity"); + assert!( + !managed.is_sender_ignored(&sender), + "un-ignore must survive a restart — the stale entry_blob snapshot \ + must not resurrect the ignore" + ); + + // Control (same loader, opposite direction): with the table row still + // present, the restore DOES ignore the sender — even when the blob + // snapshot never recorded it (blob older than the ignore). + let (persister, _tmp2, path2) = fresh_persister(); + let w2 = wid(0xC2); + ensure_wallet_meta(&persister, &w2); + let entry = identity_entry(0x61, Some(0)); // blob: no ignored senders + let mut identities = BTreeMap::new(); + identities.insert(entry.id, entry); + persister + .store( + w2, + PlatformWalletChangeSet { + identities: Some(IdentityChangeSet { + identities, + removed: Default::default(), + }), + contacts: Some(ContactChangeSet { + ignored: BTreeSet::from([(owner, sender)]), + ..Default::default() + }), + ..Default::default() + }, + ) + .unwrap(); + drop(persister); + let p3 = reopen(&path2); + let conn = p3.lock_conn_for_test(); + let state = identities::load_state(&conn, &w2).expect("load_state"); + drop(conn); + let managed = state + .wallet_identities + .get(&w2) + .and_then(|bucket| bucket.get(&0)) + .expect("restored identity"); + assert!( + managed.is_sender_ignored(&sender), + "an ignore recorded in the table restores even when the blob snapshot predates it" + ); +} diff --git a/packages/rs-platform-wallet/src/wallet/identity/crypto/contact_info.rs b/packages/rs-platform-wallet/src/wallet/identity/crypto/contact_info.rs index 9273444141f..b70b385ad68 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/crypto/contact_info.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/crypto/contact_info.rs @@ -54,6 +54,16 @@ const PRIVATE_DATA_MIN_LEN: usize = 48; /// 48-byte ciphertext floor: a 17-byte plaintext pads to 32 (CBC) + 16 (IV). const MIN_PLAINTEXT_LEN: usize = PRIVATE_DATA_MIN_LEN - 16 - 15; +/// The deployed schema's `privateData` maximum length (bytes, IV included): +/// `"privateData": { "maxItems": 2048 }` in `dashpay.schema.json`. +const PRIVATE_DATA_MAX_LEN: usize = 2048; + +/// Plaintext ceiling so `IV(16) ‖ AES-256-CBC/PKCS7(plaintext)` fits the +/// schema's 2048-byte cap: PKCS7 always adds 1..=16 padding bytes, so the +/// largest admissible plaintext is `(2048 - 16) - 1 = 2031` bytes (a +/// 2031-byte plaintext pads to a 2032-byte ciphertext; 2032 + 16 = 2048). +pub const MAX_PLAINTEXT_LEN: usize = PRIVATE_DATA_MAX_LEN - 16 - 1; + /// DIP-15 `version` for the v0 field set: `major(0) << 16 | minor(0)`. const PRIVATE_DATA_VERSION_V0: u32 = 0; @@ -245,6 +255,31 @@ pub fn encode_private_data(data: &ContactInfoPrivateData) -> Vec { out } +/// [`encode_private_data`] with the schema's size cap enforced. +/// +/// The publish path MUST use this (before persisting any local state): +/// an over-cap plaintext produces a `privateData` blob the contract's +/// 2048-byte `maxItems` rejects at broadcast — a PERMANENT failure, not a +/// transient one, so it has to surface as an error to the caller instead +/// of leaving locally-persisted metadata durably divergent from chain. +/// Unlike the account-label codec (which truncates to its 80-byte field), +/// alias/note are user-visible verbatim, so we reject rather than +/// silently truncate. +pub fn encode_private_data_bounded( + data: &ContactInfoPrivateData, +) -> Result, PlatformWalletError> { + let out = encode_private_data(data); + if out.len() > MAX_PLAINTEXT_LEN { + return Err(PlatformWalletError::InvalidIdentityData(format!( + "contactInfo privateData plaintext is {} bytes; the encrypted document would \ + exceed the contract's {PRIVATE_DATA_MAX_LEN}-byte cap (max plaintext \ + {MAX_PLAINTEXT_LEN} bytes) — shorten the alias/note", + out.len() + ))); + } + Ok(out) +} + /// Decode a `privateData` plaintext (inverse of [`encode_private_data`]). /// /// Tolerant per DIP-15 versioning: an unknown **major** version discards the @@ -384,6 +419,54 @@ mod tests { ); } + /// Over-cap payloads are rejected BEFORE encryption/persist: the + /// contract's `privateData` cap is 2048 bytes (IV + CBC ciphertext), + /// so a plaintext past [`MAX_PLAINTEXT_LEN`] would fail the broadcast + /// permanently — after the publish path already persisted local + /// metadata. The bounded encoder must error, not truncate. Was red + /// against the cap-less encoder. + #[test] + fn private_data_over_cap_is_rejected_at_cap_is_accepted() { + // Fixed overhead around the note: version(4) + alias varstr(1, empty) + // + note varint prefix + displayHidden(1) + accepted count varint(1). + // A ~2100-byte note is safely over the 2031-byte plaintext cap. + let over = ContactInfoPrivateData { + alias_name: None, + note: Some("x".repeat(2100)), + display_hidden: false, + accepted_accounts: Vec::new(), + }; + assert!( + encode_private_data_bounded(&over).is_err(), + "a plaintext past MAX_PLAINTEXT_LEN must be rejected, not encrypted" + ); + + // Boundary: size the note so the encoded plaintext lands EXACTLY on + // MAX_PLAINTEXT_LEN — must be accepted and round-trip. + let mut at_cap = ContactInfoPrivateData { + alias_name: None, + note: Some(String::new()), + display_hidden: false, + accepted_accounts: Vec::new(), + }; + // Find the note length whose encoding hits the cap exactly: encode + // once to measure the fixed overhead of a 3-byte varstr prefix + // (lengths ≥ 253 use 0xFD + u16). + let overhead = { + let probe = ContactInfoPrivateData { + alias_name: None, + note: Some("y".repeat(300)), + display_hidden: false, + accepted_accounts: Vec::new(), + }; + encode_private_data(&probe).len() - 300 + }; + at_cap.note = Some("y".repeat(MAX_PLAINTEXT_LEN - overhead)); + let encoded = encode_private_data_bounded(&at_cap).expect("at-cap payload is admissible"); + assert_eq!(encoded.len(), MAX_PLAINTEXT_LEN); + assert_eq!(decode_private_data(&encoded).expect("decode"), at_cap); + } + /// Forward-compat: a v0 decoder reading bytes with extra trailing data /// (a higher minor version's fields) parses the v0 fields and ignores /// the rest — DIP-15's minor-version rule. diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_info.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_info.rs index 646fda4c4e3..77b883a6094 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_info.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_info.rs @@ -27,7 +27,8 @@ use super::*; use crate::broadcaster::TransactionBroadcaster; use crate::error::PlatformWalletError; use crate::wallet::identity::crypto::contact_info::{ - decode_private_data, derive_contact_info_keys, encode_private_data, ContactInfoPrivateData, + decode_private_data, derive_contact_info_keys, encode_private_data_bounded, + ContactInfoPrivateData, }; /// One decrypted `contactInfo` document — the fields the sweep applies. (The @@ -531,6 +532,14 @@ impl DashPayView<'_, B> { accepted_accounts: Vec::new(), }; + // 0. Size gate BEFORE any local persist. An over-cap alias/note + // would fail the broadcast against the contract's 2048-byte + // `privateData` cap — a PERMANENT failure with no retry queue — + // after step 1 already durably persisted the local metadata, + // leaving local state divergent from chain for good. Encode once + // here; the encrypt step below reuses these bytes. + let plaintext = encode_private_data_bounded(&metadata)?; + // 1. Local state first — works offline and feeds SwiftData. let (established_count, identity_index, signing_key, root_key_id) = { let mut wm = self.wallet_manager.write().await; @@ -564,16 +573,15 @@ impl DashPayView<'_, B> { false, ) .cloned(); - let root_key_id = managed - .identity - .public_keys() - .iter() - .find(|(_, k)| { - k.purpose() == Purpose::ENCRYPTION - && k.key_type() == KeyType::ECDSA_SECP256K1 - && k.disabled_at().is_none() - }) - .map(|(_, k)| k.id()); + // Shared own-ECDH-root selector (same policy as the + // contact-request send path); `Option` preserved — a missing + // key defers the publish rather than erroring here. + let root_key_id = + crate::wallet::identity::network::contact_requests::select_own_encryption_key( + &managed.identity, + ) + .ok() + .map(|k| k.id()); (established_count, identity_index, signing_key, root_key_id) }; @@ -695,7 +703,8 @@ impl DashPayView<'_, B> { &root_path, derivation_index, &contact_id.to_buffer(), - &encode_private_data(&metadata), + // Pre-encoded (and size-gated) in step 0. + &plaintext, &iv, ) .await?; 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 21e707a8188..0b5c6221149 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 @@ -419,19 +419,12 @@ impl DashPayView<'_, B> { // 3. Resolve key indices. The sender selects its own ENCRYPTION key // (the live convention for both cohorts); ECDSA_SECP256K1 is - // required for ECDH. - let sender_encryption_key = sender_identity - .public_keys() - .iter() - .find(|(_, k)| { - k.purpose() == Purpose::ENCRYPTION && k.key_type() == KeyType::ECDSA_SECP256K1 - }) - .map(|(_, k)| k.clone()) - .ok_or_else(|| { - PlatformWalletError::InvalidIdentityData( - "Sender identity has no ECDSA_SECP256K1 encryption key".to_string(), - ) - })?; + // required for ECDH. Shared selector — same enabled-only policy + // as the contactInfo publish path, so both DashPay surfaces + // agree on the ECDH root and a disabled (rotated-away) first + // key can't hard-fail the pre-send validator below while an + // enabled replacement exists. + let sender_encryption_key = select_own_encryption_key(&sender_identity)?.clone(); let sender_key_index = sender_encryption_key.id(); let recipient_key_index = select_recipient_key_index(&recipient_identity)?; @@ -948,6 +941,35 @@ fn select_recipient_key_index(recipient_identity: &Identity) -> Result Result<&IdentityPublicKey, PlatformWalletError> { + identity + .public_keys() + .iter() + .find(|(_, k)| { + k.purpose() == Purpose::ENCRYPTION + && k.key_type() == KeyType::ECDSA_SECP256K1 + && k.disabled_at().is_none() + }) + .map(|(_, k)| k) + .ok_or_else(|| { + PlatformWalletError::InvalidIdentityData( + "Identity has no enabled ECDSA_SECP256K1 encryption key".to_string(), + ) + }) +} + // --------------------------------------------------------------------------- // Sync contact requests from platform // --------------------------------------------------------------------------- @@ -2030,6 +2052,7 @@ impl DashPayView<'_, B> { self.note_external_account_registered( &entry.owner_identity_id, &entry.contact_id, + encrypted_public_key, ) .await; } @@ -2475,10 +2498,24 @@ impl DashPayView<'_, B> { /// /// Idempotent (skips the persist when nothing changed). Takes its own /// write guard; the caller must hold no wallet-manager guard. + /// + /// `built_from_ciphertext` is the `encryptedPublicKey` blob the account + /// was actually registered from. Registration and this stamp run under + /// SEPARATE guards (the drain awaits the ECDH provider lock-free in + /// between), so a rotation sweep can advance `incoming_request` after + /// the payload was snapshotted but before this stamp. Stamping the LIVE + /// reference in that window would mark an account built from the + /// rotated-away xpub as current — `external_account_needs_rebuild` then + /// never fires and `send_payment` silently derives addresses the + /// contact no longer watches. Comparing the live ciphertext against the + /// one we built from detects the race; on mismatch the marker is left + /// stale (or `None`) so the sweep's teardown + rebuild picks up the + /// fresh request. pub(crate) async fn note_external_account_registered( &self, identity_id: &Identifier, contact_id: &Identifier, + built_from_ciphertext: &[u8], ) { let mut wm = self.wallet_manager.write().await; let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) else { @@ -2490,6 +2527,14 @@ impl DashPayView<'_, B> { let Some(contact) = managed.established_contact_mut(contact_id) else { return; }; + if contact.incoming_request.encrypted_public_key != built_from_ciphertext { + tracing::warn!( + owner = %identity_id, contact = %contact_id, + "external account registered from a superseded payload (rotation raced \ + the registration); leaving the self-heal marker stale so the sweep rebuilds" + ); + return; + } let current_reference = contact.incoming_request.account_reference; let already_current = contact.external_account_reference == Some(current_reference); if already_current && !contact.payment_channel_broken { @@ -2923,8 +2968,12 @@ impl DashPayView<'_, B> { // the sweep detects the stale marker and rebuilds; its build path // stamps + clears broken then. if registration == ExternalAccountRegistration::Built { - self.note_external_account_registered(our_identity_id, contact_id) - .await; + self.note_external_account_registered( + our_identity_id, + contact_id, + contact_encrypted_xpub, + ) + .await; } // Surface the contact's account label from the same ECDH shared key @@ -3985,6 +4034,49 @@ mod recipient_key_selection_tests { "a sole disabled key must error, got {err:?}" ); } + + /// **Own-key selector: a disabled first ENCRYPTION key is skipped in + /// favour of the enabled replacement.** After a disable-and-replace key + /// rotation the lowest-id ENCRYPTION key is disabled; selecting it + /// would hard-fail the pre-send validator on every new outgoing contact + /// request ("Sender key N is disabled") even though an enabled + /// replacement exists — while the contactInfo path (which already + /// filtered disabled keys) would use the replacement, splitting the two + /// surfaces' notion of the ECDH root. Was red against the send path's + /// unfiltered inline selection. + #[test] + fn own_key_selector_skips_disabled_first_encryption_key() { + let identity = identity_with_keys(vec![ + key(0, KeyType::ECDSA_SECP256K1, Purpose::AUTHENTICATION), + disabled_key(1, KeyType::ECDSA_SECP256K1, Purpose::ENCRYPTION), + key(2, KeyType::ECDSA_SECP256K1, Purpose::ENCRYPTION), + ]); + + let selected = select_own_encryption_key(&identity) + .expect("must skip the disabled key and select the enabled replacement"); + assert_eq!( + selected.id(), + 2, + "the disabled lowest-id ENCRYPTION key (id 1) must not be the ECDH root" + ); + } + + /// Own-key selector: enabled-only, so an identity whose ONLY + /// ENCRYPTION key is disabled errors instead of deriving ECDH from a + /// revoked key. + #[test] + fn own_key_selector_errors_when_only_encryption_key_is_disabled() { + let identity = identity_with_keys(vec![ + key(0, KeyType::ECDSA_SECP256K1, Purpose::AUTHENTICATION), + disabled_key(1, KeyType::ECDSA_SECP256K1, Purpose::ENCRYPTION), + ]); + + let err = select_own_encryption_key(&identity).unwrap_err(); + assert!( + matches!(err, PlatformWalletError::InvalidIdentityData(_)), + "a sole disabled ENCRYPTION key must error, got {err:?}" + ); + } } #[cfg(test)] @@ -4179,3 +4271,126 @@ mod contact_info_provider_tests { assert_eq!(count_account_build_ops(&[]), 0); } } + +#[cfg(test)] +mod stamp_race_tests { + //! The rotation self-heal stamp must be payload-bound, not live-state + //! bound: registration (drain / accept) and the stamp run under separate + //! guards, so a rotation sweep can advance `incoming_request` in between. + //! Stamping the live reference onto an account built from the superseded + //! payload would silence `external_account_needs_rebuild` forever. + + use super::*; + use crate::events::{EventHandler, PlatformEventHandler}; + use crate::wallet::identity::EstablishedContact; + use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; + use dpp::identity::v0::IdentityV0; + use dpp::identity::Identity; + use key_wallet::mnemonic::{Language, Mnemonic}; + use key_wallet::wallet::initialization::WalletAccountCreationOptions; + use key_wallet::Network; + use std::collections::BTreeMap; + use std::sync::Arc; + + const TEST_MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon abandon abandon about"; + + struct NoopEventHandler; + impl EventHandler for NoopEventHandler {} + impl PlatformEventHandler for NoopEventHandler {} + + fn bare_identity(id: [u8; 32]) -> Identity { + Identity::V0(IdentityV0 { + id: Identifier::from(id), + public_keys: BTreeMap::new(), + balance: 0, + revision: 0, + }) + } + + /// The stamp must be a no-op when the live incoming request's ciphertext + /// no longer matches the payload the account was registered from (a + /// rotation raced the registration), and must stamp normally when they + /// match. Was red against the live-reference stamp. + #[tokio::test] + async fn stamp_skips_when_registration_raced_a_rotation() { + let sdk = Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")); + let persister = Arc::new(NoPlatformPersistence); + let handler: Arc = Arc::new(NoopEventHandler); + let manager = Arc::new(crate::PlatformWalletManager::new( + sdk, + Arc::clone(&persister), + handler, + )); + let mnemonic = + Mnemonic::from_phrase(TEST_MNEMONIC, Language::English).expect("valid mnemonic"); + let seed = mnemonic.to_seed(""); + let wallet = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed, + WalletAccountCreationOptions::None, + Some(0), + ) + .await + .expect("wallet creation"); + let wallet_id = wallet.wallet_id(); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + // Live state: the contact ROTATED — incoming_request now carries the + // fresh ciphertext (7s) under reference 7; no marker yet. + let fresh_cipher = vec![7u8; 96]; + let stale_cipher = vec![9u8; 96]; + { + 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([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + let outgoing = ContactRequest::new(owner, contact, 0, 0, 0, vec![0u8; 96], 100, 0); + let incoming = + ContactRequest::new(contact, owner, 0, 0, 7, fresh_cipher.clone(), 100, 0); + info.identity_manager + .managed_identity_mut(&owner) + .expect("managed") + .apply_established_contact(EstablishedContact::new(contact, outgoing, incoming)); + } + + let marker = |iw: &IdentityWallet| { + let iw = iw.clone(); + async move { + let wm = iw.wallet_manager.read().await; + wm.get_wallet_info(&wallet_id) + .and_then(|info| info.identity_manager.managed_identity(&owner)) + .and_then(|m| m.dashpay().established_contacts().get(&contact).cloned()) + .expect("established contact") + .external_account_reference + } + }; + + // Drain finished registering from the STALE (pre-rotation) payload: + // the stamp must detect the mismatch and leave the marker unset so + // the sweep's teardown + rebuild path picks up the fresh request. + iw.dashpay() + .note_external_account_registered(&owner, &contact, &stale_cipher) + .await; + assert_eq!( + marker(iw).await, + None, + "a stamp from a superseded payload must NOT mark the account current" + ); + + // Registration from the LIVE payload stamps the tracked reference. + iw.dashpay() + .note_external_account_registered(&owner, &contact, &fresh_cipher) + .await; + assert_eq!( + marker(iw).await, + Some(7), + "a stamp from the live payload records the tracked reference" + ); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs index b46a750a38a..73fa4d24d90 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs @@ -53,11 +53,11 @@ impl ManagedIdentity { /// contact per sweep, and an `EstablishedContact::new` for an /// already-established pair would wipe the user's alias / note / /// hide-flag / accepted-accounts. So this method is a **no-op** when - /// the recipient is already tracked as established with the SAME - /// outgoing `accountReference` or already in the sent map (symmetric - /// to the received-side dedup in `sync_contact_requests`). When it - /// must (re-)establish against a pre-existing incoming request, it - /// MERGES into any existing `EstablishedContact` to preserve metadata. + /// the recipient is already tracked — established or pending-sent — + /// with the SAME outgoing `accountReference` (symmetric to the + /// received-side dedup in `sync_contact_requests`). When it must + /// (re-)establish against a pre-existing incoming request, it MERGES + /// into any existing `EstablishedContact` to preserve metadata. /// /// **Sent-side rotation supersede.** When we re-send to an /// already-established contact with a *different* outgoing @@ -111,13 +111,36 @@ impl ManagedIdentity { .insert(recipient_id, updated); return Ok(()); } - // Already tracked as a pending sent request → no-op (no phantom - // row, no redundant changeset write). - if self - .dashpay - .sent_contact_requests - .contains_key(&recipient_id) - { + // Already tracked as a pending sent request. Same outgoing + // reference → no-op (no phantom row, no redundant changeset + // write). Different reference → the pending mirror of the + // established-branch rotation supersede above: we rotated and + // broadcast a superseding request to a recipient who hasn't + // reciprocated yet. Without the supersede the pending map stays + // frozen at the first send's reference, so the NEXT rotation + // re-derives (un-mask → bump) from the stale version and + // reproduces an already-broadcast reference — rejected forever + // by the contract's `($ownerId, toUserId, accountReference)` + // unique index. Persist BEFORE committing to memory, same as + // the established branch and for the same retry reason. + if let Some(existing) = self.dashpay.sent_contact_requests.get(&recipient_id) { + if existing.account_reference == request.account_reference { + return Ok(()); + } + let mut cs = ContactChangeSet::default(); + cs.sent_requests.insert( + SentContactRequestKey { + owner_id, + recipient_id, + }, + ContactRequestEntry { + request: request.clone(), + }, + ); + persister.store(cs.into())?; + self.dashpay + .sent_contact_requests + .insert(recipient_id, request); return Ok(()); } @@ -180,7 +203,11 @@ impl ManagedIdentity { /// for persisting it through the same write guard it holds). pub fn ignore_sender(&mut self, sender_id: &Identifier) -> ContactChangeSet { let owner_id = self.id(); - self.dashpay.incoming_contact_requests.remove(sender_id); + let removed = self + .dashpay + .incoming_contact_requests + .remove(sender_id) + .is_some(); self.dashpay.ignored_senders.insert(*sender_id); let mut cs = ContactChangeSet::default(); @@ -193,10 +220,22 @@ impl ManagedIdentity { // user's ignore is silently undone on that backend. (The SwiftData // persister already deletes the row via its `ignored` handler, so // this makes the two backends consistent.) - cs.removed_incoming.insert(ReceivedContactRequestKey { - owner_id, - sender_id: *sender_id, - }); + // + // Guarded on an ACTUAL removal — the same `removed.is_some()` + // discipline as `remove_incoming_contact_request` / + // `remove_sent_contact_request`. Ignoring a sender who has no + // pending incoming entry (e.g. an already-established contact, or + // one that raced auto-establish) must not emit a tombstone: the + // contacts table is one row per pair, so an unconditional + // tombstone would DELETE the established row — outgoing/incoming + // blobs plus the user's alias/note/hidden/accepted-accounts — + // while memory keeps the contact established. + if removed { + cs.removed_incoming.insert(ReceivedContactRequestKey { + owner_id, + sender_id: *sender_id, + }); + } cs.ignored.insert((owner_id, *sender_id)); cs } @@ -883,6 +922,50 @@ mod tests { ); } + /// **Ignore of a sender with NO pending incoming entry must not emit a + /// `removed_incoming` tombstone.** The contacts table is one row per + /// pair, so a tombstone for an ESTABLISHED contact (ignore tapped after + /// auto-establish, or on an established pair directly) would DELETE the + /// established row — both request blobs plus the user's + /// alias/note/hidden/accepted-accounts — while memory keeps the contact + /// established. Mirrors the `removed.is_some()` guard already used by + /// `remove_incoming_contact_request`. Was red against the unconditional + /// emission. + #[test] + fn ignore_sender_without_pending_incoming_emits_no_tombstone() { + let mut managed = create_test_identity([1u8; 32]); + let owner_id = managed.id(); + let contact_id = Identifier::from([2u8; 32]); + let p = noop_persister(); + + // Establish the pair (incoming + sent → auto-establish), leaving NO + // pending incoming entry. + managed + .add_incoming_contact_request(create_contact_request(contact_id, owner_id, 1), &p) + .expect("setup persists"); + managed + .add_sent_contact_request(create_contact_request(owner_id, contact_id, 2), &p) + .expect("setup persists"); + assert_eq!(managed.dashpay.established_contacts.len(), 1); + assert!(managed.dashpay.incoming_contact_requests.is_empty()); + + let cs = managed.ignore_sender(&contact_id); + + // The suppression is recorded... + assert!( + cs.ignored.contains(&(owner_id, contact_id)), + "ignore must record the per-sender suppression" + ); + // ...but NO row tombstone is emitted — nothing pending was removed, + // and an unconditional tombstone would destroy the established row. + assert!( + cs.removed_incoming.is_empty(), + "no pending incoming entry was removed, so no tombstone may be emitted" + ); + // The established contact survives in memory untouched. + assert_eq!(managed.dashpay.established_contacts.len(), 1); + } + #[test] fn test_add_incoming_contact_request_without_reciprocal() { let mut managed = create_test_identity([1u8; 32]); @@ -1488,6 +1571,71 @@ mod tests { assert_eq!(est.alias, Some("Carol".to_string())); } + /// Pending-branch rotation supersede: re-sending to a recipient who + /// has NOT yet reciprocated (still in the pending sent map) with a + /// DIFFERENT outgoing `accountReference` must advance the pending + /// entry — the pending mirror of + /// [`add_sent_contact_request_rotation_supersedes_outgoing_reference`]. + /// Without it the pending map (and `prior_sent_account_reference`) + /// stays frozen at the first send, so the next rotation re-derives the + /// already-broadcast reference and collides on the contract's unique + /// index. Was red against the `contains_key` no-op guard. + #[test] + fn add_sent_contact_request_rotation_supersedes_pending_reference() { + let mut managed = create_test_identity([1u8; 32]); + let our_id = Identifier::from([1u8; 32]); + let contact_id = Identifier::from([2u8; 32]); + let p = noop_persister(); + + // First send to a recipient with no incoming request → pending. + let mut first_send = create_contact_request(our_id, contact_id, 1); + first_send.account_reference = 100; // R0 + managed + .add_sent_contact_request(first_send, &p) + .expect("setup persists"); + assert!(managed.dashpay.established_contacts.is_empty()); + assert_eq!(managed.prior_sent_account_reference(&contact_id), Some(100)); + + // Rotation re-send while STILL pending: bumped reference R1. + let mut rotation = create_contact_request(our_id, contact_id, 2); + rotation.account_reference = 101; // R1 + managed + .add_sent_contact_request(rotation, &p) + .expect("pending rotation persists"); + assert_eq!( + managed.prior_sent_account_reference(&contact_id), + Some(101), + "pending rotation must advance the tracked reference (not freeze at R0)" + ); + // Still pending — the supersede must not fabricate an establishment. + assert!(managed.dashpay.established_contacts.is_empty()); + assert_eq!(managed.dashpay.sent_contact_requests.len(), 1); + + // Same-reference re-ingest (the sweep re-reading the newest doc) + // stays a no-op. + let mut resend_same = create_contact_request(our_id, contact_id, 3); + resend_same.account_reference = 101; + managed + .add_sent_contact_request(resend_same, &p) + .expect("same-reference re-ingest is a no-op"); + assert_eq!(managed.prior_sent_account_reference(&contact_id), Some(101)); + + // The recipient finally reciprocates: establishment must capture + // the NEWEST outgoing reference, not the original. + managed + .add_incoming_contact_request(create_contact_request(contact_id, our_id, 4), &p) + .expect("reciprocal persists"); + let est = managed + .dashpay + .established_contacts + .get(&contact_id) + .expect("reciprocal establishes"); + assert_eq!( + est.outgoing_request.account_reference, 101, + "establishment must adopt the superseded (newest) outgoing reference" + ); + } + /// When a sent request auto-establishes against a pre-existing /// incoming, but the pair was previously established and we carry /// forward metadata — the re-establish must preserve it. (Covers the