more work

Author: QuantumExplorer

key-wallet-ffi/src/address.rs

@@ -5,7 +5,7 @@
 mod tests;
 
 use std::ffi::{CStr, CString};
-use std::os::raw::{c_char, c_uint};
+use std::os::raw::{c_char, c_uchar};
 
 use crate::error::{FFIError, FFIErrorCode};
 use crate::types::FFINetwork;
@@ -115,6 +115,12 @@ pub unsafe extern "C" fn address_validate(
 
 /// Get address type
 ///
+/// Returns:
+/// - 0: P2PKH address
+/// - 1: P2SH address
+/// - 2: Other address type
+/// - u8::MAX (255): Error occurred
+///
 /// # Safety
 ///
 /// - `address` must be a valid null-terminated C string
@@ -124,10 +130,10 @@ pub unsafe extern "C" fn address_get_type(
     address: *const c_char,
     network: FFINetwork,
     error: *mut FFIError,
-) -> c_uint {
+) -> c_uchar {
     if address.is_null() {
         FFIError::set_error(error, FFIErrorCode::InvalidInput, "Address is null".to_string());
-        return 0;
+        return u8::MAX;
     }
 
     let address_str = unsafe {
@@ -139,7 +145,7 @@ pub unsafe extern "C" fn address_get_type(
                     FFIErrorCode::InvalidInput,
                     "Invalid UTF-8 in address".to_string(),
                 );
-                return 0;
+                return u8::MAX;
             }
         }
     };
@@ -167,7 +173,7 @@ pub unsafe extern "C" fn address_get_type(
                         FFIErrorCode::InvalidAddress,
                         "Address not valid for network".to_string(),
                     );
-                    2 // Invalid
+                    u8::MAX // Error
                 }
             }
         }
@@ -177,7 +183,7 @@ pub unsafe extern "C" fn address_get_type(
                 FFIErrorCode::InvalidAddress,
                 format!("Invalid address: {}", e),
             );
-            2 // Invalid
+            u8::MAX // Error
         }
     }
 }

key-wallet-ffi/src/address_tests.rs

