feat: validate transaction label size

[xdustinface]

Just to avoid having huge data stored in the labels and i think 256 chars should be more than enough for a transaction label.

Summary by CodeRabbit

• Bug Fixes
  • Improved transaction label handling with input validation. Labels are now limited to a maximum of 256 characters, preventing invalid or oversized entries. Empty labels clear existing values, while invalid entries are properly rejected with appropriate error messages.

[coderabbitai]

📝 Walkthrough

Walkthrough

The TransactionRecord::set_label method was refactored to return Result<(), Error> instead of unit type, adding validation logic that rejects labels exceeding a 256-byte maximum, supports clearing labels with empty strings, and updates tests accordingly.

Changes

Cohort / File(s)	Summary
Transaction Record Validation key-wallet/src/managed_account/transaction_record.rs	Added MAX_LABEL_LENGTH constant (256 bytes), refactored set_label to return Result<(), Error> with validation: empty labels clear the field, oversized labels return an error, and valid labels are stored. Tests updated to handle the new return type and cover edge cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A label validator, strong and true,
Checks every string that passes through,
256 bytes is the limit we set,
Clear and precise, no regret!
With Results and Errors, all errors we catch, ✨

🚥 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 'feat: validate transaction label size' directly and clearly summarizes the main change: introducing validation for transaction label sizes with a maximum length constraint.
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 feat/transaction-label-size

---

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.}

[coderabbitai]

🧹 Nitpick comments (1)

key-wallet/src/managed_account/transaction_record.rs (1)

344-348: Consider testing that a valid label is preserved when an invalid one is rejected.

The current test verifies that label remains None after the error, but the label was already cleared at line 341. To fully exercise the atomic behavior (existing label unchanged on error), consider setting a valid label first, then attempting to set an over-limit one.

🧪 Suggested test improvement

         // Exceeding max length returns an error
+        record.set_label("Preserved Label".to_string()).unwrap();
         let long_label = "x".repeat(MAX_LABEL_LENGTH + 1);
         assert!(record.set_label(long_label).is_err());
-        assert_eq!(record.label, None); // unchanged
+        assert_eq!(record.label, Some("Preserved Label".to_string())); // unchanged from valid label
 
         // Exactly max length is fine

🤖 Prompt for AI Agents

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

In `@key-wallet/src/managed_account/transaction_record.rs` around lines 344 - 348,
Update the test to verify atomic behavior of set_label: first set a valid label
via record.set_label(valid_label) and assert it returns Ok and record.label
equals that valid value, then attempt to set the over-length label using
record.set_label(long_label) and assert it returns Err and that record.label is
still the original valid label (unchanged). Use the existing MAX_LABEL_LENGTH,
record.set_label, and record.label symbols to locate and modify the test.

🤖 Prompt for all review comments with AI agents

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

Nitpick comments:
In `@key-wallet/src/managed_account/transaction_record.rs`:
- Around line 344-348: Update the test to verify atomic behavior of set_label:
first set a valid label via record.set_label(valid_label) and assert it returns
Ok and record.label equals that valid value, then attempt to set the over-length
label using record.set_label(long_label) and assert it returns Err and that
record.label is still the original valid label (unchanged). Use the existing
MAX_LABEL_LENGTH, record.set_label, and record.label symbols to locate and
modify the test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: bfc39b86-d0ff-454c-97a5-8a5827781cd9

📥 Commits

Reviewing files that changed from the base of the PR and between 734b35f and d0add65.

📒 Files selected for processing (1)

• key-wallet/src/managed_account/transaction_record.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.58%. Comparing base (734b35f) to head (d0add65).
⚠️ Report is 5 commits behind head on v0.42-dev.

Additional details and impacted files

@@              Coverage Diff              @@
##           v0.42-dev     #617      +/-   ##
=============================================
+ Coverage      67.21%   67.58%   +0.37%     
=============================================
  Files            321      321              
  Lines          68249    68268      +19     
=============================================
+ Hits           45874    46142     +268     
+ Misses         22375    22126     -249     

Flag	Coverage Δ
core	75.21% <ø> (ø)
ffi	38.04% <ø> (+2.44%)	⬆️
rpc	19.92% <ø> (ø)
spv	83.72% <ø> (-0.04%)	⬇️
wallet	67.48% <100.00%> (+0.03%)	⬆️

Files with missing lines	Coverage Δ
...y-wallet/src/managed_account/transaction_record.rs	100.00% <100.00%> (ø)

... and 19 files with indirect coverage changes