Skip to content
Merged
156 changes: 150 additions & 6 deletions packages/rs-platform-wallet-ffi/src/identity_persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,37 @@ pub struct IdentityKeyEntryFFI {
pub derivation_indices_is_some: bool,
pub identity_index: u32,
pub key_index: u32,

// ContractBounds projection. Mirrors the DPP enum
// `ContractBounds` so the client can reconstruct the variant
// verbatim instead of dropping the document-type name:
//
// * `contract_bounds_kind == 0` — no contract bounds; the
// `id` field is zeroed and the doc-type pointer is null.
// * `contract_bounds_kind == 1` — `SingleContract`; only the
// 32-byte `id` is meaningful, doc-type pointer is null.
// * `contract_bounds_kind == 2` — `SingleContractDocumentType`;
// both the `id` and the heap-allocated UTF-8 doc-type
// C-string are meaningful. Doc-type string is released by
// [`free_identity_key_entry_ffi`].
//
// Keeping the kind tag inline (vs. always nulling fields) lets
// the Swift side switch on a single discriminant without
// probing pointer values, matching how the rest of this struct
// encodes optional payloads.
//
// Ownership: `contract_bounds_document_type` is owned by this
// struct EXCLUSIVELY when it is populated by
// [`IdentityKeyEntryFFI::from_entry`]. Consumers MUST NOT
// copy the struct value and then free both copies — the second
// free is a use-after-free / double-free. The Swift binding
// copies the doc-type into an owned Swift `String` inside the
// callback (per `persistIdentityKeysCallback`) and never
// retains the raw pointer past the callback window, which is
// the only supported consumption pattern.
pub contract_bounds_kind: u8,
pub contract_bounds_id: [u8; 32],
pub contract_bounds_document_type: *const c_char,
}

