Skip to content

Add test of sync callbacks#1022

Draft
thunderbiscuit wants to merge 1 commit into
bitcoindevkit:masterfrom
thunderbiscuit:feat/sync-callbacks
Draft

Add test of sync callbacks#1022
thunderbiscuit wants to merge 1 commit into
bitcoindevkit:masterfrom
thunderbiscuit:feat/sync-callbacks

Conversation

@thunderbiscuit

Copy link
Copy Markdown
Member

WIP. I'd like to add one for the FullScanScriptInspector as well.

Description

Notes to the reviewers

Documentation

Changelog

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
  • 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

@thunderbiscuit

thunderbiscuit commented Jun 18, 2026

Copy link
Copy Markdown
Member Author

Just putting this here to remember: our current implementation of the callback for sync is not perfect, because if you just want to know how many scripts are being synced, you need to do a bit of workaround to do it instead of just pulling the data out of the type. This is not an issue at the ffi layer but the Rust layer. Issue here: bitcoindevkit/bdk#2220.

Currently logging the number of scripts being synced for Tatooine with this (works but is clearly not great).

class SyncCallback : SyncScriptInspector {
    private val logger = KotlinLogging.logger {}
    var totalSynced = 0

    // On the first run of the callback, log the number of scripts that will be inspected.
    override fun inspect(script: Script, total: ULong) {
        if (totalSynced == 0) logger.info { "Syncing $total scripts in this sync" }
        totalSynced++
    }
}

Note that this triggers only once the sync is started, and so the number of scripts is unknown until the sync starts.

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

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

1 participant