Skip to content

feat(key-wallet): add Signer::extended_public_key#817

Open
shumkov wants to merge 1 commit into
devfrom
feat/signer-extended-public-key
Open

feat(key-wallet): add Signer::extended_public_key#817
shumkov wants to merge 1 commit into
devfrom
feat/signer-extended-public-key

Conversation

@shumkov

@shumkov shumkov commented Jun 22, 2026

Copy link
Copy Markdown
Collaborator

What

Adds a required extended_public_key(path) -> ExtendedPubKey method to the key_wallet::signer::Signer trait.

Why

The trait exposes sign_ecdsa + public_key, the latter returning only the compressed leaf point. A caller that needs a BIP-32 extended public key at a hardened path — e.g. DashPay contact-xpub derivation under m/9'/coin'/15'/account'/sender/recipient — could not obtain the chain code without the wallet's resident root private key, forcing the seed to be made resident.

This lets a Keychain/hardware-backed signer return the point + chain code + parent fingerprint at a (possibly hardened) path, so the caller can non-hardened-derive descendants offline with no resident seed. It unblocks the dashpay-platform work to delete the attach_wallet_seed workaround (the wallet stays external-signable; the signer answers xpub requests).

Details

  • A signer that cannot export an xpub at a hardened path returns an error rather than panicking.
  • The two in-repo InMemorySigner test signers derive it from their root xprv (ExtendedPubKey::from_priv); the NoDigestSigner test stubs are unreachable!().
  • Pinned by a test asserting the signer's xpub at a hardened path equals the wallet's own derive_extended_public_key.
  • Required, not defaulted: the trait's associated Error type is opaque, so a default body cannot construct a Self::Error to return; there are no production Signer impls in this crate.

Test

cargo test -p key-wallet --lib — 497 passed, 0 failed. fmt + clippy clean.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Wallet now supports extended public key derivation and export capabilities, enabling advanced key management scenarios, multi-signature workflows, and improved cross-wallet interoperability.
  • Tests

    • Comprehensive tests added to verify extended public key derivation functionality works correctly across various wallet operations and scenarios.

The Signer trait exposes sign_ecdsa + public_key, the latter returning only
the compressed leaf point. A caller that needs a BIP-32 extended public key
at a hardened path — e.g. DashPay contact-xpub derivation under
m/9'/coin'/15'/account'/sender/recipient — could not obtain the chain code
without the wallet's resident root private key, forcing the seed to be made
resident.

Add a required extended_public_key(path) -> ExtendedPubKey method so a
Keychain/hardware-backed signer can return the point + chain code + parent
fingerprint at a (possibly hardened) path, letting the caller
non-hardened-derive descendants offline with no resident seed. A signer
that cannot export an xpub at a hardened path returns an error rather than
panicking.

The two in-repo InMemorySigner test signers derive it from their root xprv
(ExtendedPubKey::from_priv); the NoDigestSigner test stubs are unreachable.
Pinned by a test asserting the signer's xpub at a hardened path equals the
wallet's own derive_extended_public_key.

Required (not defaulted): the trait's associated Error type is opaque, so a
default body cannot construct a Self::Error to return; there are no
production Signer impls in this crate.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4f6a4c8d-1359-4a16-9555-020ebb3929e0

📥 Commits

Reviewing files that changed from the base of the PR and between ceee4a9 and b4779fc.

📒 Files selected for processing (3)
  • key-wallet/src/signer.rs
  • key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs
  • key-wallet/src/wallet/managed_wallet_info/transaction_building.rs

📝 Walkthrough

Walkthrough

Adds a new async extended_public_key method to the Signer trait returning a BIP-32 ExtendedPubKey (with chain code and parent fingerprint). The method is implemented in test InMemorySigner stubs in two files via private-key derivation, and NoDigestSigner stubs are updated with unreachable/panic guards. A new Tokio test verifies the signer's output matches the wallet's own derivation.

Changes

Signer::extended_public_key addition

Layer / File(s) Summary
Signer trait: import and method declaration
key-wallet/src/signer.rs
Adds ExtendedPubKey to imports and declares the async extended_public_key(&self, path: &DerivationPath) -> Result<ExtendedPubKey, Self::Error> method on the Signer trait, with docs covering BIP-32 semantics and hardened-path error requirements.
InMemorySigner and NoDigestSigner implementations + test
key-wallet/src/wallet/managed_wallet_info/asset_lock_builder.rs, key-wallet/src/wallet/managed_wallet_info/transaction_building.rs
Implements extended_public_key on InMemorySigner in both files by deriving an xpriv at the given path and returning the corresponding ExtendedPubKey. Adds unreachable/panic stubs on NoDigestSigner in both files. Adds a Tokio test asserting the signer's output for a hardened path equals wallet.derive_extended_public_key.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐇 A new key sprouts from the BIP-32 tree,
Chain codes and fingerprints, neat as can be.
The signer now stretches with paths long and deep,
Hardened paths graceful—no panics to weep.
Hop, hop, the wallet and signer agree! 🌿

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 23.08% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(key-wallet): add Signer::extended_public_key' clearly and specifically summarizes the main change—adding a new method to the Signer trait.
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 feat/signer-extended-public-key

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.

@codecov

codecov Bot commented Jun 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.00000% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.03%. Comparing base (ceee4a9) to head (b4779fc).

Files with missing lines Patch % Lines
...c/wallet/managed_wallet_info/asset_lock_builder.rs 0.00% 5 Missing ⚠️
...wallet/managed_wallet_info/transaction_building.rs 85.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              dev     #817      +/-   ##
==========================================
+ Coverage   72.82%   73.03%   +0.21%     
==========================================
  Files         323      323              
  Lines       72000    72025      +25     
==========================================
+ Hits        52432    52602     +170     
+ Misses      19568    19423     -145     
Flag Coverage Δ
core 76.74% <ø> (ø)
ffi 47.35% <ø> (+1.34%) ⬆️
rpc 20.00% <ø> (ø)
spv 90.29% <ø> (+0.11%) ⬆️
wallet 71.83% <68.00%> (+0.03%) ⬆️
Files with missing lines Coverage Δ
key-wallet/src/signer.rs 100.00% <ø> (ø)
...wallet/managed_wallet_info/transaction_building.rs 88.41% <85.00%> (-0.19%) ⬇️
...c/wallet/managed_wallet_info/asset_lock_builder.rs 81.64% <0.00%> (-1.14%) ⬇️

... and 23 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant