Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 41 additions & 2 deletions packages/rs-sdk-ffi/src/contested_resource/queries/vote_state.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
use crate::types::SDKHandle;
use crate::{DashSDKError, DashSDKErrorCode, DashSDKResult, DashSDKResultDataType};
use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
use dash_sdk::dpp::document::DocumentV0Getters;
use dash_sdk::dpp::platform_value::Value;
use dash_sdk::dpp::voting::contender_structs::ContenderWithSerializedDocument;
use dash_sdk::dpp::voting::vote_info_storage::contested_document_vote_poll_winner_info::ContestedDocumentVotePollWinnerInfo;
use dash_sdk::dpp::voting::vote_polls::contested_document_resource_vote_poll::ContestedDocumentResourceVotePoll;
use dash_sdk::drive::query::vote_poll_vote_state_query::ContestedDocumentVotePollDriveQuery;
use dash_sdk::platform::FetchMany;
use dash_sdk::platform::{DataContract, Fetch};
use dash_sdk::query_types::Contenders;
use std::ffi::{c_char, c_void, CStr, CString};

Expand Down Expand Up @@ -235,6 +238,19 @@ fn get_contested_resource_vote_state(
}
// Add contenders
if result_type.has_documents() {
// Decode each contender's document so callers get the
// label the requester actually typed ("pizza") next to the
// homograph-normalized index value ("p1zza"). Without this
// a UI can only show the normalized form, which reads as a
// typo to the person who submitted it.
//
// Best-effort: the contract fetch or a single decode
// failing must not fail the whole query, so `label` is
// simply absent for rows that could not be decoded and the
// caller falls back to the normalized value.
let contract: Option<DataContract> =
DataContract::fetch(&sdk, contract_id).await.ok().flatten();

Comment on lines 238 to +253

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: New label-decoding logic has no Rust-side test coverage

Verified: the only tests in this file's #[cfg(test)] module (lines 306-347) cover null-handle and null-contract-id guard paths — nothing exercises the new label-decoding logic added in this PR (contract fetch, document_type_for_name, try_to_contender, JSON-embedded label extraction), nor the mirrored logic in dpns/queries/contested.rs. The new Swift tests inject already-decoded label JSON directly, so they validate JSON consumption but not the Rust-side production logic — e.g. that a document lacking a label field, or one that fails to deserialize against the fetched contract, is silently omitted rather than producing malformed JSON or panicking.

source: ['claude', 'codex']

let contenders_json: Vec<String> = contenders.contenders
.iter()
.map(|(id, contender)| {
Expand All @@ -245,13 +261,36 @@ fn get_contested_resource_vote_state(
r#""document":null"#.to_string()
};

let label_json = contract
.as_ref()
.and_then(|contract| {
let doc_type = contract
.document_type_for_name(document_type_name_str)
.ok()?;
let decoded = contender
.try_to_contender(doc_type, sdk.version())
.ok()?;
let label = decoded
.document()
.as_ref()?
.get("label")?
.as_str()?
.to_string();
Some(format!(
r#","label":{}"#,
serde_json::to_string(&label).ok()?
))
})
.unwrap_or_default();
Comment on lines +264 to +284

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.

💬 Nitpick: document_type_for_name looked up per contender instead of once per query

contract.document_type_for_name(document_type_name_str) is called inside the per-contender .map() closure (verified at line ~264), repeating a name-keyed lookup into the contract's document type map for every contender instead of resolving it once before the loop. Contests are typically small so the practical cost is negligible, but hoisting it out avoids the repeated lookup.

source: ['claude']


let vote_count = contender.vote_tally().unwrap_or(0);

format!(
r#"{{"identity_id":"{}","vote_count":{},{}}}"#,
r#"{{"identity_id":"{}","vote_count":{},{}{}}}"#,
bs58::encode(id.as_bytes()).into_string(),
vote_count,
document_json
document_json,
label_json
)
})
.collect();
Expand Down
51 changes: 51 additions & 0 deletions packages/rs-sdk-ffi/src/dpns/queries/contested.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
//! FFI bindings for contested DPNS username queries

use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters;
use dash_sdk::dpp::document::DocumentV0Getters;
use dash_sdk::dpp::system_data_contracts::{load_system_data_contract, SystemDataContract};
use std::ffi::{CStr, CString};
use std::os::raw::c_char;

Expand Down Expand Up @@ -488,6 +491,14 @@ pub unsafe extern "C" fn dash_sdk_dpns_get_non_resolved_contests_for_identity(

match result {
Ok(names_with_contest_info) => {
// Baked-in system contract: no network round trip, and it is the
// schema these documents were written against.
let platform_version = sdk.version();
let dpns_domain_type =
load_system_data_contract(SystemDataContract::DPNS, platform_version)
.ok()
.and_then(|contract| contract.document_type_cloned_for_name("domain").ok());

let count = names_with_contest_info.len();
let mut names = Vec::with_capacity(count);

Expand All @@ -512,9 +523,25 @@ pub unsafe extern "C" fn dash_sdk_dpns_get_non_resolved_contests_for_identity(
// Extract actual vote tally from ContenderWithSerializedDocument
let vote_count = votes.vote_tally().unwrap_or(0);

// The requester's own spelling ("pizza"), which the
// normalized contest name ("p1zza") does not preserve.
// Null when the document can't be decoded — callers fall
// back to the normalized name rather than guessing.
let c_label = dpns_domain_type
.as_ref()
.and_then(|doc_type| {
let decoded = votes
.try_to_contender(doc_type.as_ref(), platform_version)
.ok()?;
let label = decoded.document().as_ref()?.get("label")?.as_str()?;
CString::new(label).ok().map(|s| s.into_raw())
})
.unwrap_or(std::ptr::null_mut());

contenders.push(DashSDKContender {
identity_id: c_id,
vote_count,
label: c_label,
});
}

Expand Down Expand Up @@ -587,6 +614,14 @@ pub unsafe extern "C" fn dash_sdk_dpns_get_contested_non_resolved_usernames(

match result {
Ok(names_with_contest_info) => {
// Baked-in system contract: no network round trip, and it is the
// schema these documents were written against.
let platform_version = sdk.version();
let dpns_domain_type =
load_system_data_contract(SystemDataContract::DPNS, platform_version)
.ok()
.and_then(|contract| contract.document_type_cloned_for_name("domain").ok());

let count = names_with_contest_info.len();
let mut names = Vec::with_capacity(count);

Expand All @@ -611,9 +646,25 @@ pub unsafe extern "C" fn dash_sdk_dpns_get_contested_non_resolved_usernames(
// Extract actual vote tally from ContenderWithSerializedDocument
let vote_count = votes.vote_tally().unwrap_or(0);

// The requester's own spelling ("pizza"), which the
// normalized contest name ("p1zza") does not preserve.
// Null when the document can't be decoded — callers fall
// back to the normalized name rather than guessing.
let c_label = dpns_domain_type
.as_ref()
.and_then(|doc_type| {
let decoded = votes
.try_to_contender(doc_type.as_ref(), platform_version)
.ok()?;
let label = decoded.document().as_ref()?.get("label")?.as_str()?;
CString::new(label).ok().map(|s| s.into_raw())
})
.unwrap_or(std::ptr::null_mut());

contenders.push(DashSDKContender {
identity_id: c_id,
vote_count,
label: c_label,
});
}

Expand Down
191 changes: 158 additions & 33 deletions packages/rs-sdk-ffi/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1032,6 +1032,12 @@ pub struct DashSDKContender {
pub identity_id: *mut c_char,
/// Vote count for this contender
pub vote_count: u32,
/// The label this contender actually requested ("pizza"), decoded from
/// their `domain` document — the contest itself is keyed by the
/// homograph-normalized form ("p1zza"). Null when the document could not
/// be decoded; callers fall back to the normalized name rather than
/// guessing a spelling.
pub label: *mut c_char,
}

/// Represents contest information for a DPNS name
Expand Down Expand Up @@ -1103,8 +1109,41 @@ pub unsafe extern "C" fn dash_sdk_contender_free(contender: *mut DashSDKContende
return;
}

let contender = Box::from_raw(contender);
dash_sdk_string_free(contender.identity_id);
let mut contender = Box::from_raw(contender);
free_contender_fields(&mut *contender);
}

/// Release the heap strings owned by one contender.
///
/// Every destructor that owns contenders funnels through this: the fields were
/// duplicated across four free paths, and adding `label` to the struct without
/// updating all of them leaked one allocation per decoded contender on three of
/// them. A single helper makes the next added field a one-line change.
///
/// # Safety
/// - `contender` must point to a live `DashSDKContender` whose strings were
/// allocated by this SDK and not yet freed.
unsafe fn free_contender_fields(contender: *mut DashSDKContender) {
dash_sdk_string_free((*contender).identity_id);
// Null whenever the contender document could not be decoded;
// `dash_sdk_string_free` is a no-op on null.
dash_sdk_string_free((*contender).label);
}

/// Free the contender array owned by a `DashSDKContestInfo`, including each
/// contender's strings.
///
/// # Safety
/// - `contenders` must be either null or a pointer obtained from
/// `Box::into_raw` over a boxed slice of exactly `count` contenders.
unsafe fn free_contender_array(contenders: *mut DashSDKContender, count: usize) {
if contenders.is_null() || count == 0 {
return;
}
for i in 0..count {
free_contender_fields(contenders.add(i));
}
let _ = Vec::from_raw_parts(contenders, count, count);
}

/// Free contest info structure
Expand All @@ -1120,13 +1159,7 @@ pub unsafe extern "C" fn dash_sdk_contest_info_free(info: *mut DashSDKContestInf
}

let info = Box::from_raw(info);
if !info.contenders.is_null() && info.contender_count > 0 {
for i in 0..info.contender_count {
let contender = info.contenders.add(i);
dash_sdk_string_free((*contender).identity_id);
}
let _ = Vec::from_raw_parts(info.contenders, info.contender_count, info.contender_count);
}
free_contender_array(info.contenders, info.contender_count);
}

/// Free a contested name structure
Expand All @@ -1145,17 +1178,10 @@ pub unsafe extern "C" fn dash_sdk_contested_name_free(name: *mut DashSDKConteste
dash_sdk_string_free(name.name);

// Free contest info contents (but not the struct itself as it's embedded)
if !name.contest_info.contenders.is_null() && name.contest_info.contender_count > 0 {
for i in 0..name.contest_info.contender_count {
let contender = name.contest_info.contenders.add(i);
dash_sdk_string_free((*contender).identity_id);
}
let _ = Vec::from_raw_parts(
name.contest_info.contenders,
name.contest_info.contender_count,
name.contest_info.contender_count,
);
}
free_contender_array(
name.contest_info.contenders,
name.contest_info.contender_count,
);
}

/// Free a contested names list
Expand All @@ -1177,19 +1203,10 @@ pub unsafe extern "C" fn dash_sdk_contested_names_list_free(list: *mut DashSDKCo
dash_sdk_string_free((*name).name);

// Free contest info contents
if !(*name).contest_info.contenders.is_null()
&& (*name).contest_info.contender_count > 0
{
for j in 0..(*name).contest_info.contender_count {
let contender = (*name).contest_info.contenders.add(j);
dash_sdk_string_free((*contender).identity_id);
}
let _ = Vec::from_raw_parts(
(*name).contest_info.contenders,
(*name).contest_info.contender_count,
(*name).contest_info.contender_count,
);
}
free_contender_array(
(*name).contest_info.contenders,
(*name).contest_info.contender_count,
);
}
let _ = Vec::from_raw_parts(list.names, list.count, list.count);
}
Expand Down Expand Up @@ -1674,4 +1691,112 @@ mod tests {
assert!(result.data.is_null());
}
}

// MARK: contender destructors
//
// These run under Miri/ASan in CI if enabled, but even without a leak
// checker they pin the shape that regressed: `label` was added to
// `DashSDKContender` and only one of the four free paths was updated, so
// three public destructors leaked one allocation per decoded contender.
// Every destructor now funnels through `free_contender_fields`.

/// Build a heap contender the way the DPNS producers do.
unsafe fn make_contender(id: &str, label: Option<&str>) -> DashSDKContender {
DashSDKContender {
identity_id: CString::new(id).unwrap().into_raw(),
vote_count: 7,
label: match label {
Some(l) => CString::new(l).unwrap().into_raw(),
None => std::ptr::null_mut(),
},
}
}

unsafe fn make_contest_info(contenders: Vec<DashSDKContender>) -> DashSDKContestInfo {
let count = contenders.len();
let ptr = if count == 0 {
std::ptr::null_mut()
} else {
Box::into_raw(contenders.into_boxed_slice()) as *mut DashSDKContender
};
DashSDKContestInfo {
contenders: ptr,
contender_count: count,
abstain_votes: 1,
lock_votes: 2,
end_time: 1_700_000_000_000,
has_winner: false,
}
}

#[test]
fn contender_free_releases_label() {
unsafe {
let contender = Box::into_raw(Box::new(make_contender("Ab1", Some("pizza"))));
dash_sdk_contender_free(contender);
}
}

#[test]
fn contender_free_tolerates_absent_label() {
unsafe {
let contender = Box::into_raw(Box::new(make_contender("Ab1", None)));
dash_sdk_contender_free(contender);
}
}

#[test]
fn contender_free_is_null_safe() {
unsafe { dash_sdk_contender_free(std::ptr::null_mut()) };
}

#[test]
fn contest_info_free_releases_every_contender_label() {
unsafe {
let info = make_contest_info(vec![
make_contender("Ab1", Some("pizza")),
make_contender("Cd2", Some("p1zza")),
make_contender("Ef3", None),
]);
dash_sdk_contest_info_free(Box::into_raw(Box::new(info)));
}
}

#[test]
fn contest_info_free_handles_no_contenders() {
unsafe {
let info = make_contest_info(vec![]);
dash_sdk_contest_info_free(Box::into_raw(Box::new(info)));
}
}

#[test]
fn contested_name_free_releases_labels() {
unsafe {
let name = DashSDKContestedName {
name: CString::new("p1zza").unwrap().into_raw(),
contest_info: make_contest_info(vec![
make_contender("Ab1", Some("pizza")),
make_contender("Cd2", None),
]),
};
dash_sdk_contested_name_free(Box::into_raw(Box::new(name)));
}
}

#[test]
fn contested_names_list_free_releases_labels() {
unsafe {
let names = vec![DashSDKContestedName {
name: CString::new("p1zza").unwrap().into_raw(),
contest_info: make_contest_info(vec![make_contender("Ab1", Some("pizza"))]),
}];
let count = names.len();
let list = DashSDKContestedNamesList {
names: Box::into_raw(names.into_boxed_slice()) as *mut DashSDKContestedName,
count,
};
dash_sdk_contested_names_list_free(Box::into_raw(Box::new(list)));
}
}
}
Loading
Loading