feature: Add Caravan wallet format import/export support #205

vkprogrammer-001 · 2025-04-09T16:01:24Z

Description

This PR implements support for importing and exporting wallets in the Caravan wallet format as requested in issue #5.

Changes

Added CaravanExport struct to handle Caravan wallet format
Implemented methods to export a wallet to Caravan JSON format
Added functionality to import Caravan wallet configs and convert to BDK descriptors
Added documentation with usage examples
Added comprehensive test coverage for all supported multisig types (P2SH, P2WSH, P2SH-P2WSH)

Implementation Details

All code is contained in the wallet/export.rs module alongside the existing FullyNoded export functionality
Support for different address types required by Caravan (P2SH, P2WSH, P2SH-P2WSH)
Handles the mapping between BDK descriptor format and Caravan's JSON configuration format

Testing

The implementation includes tests for:

Exporting BDK wallets to Caravan format
Importing Caravan configurations to BDK wallets
All supported address types (P2SH, P2WSH, P2SH-P2WSH)
Network detection (mainnet/testnet)

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing

Closes #5

coveralls · 2025-04-10T21:03:20Z

Pull Request Test Coverage Report for Build 18456006387

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

For more information on this, see Tracking coverage changes with pull request builds.
To avoid this issue with future PRs, see these Recommended CI Configurations.
For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

353 of 386 (91.45%) changed or added relevant lines in 2 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.1%) to 85.111%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/wallet/export.rs	352	385	91.43%

Totals
Change from base Build 18099939060:	-0.1%
Covered Lines:	7260
Relevant Lines:	8530

💛 - Coveralls

ValuedMammal

I think this will be a good addition to have. After doing a first pass my main concern is the amount of string manipulation that's required, and would prefer to use stronger types where possible, for example Fingerprint, DerivationPath, and Xpub.

src/wallet/export.rs

wallet/src/wallet/export.rs

src/wallet/export.rs

wallet/src/wallet/export.rs

ValuedMammal · 2025-09-03T15:25:42Z

@vkprogrammer-001 Any interest in keeping this PR up to date? If not we can probably find someone to pick it up.

thunderbiscuit · 2025-09-03T15:27:11Z

I had been thinking about this in the context of the multi-keychain wallet PRs and the new KeyRing type. This sort of stuff is a really good fit for the keyring, because it could simply have a new constructor on the type which passes these backup formats (similar to how we think of doing it for bitcoin core) as an argument, and bam you're done. This can be extended to other export formats too on popular wallets.

wallet/src/wallet/export.rs

vkprogrammer-001 · 2025-09-21T13:36:32Z

@vkprogrammer-001 Any interest in keeping this PR up to date? If not we can probably find someone to pick it up.

Yes @ValuedMammal , I am interested in completing this PR. I have addressed all the comments and made the necessary changes. PTAL and let me know if there are any further changes needed.

ValuedMammal

Thank you @vkprogrammer-001

ValuedMammal · 2025-09-21T14:44:40Z

src/wallet/export.rs

