fix(ffi): free transaction byte buffers correctly

key-wallet-ffi/FFI_API.md

@@ -277,7 +277,7 @@ Functions: 14
 |----------|-------------|--------|
 | `transaction_add_input` | Add an input to a transaction  # Safety - `tx` must be a valid pointer to an... | transaction |
 | `transaction_add_output` | Add an output to a transaction  # Safety - `tx` must be a valid pointer to... | transaction |
-| `transaction_bytes_free` | Free transaction bytes  # Safety  - `tx_bytes` must be a valid pointer... | transaction |
+| `transaction_bytes_free` | Free transaction bytes returned by transaction-building FFI functions | transaction |
 | `transaction_check_result_free` | Free a transaction check result  # Safety  - `result` must be a valid... | transaction_checking |
 | `transaction_classify` | Get the transaction classification for routing  Returns a string describing... | transaction_checking |
 | `transaction_create` | Create a new empty transaction  # Returns - Pointer to FFITransaction on... | transaction |
@@ -1289,10 +1289,10 @@ wallet_build_and_sign_asset_lock_transaction(manager: *const FFIWalletManager, w
 ```
 
 **Description:**
-Build and sign an asset lock transaction for Core to Platform transfers.  Creates a special transaction (type 8) with `AssetLockPayload` that locks Dash for Platform credits. Derives one unique private key per credit output from the specified funding account types.  # Parameters  - `funding_types`: Array of `credit_outputs_count` funding account types, one per credit output (registration, top-up, invitation, etc.) - `identity_indices`: Array of `credit_outputs_count` identity indices. Only used for `IdentityTopUp` entries; ignored for other funding types. - `private_keys_out`: Caller-allocated array of `credit_outputs_count` × 32-byte buffers. On success, each `private_keys_out[i]` receives the one-time private key corresponding to `credit_output_scripts[i]`.  # Safety  - All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free`
+Build and sign an asset lock transaction for Core to Platform transfers.  Creates a special transaction (type 8) with `AssetLockPayload` that locks Dash for Platform credits. Derives one unique private key per credit output from the specified funding account types.  # Parameters  - `funding_types`: Array of `credit_outputs_count` funding account types, one per credit output (registration, top-up, invitation, etc.) - `identity_indices`: Array of `credit_outputs_count` identity indices. Only used for `IdentityTopUp` entries; ignored for other funding types. - `private_keys_out`: Caller-allocated array of `credit_outputs_count` × 32-byte buffers. On success, each `private_keys_out[i]` receives the one-time private key corresponding to `credit_output_scripts[i]`.  # Safety  - All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - On success, the caller must free the returned transaction bytes by calling `transaction_bytes_free(*tx_bytes_out)`.
 
 **Safety:**
-- All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - Caller must free `tx_bytes_out` with `transaction_bytes_free`
+- All pointer parameters must be valid and non-null - All parallel arrays must have at least `credit_outputs_count` elements - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers - On success, the caller must free the returned transaction bytes by calling `transaction_bytes_free(*tx_bytes_out)`.
 
 **Module:** `transaction`
 
@@ -1305,10 +1305,10 @@ wallet_build_and_sign_transaction(manager: *const FFIWalletManager, wallet: *con
 ```
 
 **Description:**
-Build and sign a transaction using the wallet's managed info  This is the recommended way to build transactions. It handles: - UTXO selection using coin selection algorithms - Fee calculation - Change address generation - Transaction signing  # Safety  - `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_rate` must be a valid variant of FFIFeeRate - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free`
+Build and sign a transaction using the wallet's managed info  This is the recommended way to build transactions. It handles: - UTXO selection using coin selection algorithms - Fee calculation - Change address generation - Transaction signing  # Safety  - `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_per_kb` is the requested fee rate in duffs per kilobyte - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - On success, the returned transaction bytes must be freed by calling `transaction_bytes_free(*tx_bytes_out)`.
 
 **Safety:**
-- `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_rate` must be a valid variant of FFIFeeRate - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - The returned transaction bytes must be freed with `transaction_bytes_free`
+- `manager` must be a valid pointer to an FFIWalletManager - `wallet` must be a valid pointer to an FFIWallet - `account_index` must be a valid BIP44 account index present in the wallet - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements - `fee_per_kb` is the requested fee rate in duffs per kilobyte - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the calculated transaction fee in duffs - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer - `tx_len_out` must be a valid pointer to store the transaction length - `error` must be a valid pointer to an FFIError - On success, the returned transaction bytes must be freed by calling `transaction_bytes_free(*tx_bytes_out)`.
 
 **Module:** `transaction`
 
@@ -3579,10 +3579,10 @@ transaction_bytes_free(tx_bytes: *mut u8) -> ()
 ```
 
 **Description:**
-Free transaction bytes  # Safety  - `tx_bytes` must be a valid pointer created by transaction functions or null - After calling this function, the pointer becomes invalid
+Free transaction bytes returned by transaction-building FFI functions.  The returned pointer carries a small hidden length prefix so the C ABI can keep the historical one-argument free function while still reconstructing the original boxed slice layout correctly.  # Safety  - `tx_bytes` must either be null, or a pointer previously returned by a `wallet_build_and_sign_*` FFI function and not yet freed. - After calling this function, the pointer becomes invalid and must not be used again.
 
 **Safety:**
-- `tx_bytes` must be a valid pointer created by transaction functions or null - After calling this function, the pointer becomes invalid
+- `tx_bytes` must either be null, or a pointer previously returned by a `wallet_build_and_sign_*` FFI function and not yet freed. - After calling this function, the pointer becomes invalid and must not be used again.
 
 **Module:** `transaction`
 

key-wallet-ffi/src/transaction.rs

@@ -75,13 +75,14 @@ pub struct FFITxOutput {
 /// - `wallet` must be a valid pointer to an FFIWallet
 /// - `account_index` must be a valid BIP44 account index present in the wallet
 /// - `outputs` must be a valid pointer to an array of FFITxOutput with at least `outputs_count` elements
-/// - `fee_rate` must be a valid variant of FFIFeeRate
+/// - `fee_per_kb` is the requested fee rate in duffs per kilobyte
 /// - `fee_out` must be a valid, non-null pointer to a `u64`; on success it receives the
 ///   calculated transaction fee in duffs
 /// - `tx_bytes_out` must be a valid pointer to store the transaction bytes pointer
 /// - `tx_len_out` must be a valid pointer to store the transaction length
 /// - `error` must be a valid pointer to an FFIError
-/// - The returned transaction bytes must be freed with `transaction_bytes_free`
+/// - On success, the returned transaction bytes must be freed by calling
+///   `transaction_bytes_free(*tx_bytes_out)`.
 #[no_mangle]
 pub unsafe extern "C" fn wallet_build_and_sign_transaction(
     manager: *const FFIWalletManager,
@@ -146,10 +147,7 @@ pub unsafe extern "C" fn wallet_build_and_sign_transaction(
 
             // Serialize the transaction
             let serialized = consensus::serialize(&transaction);
-            let size = serialized.len();
-
-            let boxed = serialized.into_boxed_slice();
-            let tx_bytes = Box::into_raw(boxed) as *mut u8;
+            let (tx_bytes, size) = transaction_bytes_into_ffi_buffer(serialized);
 
             *tx_bytes_out = tx_bytes;
             *tx_len_out = size;
@@ -244,18 +242,45 @@ pub unsafe extern "C" fn wallet_check_transaction(
     }
 }
 
-/// Free transaction bytes
+const TRANSACTION_BYTES_LEN_PREFIX_SIZE: usize = std::mem::size_of::<usize>();
+
+fn transaction_bytes_into_ffi_buffer(serialized: Vec<u8>) -> (*mut u8, usize) {
+    let len = serialized.len();
+    let mut allocation = Vec::with_capacity(TRANSACTION_BYTES_LEN_PREFIX_SIZE + len);
+    allocation.resize(TRANSACTION_BYTES_LEN_PREFIX_SIZE, 0);
+    unsafe {
+        ptr::write_unaligned(allocation.as_mut_ptr() as *mut usize, len);
+    }
+    allocation.extend_from_slice(&serialized);
+
+    let boxed = allocation.into_boxed_slice();
+    let data_ptr =
+        unsafe { (Box::into_raw(boxed) as *mut u8).add(TRANSACTION_BYTES_LEN_PREFIX_SIZE) };
+    (data_ptr, len)
+}
+
+/// Free transaction bytes returned by transaction-building FFI functions.
+///
+/// The returned pointer carries a small hidden length prefix so the C ABI can
+/// keep the historical one-argument free function while still reconstructing
+/// the original boxed slice layout correctly.
 ///
 /// # Safety
 ///
-/// - `tx_bytes` must be a valid pointer created by transaction functions or null
-/// - After calling this function, the pointer becomes invalid
+/// - `tx_bytes` must either be null, or a pointer previously returned by a
+///   `wallet_build_and_sign_*` FFI function and not yet freed.
+/// - After calling this function, the pointer becomes invalid and must not be
+///   used again.
 #[no_mangle]
 pub unsafe extern "C" fn transaction_bytes_free(tx_bytes: *mut u8) {
-    if !tx_bytes.is_null() {
-        unsafe {
-            let _ = Box::from_raw(tx_bytes);
-        }
+    if tx_bytes.is_null() {
+        return;
+    }
+    unsafe {
+        let allocation_ptr = tx_bytes.sub(TRANSACTION_BYTES_LEN_PREFIX_SIZE);
+        let tx_len = ptr::read_unaligned(allocation_ptr as *const usize);
+        let allocation_len = TRANSACTION_BYTES_LEN_PREFIX_SIZE + tx_len;
+        let _ = Box::from_raw(ptr::slice_from_raw_parts_mut(allocation_ptr, allocation_len));
     }
 }
 
@@ -761,7 +786,8 @@ impl From<FFIAssetLockFundingType> for AssetLockFundingType {
 /// - All pointer parameters must be valid and non-null
 /// - All parallel arrays must have at least `credit_outputs_count` elements
 /// - `private_keys_out` must point to an array of `credit_outputs_count` × `[u8; 32]` buffers
-/// - Caller must free `tx_bytes_out` with `transaction_bytes_free`
+/// - On success, the caller must free the returned transaction bytes by calling
+///   `transaction_bytes_free(*tx_bytes_out)`.
 #[no_mangle]
 pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction(
     manager: *const FFIWalletManager,
@@ -861,13 +887,16 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction(
             }
 
             let serialized = consensus::serialize(&result.transaction);
-            let size = serialized.len();
-            let boxed = serialized.into_boxed_slice();
-            *tx_bytes_out = Box::into_raw(boxed) as *mut u8;
+            let (tx_bytes, size) = transaction_bytes_into_ffi_buffer(serialized);
+            *tx_bytes_out = tx_bytes;
             *tx_len_out = size;
 
             (*error).clean();
             true
         })
     }
 }
+
+#[cfg(test)]
+#[path = "transaction_tests.rs"]
+mod tests;

key-wallet-ffi/src/transaction_tests.rs

@@ -0,0 +1,43 @@
+#[cfg(test)]
+mod transaction_tests {
+    use super::super::{transaction_bytes_free, transaction_bytes_into_ffi_buffer};
+    use std::ptr;
+
+    /// Freeing a null pointer must be a safe no-op.
+    #[test]
+    fn transaction_bytes_free_null_is_noop() {
+        unsafe {
+            transaction_bytes_free(ptr::null_mut());
+        }
+    }
+
+    /// Freeing a pointer produced by the FFI transaction buffer helper must
+    /// recover the hidden length prefix and drop the original boxed slice.
+    #[test]
+    fn transaction_bytes_free_releases_prefixed_buffer() {
+        let payload: Vec<u8> = (0u8..64).collect();
+        let expected_len = payload.len();
+        let (raw, len) = transaction_bytes_into_ffi_buffer(payload);
+        assert_eq!(len, expected_len);
+        assert!(!raw.is_null());
+
+        unsafe {
+            transaction_bytes_free(raw);
+        }
+        // Reaching this line without aborting is the success criterion;
+        // double-free or layout mismatches would have tripped the allocator.
+    }
+
+    /// A zero-length transaction payload still has a hidden prefix allocation
+    /// and must be freeable through the same ABI-stable function.
+    #[test]
+    fn transaction_bytes_free_handles_empty_buffer() {
+        let (raw, len) = transaction_bytes_into_ffi_buffer(Vec::new());
+        assert_eq!(len, 0);
+        assert!(!raw.is_null());
+
+        unsafe {
+            transaction_bytes_free(raw);
+        }
+    }
+}