@@ -5,25 +5,9 @@ mod address_tests {
     use crate::address::{address_array_free, address_free, address_get_type, address_validate};
     use crate::error::{FFIError, FFIErrorCode};
     use crate::types::FFINetwork;
-    use crate::wallet;
     use std::ffi::CString;
     use std::ptr;
 
-    unsafe fn create_test_wallet() -> (*mut crate::types::FFIWallet, *mut FFIError) {
-        let mut error = FFIError::success();
-        let error_ptr = &mut error as *mut FFIError;
-
-        let seed = [0x01u8; 64];
-        let wallet = wallet::wallet_create_from_seed(
-            seed.as_ptr(),
-            seed.len(),
-            FFINetwork::Testnet,
-            error_ptr,
-        );
-
-        (wallet, error_ptr)
-    }
-
     #[test]
     fn test_address_validation() {
         let mut error = FFIError::success();
@@ -57,7 +41,7 @@ mod address_tests {
         let addr_type =
             unsafe { address_get_type(p2pkh_addr.as_ptr(), FFINetwork::Testnet, error) };
         assert_eq!(unsafe { (*error).code }, FFIErrorCode::Success);
-        // Currently returns 0 for P2PKH (placeholder implementation)
+        // Returns 0 for P2PKH
         assert_eq!(addr_type, 0);
     }
 
@@ -100,13 +84,19 @@ mod address_tests {
     fn test_address_get_type_valid() {
         let mut error = FFIError::success();
 
-        // Test P2PKH address type
-        let addr_str = CString::new("yXdxAYfK7KGx7gNpVHUfRsQMNpMj5cAadG").unwrap();
+        // Test P2PKH address type (use same known-valid address from other tests)
+        let addr_str = CString::new("yRd4FhXfVGHXpsuZXPNkMrfD9GVj46pnjt").unwrap();
         let addr_type =
             unsafe { address_get_type(addr_str.as_ptr(), FFINetwork::Testnet, &mut error) };
 
-        // Type may vary based on library version, just verify it runs
-        assert!(addr_type <= 2);
+        // Type should be 0, 1, or 2 for valid addresses
+        // If it's invalid (255), the address might not be valid for testnet
+        if addr_type == 255 {
+            assert_eq!(error.code, FFIErrorCode::InvalidAddress);
+        } else {
+            assert!(addr_type <= 2);
+            assert_eq!(error.code, FFIErrorCode::Success);
+        }
     }
 
     #[test]
@@ -117,8 +107,8 @@ mod address_tests {
         let addr_type =
             unsafe { address_get_type(addr_str.as_ptr(), FFINetwork::Testnet, &mut error) };
 
-        // Should return 2 for invalid
-        assert_eq!(addr_type, 2);
+        // Should return 255 (u8::MAX) for invalid
+        assert_eq!(addr_type, 255);
         assert_eq!(error.code, FFIErrorCode::InvalidAddress);
     }
 
@@ -128,8 +118,8 @@ mod address_tests {
 
         let addr_type = unsafe { address_get_type(ptr::null(), FFINetwork::Testnet, &mut error) };
 
-        // Should return 0 for null input
-        assert_eq!(addr_type, 0);
+        // Should return 255 (u8::MAX) for null input
+        assert_eq!(addr_type, 255);
         assert_eq!(error.code, FFIErrorCode::InvalidInput);
     }
 
@@ -207,8 +197,8 @@ mod address_tests {
                 let addr_type =
                     address_get_type(addr_str.as_ptr(), FFINetwork::Testnet, &mut error);
 
-                // Should return a valid type (0, 1, or 2 for invalid)
-                assert!(addr_type <= 2);
+                // Should return a valid type (0, 1, 2) or 255 for error
+                assert!(addr_type <= 2 || addr_type == 255);
             }
         }
     }

key-wallet-ffi/src/derivation.rs

@@ -773,21 +773,70 @@ pub unsafe extern "C" fn dip9_derive_identity_key(
     let additional_path = match key_type {
         FFIDerivationPathType::BlockchainIdentities => {
             // Authentication: identity_index'/key_index'
-            DerivationPath::from(vec![
-                ChildNumber::from_hardened_idx(identity_index).unwrap(),
-                ChildNumber::from_hardened_idx(key_index).unwrap(),
-            ])
+            let cn1 = match ChildNumber::from_hardened_idx(identity_index) {
+                Ok(v) => v,
+                Err(e) => {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidDerivationPath,
+                        format!("Invalid identity_index: {}", e),
+                    );
+                    return ptr::null_mut();
+                }
+            };
+            let cn2 = match ChildNumber::from_hardened_idx(key_index) {
+                Ok(v) => v,
+                Err(e) => {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidDerivationPath,
+                        format!("Invalid key_index: {}", e),
+                    );
+                    return ptr::null_mut();
+                }
+            };
+            DerivationPath::from(vec![cn1, cn2])
         }
         FFIDerivationPathType::BlockchainIdentityCreditRegistrationFunding => {
             // Registration: index'
-            DerivationPath::from(vec![ChildNumber::from_hardened_idx(identity_index).unwrap()])
+            let cn = match ChildNumber::from_hardened_idx(identity_index) {
+                Ok(v) => v,
+                Err(e) => {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidDerivationPath,
+                        format!("Invalid identity_index: {}", e),
+                    );
+                    return ptr::null_mut();
+                }
+            };
+            DerivationPath::from(vec![cn])
         }
         FFIDerivationPathType::BlockchainIdentityCreditTopupFunding => {
             // Top-up: identity_index'/topup_index'
-            DerivationPath::from(vec![
-                ChildNumber::from_hardened_idx(identity_index).unwrap(),
-                ChildNumber::from_hardened_idx(key_index).unwrap(), // key_index used as topup_index
-            ])
+            let cn1 = match ChildNumber::from_hardened_idx(identity_index) {
+                Ok(v) => v,
+                Err(e) => {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidDerivationPath,
+                        format!("Invalid identity_index: {}", e),
+                    );
+                    return ptr::null_mut();
+                }
+            };
+            let cn2 = match ChildNumber::from_hardened_idx(key_index) {
+                Ok(v) => v,
+                Err(e) => {
+                    FFIError::set_error(
+                        error,
+                        FFIErrorCode::InvalidDerivationPath,
+                        format!("Invalid topup_index: {}", e),
+                    );
+                    return ptr::null_mut();
+                }
+            };
+            DerivationPath::from(vec![cn1, cn2])
         }
         _ => {
             FFIError::set_error(error, FFIErrorCode::InvalidInput, "Invalid key type".to_string());

key-wallet-ffi/src/lib.rs

@@ -45,5 +45,7 @@ pub extern "C" fn key_wallet_ffi_initialize() -> bool {
 /// Get library version
 #[no_mangle]
 pub extern "C" fn key_wallet_ffi_version() -> *const c_char {
-    CString::new(env!("CARGO_PKG_VERSION")).unwrap().into_raw()
+    CString::new(env!("CARGO_PKG_VERSION"))
+        .expect("Version string should never fail")
+        .into_raw()
 }

key-wallet-ffi/src/managed_wallet.rs

@@ -288,7 +288,7 @@ pub unsafe extern "C" fn managed_wallet_get_next_bip44_change_address(
 /// - `addresses_out` must be a valid pointer to store the address array pointer
 /// - `count_out` must be a valid pointer to store the count
 /// - `error` must be a valid pointer to an FFIError
-/// - The returned addresses must be freed individually and the array must be freed
+/// - Free the result with address_array_free(addresses_out, count_out)
 #[no_mangle]
 pub unsafe extern "C" fn managed_wallet_get_bip_44_external_address_range(
     managed_wallet: *mut FFIManagedWalletInfo,
@@ -447,10 +447,10 @@ pub unsafe extern "C" fn managed_wallet_get_bip_44_external_address_range(
         }
     }
 
-    // Convert to C array
-    let len = c_addresses.len();
-    let ptr = c_addresses.as_mut_ptr();
-    std::mem::forget(c_addresses);
+    // Convert Vec to Box<[*mut c_char]> and leak it properly
+    let boxed_slice = c_addresses.into_boxed_slice();
+    let len = boxed_slice.len();
+    let ptr = Box::into_raw(boxed_slice) as *mut *mut c_char;
 
     *count_out = len;
     *addresses_out = ptr;
@@ -470,7 +470,7 @@ pub unsafe extern "C" fn managed_wallet_get_bip_44_external_address_range(
 /// - `addresses_out` must be a valid pointer to store the address array pointer
 /// - `count_out` must be a valid pointer to store the count
 /// - `error` must be a valid pointer to an FFIError
-/// - The returned addresses must be freed individually and the array must be freed
+/// - Free the result with address_array_free(addresses_out, count_out)
 #[no_mangle]
 pub unsafe extern "C" fn managed_wallet_get_bip_44_internal_address_range(
     managed_wallet: *mut FFIManagedWalletInfo,
@@ -629,10 +629,10 @@ pub unsafe extern "C" fn managed_wallet_get_bip_44_internal_address_range(
         }
     }
 
-    // Convert to C array
-    let len = c_addresses.len();
-    let ptr = c_addresses.as_mut_ptr();
-    std::mem::forget(c_addresses);
+    // Convert Vec to Box<[*mut c_char]> and leak it properly
+    let boxed_slice = c_addresses.into_boxed_slice();
+    let len = boxed_slice.len();
+    let ptr = Box::into_raw(boxed_slice) as *mut *mut c_char;
 
     *count_out = len;
     *addresses_out = ptr;
@@ -712,6 +712,21 @@ pub unsafe extern "C" fn managed_wallet_free(managed_wallet: *mut FFIManagedWall
     }
 }
 
+/// Free managed wallet info returned by wallet_manager_get_managed_wallet_info
+///
+/// # Safety
+///
+/// - `wallet_info` must be a valid pointer returned by wallet_manager_get_managed_wallet_info or null
+/// - After calling this function, the pointer becomes invalid and must not be used
+#[no_mangle]
+pub unsafe extern "C" fn managed_wallet_info_free(wallet_info: *mut FFIManagedWalletInfo) {
+    if !wallet_info.is_null() {
+        unsafe {
+            let _ = Box::from_raw(wallet_info);
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use crate::error::{FFIError, FFIErrorCode};
@@ -1083,9 +1098,10 @@ mod tests {
                 let addr_str = CStr::from_ptr(addr_ptr).to_string_lossy();
                 assert!(!addr_str.is_empty());
                 println!("Internal address: {}", addr_str);
-                let _ = CString::from_raw(addr_ptr);
+                // Don't manually free individual strings - address_array_free handles it
             }
-            libc::free(addresses_out as *mut libc::c_void);
+            // Use the proper FFI function to free the array and all strings
+            crate::address::address_array_free(addresses_out, count_out);
         }
 
         // Clean up

key-wallet-ffi/src/types.rs

@@ -85,8 +85,10 @@ impl FFIAccountResult {
     /// Create an error result
     pub fn error(code: crate::error::FFIErrorCode, message: String) -> Self {
         use std::ffi::CString;
-        let c_message =
-            CString::new(message).unwrap_or_else(|_| CString::new("Unknown error").unwrap());
+        let c_message = CString::new(message).unwrap_or_else(|_| {
+            // Fallback to a safe literal that cannot fail
+            CString::new("Unknown error").expect("Hardcoded string should never fail")
+        });
         FFIAccountResult {
             account: std::ptr::null_mut(),
             error_code: code as i32,

key-wallet-ffi/src/wallet_manager.rs

@@ -5,7 +5,7 @@
 mod tests;
 
 use std::ffi::{CStr, CString};
-use std::os::raw::{c_char, c_uint, c_ulong};
+use std::os::raw::{c_char, c_uint};
 use std::ptr;
 use std::sync::Mutex;
 
@@ -265,7 +265,7 @@ pub unsafe extern "C" fn wallet_manager_get_wallet_ids(
 /// - `wallet_id` must be a valid pointer to a 32-byte wallet ID
 /// - `error` must be a valid pointer to an FFIError structure or null
 /// - The caller must ensure all pointers remain valid for the duration of this call
-/// - The returned wallet pointer is only valid while the manager exists
+/// - The returned wallet must be freed with wallet_free()
 #[no_mangle]
 pub unsafe extern "C" fn wallet_manager_get_wallet(
     manager: *const FFIWalletManager,
@@ -327,7 +327,7 @@ pub unsafe extern "C" fn wallet_manager_get_wallet(
 /// - `wallet_id` must be a valid pointer to a 32-byte wallet ID
 /// - `error` must be a valid pointer to an FFIError structure or null
 /// - The caller must ensure all pointers remain valid for the duration of this call
-/// - The returned managed wallet info pointer is only valid while the manager exists
+/// - The returned managed wallet info must be freed with managed_wallet_info_free()
 #[no_mangle]
 pub unsafe extern "C" fn wallet_manager_get_managed_wallet_info(
     manager: *const FFIWalletManager,
@@ -551,16 +551,16 @@ pub unsafe extern "C" fn wallet_manager_get_change_address(
 ///
 /// - `manager` must be a valid pointer to an FFIWalletManager instance
 /// - `wallet_id` must be a valid pointer to a 32-byte wallet ID
-/// - `confirmed_out` must be a valid pointer to a c_ulong
-/// - `unconfirmed_out` must be a valid pointer to a c_ulong
+/// - `confirmed_out` must be a valid pointer to a u64 (maps to C uint64_t)
+/// - `unconfirmed_out` must be a valid pointer to a u64 (maps to C uint64_t)
 /// - `error` must be a valid pointer to an FFIError structure or null
 /// - The caller must ensure all pointers remain valid for the duration of this call
 #[no_mangle]
 pub unsafe extern "C" fn wallet_manager_get_wallet_balance(
     manager: *const FFIWalletManager,
     wallet_id: *const u8,
-    confirmed_out: *mut c_ulong,
-    unconfirmed_out: *mut c_ulong,
+    confirmed_out: *mut u64,
+    unconfirmed_out: *mut u64,
     error: *mut FFIError,
 ) -> bool {
     if manager.is_null()
@@ -598,8 +598,8 @@ pub unsafe extern "C" fn wallet_manager_get_wallet_balance(
     match manager_guard.get_wallet_balance(&wallet_id_array) {
         Ok(balance) => {
             unsafe {
-                *confirmed_out = balance.confirmed as c_ulong;
-                *unconfirmed_out = balance.unconfirmed as c_ulong;
+                *confirmed_out = balance.confirmed;
+                *unconfirmed_out = balance.unconfirmed;
             }
             FFIError::set_success(error);
             true