/// Composite identifier for [`IdentityKeysChangeSet::removed`] entries
Expand Down Expand Up @@ -228,9 +259,13 @@ pub struct IdentityKeyRemovalFFI {
// 126..=127 (padding to 4)
// 128..=131 identity_index u32
// 132..=135 key_index u32
// 136 contract_bounds_kind u8
// 137..=168 contract_bounds_id [u8; 32]
// 169..=175 (padding to 8 for pointer alignment)
// 176..=183 contract_bounds_document_type *const c_char
//
// Total size = 136, alignment = 8 (from u64 / pointer).
const _: [u8; 136] = [0u8; std::mem::size_of::<IdentityKeyEntryFFI>()];
// Total size = 184, alignment = 8 (from u64 / pointer).
const _: [u8; 184] = [0u8; std::mem::size_of::<IdentityKeyEntryFFI>()];
const _: [u8; 8] = [0u8; std::mem::align_of::<IdentityKeyEntryFFI>()];

// Compile-time guard for `IdentityEntryFFI`. Same rationale as the
Expand Down Expand Up @@ -451,10 +486,12 @@ fn allocate_dpns_arrays(
impl IdentityKeyEntryFFI {
/// Copy an [`IdentityKeyEntry`] into a fresh FFI struct. The
/// caller owns the heap-allocated `public_key_data_ptr` byte
/// buffer and (when present) the `private_key_derivation_path`
/// C-string; release both via [`free_identity_key_entry_ffi`].
/// buffer and (when present) the
/// `contract_bounds_document_type` C-string; release both via
/// [`free_identity_key_entry_ffi`].
pub fn from_entry(entry: &IdentityKeyEntry) -> Self {
use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0;
use dpp::identity::identity_public_key::contract_bounds::ContractBounds;

let pk_bytes = entry.public_key.data().as_slice().to_vec();
let pk_len = pk_bytes.len();
Expand All @@ -477,6 +514,30 @@ impl IdentityKeyEntryFFI {
None => (false, 0, 0),
};

// Project the DPP `ContractBounds` enum into the kind /
// id / doc-type-cstring trio so the Swift side can switch
// on a single discriminant. Strings containing interior
// NULs (impossible in practice — DPP rejects them) keep
// the discriminant + payload self-consistent by falling
// back to `SingleContract { id }` (kind=1 + null doc-type
// pointer); emitting kind=2 with a null doc-type pointer
// would silently strip the bound on the Swift side, so
// demoting to `SingleContract` is the closest faithful
// representation — the document-type qualifier is the
// only thing lost, the contract id is preserved.
let (contract_bounds_kind, contract_bounds_id, contract_bounds_document_type) =
match entry.public_key.contract_bounds() {
Some(ContractBounds::SingleContract { id }) => (1u8, id.to_buffer(), ptr::null()),
Some(ContractBounds::SingleContractDocumentType {
id,
document_type_name,
}) => match CString::new(document_type_name.as_str()) {
Ok(c) => (2u8, id.to_buffer(), c.into_raw() as *const c_char),
Err(_) => (1u8, id.to_buffer(), ptr::null()),
},
None => (0u8, [0u8; 32], ptr::null()),
};
Comment thread
QuantumExplorer marked this conversation as resolved.

Self {
identity_id: entry.identity_id.to_buffer(),
key_id: entry.key_id,
Expand All @@ -494,6 +555,9 @@ impl IdentityKeyEntryFFI {
derivation_indices_is_some,
identity_index,
key_index,
contract_bounds_kind,
contract_bounds_id,
contract_bounds_document_type,
}
}
}
Expand Down Expand Up @@ -601,8 +665,12 @@ unsafe fn free_optional_c_string(slot: &mut *const c_char) {
}

/// Release heap allocations owned by an [`IdentityKeyEntryFFI`] —
/// the public-key data buffer and, when present, the derivation-path
/// string for the `AtWalletDerivationPath` variant.
/// the public-key data buffer and, when present, the contract-bounds
/// document-type C-string (set when
/// `contract_bounds_kind == 2`, i.e. SingleContractDocumentType).
///
/// Idempotent: pointers are nulled and length zeroed after release,
/// so a second call is a no-op.
///
/// # Safety
///
Expand All @@ -621,6 +689,13 @@ pub unsafe fn free_identity_key_entry_ffi(entry: &mut IdentityKeyEntryFFI) {
entry.public_key_data_ptr = ptr::null_mut();
entry.public_key_data_len = 0;
}
// Release the contract-bounds doc-type C-string. Only allocated
// when the original entry carried `SingleContractDocumentType`
// bounds (and the doc-type name didn't contain interior NULs).
if !entry.contract_bounds_document_type.is_null() {
let _ = unsafe { CString::from_raw(entry.contract_bounds_document_type as *mut c_char) };
entry.contract_bounds_document_type = ptr::null();
}
// No private-key heap allocations to reclaim — the new FFI shape
// carries only scalar derivation breadcrumbs, not an owned path
// string or key-material buffer.
Expand Down Expand Up @@ -878,6 +953,75 @@ mod tests {
assert!(ffi.read_only);
assert!(ffi.disabled_at_is_some);
assert_eq!(ffi.disabled_at, 1_700_000_000);
assert_eq!(ffi.contract_bounds_kind, 0);
assert!(ffi.contract_bounds_document_type.is_null());
unsafe { free_identity_key_entry_ffi(&mut ffi) };
}

#[test]
fn test_identity_key_entry_ffi_contract_bounds_single_contract() {
use dpp::identity::identity_public_key::contract_bounds::ContractBounds;
let contract_id = Identifier::from([0xAB; 32]);
let public_key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id: 1,
purpose: Purpose::AUTHENTICATION,
security_level: SecurityLevel::HIGH,
contract_bounds: Some(ContractBounds::SingleContract { id: contract_id }),
key_type: KeyType::ECDSA_SECP256K1,
read_only: false,
data: BinaryData::new(vec![0x01; 33]),
disabled_at: None,
});
let entry = IdentityKeyEntry {
identity_id: Identifier::from([1u8; 32]),
key_id: 1,
public_key,
public_key_hash: [0x11; 20],
wallet_id: None,
derivation_indices: None,
};
let mut ffi = IdentityKeyEntryFFI::from_entry(&entry);
assert_eq!(ffi.contract_bounds_kind, 1);
assert_eq!(ffi.contract_bounds_id, [0xAB; 32]);
assert!(ffi.contract_bounds_document_type.is_null());
unsafe { free_identity_key_entry_ffi(&mut ffi) };
}

#[test]
fn test_identity_key_entry_ffi_contract_bounds_single_doc_type() {
use dpp::identity::identity_public_key::contract_bounds::ContractBounds;
let contract_id = Identifier::from([0xCD; 32]);
let public_key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id: 2,
purpose: Purpose::ENCRYPTION,
security_level: SecurityLevel::MEDIUM,
contract_bounds: Some(ContractBounds::SingleContractDocumentType {
id: contract_id,
document_type_name: "contactRequest".to_string(),
}),
key_type: KeyType::ECDSA_SECP256K1,
read_only: false,
data: BinaryData::new(vec![0x02; 33]),
disabled_at: None,
});
let entry = IdentityKeyEntry {
identity_id: Identifier::from([4u8; 32]),
key_id: 2,
public_key,
public_key_hash: [0x22; 20],
wallet_id: None,
derivation_indices: None,
};
let mut ffi = IdentityKeyEntryFFI::from_entry(&entry);
assert_eq!(ffi.contract_bounds_kind, 2);
assert_eq!(ffi.contract_bounds_id, [0xCD; 32]);
assert!(!ffi.contract_bounds_document_type.is_null());
// Verify the doc-type CString round-trips.
let cstr = unsafe { std::ffi::CStr::from_ptr(ffi.contract_bounds_document_type) };
assert_eq!(cstr.to_str().unwrap(), "contactRequest");
unsafe { free_identity_key_entry_ffi(&mut ffi) };
// Idempotent free.
assert!(ffi.contract_bounds_document_type.is_null());
unsafe { free_identity_key_entry_ffi(&mut ffi) };
}
}
38 changes: 37 additions & 1 deletion packages/rs-platform-wallet-ffi/src/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3101,6 +3101,7 @@ fn build_wallet_identity_bucket(
unsafe fn build_identity_public_keys(
spec: &IdentityRestoreEntryFFI,
) -> BTreeMap<KeyID, IdentityPublicKey> {
use dpp::identity::identity_public_key::contract_bounds::ContractBounds;
let mut map: BTreeMap<KeyID, IdentityPublicKey> = BTreeMap::new();
if spec.keys.is_null() || spec.keys_count == 0 {
return map;
Expand All @@ -3120,11 +3121,46 @@ unsafe fn build_identity_public_keys(
continue;
}
let bytes: Vec<u8> = slice::from_raw_parts(row.data, row.data_len).to_vec();

// Reconstruct the ContractBounds variant from the kind tag
// + id + optional doc-type C-string trio. Mirrors the
// encoding in `IdentityKeyEntryFFI::from_entry`. A kind=2
// row with a null doc-type pointer is an FFI-side
// inconsistency (the writer is supposed to demote to
// kind=1 in that case — see identity_persistence.rs); we
// demote it here too rather than fabricating an empty doc-
// type name. Invalid kind tags load as unbounded so a
// forward-compatible writer doesn't lock us out.
let contract_bounds: Option<ContractBounds> = match row.contract_bounds_kind {
0 => None,
1 => Some(ContractBounds::SingleContract {
id: row.contract_bounds_id.into(),
}),
2 => {
if row.contract_bounds_document_type.is_null() {
Some(ContractBounds::SingleContract {
id: row.contract_bounds_id.into(),
})
} else {
match CStr::from_ptr(row.contract_bounds_document_type).to_str() {
Ok(name) => Some(ContractBounds::SingleContractDocumentType {
id: row.contract_bounds_id.into(),
document_type_name: name.to_string(),
}),
Err(_) => Some(ContractBounds::SingleContract {
id: row.contract_bounds_id.into(),
}),
}
}
}
_ => None,
};

let pk = IdentityPublicKey::V0(IdentityPublicKeyV0 {
id: row.key_id,
purpose,
security_level,
contract_bounds: None,
contract_bounds,
key_type,
read_only: row.read_only,
data: BinaryData::new(bytes),
Expand Down
29 changes: 23 additions & 6 deletions packages/rs-platform-wallet-ffi/src/wallet_restore_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -182,12 +182,16 @@ pub struct AccountSpecFFI {
/// BLS → 48; etc.). The pointer is Swift-owned and valid only for the
/// duration of the load callback.
///
/// Disabled-at, contract-bounds and other non-essential fields are
/// intentionally omitted — they're either always `None` for newly
/// derived identity-auth keys or get re-populated by the next
/// identity sync round if they exist on chain. The scope of this
/// restore is narrowly "make `Identity.public_keys` non-empty so
/// auth-key gates pass".
/// `contract_bounds_*` mirror the [`IdentityKeyEntryFFI`]
/// projection of DPP's `ContractBounds` enum (kind tag: 0=none,
/// 1=SingleContract, 2=SingleContractDocumentType). Including them
/// here closes the persist↔restore round-trip — without it, scoped
/// DashPay keys (registered with `SingleContractDocumentType`) come
/// back as unbounded on cold restart.
///
/// Disabled-at and other non-essential fields remain omitted —
/// they're either always `None` for newly derived identity-auth
/// keys or get re-populated by the next identity sync round.
#[repr(C)]
pub struct IdentityKeyRestoreFFI {
pub key_id: u32,
Expand All @@ -199,6 +203,19 @@ pub struct IdentityKeyRestoreFFI {
/// Valid for callback duration only; Swift owns the allocation.
pub data: *const u8,
pub data_len: usize,
/// ContractBounds discriminant: 0=none, 1=SingleContract,
/// 2=SingleContractDocumentType. Mirrors the encoding in
/// [`crate::identity_persistence::IdentityKeyEntryFFI`].
pub contract_bounds_kind: u8,
/// 32-byte contract identifier. Zeroed when
/// `contract_bounds_kind == 0`; otherwise the contract id the
/// key is bound to.
pub contract_bounds_id: [u8; 32],
/// NUL-terminated UTF-8 doc-type name. Non-null iff
/// `contract_bounds_kind == 2`. Swift-owned (released by the
/// same load-callback allocation arena that frees the public-
/// key data buffer).
pub contract_bounds_document_type: *const c_char,
}

/// Per-identity entry attached to a [`WalletRestoreEntryFFI`].
Expand Down
8 changes: 6 additions & 2 deletions packages/rs-sdk/src/platform/dpns_usernames/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,12 @@ impl Sdk {

let documents = Document::fetch_many(self, query).await?;

// If no documents found, the name is available
Ok(documents.is_empty())
// `Document::fetch_many` returns `BTreeMap<Identifier, Option<Document>>`
// — a non-existence proof comes back as a non-empty map whose values are
// all `None`. Checking `documents.is_empty()` would treat a proven
// non-existence as "taken". The name is available iff no entry in the
// map carries an actual document.
Ok(documents.values().all(|d| d.is_none()))
Comment on lines 406 to +413

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Suggestion: is_dpns_name_available correctness fix has no automated test

The semantic change from documents.is_empty() to documents.values().all(|d| d.is_none()) is the central correctness fix in the PR and drives user-visible DPNS registration UX, but is exercised only manually via the iOS app. Commit 2c9e9af just relocated DPNS network tests under tests/, so there is now a natural home for a positive case (proven-non-existence map → Ok(true)) and a negative case (Some(Document) entry → Ok(false)). Add both so a future change to Document::fetch_many's return shape can't regress the semantics silently.

source: ['claude']

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acknowledged, not addressed in this commit. An automated test for is_dpns_name_available (positive Ok(true) on proven-non-existence Some(None) map + negative Ok(false) on Some(Document)) is the right gate against silent regressions in Document::fetch_many semantics. Filed as a follow-up — slotting it into the existing tests/ directory is cheap once the iOS PR is in; doing it here would balloon the diff with test-only infra unrelated to the DashPay flow.

}

/// Resolve a DPNS name to an identity ID
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,19 @@ public final class PersistentAccount {
/// Parent wallet. Every account currently belongs to a wallet. If
/// standalone non-wallet accounts are introduced later, this
/// becomes optional again.
///
/// Kept non-optional. SwiftData would otherwise fatal during
/// the `save()` phase of a wallet delete
/// (`Cannot remove PersistentWallet from relationship wallet on
/// PersistentAccount because an appropriate default value is
/// not configured`); the workaround is in
/// `PlatformWalletPersistenceHandler.deleteWalletData`, which
/// deletes all of the wallet's accounts in a separate
/// `save()` BEFORE deleting the wallet itself. By the time the
/// wallet row is deleted, its `accounts` collection is empty
/// and SwiftData has no inverse to null out. This costs
/// atomicity (two saves instead of one) — acceptable for a
/// user-initiated wipe.
public var wallet: PersistentWallet

/// Addresses from this account's address pools (external +
Expand Down
Loading
Loading