Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,25 @@ internal object WalletManagerNative {
external fun spvIsRunning(managerHandle: Long): Boolean
external fun spvStop(managerHandle: Long)

/**
* The proTxHashes of every masternode in the current-tip deterministic
* masternode list whose voting-key hash matches the 20-byte [votingKeyId]
* (hash160 of a voting public key), as a flat `byte[]` of concatenated
* 32-byte proTxHashes (internal byte order) — the caller splits into
* 32-byte rows. Replaces dashj's
* `MasternodeListManager.getMasternodesByVotingKey(votingKeyId)` used by
* contested-username voting. Returns an EMPTY (non-null) `byte[]` when the
* masternode list hasn't synced (SPV client not running / DML unavailable)
* or no masternode uses the key; throws only on a structural FFI error.
* JNI symbol:
* `Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_masternodesByVotingKey`;
* bridges `platform_wallet_manager_masternodes_by_voting_key`.
*/
external fun masternodesByVotingKey(
managerHandle: Long,
votingKeyId: ByteArray,
): ByteArray
Comment on lines 496 to +516

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: Expose masternodesByVotingKey through the public PlatformWalletManager API

WalletManagerNative is declared internal object (line 23), and masternodesByVotingKey is added here as a raw external fun with no corresponding public wrapper in PlatformWalletManager.kt. Every other native call in this class — createWallet, removeWallet, platformAddressSyncStart, identitySyncStart, shieldedSyncStart, trackedIdentityRecoveryAssetLocks, etc. — has a matching public suspend fun on PlatformWalletManager that validates input, dispatches via withContext(Dispatchers.IO), and calls through mapNativeErrors. This lookup breaks that pattern: it is unreachable from outside the kotlin-sdk module (managerHandle is also private on PlatformWalletManager), and even the raw binding bypasses the IO-dispatch convention even though the underlying Rust accessor documents itself as blocking via two tokio::RwLock::blocking_read calls. Since this PR's stated purpose is surfacing the lookup to Kotlin/dash-wallet consumers, the feature isn't actually usable through supported API without this wrapper.

Suggested change
external fun spvIsRunning(managerHandle: Long): Boolean
external fun spvStop(managerHandle: Long)
/**
* The proTxHashes of every masternode in the current-tip deterministic
* masternode list whose voting-key hash matches the 20-byte [votingKeyId]
* (hash160 of a voting public key), as a flat `byte[]` of concatenated
* 32-byte proTxHashes (internal byte order) — the caller splits into
* 32-byte rows. Replaces dashj's
* `MasternodeListManager.getMasternodesByVotingKey(votingKeyId)` used by
* contested-username voting. Returns an EMPTY (non-null) `byte[]` when the
* masternode list hasn't synced (SPV client not running / DML unavailable)
* or no masternode uses the key; throws only on a structural FFI error.
* JNI symbol:
* `Java_org_dashfoundation_dashsdk_ffi_WalletManagerNative_masternodesByVotingKey`;
* bridges `platform_wallet_manager_masternodes_by_voting_key`.
*/
external fun masternodesByVotingKey(
managerHandle: Long,
votingKeyId: ByteArray,
): ByteArray
suspend fun masternodesByVotingKey(votingKeyId: ByteArray): List<ByteArray> = withContext(Dispatchers.IO) {
require(votingKeyId.size == 20) { "votingKeyId must be exactly 20 bytes" }
val flat = mapNativeErrors { WalletManagerNative.masternodesByVotingKey(managerHandle, votingKeyId) }
check(flat.size % 32 == 0) { "masternodesByVotingKey returned a non-multiple-of-32 buffer" }
flat.toList().chunked(32).map { it.toByteArray() }
}

source: ['codex']


// ── Wallet-memory snapshots (Wave-1B) ─────────────────────────────

/**
Expand Down
98 changes: 97 additions & 1 deletion packages/rs-platform-wallet-ffi/src/spv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,17 @@
use std::ffi::{CStr, CString};
use std::os::raw::c_char;

use dashcore::hashes::Hash;
use dashcore::sml::llmq_type::LlmqDevnetParams;
use dashcore::PubkeyHash;
use platform_wallet::spv::{
ClientConfig, DevnetConfig, ProgressPercentage, SpvPeerNodeType, SyncProgress, SyncState,
};

use crate::error::*;
use crate::handle::*;
use crate::runtime::{block_on_worker, runtime};
use crate::types::FFINetwork;
use crate::types::{FFINetwork, IdentifierArray};
use crate::{check_ptr, unwrap_option_or_return, unwrap_result_or_return};

pub const SPV_SYNC_STATE_WAIT_FOR_EVENTS: u32 = 0;
Expand Down Expand Up @@ -152,6 +154,100 @@ pub unsafe extern "C" fn platform_wallet_manager_sync_progress(
PlatformWalletFFIResult::ok()
}

/// The proTxHashes of every masternode in the current-tip deterministic
/// masternode list whose voting key hash matches the 20-byte `voting_key_id`.
///
/// Replaces dashj's `MasternodeListManager.getMasternodesByVotingKey(...)`,
/// the lookup contested-username voting uses. On success `*out_array` owns a
/// flat `[[u8; 32]]` of `count` proTxHashes (internal byte order), which the
/// caller must release via [`crate::platform_wallet_identifier_array_free`].
/// An unsynced/stopped client (or a voting key no masternode uses) returns
/// `ok()` with the empty `(null, 0)` sentinel.
///
/// # Safety
/// - `voting_key_id` must point at 20 readable bytes.
/// - `out_array` must be a valid `*mut IdentifierArray`.
#[no_mangle]
pub unsafe extern "C" fn platform_wallet_manager_masternodes_by_voting_key(
handle: Handle,
voting_key_id: *const u8,
out_array: *mut IdentifierArray,
) -> PlatformWalletFFIResult {
// Validate and publish the sentinel BEFORE any other guard. `check_ptr!`
// returns early, so validating `voting_key_id` first meant a null input
// pointer returned `ErrorNullPointer` with `*out_array` still holding
// whatever the caller's stack had — breaking this function's documented
// promise that the out-param is initialized on every path. A C or Swift
// caller that frees unconditionally would then hand
// `platform_wallet_identifier_array_free` an arbitrary pointer
// (dashpay/platform#4258 review).
check_ptr!(out_array);
*out_array = IdentifierArray::empty();
check_ptr!(voting_key_id);

let key_bytes: [u8; 20] = std::ptr::read(voting_key_id as *const [u8; 20]);
let voting_key = PubkeyHash::from_byte_array(key_bytes);

let option = PLATFORM_WALLET_MANAGER_STORAGE.with_item(handle, |manager| {
manager
.spv()
.masternodes_by_voting_key_blocking(&voting_key)
});
let hashes = unwrap_option_or_return!(option);
*out_array = IdentifierArray::from_hashes(hashes);
PlatformWalletFFIResult::ok()
}

#[cfg(test)]
mod masternodes_by_voting_key_tests {
use super::*;
use crate::error::platform_wallet_ffi_result_free;

/// The function documents that `*out_array` is initialized on EVERY path.
/// A null `voting_key_id` must therefore still leave the sentinel behind:
/// a C or Swift caller that frees unconditionally on error would otherwise
/// hand `platform_wallet_identifier_array_free` whatever its uninitialized
/// stack slot happened to hold (dashpay/platform#4258 review).
#[test]
fn null_input_pointer_still_initializes_the_out_param() {
// Pre-poison the out-param the way an uninitialized C local looks:
// non-null pointer, non-zero count. If the guard order regresses, this
// survives the call and a cleanup-on-error caller frees it.
let mut out = IdentifierArray {
items: 0xdead_beef_usize as *mut [u8; 32],
count: 9,
};

let mut result = unsafe {
platform_wallet_manager_masternodes_by_voting_key(0, std::ptr::null(), &mut out)
};

assert_eq!(result.code, PlatformWalletFFIResultCode::ErrorNullPointer);
assert!(
out.items.is_null() && out.count == 0,
"the empty sentinel must be published before the input-pointer \
guard returns (got items={:?}, count={})",
out.items,
out.count,
);

unsafe { platform_wallet_ffi_result_free(&mut result) };
}

/// A null `out_array` has nowhere to publish the sentinel, so it must be
/// rejected — and must not be dereferenced on the way out.
#[test]
fn null_out_param_is_rejected() {
let mut result = unsafe {
let key = [0u8; 20];
platform_wallet_manager_masternodes_by_voting_key(0, key.as_ptr(), std::ptr::null_mut())
};

assert_eq!(result.code, PlatformWalletFFIResultCode::ErrorNullPointer);
unsafe { platform_wallet_ffi_result_free(&mut result) };
}
}

pub const SPV_PEER_NODE_TYPE_UNKNOWN: u32 = 0;
pub const SPV_PEER_NODE_TYPE_NORMAL: u32 = 1;
pub const SPV_PEER_NODE_TYPE_MASTERNODE: u32 = 2;
Expand Down
99 changes: 92 additions & 7 deletions packages/rs-platform-wallet-ffi/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,17 +124,32 @@ impl IdentifierArray {
}

pub fn new(identifiers: Vec<dpp::prelude::Identifier>) -> Self {
let count = identifiers.len();
Self::from_hashes(identifiers.into_iter().map(|id| id.to_buffer()).collect())
}

/// Build the array from raw 32-byte rows (e.g. proTxHashes), which are not
/// platform `Identifier`s. The buffer is heap-leaked here and reclaimed by
/// [`platform_wallet_identifier_array_free`].
///
/// The rows are moved into an exact-length **boxed slice** before ownership
/// is transferred, so the allocation is exactly `count` elements wide and
/// the free path can reconstruct it with the identical layout. Leaking the
/// `Vec` directly would export only `(ptr, len)` while the allocation kept
/// whatever spare capacity the producer happened to have — and freeing an
/// allocation with a capacity it was not created with is undefined
/// behaviour. That is not hypothetical: `masternodes_by_voting_key`
/// collects through a `filter`, whose `size_hint` lower bound is 0, so an
/// ordinary one-match lookup routinely produces `capacity != len`
/// (dashpay/platform#4258 review).
pub fn from_hashes(hashes: Vec<[u8; 32]>) -> Self {
let count = hashes.len();
if count == 0 {
return Self::empty();
}

let mut items: Vec<[u8; 32]> = identifiers.into_iter().map(|id| id.to_buffer()).collect();
let items = Box::into_raw(hashes.into_boxed_slice()) as *mut [u8; 32];

let ptr = items.as_mut_ptr();
std::mem::forget(items);

Self { items: ptr, count }
Self { items, count }
}
}

Expand All @@ -151,8 +166,12 @@ pub unsafe extern "C" fn platform_wallet_identifier_array_free(array: *mut Ident
}
let array = unsafe { &mut *array };
if !array.items.is_null() && array.count > 0 {
// Mirror image of [`IdentifierArray::from_hashes`]: the allocation was
// handed over as an exact-length boxed slice, so reclaim it as one.
// (The previous `Vec::from_raw_parts(items, count, count)` asserted a
// capacity the producer never guaranteed — see that constructor.)
unsafe {
let _ = Vec::from_raw_parts(array.items, array.count, array.count);
let _ = Box::from_raw(std::ptr::slice_from_raw_parts_mut(array.items, array.count));
}
}
array.items = std::ptr::null_mut();
Expand Down Expand Up @@ -215,6 +234,72 @@ mod tests {
}
}

/// The production `masternodes_by_voting_key` path collects through a
/// `filter`, so its `Vec` routinely carries spare capacity — a one-match
/// lookup over many masternodes is the common case. Creation and free must
/// agree on the allocation layout regardless; under Miri (or a
/// capacity-checking allocator) the old `Vec::from_raw_parts(p, n, n)`
/// free of such a buffer is undefined behaviour
/// (dashpay/platform#4258 review).
#[test]
fn identifier_array_frees_rows_collected_with_spare_capacity() {
unsafe {
// Exactly the production shape: filter many rows down to one.
let hashes: Vec<[u8; 32]> = (0u8..64)
.map(|i| [i; 32])
.filter(|row| row[0] == 7)
.collect();
assert_eq!(hashes.len(), 1);
assert!(
hashes.capacity() > hashes.len(),
"precondition: a filtered collect must over-allocate for this \
test to exercise the layout mismatch (len {}, capacity {})",
hashes.len(),
hashes.capacity(),
);

let mut array = IdentifierArray::from_hashes(hashes);
assert_eq!(array.count, 1);
assert!(!array.items.is_null());
assert_eq!(*array.items, [7u8; 32], "the row must survive intact");

platform_wallet_identifier_array_free(&mut array);
assert!(array.items.is_null());
assert_eq!(array.count, 0);
}
}

/// Multi-row round trip: every row must come back byte-for-byte in order,
/// and the whole buffer must free cleanly.
#[test]
fn identifier_array_from_hashes_round_trips_every_row() {
unsafe {
let rows: Vec<[u8; 32]> = (0u8..5).map(|i| [i; 32]).collect();
let mut array = IdentifierArray::from_hashes(rows.clone());
assert_eq!(array.count, rows.len());

let seen = std::slice::from_raw_parts(array.items, array.count);
assert_eq!(seen, rows.as_slice());

platform_wallet_identifier_array_free(&mut array);
assert!(array.items.is_null());
}
}

/// `from_hashes` with no rows must produce the `(null, 0)` sentinel, which
/// the free path skips — an empty lookup result is not an allocation.
#[test]
fn identifier_array_from_hashes_empty_is_the_sentinel() {
unsafe {
let mut array = IdentifierArray::from_hashes(Vec::new());
assert!(array.items.is_null());
assert_eq!(array.count, 0);
// Freeing the sentinel is a no-op, and idempotent.
platform_wallet_identifier_array_free(&mut array);
platform_wallet_identifier_array_free(&mut array);
}
}

#[test]
fn test_read_identifier_round_trip() {
unsafe {
Expand Down
Loading
Loading