Skip to content

fix: clean up ffi methods #124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
QuantumExplorer merged 2 commits into from
Aug 29, 2025
Merged

fix: clean up ffi methods #124

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f663521
fix: rename networks
QuantumExplorer Aug 29, 2025
18a17a6
more work
QuantumExplorer Aug 29, 2025
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
2 changes: 1 addition & 1 deletion hashes/src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ macro_rules! hash_newtype_known_attrs {
#[cfg(feature = "schemars")]
pub mod json_hex_string {
use schemars::SchemaGenerator;
use schemars::{Schema, json_schema};
use schemars::{json_schema, Schema};
macro_rules! define_custom_hex {
($name:ident, $len:expr) => {
pub fn $name(_generator: &mut SchemaGenerator) -> Schema {
Expand Down
191 changes: 83 additions & 108 deletions key-wallet-ffi/include/key_wallet_ffi.h
View file
Open in desktop

Large diffs are not rendered by default.

20 changes: 10 additions & 10 deletions key-wallet-ffi/src/account.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::os::raw::c_uint;
use std::sync::Arc;

use crate::error::{FFIError, FFIErrorCode};
use crate::types::{FFIAccountResult, FFIAccountType, FFINetwork, FFIWallet};
use crate::types::{FFIAccountResult, FFIAccountType, FFINetworks, FFIWallet};
#[cfg(feature = "bls")]
use key_wallet::account::BLSAccount;
#[cfg(feature = "eddsa")]
Expand Down Expand Up @@ -81,7 +81,7 @@ impl FFIEdDSAAccount {
#[no_mangle]
pub unsafe extern "C" fn wallet_get_account(
wallet: *const FFIWallet,
network: FFINetwork,
network: FFINetworks,
account_index: c_uint,
account_type: FFIAccountType,
) -> FFIAccountResult {
Expand Down Expand Up @@ -126,7 +126,7 @@ pub unsafe extern "C" fn wallet_get_account(
#[no_mangle]
pub unsafe extern "C" fn wallet_get_top_up_account_with_registration_index(
wallet: *const FFIWallet,
network: FFINetwork,
network: FFINetworks,
registration_index: c_uint,
) -> FFIAccountResult {
if wallet.is_null() {
Expand Down Expand Up @@ -262,9 +262,9 @@ pub unsafe extern "C" fn account_get_extended_public_key_as_string(
/// - `account` must be a valid pointer to an FFIAccount instance
/// - Returns FFINetwork::NoNetworks if the account is null
#[no_mangle]
pub unsafe extern "C" fn account_get_network(account: *const FFIAccount) -> FFINetwork {
pub unsafe extern "C" fn account_get_network(account: *const FFIAccount) -> FFINetworks {
if account.is_null() {
return FFINetwork::NoNetworks;
return FFINetworks::NoNetworks;
}

let account = &*account;
Expand Down Expand Up @@ -374,9 +374,9 @@ pub unsafe extern "C" fn bls_account_get_extended_public_key_as_string(
/// - Returns FFINetwork::NoNetworks if the account is null
#[cfg(feature = "bls")]
#[no_mangle]
pub unsafe extern "C" fn bls_account_get_network(account: *const FFIBLSAccount) -> FFINetwork {
pub unsafe extern "C" fn bls_account_get_network(account: *const FFIBLSAccount) -> FFINetworks {
if account.is_null() {
return FFINetwork::NoNetworks;
return FFINetworks::NoNetworks;
}

let account = &*account;
Expand Down Expand Up @@ -491,9 +491,9 @@ pub unsafe extern "C" fn eddsa_account_get_extended_public_key_as_string(
/// - Returns FFINetwork::NoNetworks if the account is null
#[cfg(feature = "eddsa")]
#[no_mangle]
pub unsafe extern "C" fn eddsa_account_get_network(account: *const FFIEdDSAAccount) -> FFINetwork {
pub unsafe extern "C" fn eddsa_account_get_network(account: *const FFIEdDSAAccount) -> FFINetworks {
if account.is_null() {
return FFINetwork::NoNetworks;
return FFINetworks::NoNetworks;
}

let account = &*account;
Expand Down Expand Up @@ -581,7 +581,7 @@ pub unsafe extern "C" fn eddsa_account_get_is_watch_only(account: *const FFIEdDS
#[no_mangle]
pub unsafe extern "C" fn wallet_get_account_count(
wallet: *const FFIWallet,
network: FFINetwork,
network: FFINetworks,
error: *mut FFIError,
) -> c_uint {
if wallet.is_null() {
Expand Down
130 changes: 66 additions & 64 deletions key-wallet-ffi/src/account_collection.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use std::ptr;

use crate::account::FFIAccount;
use crate::error::{FFIError, FFIErrorCode};
use crate::types::{FFINetwork, FFIWallet};
use crate::types::{FFINetworks, FFIWallet};

/// Opaque handle to an account collection
pub struct FFIAccountCollection {
Expand Down Expand Up @@ -83,7 +83,7 @@ pub struct FFIAccountCollectionSummary {
#[no_mangle]
pub unsafe extern "C" fn wallet_get_account_collection(
wallet: *const FFIWallet,
network: FFINetwork,
network: FFINetworks,
error: *mut FFIError,
) -> *mut FFIAccountCollection {
if wallet.is_null() {
Expand Down Expand Up @@ -606,39 +606,38 @@ pub unsafe extern "C" fn account_collection_has_provider_owner_keys(
}

/// Get the provider operator keys account if it exists
/// Note: This function is only available when the `bls` feature is enabled
/// Note: Returns null if the `bls` feature is not enabled
///
/// # Safety
///
/// - `collection` must be a valid pointer to an FFIAccountCollection
/// - The returned pointer must be freed with `bls_account_free` when no longer needed
#[cfg(feature = "bls")]
/// - The returned pointer must be freed with `bls_account_free` when no longer needed (when BLS is enabled)
#[no_mangle]
pub unsafe extern "C" fn account_collection_get_provider_operator_keys(
collection: *const FFIAccountCollection,
) -> *mut crate::account::FFIBLSAccount {
if collection.is_null() {
return ptr::null_mut();
}
) -> *mut std::os::raw::c_void {
#[cfg(feature = "bls")]
{
if collection.is_null() {
return ptr::null_mut();
}

let collection = &*collection;
match &collection.collection.provider_operator_keys {
Some(account) => {
let ffi_account = crate::account::FFIBLSAccount::new(account);
Box::into_raw(Box::new(ffi_account))
let collection = &*collection;
match &collection.collection.provider_operator_keys {
Some(account) => {
let ffi_account = crate::account::FFIBLSAccount::new(account);
Box::into_raw(Box::new(ffi_account)) as *mut std::os::raw::c_void
}
None => ptr::null_mut(),
}
None => ptr::null_mut(),
}
}

/// Get the provider operator keys account if it exists (stub when BLS is disabled)
#[cfg(not(feature = "bls"))]
#[no_mangle]
pub unsafe extern "C" fn account_collection_get_provider_operator_keys(
_collection: *const FFIAccountCollection,
) -> *mut std::os::raw::c_void {
// BLS feature not enabled, always return null
ptr::null_mut()
#[cfg(not(feature = "bls"))]
{
// BLS feature not enabled, always return null
let _ = collection; // Avoid unused parameter warning
ptr::null_mut()
}
}
Comment on lines 616 to 641
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Returning void for BLS operator account weakens type safety*

ABI now requires consumers to cast and call the correct free, risking UB if mismatched. Prefer typed return with a stable opaque type and return null when the feature is disabled.

If keeping this ABI, at minimum add a runtime type tag alongside the pointer or provide feature-gated typed getters (and deprecate the void* versions). I can draft a safe-ABI variant if desired.

🤖 Prompt for AI Agents 
In key-wallet-ffi/src/account_collection.rs around lines 616–641, the FFI
currently returns a raw void* for a BLS operator account which weakens type
safety; instead add a feature-gated, typed getter and keep the void* API
deprecated (or add a tagged opaque wrapper). Concretely: add a new function pub
unsafe extern "C" fn
account_collection_get_provider_operator_bls_account(collection: *const
FFIAccountCollection) -> *mut crate::account::FFIBLSAccount that is
#[cfg(feature = "bls")] and returns a boxed FFIBLSAccount pointer (or null if
collection/account is null); provide an #[cfg(not(feature = "bls"))]
implementation that returns null; mark the existing void* function deprecated
(or modify it to return a small tagged opaque struct containing a type tag and
pointer) so consumers can safely detect type at runtime. Ensure null checks and
ownership semantics match existing allocation/free conventions.


/// Check if provider operator keys account exists
Expand Down Expand Up @@ -667,39 +666,38 @@ pub unsafe extern "C" fn account_collection_has_provider_operator_keys(
}

/// Get the provider platform keys account if it exists
/// Note: This function is only available when the `eddsa` feature is enabled
/// Note: Returns null if the `eddsa` feature is not enabled
///
/// # Safety
///
/// - `collection` must be a valid pointer to an FFIAccountCollection
/// - The returned pointer must be freed with `eddsa_account_free` when no longer needed
#[cfg(feature = "eddsa")]
/// - The returned pointer must be freed with `eddsa_account_free` when no longer needed (when EdDSA is enabled)
#[no_mangle]
pub unsafe extern "C" fn account_collection_get_provider_platform_keys(
collection: *const FFIAccountCollection,
) -> *mut crate::account::FFIEdDSAAccount {
if collection.is_null() {
return ptr::null_mut();
}
) -> *mut std::os::raw::c_void {
#[cfg(feature = "eddsa")]
{
if collection.is_null() {
return ptr::null_mut();
}

let collection = &*collection;
match &collection.collection.provider_platform_keys {
Some(account) => {
let ffi_account = crate::account::FFIEdDSAAccount::new(account);
Box::into_raw(Box::new(ffi_account))
let collection = &*collection;
match &collection.collection.provider_platform_keys {
Some(account) => {
let ffi_account = crate::account::FFIEdDSAAccount::new(account);
Box::into_raw(Box::new(ffi_account)) as *mut std::os::raw::c_void
}
None => ptr::null_mut(),
}
None => ptr::null_mut(),
}
}

/// Get the provider platform keys account if it exists (stub when EdDSA is disabled)
#[cfg(not(feature = "eddsa"))]
#[no_mangle]
pub unsafe extern "C" fn account_collection_get_provider_platform_keys(
_collection: *const FFIAccountCollection,
) -> *mut std::os::raw::c_void {
// EdDSA feature not enabled, always return null
ptr::null_mut()
#[cfg(not(feature = "eddsa"))]
{
// EdDSA feature not enabled, always return null
let _ = collection; // Avoid unused parameter warning
ptr::null_mut()
}
}
Comment on lines 676 to 701
Copy link
Contributor

@coderabbitai coderabbitai bot Aug 29, 2025

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

⚠️ Potential issue

Same concern for EdDSA platform account void*

Mirror the fix for operator keys to keep the ABI type-safe.

🤖 Prompt for AI Agents 
In key-wallet-ffi/src/account_collection.rs around lines 676 to 701, the
function currently returns a bare *mut c_void for the EdDSA
provider_platform_keys which is not ABI type-safe; mirror the operator-keys fix
by changing the function signature and returns to use the concrete FFI pointer
type used for platform accounts (the same FFI platform/account struct used for
operator keys — e.g., *mut FFIPlatformAccount or the concrete FFIEdDSAAccount
pointer type), construct and Box::into_raw the appropriate FFI wrapper instance
and return it as that concrete pointer (or return null_mut() when absent), and
update the not(feature = "eddsa") branch to avoid unused parameter warnings
while preserving the concrete return type.


/// Check if provider platform keys account exists
Expand Down Expand Up @@ -1100,7 +1098,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null(),
ptr::null_mut(),
);
Expand All @@ -1109,7 +1107,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand Down Expand Up @@ -1158,7 +1156,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1167,7 +1165,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand All @@ -1179,7 +1177,9 @@ mod tests {
assert!(!operator_account.is_null());

// Free the BLS account
crate::account::bls_account_free(operator_account);
crate::account::bls_account_free(
operator_account as *mut crate::account::FFIBLSAccount,
);
}

// Clean up
Expand All @@ -1206,7 +1206,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1215,7 +1215,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand All @@ -1227,7 +1227,9 @@ mod tests {
assert!(!platform_account.is_null());

// Free the EdDSA account
crate::account::eddsa_account_free(platform_account);
crate::account::eddsa_account_free(
platform_account as *mut crate::account::FFIEdDSAAccount,
);
}

// Clean up
Expand Down Expand Up @@ -1278,7 +1280,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1287,7 +1289,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand Down Expand Up @@ -1334,7 +1336,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1343,7 +1345,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);

Expand Down Expand Up @@ -1422,7 +1424,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1431,7 +1433,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand Down Expand Up @@ -1512,7 +1514,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
&options,
ptr::null_mut(),
);
Expand All @@ -1521,7 +1523,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);

Expand Down Expand Up @@ -1586,7 +1588,7 @@ mod tests {
let wallet = wallet_create_from_mnemonic_with_options(
mnemonic.as_ptr(),
ptr::null(),
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null(),
ptr::null_mut(),
);
Expand All @@ -1595,7 +1597,7 @@ mod tests {
// Get account collection
let collection = wallet_get_account_collection(
wallet,
crate::types::FFINetwork::Testnet,
crate::types::FFINetworks::Testnet,
ptr::null_mut(),
);
assert!(!collection.is_null());
Expand Down
Loading
Loading