refactor: use String for TransactionRecord::label

[xdustinface]

Instead of Option<String>. Empty string represents "no label".

Summary by CodeRabbit

• Refactor
  • Transaction label handling made consistent across layers.
  • Clearing a label now results in an empty label (""), not an unset value; this affects how labels display and persist.
  • Empty labels are treated as absent when communicating with external interfaces.

[coderabbitai]

📝 Walkthrough

Walkthrough

Refactors label handling: TransactionRecord.label changed from Option<String> to String; FFI layers now treat empty labels as unset and marshal empty strings to NULL for C consumers.

Changes

Cohort / File(s)	Summary
TransactionRecord model key-wallet/src/managed_account/transaction_record.rs	Changed pub label: Option<String> → pub label: String; new initializes empty string; set_label assigns directly and validates length; tests updated to expect empty string for unset.
FFI marshaling / callbacks dash-spv-ffi/src/callbacks.rs	Reworked label CString creation and pointer assignment: always create CString from record.label.as_str() and set label pointer to NULL when record.label.is_empty(); removed previous Option-based mapping.
FFI managed account exports key-wallet-ffi/src/managed_account.rs	Updated managed_core_account_get_transactions to treat empty record.label as unset (return NULL), creating CString only for non-empty labels; preserved fallback on CString::new failure.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
A label hopped from Some to plain String's field,
Empty now quiet where Option once kneeled.
Marshals whisper NULL when the string is bare,
C and Rust nod, tidy and fair,
I nibble carrots and cheer the new deal.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: replacing Option<String> with String for TransactionRecord::label, which is the primary structural refactoring across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch refactor/transaction-label

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[codecov]

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 67.66%. Comparing base (253c758) to head (993b6ac).
⚠️ Report is 5 commits behind head on v0.42-dev.

Files with missing lines	Patch %	Lines
key-wallet-ffi/src/managed_account.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@              Coverage Diff              @@
##           v0.42-dev     #624      +/-   ##
=============================================
+ Coverage      67.62%   67.66%   +0.03%     
=============================================
  Files            321      318       -3     
  Lines          68253    67826     -427     
=============================================
- Hits           46159    45896     -263     
+ Misses         22094    21930     -164     

Flag	Coverage Δ
core	75.21% <ø> (ø)
ffi	38.28% <87.50%> (+0.28%)	⬆️
rpc	19.92% <ø> (ø)
spv	84.67% <ø> (+0.72%)	⬆️
wallet	67.56% <100.00%> (+0.05%)	⬆️

Files with missing lines	Coverage Δ
dash-spv-ffi/src/callbacks.rs	73.79% <100.00%> (-8.47%)	⬇️
...y-wallet/src/managed_account/transaction_record.rs	100.00% <100.00%> (ø)
key-wallet-ffi/src/managed_account.rs	54.85% <66.66%> (+2.23%)	⬆️

... and 42 files with indirect coverage changes

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@dash-spv-ffi/src/callbacks.rs`:
- Around line 733-737: The code builds c_label from record.label using
CString::new(...).unwrap_or_default(), which silently masks interior NUL errors;
replace that with a fallible conversion so failures become None: call
CString::new(record.label.as_str()).ok() and map that into the Option<CString>
(i.e., use Some(...) only when ok() yields Some), removing unwrap_or_default()
and any unwrap/expect usage around CString::new to properly propagate conversion
failure for c_label and preserve safety in marshaling.

In `@key-wallet-ffi/src/managed_account.rs`:
- Around line 781-785: The code currently uses unwrap_or_default() when
converting record.label to a CString, which silently coerces invalid input
(e.g., interior NULs) into an empty string; change the conversion for
ffi_record.label to use
CString::new(record.label.as_str()).ok().map_or(std::ptr::null_mut(), |c|
c.into_raw()) so invalid labels return null instead of being treated as empty,
and apply the same pattern to the address conversion noted (replace its
unwrap_or_default path with ok().map_or(std::ptr::null_mut(), |c|
c.into_raw())); this preserves the FFI contract and matches the empty-label
handling.

In `@key-wallet/src/managed_account/transaction_record.rs`:
- Around line 158-165: The set_label method currently only checks length and
allows strings containing NUL ('\0') bytes which later get lost when converted
for FFI; update the set_label(&mut self, label: String) -> Result<(), Error> in
transaction_record.rs to also scan the input for any NUL bytes (e.g.,
label.contains('\0')) and return Err(Error::InvalidParameter("Label contains NUL
byte".to_string())) if found, before assigning self.label = label; keep the
existing MAX_LABEL_LENGTH check and error behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 72fb5c48-8dea-4bb7-828b-708a19242d49

📥 Commits

Reviewing files that changed from the base of the PR and between 253c758 and 7c50283.

📒 Files selected for processing (3)

• dash-spv-ffi/src/callbacks.rs
• key-wallet-ffi/src/managed_account.rs
• key-wallet/src/managed_account/transaction_record.rs

[ZocoLini]

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 _),

[ZocoLini]

I missclicked, I dint mean to approve xd

[coderabbitai]

♻️ Duplicate comments (1)

dash-spv-ffi/src/callbacks.rs (1)

733-738: ⚠️ Potential issue | 🟠 Major

Stop masking CString::new failures for label.

unwrap_or_default() hides interior-NUL conversion errors and may pass a non-null empty C string for an invalid non-empty label. Use a fallible conversion and map failures to NULL explicitly.

Suggested fix

-                    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 _
-                    };
+                    let c_label = if record.label.is_empty() {
+                        None
+                    } else {
+                        CString::new(record.label.as_str()).ok()
+                    };
+                    let c_label_ptr = c_label
+                        .as_ref()
+                        .map_or(std::ptr::null_mut(), |label| label.as_ptr() as *mut _);

As per coding guidelines, "**/*.rs: Use proper error types with thiserror crate and propagate errors appropriately."

Also applies to: 765-765

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@dash-spv-ffi/src/callbacks.rs` around lines 733 - 738, The code is masking
CString::new failures for record.label by using unwrap_or_default; instead
perform a fallible conversion and set c_label_ptr to NULL on error or empty
input: call CString::new(record.label.as_str()) and match the Result, treating
Err(_) the same as an empty label by using std::ptr::null_mut() for c_label_ptr;
ensure you don’t produce a pointer to an empty internal CString created by
unwrap_or_default and apply the same fix to the other occurrence handling
record.label (the one using c_label and c_label_ptr).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@dash-spv-ffi/src/callbacks.rs`:
- Around line 733-738: The code is masking CString::new failures for
record.label by using unwrap_or_default; instead perform a fallible conversion
and set c_label_ptr to NULL on error or empty input: call
CString::new(record.label.as_str()) and match the Result, treating Err(_) the
same as an empty label by using std::ptr::null_mut() for c_label_ptr; ensure you
don’t produce a pointer to an empty internal CString created by
unwrap_or_default and apply the same fix to the other occurrence handling
record.label (the one using c_label and c_label_ptr).

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 13b1e164-6a86-4f3b-a411-a03592e3375b

📥 Commits

Reviewing files that changed from the base of the PR and between 7c50283 and 993b6ac.

📒 Files selected for processing (1)

• dash-spv-ffi/src/callbacks.rs