Skip to content

refactor(key-wallet-ffi): remove and replaced FFIExtendedPublicKey with FFIExtendedPubKey#646

Merged
xdustinface merged 1 commit intov0.42-devfrom
refactor/remove-FFIExtendedPubKey
Apr 14, 2026
Merged

refactor(key-wallet-ffi): remove and replaced FFIExtendedPublicKey with FFIExtendedPubKey#646
xdustinface merged 1 commit intov0.42-devfrom
refactor/remove-FFIExtendedPubKey

Conversation

@ZocoLini
Copy link
Copy Markdown
Collaborator

@ZocoLini ZocoLini commented Apr 13, 2026

Same issue I am fixing in PR #645 but with a different structure

Summary by CodeRabbit

  • Refactor
    • Internal API improvements and standardization of type naming conventions to enhance code maintainability.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 13, 2026

📝 Walkthrough

Walkthrough

The changes rename the opaque FFI type FFIExtendedPublicKey to FFIExtendedPubKey consistently across documentation, build configuration, and implementation files. A new impl block provides accessor methods inner() and from_inner() to standardize access patterns to the underlying key data.

Changes

Cohort / File(s) Summary
Documentation & Configuration
key-wallet-ffi/FFI_API.md, key-wallet-ffi/cbindgen.toml
Updated opaque type references from FFIExtendedPublicKey to FFIExtendedPubKey in documented C ABI function signatures and Swift export configuration.
FFI Bindings Implementation
key-wallet-ffi/src/keys.rs
Renamed struct from FFIExtendedPublicKey to FFIExtendedPubKey across all function signatures (extended_public_key_free, extended_public_key_to_string, extended_public_key_get_public_key, wallet_derive_extended_public_key). Added impl block with inner() and from_inner() accessor methods for consistent key access.
Derivation Module
key-wallet-ffi/src/derivation.rs
Removed local struct definition and updated callers to use FFIExtendedPubKey::from_inner() constructor and inner() accessor method instead of direct field access.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 A hop, a skip, a type rename fair,
From PublicKey long to PubKey spare,
With inner() and from_inner() in place,
The FFI types now embrace,
Consistency swift through the codebase race! 🚀

🚥 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 refactoring change: replacing FFIExtendedPublicKey with FFIExtendedPubKey across multiple 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/remove-FFIExtendedPubKey

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

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

@ZocoLini ZocoLini requested a review from xdustinface April 13, 2026 20:56
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 13, 2026

Codecov Report

❌ Patch coverage is 56.25000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.99%. Comparing base (2089aaa) to head (10a7c7d).
⚠️ Report is 2 commits behind head on v0.42-dev.

Files with missing lines Patch % Lines
key-wallet-ffi/src/keys.rs 61.53% 5 Missing ⚠️
key-wallet-ffi/src/derivation.rs 33.33% 2 Missing ⚠️
Additional details and impacted files
@@              Coverage Diff              @@
##           v0.42-dev     #646      +/-   ##
=============================================
- Coverage      68.08%   67.99%   -0.09%     
=============================================
  Files            318      318              
  Lines          67800    67806       +6     
=============================================
- Hits           46164    46108      -56     
- Misses         21636    21698      +62     
Flag Coverage Δ
core 75.21% <ø> (ø)
ffi 39.44% <56.25%> (-0.67%) ⬇️
rpc 20.00% <ø> (ø)
spv 85.41% <ø> (+0.07%) ⬆️
wallet 67.62% <ø> (ø)
Files with missing lines Coverage Δ
key-wallet-ffi/src/derivation.rs 23.79% <33.33%> (-6.56%) ⬇️
key-wallet-ffi/src/keys.rs 14.01% <61.53%> (-1.74%) ⬇️

... and 17 files with indirect coverage changes

