diff --git a/key-wallet-ffi/FFI_API.md b/key-wallet-ffi/FFI_API.md index 9b9885165..d81cdc7c3 100644 --- a/key-wallet-ffi/FFI_API.md +++ b/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` diff --git a/key-wallet-ffi/src/transaction.rs b/key-wallet-ffi/src/transaction.rs index 2f38b742e..09d935aea 100644 --- a/key-wallet-ffi/src/transaction.rs +++ b/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::(); + +fn transaction_bytes_into_ffi_buffer(serialized: Vec) -> (*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 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,9 +887,8 @@ 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(); @@ -871,3 +896,7 @@ pub unsafe extern "C" fn wallet_build_and_sign_asset_lock_transaction( }) } } + +#[cfg(test)] +#[path = "transaction_tests.rs"] +mod tests; diff --git a/key-wallet-ffi/src/transaction_tests.rs b/key-wallet-ffi/src/transaction_tests.rs new file mode 100644 index 000000000..55b184d62 --- /dev/null +++ b/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 = (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); + } + } +}