Add custom sync methods by thunderbiscuit · Pull Request #1012 · bitcoindevkit/bdk-ffi

thunderbiscuit · 2026-05-26T18:21:30Z

This PR introduces two new methods:

Wallet::start_sync_with_spk_indices(external_indices, internal_indices)
start_sync_with_last_revealed_spks(external_count, internal_count)

Note that when using these methods, invalid inputs can be provided, and so the methods actually return a Result (for example if you give an index you haven't revealed yet or if you ask for indices on a keychain the wallet doesn't have). This required a new error that doesn't exist upstream: SpkIndexError.

Why adding methods to the wallet instead of exposing the whole builder?

I looked at exposing SyncRequest::builder() to the bindings instead because that could potentially be the most flexible option, but it turned out to be super clumsy:

chain_tip takes a CheckPoint, which the bindings do not expose. A request built without a tip produces no chain update, so confirmed transactions cannot be connected to the local chain and show up as unconfirmed. Exposing the CheckPoint is not impossible, but it's a linked list of unknown size so it ends up being a lot more than we probably want, especially that you don't actually need it, you just need to pass it to your builder!
expected_spk_txids is weird to compute. It's a reimplementation of internal bookkeeping that every caller would write, and keep in step with upstream if the definition of "relevant spk" changes. Getting it wrong, or skipping it, fails silently, and a dropped or replaced transaction just stays pending forever. No error, no missing-argument complaint. Also, the wallet already has the answer, so having each app recompute it is wasted effort.

Keeping the entry point on Wallet means both are always filled in. The cost is that we own two methods upstream doesn't have.

Notes on API shape

There is a bit of bikeshedding to be done if you want to around certain choices I made:

Right now the arguments are all optional and default to None, so you can call a sync for the last 3 addresses on your external keychain like so:

wallet.startSyncWithLastRevealedSpks(externalCount = 3)

Both being optional means you might also mistakenly call

wallet.startSyncWithLastRevealedSpks()

But for that you'll get an error (can't construct an empty SyncRequest)
3. None is a bit different than an empty list, and if you try to sync an empty list on an internal keychain but you have a single-keychain wallet you get an error.

Documentation

There are no direct upstream equivalent to these methods.

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 exactly one changelog:* label

New Features:

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

j-kon · 2026-06-16T11:51:02Z

+                    .map(|spk| ((KeychainKind::Internal, i), spk))
+            }));
+
+        let builder = BdkSyncRequest::builder()


Could we preserve the expected_spk_txids behavior that BDK Wallet’s normal start_sync_with_revealed_spks{,_at} path adds? Those expected txids are used by sync backends to detect previously-seen mempool txs that disappear from SPK history. As written, this custom SyncRequest builder only sets the chain tip and selected SPKs, so these selective sync APIs can silently miss eviction/replacement updates compared with the existing sync path.

We totally could and it's fairly easy to add, but I'm just not sure those are as useful as we think they are. They sort of imply you have a faulty clock (or don't have access to one?), and you need to set the time directly.

The other methods simply take the time at the moment the builder is created using something like now() I think, and you get exactly the same stuff regarding evictions and whatnot.

Rechecked the updated implementation and verified locally that the custom request preserves the chain tip and expected transaction IDs for a selected script. This addresses my original eviction-metadata concern. The concern was about expected_spk_txids, rather than exposing an explicit timestamp.

j-kon · 2026-06-16T11:51:09Z

        Arc::new(SyncRequestBuilder(Mutex::new(Some(builder))))
    }

+    /// Create a partial [`SyncRequest`] for a specific set of derivation indices.


Since this is a new public FFI API, could we add tests that reveal a known set of external/internal addresses and assert the request contains only the selected indices / last N revealed SPKs? That would also help guard this custom builder against drifting from BDK Wallet’s normal sync-request semantics.

+1 for adding tests. I just did that!

reez · 2026-10-14T20:47:13Z

+            if spk_index.get_descriptor(keychain).is_none() {
+                return Err(SpkIndexError::KeychainNotFound { keychain });
+            }
+            if spk_index.last_revealed_index(keychain).is_none() {


Could we only error when the combined selection is empty? Example: externalCount = 5, internalCount = 5 currently fails when external addresses are revealed but no change addresses are.

And maybe we could cover this with tests where only external addresses are revealed, and vice versa?

reez · 2026-10-14T20:50:39Z

+            fetchPrevTxouts = true
+        )
+
+        assertEquals(7, counter.inspected, "Only the 5 external and 2 internal scripts should be synced")


Could we also assert which scripts were synced because checking only the count would also pass if we accidentally selected the first scripts instead of the last I think

reez · 2026-10-14T20:51:37Z

+    BdkSyncRequest::builder()
+        .chain_tip(chain_tip)
+        .spks_with_indexes(spks)
+        .expected_spk_txids(expected_spk_txids)


Might want to add a rust test checking that the custom request preserves the wallets chain tip and expected transaction IDs for the selected scripts to possibly would help catch regressions in the metadata needed for eviction tracking? Non-blocking

reez · 2026-10-14T20:54:09Z

+    /// an unrevealed index would be misleading: the wallet's indexer does not track it, so the
+    /// outputs of any transaction found there would not be recognized as the wallet's own.


Can we keep the “every index must have been revealed” requirement and drop the explanation about outputs not being recognized?

reez · 2026-10-14T20:56:28Z

+        external_indices: Option<Vec<u32>>,
+        internal_indices: Option<Vec<u32>>,


non blocking: what do you think about these using plain lists defaulting to empty and the count arguments default to zero, just since None has the same meaning, or is there a better reason we need the optional types?

reez · 2026-10-14T20:57:12Z

+                continue;
+            }
+            if spk_index.get_descriptor(keychain).is_none() {
+                return Err(SpkIndexError::KeychainNotFound { keychain });


Is rejecting the internal keychain intentional for single keychain wallets? Upstream methods like reveal_next_address map Internal to External in that case so this behaves differently is all.

reez · 2026-10-14T20:59:35Z

concept looking good, added some comments/questions.

in original description we can update 3. None is a bit different than an empty list, and if you try to sync an empty list on an internal keychain but you have a single-keychain wallet you get an error. because None and an empty list now behave the same (including when the wallet has no internal keychain)? Not that update needed before merge but partially just to make sure I'm understanding everything properly.

github-project-automation Bot added this to BDK-Bindings May 26, 2026

thunderbiscuit added the changelog: added label May 26, 2026

j-kon reviewed Jun 16, 2026

View reviewed changes

thunderbiscuit added this to the 3.2.0 milestone Sep 11, 2026

thunderbiscuit changed the title ~~Exploration: add custom sync methods~~ Exploration: custom sync methods Sep 15, 2026

thunderbiscuit force-pushed the feat/custom-sync branch 2 times, most recently from ceefaac to 2e9dc1a Compare September 15, 2026 19:42

thunderbiscuit modified the milestones: 3.1.1, 3.2.0 Sep 17, 2026

thunderbiscuit force-pushed the feat/custom-sync branch from 2e9dc1a to 831fca7 Compare October 13, 2026 19:49

thunderbiscuit requested a review from a team as a code owner October 13, 2026 19:49

feat: add custom sync methods

dcd0127

thunderbiscuit force-pushed the feat/custom-sync branch from 831fca7 to ebff0bf Compare October 13, 2026 19:59

test: add kotlin and swift tests for custom sync

5d88586

thunderbiscuit force-pushed the feat/custom-sync branch from ebff0bf to 5d88586 Compare October 14, 2026 19:12

reez reviewed Oct 14, 2026

View reviewed changes

thunderbiscuit changed the title ~~Exploration: custom sync methods~~ Add custom sync methods Oct 17, 2026

thunderbiscuit mentioned this pull request Oct 19, 2026

Expose new SyncRequest methods #1147

Open

reez mentioned this pull request Oct 20, 2026

Sync a caller-chosen set of scripts with ElectrumClient #1148

Open

This was referenced Oct 27, 2026

feat(electrum): add ElectrumClient.populate_tx_cache #1150

Merged

feat(electrum): add ElectrumClient.populate_anchor_cache #1151

Open

		/// an unrevealed index would be misleading: the wallet's indexer does not track it, so the
		/// outputs of any transaction found there would not be recognized as the wallet's own.

		external_indices: Option<Vec<u32>>,
		internal_indices: Option<Vec<u32>>,

Conversation

thunderbiscuit commented May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notes on API shape

Documentation

Checklists

All Submissions:

New Features:

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!

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!

Choose a reason for hiding this comment

Uh oh!

reez commented Oct 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thunderbiscuit commented May 26, 2026 •

edited

Loading