refactor: change DerivedAddress::public_key to PublicKey

[xdustinface]

Replace the raw [u8; 33] field with dashcore::PublicKey so the event payload carries a validated curve point instead of opaque bytes. The projection in DerivedAddress::from_info now parses via PublicKey::from_slice and logs a warn on failure, matching the existing skip-on-bad-input pattern.

The FFI surface in dash-spv-ffi keeps its [u8; 33] field for ABI stability and serializes the parsed key back to compressed bytes at the boundary.

Summary by CodeRabbit

• Bug Fixes

  • Improved public key validation during address derivation; invalid keys are now gracefully skipped instead of causing failures.

• Refactor

  • Updated internal address derivation logic for more robust public key serialization and handling.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request updates DerivedAddress to carry a dashcore::PublicKey object instead of a raw 33-byte array. Event projection logic now parses ECDSA public key bytes into structured PublicKey objects with validation, FFI serialization adapts to the new type, and all tests are refactored to construct and verify fixtures using the updated type.

Changes

DerivedAddress Public Key Type Upgrade

Layer / File(s)	Summary
DerivedAddress public key type definition key-wallet-manager/src/events.rs	DerivedAddress struct field changes from [u8; 33] to dashcore::PublicKey; documentation updated to reflect ECDSA public key payload and non-ECDSA pool handling.
Event projection with public key parsing key-wallet-manager/src/events.rs	DerivedAddress::from_info now parses ECDSA public key bytes via PublicKey::from_slice, logging warnings on failure and dropping invalid entries; replaces prior length-specific handling.
FFI public key serialization dash-spv-ffi/src/callbacks.rs	FFIDerivedAddress::from_slice updates to call d.public_key.inner.serialize() for proper 33-byte FFI representation instead of direct copy.
Test fixtures, imports, and verification scenarios key-wallet-manager/src/event_tests.rs, key-wallet-manager/src/events.rs	Test helpers refactored with alt_key boolean and dual on-curve pubkey constants; dedup and distinct indices assertions updated to compare PublicKey objects; invalid pubkey coverage extended; event_tests imports consolidated.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 A public key dressed in structured form,
No longer raw bytes in an old-fashioned norm!
Parse and validate with dashcore's might,
FFI and tests all shining bright.
One type to rule them, one type to bind!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the main change: converting DerivedAddress::public_key from a raw byte array to a PublicKey type.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ 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/derived-address-pubkey

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

• Generate code and open pull requests
• Plan features and break down work
• Investigate incidents and troubleshoot customer tickets together
• Automate recurring tasks and respond to alerts with triggers
• Summarize progress and report instantly

Built for teams:

• Shared memory across your entire org—no repeating context
• Per-thread sandboxes to safely plan and execute work
• Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started

---

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]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

key-wallet-manager/src/events.rs (1)

127-133: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Insert dedup keys only after successful projection.

With parse failures now possible in DerivedAddress::from_info, marking a key as seen before projection can drop a later valid duplicate for the same (account_type, pool_type, derivation_index). That can suppress a valid DerivedAddress emission.

Suggested fix

     for info in infos {
         let key = (info.account_type, info.pool_type, info.info.index);
-        if !seen.insert(key) {
+        if seen.contains(&key) {
             continue;
         }
         if let Some(d) = DerivedAddress::from_info(info) {
+            seen.insert(key);
             out.push(d);
         }
     }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@key-wallet-manager/src/events.rs` around lines 127 - 133, In the loop over
infos in events.rs, don't mark the dedup key as seen before attempting
projection: change the logic so you first call DerivedAddress::from_info(info)
and only if it returns Some(d) check/insert the key into seen (e.g., check
seen.contains(&key) then insert(&key) after successful projection) before
pushing d into out; this ensures parse failures in DerivedAddress::from_info
won't cause a key to be prematurely consumed and drop later valid DerivedAddress
instances.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@key-wallet-manager/src/events.rs`:
- Around line 127-133: In the loop over infos in events.rs, don't mark the dedup
key as seen before attempting projection: change the logic so you first call
DerivedAddress::from_info(info) and only if it returns Some(d) check/insert the
key into seen (e.g., check seen.contains(&key) then insert(&key) after
successful projection) before pushing d into out; this ensures parse failures in
DerivedAddress::from_info won't cause a key to be prematurely consumed and drop
later valid DerivedAddress instances.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 24a52284-98ad-4c15-90a2-9bad74743995

📥 Commits

Reviewing files that changed from the base of the PR and between 5297d61 and 9cafe32.

📒 Files selected for processing (3)

• dash-spv-ffi/src/callbacks.rs
• key-wallet-manager/src/event_tests.rs
• key-wallet-manager/src/events.rs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.28%. Comparing base (5297d61) to head (9cafe32).

Additional details and impacted files

@@              Coverage Diff              @@
##           v0.42-dev     #765      +/-   ##
=============================================
+ Coverage      72.26%   72.28%   +0.01%     
=============================================
  Files            320      320              
  Lines          70275    70271       -4     
=============================================
+ Hits           50785    50793       +8     
+ Misses         19490    19478      -12     

Flag	Coverage Δ
core	76.30% <ø> (ø)
ffi	46.10% <100.00%> (+0.01%)	⬆️
rpc	20.00% <ø> (ø)
spv	89.80% <ø> (+0.04%)	⬆️
wallet	71.27% <100.00%> (+0.01%)	⬆️

Files with missing lines	Coverage Δ
dash-spv-ffi/src/callbacks.rs	74.36% <100.00%> (+0.21%)	⬆️
key-wallet-manager/src/events.rs	68.81% <100.00%> (+1.33%)	⬆️

... and 3 files with indirect coverage changes