@ZocoLini ZocoLini changed the title refactor(key-wallet-ffi); remove and replaced FFIExtendedPublicKey with FFIExtendedPubKey refactor(key-wallet-ffi): remove and replaced FFIExtendedPublicKey with FFIExtendedPubKey Apr 13, 2026
@QuantumExplorer
Copy link
Copy Markdown
Member

@ZocoLini conflicts

@github-actions
Copy link
Copy Markdown

This PR has merge conflicts with the base branch. Please rebase or merge the base branch into your branch to resolve them.

@github-actions github-actions bot added the merge-conflict The PR conflicts with the target branch. label Apr 14, 2026
@ZocoLini ZocoLini force-pushed the refactor/remove-FFIExtendedPubKey branch from f024521 to 10a7c7d Compare April 14, 2026 19:18
@github-actions github-actions bot removed the merge-conflict The PR conflicts with the target branch. label Apr 14, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
key-wallet-ffi/src/keys.rs (1)

641-643: Use the new constructor here as well.

This is the one remaining direct FFIExtendedPubKey struct literal after adding from_inner(), so wrapper construction is still split across call sites.

♻️ Suggested cleanup
-                Box::into_raw(Box::new(FFIExtendedPubKey {
-                    inner: extended_public_key,
-                }))
+                Box::into_raw(Box::new(FFIExtendedPubKey::from_inner(extended_public_key)))
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@key-wallet-ffi/src/keys.rs` around lines 641 - 643, Replace the direct struct
literal construction of FFIExtendedPubKey with the new constructor: instead of
creating FFIExtendedPubKey { inner: extended_public_key } wrap the inner value
using FFIExtendedPubKey::from_inner(extended_public_key) and then
Box::into_raw(Box::new(...)) as before so all call sites uniformly use
from_inner; update the occurrence that currently returns
Box::into_raw(Box::new(FFIExtendedPubKey { inner: extended_public_key })) to use
FFIExtendedPubKey::from_inner.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@key-wallet-ffi/src/keys.rs`:
- Around line 25-27: You renamed the exported opaque handle from
FFIExtendedPublicKey to FFIExtendedPubKey which is a breaking FFI change;
restore source-compatibility by adding a temporary compatibility alias named
FFIExtendedPublicKey that maps to the new struct (FFIExtendedPubKey) or, if you
intend to break the API, update the crate's public docs and CHANGELOG with
explicit migration guidance referencing both FFIExtendedPubKey and the former
FFIExtendedPublicKey so consumers know to update their bindings; apply the same
alias/documentation approach for the other affected symbols mentioned in the
review (lines referenced).

---

Nitpick comments:
In `@key-wallet-ffi/src/keys.rs`:
- Around line 641-643: Replace the direct struct literal construction of
FFIExtendedPubKey with the new constructor: instead of creating
FFIExtendedPubKey { inner: extended_public_key } wrap the inner value using
FFIExtendedPubKey::from_inner(extended_public_key) and then
Box::into_raw(Box::new(...)) as before so all call sites uniformly use
from_inner; update the occurrence that currently returns
Box::into_raw(Box::new(FFIExtendedPubKey { inner: extended_public_key })) to use
FFIExtendedPubKey::from_inner.
🪄 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: 6622f978-c8cb-462b-a3db-ac432ae7b07b

📥 Commits

Reviewing files that changed from the base of the PR and between 2089aaa and 10a7c7d.

📒 Files selected for processing (4)
  • key-wallet-ffi/FFI_API.md
  • key-wallet-ffi/cbindgen.toml
  • key-wallet-ffi/src/derivation.rs
  • key-wallet-ffi/src/keys.rs

Comment thread key-wallet-ffi/src/keys.rs
@github-actions github-actions bot added the ready-for-review CodeRabbit has approved this PR label Apr 14, 2026
@xdustinface xdustinface merged commit a5680cd into v0.42-dev Apr 14, 2026
42 checks passed
@xdustinface xdustinface deleted the refactor/remove-FFIExtendedPubKey branch April 14, 2026 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-review CodeRabbit has approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants