test: add offline wallet smoke tests by j-kon · Pull Request #974 · bitcoindevkit/bdk-ffi

j-kon · 2026-03-24T11:46:55Z

Description

Add small Rust-side wallet smoke tests covering realistic wallet usage flows.

The tests focus on:

creating a wallet
revealing the next address
checking initial balance
verifying derivation index progression
validating simple create_single behavior

Notes to the reviewers

I updated the original wallet tests based on review feedback:

removed the incorrect create_single external/internal keychain comparison
dropped the _offline suffix from test names
reworked the tests to read more like usage flows rather than isolated assertions
explicitly verified derivation index behavior before and after the first reveal

Scope is limited to Rust wallet tests.

Documentation

Checklists

All Submissions:

I've signed all my commits
I followed the contribution guidelines
I ran cargo fmt and cargo clippy before committing
I've added a changelog in the next release tracking issue (see example)
I've linked the relevant upstream docs or specs above

New Features:

I've added tests for the new feature
I've added docs for the new feature

Bugfixes:

This pull request breaks the existing API
I've added tests to reproduce the issue which are now passing
I'm linking the issue being fixed by this PR

cc @thunderbiscuit and @reez

ItoroD

Thanks for the test! We have also being leaning more towards adding some tests on the bindings side which are a little more comprehensive. See example here. The addition of a regtest setup into our CI for the various bindings also allows us simulate some "live" test. Currently we have that for bdk-jvm and android, hopefully others will have have that too. Here is an issue we previously had just for context.

ItoroD · 2026-03-24T16:01:11Z

bdk-ffi/src/tests/wallet.rs

+
+#[test]
+fn test_single_descriptor_wallet_external_and_internal_peek_match() {
+    let wallet = build_single_descriptor_wallet();


Hi @j-kon! Thanks for the test.

Since you are doing a create_single here maybe we do not want to use a second KeychainKind for comparison. See doc for create_single.

ItoroD · 2026-03-24T16:05:04Z

bdk-ffi/src/tests/wallet.rs

+}
+
+#[test]
+fn test_reveal_next_address_offline() {


Small nit:

Maybe we drop the offline suffix from some of the function names?

ItoroD · 2026-03-24T16:24:39Z

@j-kon will also be good if you could check the checklist in PR description. And a signed commit as well

j-kon · 2026-03-25T01:01:37Z

Thanks for the detailed feedback, really helpful.

I see your point about create_single and the KeychainKind usage, I’ll adjust the test to better reflect single-descriptor behavior.

Also makes sense on leaning toward bindings-side tests. I’ll review the bdk-jvm example you shared and align with that direction.

I’ll clean up the naming, checklist, and signed commit as well.

j-kon · 2026-03-25T01:54:38Z

Thanks for the feedback.

I updated the wallet tests to align more closely with the direction you suggested.

Changes made:

removed the incorrect create_single external/internal keychain comparison
dropped the _offline suffix from the test names
reworked the tests into simple wallet usage flows
explicitly verified derivation index behavior before and after the first reveal
kept the scope limited to bdk-ffi/src/tests/wallet.rs

Verification:

cargo fmt
cargo test wallet
cargo clippy --all-targets --all-features

The PR now includes these updates.

ItoroD

Thanks for the updates. For completeness I think there is another create method for wallet create_from_two_path_descriptor maybe we can add that too. Also, I think your commit is still not signed.

ItoroD · 2026-03-26T10:33:03Z

bdk-ffi/src/tests/wallet.rs

    assert_eq!(address_info.index, 0);
    assert_eq!(address_info.keychain, KeychainKind::External);
    assert_eq!(address_info.address.to_string(), EXPECTED_FIRST_ADDRESS);
+    assert_eq!(wallet.derivation_index(KeychainKind::External), Some(0));


Maybe the asserts should only have to do with address_info? I think line 52 to 54 are good for this test. line 55 and 56 already show up in the other 2 tests its best to leave them out of here IMO

@j-kon I believe the lines I mentioned here were in test_reveal_next_address not create_single test. You seem to have changed create_single instead.

j-kon · 2026-03-26T11:21:18Z

Thanks for the follow-up.

I pushed another small update to keep the create_single test focused on the returned address_info, and added a separate create_from_two_path_descriptor usage-flow test so the constructor coverage stays split by wallet shape.

Verification:

cargo fmt --check
cargo test wallet

Scope is still limited to bdk-ffi/src/tests/wallet.rs.

j-kon · 2026-03-26T11:48:55Z

I force-pushed a signed commit for the latest change set, and GitHub is now recognizing the current PR head commit as verified.

ItoroD · 2026-03-31T10:53:35Z

I force-pushed a signed commit for the latest change set, and GitHub is now recognizing the current PR head commit as verified.

Cool. Maybe you could also squash your commits. Typically, commit messages do not contain the details of requested changes by the reviewer. You want the commit message describing what you are adding just like the message of your first commit. You might find it easier amending your first commit and doing a force push when reviews require you to make a change, instead of sending different commits with the different changes.

j-kon · 2026-03-31T11:02:43Z

I force-pushed a signed commit for the latest change set, and GitHub is now recognizing the current PR head commit as verified.

Cool. Maybe you could also squash your commits. Typically, commit messages do not contain the details of requested changes by the reviewer. You want the commit message describing what you are adding just like the message of your first commit. You might find it easier amending your first commit and doing a force push when reviews require you to make a change, instead of sending different commits with the different changes.

Thanks, that makes sense.

I understand. I’ll squash the PR history down to a single signed commit and keep the commit message focused on the actual addition rather than the review iterations.

j-kon · 2026-03-31T11:07:35Z

Thanks, I squashed the PR down to a single signed commit and kept the commit message focused on the actual change:

test: add offline wallet smoke tests

ItoroD · 2026-04-06T12:51:00Z

@j-kon FYI

I will look at this today. Sorry for the delay

j-kon · 2026-04-07T10:37:29Z

Thanks for catching that you were right, I had applied that change to the wrong test.

I’ve updated it so test_reveal_next_address now only checks the returned address_info, and I moved the derivation-state assertions back to test_create_single_wallet.

I force-pushed the update, and the PR is still a single signed commit.

ItoroD

ACK ba7a31f

reez

ACK ba7a31f

github-project-automation bot added this to BDK-Bindings Mar 24, 2026

ItoroD requested changes Mar 24, 2026

View reviewed changes

ItoroD reviewed Mar 26, 2026

View reviewed changes

j-kon force-pushed the master branch from 1630b71 to 0f34b6e Compare March 26, 2026 11:45

j-kon force-pushed the master branch from 0f34b6e to c404aea Compare March 31, 2026 11:04

test: add offline wallet smoke tests

ba7a31f

j-kon force-pushed the master branch from c404aea to ba7a31f Compare April 7, 2026 10:31

j-kon requested a review from ItoroD April 7, 2026 10:32

ItoroD approved these changes Apr 8, 2026

View reviewed changes

reez approved these changes Apr 8, 2026

View reviewed changes

Conversation

j-kon commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Notes to the reviewers

Documentation

Checklists

All Submissions:

New Features:

Bugfixes:

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

ItoroD Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

ItoroD Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

ItoroD commented Mar 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

j-kon commented Mar 25, 2026

Uh oh!

j-kon commented Mar 25, 2026

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

ItoroD Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

ItoroD Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

j-kon commented Mar 26, 2026

Uh oh!

j-kon commented Mar 26, 2026

Uh oh!

ItoroD commented Mar 31, 2026

Uh oh!

j-kon commented Mar 31, 2026

Uh oh!

j-kon commented Mar 31, 2026

Uh oh!

ItoroD commented Apr 6, 2026

Uh oh!

j-kon commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ItoroD left a comment

Choose a reason for hiding this comment

Uh oh!

reez left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

j-kon commented Mar 24, 2026 •

edited

Loading

ItoroD commented Mar 24, 2026 •

edited

Loading

j-kon commented Apr 7, 2026 •

edited

Loading