Skip to content

refactor: use String for TransactionRecord::label #624

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
xdustinface merged 2 commits into from
Apr 6, 2026
Merged

refactor: use String for TransactionRecord::label #624

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
7c50283
refactor: use `String` for `TransactionRecord::label`
xdustinface Apr 2, 2026
993b6ac
refactor: remove unnecessary `Option` wrapper for `c_label`
xdustinface Apr 6, 2026
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
12 changes: 7 additions & 5 deletions dash-spv-ffi/src/callbacks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -730,8 +730,12 @@ impl FFIWalletEventCallbacks {
})
.collect();

let c_label =
record.label.as_ref().map(|l| CString::new(l.as_str()).unwrap_or_default());
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 {
c_label.as_ptr() as *mut _
};

@ZocoLini ZocoLini Apr 3, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

line 764 we are mapping empty (or None after this step) to a null pointer and Some to a not null ptr, we can skip this mapping to an Option enum:

This is line 764

              label: c_label
                            .as_ref()
                            .map_or(std::ptr::null_mut(), |l| l.as_ptr() as *mut _),


let ffi_record = FFITransactionRecord {
txid: record.txid.to_byte_array(),
Expand All @@ -758,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(
Expand Down
7 changes: 4 additions & 3 deletions key-wallet-ffi/src/managed_account.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};
Comment thread
xdustinface marked this conversation as resolved.

// Output details
Expand Down
23 changes: 9 additions & 14 deletions key-wallet/src/managed_account/transaction_record.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub struct TransactionRecord {
/// Fee paid (if we created it)
pub fee: Option<u64>,
/// Transaction label
pub label: Option<String>,
pub label: String,
}

impl TransactionRecord {
Expand All @@ -115,7 +115,7 @@ impl TransactionRecord {
output_details,
net_amount,
fee: None,
label: None,
label: String::new(),
}
}

Expand Down Expand Up @@ -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;
Comment thread
xdustinface marked this conversation as resolved.
Ok(())
}

Expand Down Expand Up @@ -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);
}
}
Loading