+/// ExtendedPublicKey structure for Caravan wallet format
+#[derive(Debug, Clone)]
+pub struct CaravanExtendedPublicKey {


We can still derive Deserialize for this right?

Yes, definitely! Deserialize can still be derived for CaravanExtendedPublicKey. The custom implementation I added ensures proper handling of the "m/" prefix in the JSON format while internally using correct DerivationPath types.

Right but I think rust-bitcoin can already parse a DerivationPath from a string with or without the m/ prefix.

You are right about this. Rust-bitcoin can parse a DerivationPath from a string with or without the m/ prefix. I made the changes.

In that case I think it would be better to derive Deserialize for CaravanExtendedPublicKey rather than manually implementing it. And probably also include a #[serde(rename = "bip32Path")] attribute.

ValuedMammal

I added a few comments.

ValuedMammal · 2025-09-22T00:39:18Z

wallet/src/wallet/export.rs

+                        // Combine with origin path if present
+                        match &xpub_key.origin {
+                            Some((_, origin_path)) => origin_path.extend(&base),
+                            None => base,
+                        }


I had another question about the derivation path. I think we only care about the origin path here. If there are any steps after the xpub, then I would think they'd be appended to the end of the xpub rather than the origin. but I'm not even sure this is supported by caravan.

Are you testing this on a descriptor with more than one step after the xpub (excluding wildcard)? because so far it looks like base_vec is always empty.

To illustrate what I mean you can refer to a similar implementation I did while reviewing this. master...ValuedMammal:bdk_wallet:feat/caravan_export

@vkprogrammer-001 I might have had that wrong. I think it makes sense to combine the paths. Still a test would probably help.

If it's relevant maybe we add another field derivation_path to CaravanExtendedPublicKey for storing the xpub.derivation_path and reserve bip32_path for the origin.

Thanks for the follow-up! I agree, combining the origin and derivation paths matches Caravan’s expectations. I’ve added a test to confirm the path handling works correctly, including cases with multiple derivation steps. For now, I’m keeping a single bip32_path field since Caravan expects the full combined path, but I’ve documented the option to split into origin and derivation paths if needed in the future.

The issue I'm seeing is that this won't effectively "round-trip" back to same descriptors, e.g. descriptor_str -> Wallet -> CaravanExport -> to_descriptors. But I don't think it's a deal breaker for this PR as long as we communicate that the import/export is a 1-way operation, i.e.

Wallet -> CaravanExport

Or

JSON -> CaravanExport -> to_descriptors

You're absolutely right about the round-trip limitation. I understand that the derivation path handling means descriptor_str -> Wallet -> CaravanExport -> to_descriptors won't produce identical descriptors due to how we combine paths for Caravan compatibility.

I agree this is acceptable for the intended use cases - the functionality is designed for one-way operations (either exporting to Caravan or importing from Caravan), not for round-trip preservation. I'll add documentation to clearly communicate this limitation so users understand these are one-way operations.

wallet/src/wallet/export.rs

ValuedMammal · 2025-09-22T00:47:35Z

When this is close to being ready would you mind please dropping the merge commit 9f86668 and just rebase the changes onto current master.

vkprogrammer-001 · 2025-09-27T18:40:25Z

When this is close to being ready would you mind please dropping the merge commit 9f86668 and just rebase the changes onto current master.

@ValuedMammal I checked my branch history and the merge commit 9f86668 is not present, so there’s nothing to drop. My branch is now rebased onto the latest master and ready for review. PTAL

oleonardolima · 2025-10-01T04:28:40Z

Sorry, but it looks like it needs yet another rebase.

vkprogrammer-001 · 2025-10-02T10:33:19Z

Sorry, but it looks like it needs yet another rebase.

I’ve rebased it again. Everything looks good now. PTAL

thunderbiscuit · 2025-10-02T17:04:02Z

This PR has 13 commits, and I think only the first 3 are actually meant to be here. A complete cleanup of the PR would require rebasing (not merging master back into this PR branch) and dropping all unnecessary commits. You should end up with 3 commits that extend e2cf34d (currently latest on master).

@ValuedMammal

…an export This commit addresses @ValuedMammal feedback by replacing extensive string manipulation with proper Bitcoin types throughout the Caravan wallet export functionality, making the code more type-safe and less error-prone. Key Changes: - Type-safe BIP32 handling: Updated CaravanExtendedPublicKey to use bitcoin::bip32::{DerivationPath, Fingerprint, Xpub} instead of strings. Added custom serde implementation that properly handles "m/" prefix for JSON compatibility while using proper types internally. - Enhanced descriptor parsing: Changed extract_xpubs_from_multi() to accept SortedMultiVec<DescriptorPublicKey, Ctx> instead of SortedMultiVec<String, Ctx>, eliminating string parsing errors and making the code more robust. - Enum-based address types: Replaced string-based address type handling with CaravanAddressType enum (P2SH, P2WSH, P2SHWrappedP2WSH) for better type safety and validation. - Proper descriptor construction: Updated to_descriptors() to use Descriptor::new_sh_sortedmulti(), new_wsh_sortedmulti(), and new_sh_wsh_sortedmulti() construction methods instead of string building, ensuring standard format with checksums. - Network type conversion: Replaced string-based network handling with CaravanNetwork enum and From<bitcoin::Network> implementation using NetworkKind pattern. - Comprehensive validation: Enhanced test suite to verify that exported descriptors can create functional BDK wallets with proper address generation, ensuring practical usability.

- Simplified DerivationPath parsing by relying on rust-bitcoin's native support for m/ prefix. - Used wallet.public_descriptor directly to avoid unnecessary descriptor re-parsing. - Clarified and documented logic for combining origin and derivation paths for Caravan compatibility. - Added tests to verify correct handling of complex derivation paths and serialization/deserialization of CaravanExtendedPublicKey. - Ensured all changes are backward compatible and maintain expected Caravan export behavior.

vkprogrammer-001 · 2025-10-04T09:39:55Z

This PR has 13 commits, and I think only the first 3 are actually meant to be here. A complete cleanup of the PR would require rebasing (not merging master back into this PR branch) and dropping all unnecessary commits. You should end up with 3 commits that extend e2cf34d (currently latest on master).

@thunderbiscuit Thanks for the feedback! I've cleaned up the PR by rebasing and keeping only the 3 relevant commits extending from the latest master (e2cf34d). The branch now has a clean history with just the Caravan wallet export functionality. Ready for review. PTAL

ValuedMammal · 2025-10-07T16:30:34Z

ACK 0b67cd1

I left some remaining comments #205 (comment) #205 (comment).

Add documentation explaining that Caravan export/import operations are designed as one-way operations and that full round-trip conversions (Descriptor → Wallet → CaravanExport → to_descriptors) will not produce identical descriptors due to derivation path processing for Caravan compatibility.

vkprogrammer-001 · 2025-10-13T05:32:15Z

Hey @ValuedMammal, I've added documentation about the round-trip limitation we discussed. The note explains that these are intended as one-way operations for Caravan compatibility. PTAL

ValuedMammal · 2025-11-11T15:46:47Z

ACK c624e85

ValuedMammal added this to BDK Wallet Apr 10, 2025

ValuedMammal added the new feature New feature or request label Apr 10, 2025

ValuedMammal assigned vkprogrammer-001 Apr 10, 2025

notmandatory moved this to In Progress in BDK Wallet Apr 21, 2025

ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Aug 5, 2025

ValuedMammal added this to the Wallet 3.0.0 milestone Aug 5, 2025

ValuedMammal reviewed Sep 3, 2025

View reviewed changes

ValuedMammal requested changes Sep 3, 2025

View reviewed changes

src/wallet/export.rs Show resolved Hide resolved

wallet/src/wallet/export.rs Outdated Show resolved Hide resolved

src/wallet/export.rs Show resolved Hide resolved

wallet/src/wallet/export.rs Outdated Show resolved Hide resolved

wallet/src/wallet/export.rs Outdated Show resolved Hide resolved

ValuedMammal reviewed Sep 3, 2025

View reviewed changes

wallet/src/wallet/export.rs Show resolved Hide resolved

wallet/src/wallet/export.rs Outdated Show resolved Hide resolved

ValuedMammal moved this from Needs Review to In Progress in BDK Wallet Sep 11, 2025

vkprogrammer-001 force-pushed the feature/caravan-wallet-export branch from 0b17dbd to e95df66 Compare September 21, 2025 13:21

ValuedMammal reviewed Sep 21, 2025

View reviewed changes

ValuedMammal reviewed Sep 22, 2025

View reviewed changes

vkprogrammer-001 force-pushed the feature/caravan-wallet-export branch 3 times, most recently from 2b654a1 to 7a90425 Compare September 27, 2025 18:31

ValuedMammal moved this from In Progress to Needs Review in BDK Wallet Sep 27, 2025

ValuedMammal self-requested a review September 27, 2025 19:24

vkprogrammer-001 force-pushed the feature/caravan-wallet-export branch from 3de7c59 to 7a90425 Compare October 4, 2025 09:04

vkprogrammer-001 added 2 commits October 4, 2025 15:01

Add Caravan wallet format support

a55a2a8

vkprogrammer-001 force-pushed the feature/caravan-wallet-export branch from 7a90425 to 0b67cd1 Compare October 4, 2025 09:31

ValuedMammal approved these changes Oct 7, 2025

View reviewed changes

This was referenced Nov 6, 2025

Lining Up The Big 3.0 PRs #339

Open

Release tracking wallet-2.3.0 #342

Open

feature: Add Caravan wallet format import/export support #205

Are you sure you want to change the base?

feature: Add Caravan wallet format import/export support #205

Uh oh!

Conversation

vkprogrammer-001 commented Apr 9, 2025

Description

Changes

Implementation Details

Testing

Checklists

All Submissions:

Uh oh!

coveralls commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 18456006387

Warning: This coverage report may be inaccurate.

Details

💛 - Coveralls

Uh oh!

ValuedMammal left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValuedMammal commented Sep 3, 2025

Uh oh!

thunderbiscuit commented Sep 3, 2025

Uh oh!

Uh oh!

Uh oh!

vkprogrammer-001 commented Sep 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ValuedMammal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValuedMammal left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ValuedMammal Sep 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ValuedMammal commented Sep 22, 2025

Uh oh!

vkprogrammer-001 commented Sep 27, 2025

Uh oh!

oleonardolima commented Oct 1, 2025

Uh oh!

vkprogrammer-001 commented Oct 2, 2025

Uh oh!

thunderbiscuit commented Oct 2, 2025

Uh oh!

vkprogrammer-001 commented Oct 4, 2025

Uh oh!

ValuedMammal commented Oct 7, 2025

coveralls commented Apr 10, 2025 •

edited

Loading

ValuedMammal left a comment •

edited

Loading

vkprogrammer-001 commented Sep 21, 2025 •

edited

Loading

ValuedMammal Sep 23, 2025 •

edited

Loading