Skip to content

Add test of sync callbacks#1022

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

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

Conversation

@thunderbiscuit

@thunderbiscuit thunderbiscuit commented Jun 4, 2026

Copy link
Copy Markdown
Member

This PR adds 2 tests showcasing the use of the sync and full_scan inspector callbacks.

Documentation

Changelog

No 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

@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.

@thunderbiscuit thunderbiscuit force-pushed the feat/sync-callbacks branch 2 times, most recently from 1a9f569 to 11bb5e5 Compare June 30, 2026 18:52
@thunderbiscuit thunderbiscuit marked this pull request as ready for review June 30, 2026 18:52
.inspectSpks(SyncCallback())
.build()

electrumClient.sync(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we retain the callback instance and assert what it observed after sync/fullScan? Right now these tests pass even if the callback is never called.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it occured to me after I pushed that this is not actually testing anything yet so I'll add at least an assertion or two on those.

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.

2 participants