From 7c5028356131370f765374769508dc6a09670b17 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Fri, 3 Apr 2026 09:37:35 +1100 Subject: [PATCH 1/2] refactor: use `String` for `TransactionRecord::label` Instead of `Option`. Empty string represents "no label". --- dash-spv-ffi/src/callbacks.rs | 7 ++++-- key-wallet-ffi/src/managed_account.rs | 7 +++--- .../src/managed_account/transaction_record.rs | 23 ++++++++----------- 3 files changed, 18 insertions(+), 19 deletions(-) diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index 8aefa815f..6e2e97c90 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -730,8 +730,11 @@ impl FFIWalletEventCallbacks { }) .collect(); - let c_label = - record.label.as_ref().map(|l| CString::new(l.as_str()).unwrap_or_default()); + let c_label = if record.label.is_empty() { + None + } else { + Some(CString::new(record.label.as_str()).unwrap_or_default()) + }; let ffi_record = FFITransactionRecord { txid: record.txid.to_byte_array(), diff --git a/key-wallet-ffi/src/managed_account.rs b/key-wallet-ffi/src/managed_account.rs index 495430a4b..0e39c108d 100644 --- a/key-wallet-ffi/src/managed_account.rs +++ b/key-wallet-ffi/src/managed_account.rs @@ -778,9 +778,10 @@ pub unsafe extern "C" fn managed_core_account_get_transactions( }; // Label - ffi_record.label = match &record.label { - Some(label) => std::ffi::CString::new(label.as_str()).unwrap_or_default().into_raw(), - None => std::ptr::null_mut(), + ffi_record.label = if record.label.is_empty() { + std::ptr::null_mut() + } else { + std::ffi::CString::new(record.label.as_str()).unwrap_or_default().into_raw() }; // Output details diff --git a/key-wallet/src/managed_account/transaction_record.rs b/key-wallet/src/managed_account/transaction_record.rs index e67fbf1a5..478c1970e 100644 --- a/key-wallet/src/managed_account/transaction_record.rs +++ b/key-wallet/src/managed_account/transaction_record.rs @@ -90,7 +90,7 @@ pub struct TransactionRecord { /// Fee paid (if we created it) pub fee: Option, /// Transaction label - pub label: Option, + pub label: String, } impl TransactionRecord { @@ -115,7 +115,7 @@ impl TransactionRecord { output_details, net_amount, fee: None, - label: None, + label: String::new(), } } @@ -154,20 +154,15 @@ impl TransactionRecord { /// Set the label for this transaction. /// - /// Empty strings clear the label. Returns an error if the label - /// exceeds [`MAX_LABEL_LENGTH`] bytes. + /// Returns an error if the label exceeds [`MAX_LABEL_LENGTH`] bytes. pub fn set_label(&mut self, label: String) -> Result<(), Error> { - if label.is_empty() { - self.label = None; - return Ok(()); - } if label.len() > MAX_LABEL_LENGTH { return Err(Error::InvalidParameter(format!( "Label exceeds {} bytes", MAX_LABEL_LENGTH ))); } - self.label = Some(label); + self.label = label; Ok(()) } @@ -329,26 +324,26 @@ mod tests { let mut record = simple_record(tx, TransactionContext::Mempool, -50000); assert_eq!(record.fee, None); - assert_eq!(record.label, None); + assert!(record.label.is_empty()); record.set_fee(226); record.set_label("Payment to Bob".to_string()).unwrap(); assert_eq!(record.fee, Some(226)); - assert_eq!(record.label, Some("Payment to Bob".to_string())); + assert_eq!(record.label, "Payment to Bob"); // Empty string clears the label record.set_label(String::new()).unwrap(); - assert_eq!(record.label, None); + assert!(record.label.is_empty()); // Exceeding max length returns an error let long_label = "x".repeat(MAX_LABEL_LENGTH + 1); assert!(record.set_label(long_label).is_err()); - assert_eq!(record.label, None); // unchanged + assert!(record.label.is_empty()); // unchanged // Exactly max length is fine let max_label = "x".repeat(MAX_LABEL_LENGTH); record.set_label(max_label.clone()).unwrap(); - assert_eq!(record.label, Some(max_label)); + assert_eq!(record.label, max_label); } } From 993b6acd4acb8ec92f31e13261d6532bbaf9ae69 Mon Sep 17 00:00:00 2001 From: xdustinface Date: Mon, 6 Apr 2026 20:24:15 +1000 Subject: [PATCH 2/2] refactor: remove unnecessary `Option` wrapper for `c_label` Addresses ZocoLini review comment on PR #624 https://github.com/dashpay/rust-dashcore/pull/624#discussion_r3033307546 --- dash-spv-ffi/src/callbacks.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index 6e2e97c90..4be728f34 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -730,10 +730,11 @@ impl FFIWalletEventCallbacks { }) .collect(); - let c_label = if record.label.is_empty() { - None + let c_label = CString::new(record.label.as_str()).unwrap_or_default(); + let c_label_ptr = if record.label.is_empty() { + std::ptr::null_mut() } else { - Some(CString::new(record.label.as_str()).unwrap_or_default()) + c_label.as_ptr() as *mut _ }; let ffi_record = FFITransactionRecord { @@ -761,9 +762,7 @@ impl FFIWalletEventCallbacks { tx_bytes.as_ptr() as *mut _ }, tx_len: tx_bytes.len(), - label: c_label - .as_ref() - .map_or(std::ptr::null_mut(), |l| l.as_ptr() as *mut _), + label: c_label_ptr, }; cb(