BDK Kyoto Dream API Discussion

[thunderbiscuit]

I've been writing down some notes while looking at the library and implementing it in the example Android wallet.

Here are some thoughts, meant to start discussions mostly to sharpen my understanding of Kyoto and what's maybe possible vs maybe not. I could have opened this issue in the Kyoto repo too, I just figured it was mostly related to how I would use Kyoto through BDK as one of the blockchain clients.

My Dream Kyoto API?

I'm trying to clean up my mental model for kyoto, bdk_kyoto, and the bindings for both. This is very much in brainstorming mode but I've been meaning to write it down so I can iterate. Here is my dream API for Kyoto from the Kotlin perspective (I'm not familiar enough with tokio to write it in Rust).

// My understanding is that there is currently a utility function that builds both of those in one go but I imagine it's roughly the same
val node: KyotoNode = KyotoNode()
    .configure1()
    .configure2()
    .build() // build a custom CBF node optimized for user requirements

val client: KyotoClient = KyotoClient()
    .node(nodeToSpeakTo)
    .configureX()
    .configureY()
    .build()

// The node emits X types of events (say Log, StartSync, UpToDate, NewBlock)

In this workflow, there are no full scans nor syncs, no sync button to press.

The Node emits events, and the users use those as triggers for different kinds of wallet and UI operations.

sealed class KyotoEvent {
    data class LogInfo(message: String) : KyotoEvent
    object NodeSyncedTip : KyotoEvent
    object NewBlock : KyotoEvent
    data class WalletUpdate(update: BdkUpdate) : KyotoEvent 
}

Question for consideration: is it better to pass in a lambda to be triggered by the Node whenever an event happens on is it better to listen for events and trigger our own operations? In general I think passing callbacks is less flexible and increases reading complexity; on the other hand on simple tasks (say events that are simply logs) that's a good approach, because you just let the node know what to do with this event upon construction and don't need to think about it afterwards. For anything that needs more complex domain logic it's not great though. Anyway just a thought.

Top API: the Node emits a flow of events which I react to through wallet updates, persistence, UI messages, etc. Unfortunately I don't think we can expose Flow directly because it's a Kotlin coroutines' construct that doesn't have a direct equivalent in Swift and Python. But I do think that as is (see my option 2, standard suspension in a while loop) I can probably refactor this loop into emitting these events so they can be collected as flows by the application.

1. Kotlin Flows

val eventFlow: Flow<KyotoEvent> = client.getEventFlow()

coroutineScope.launch {
    eventFlow.collect { event ->
	when (event) {
	    is LogInfo       -> logger.info(event.message)
	    is NodeSyncedTip -> showSnackbar("All synced up!")
            is NewBlock      -> showSnackbar("New block")
            is WalletUpdate  -> wallet.applyUpdate(event.update)
	}
    }
}

2. Standard while loop awaiting the updates (I think this is roughly the current approach?)

coroutineScope.launch(Dispatchers.IO) {
    launch {
	while (nodeIsLive) {
            val event: KyotoEvent = client.update() // Suspend and wait for events
	    event?.let {
	        wallet.applyUpdate(it)
	        wallet.persist()
	        triggerCommunicationWithUser()
            }
	}
    }
   otherInterleavingAsynchronousWork()
}

[thunderbiscuit]

The above is just throwing my notes here to start he conversation. But attempting to write down what I was thinking about the new bdk_kyoto client opened up a few questions I'll leave here:

• What is the Kyoto client doing for me? Why doesn't the Kyoto node communicate directly with me when events happen to it?
• Alternatively, if the only important construct for the wallet is the Kyoto client, can this library hide it from the user? I imagine the reason to expose both the client and the node to the user is to allow for tuning on/off the node? Can that be done through the client? I guess the distinction between these two (as is evident from my question above 😅) are not yet clear in my mind.
• In a situation where the node/client emits events, there is always the discussion of "what happens when these events are lost". I haven't explored that at all. I know ldk_node forces you to acknowledge events; not sure if that's too heavy-handed for us here.
• If I remember correctly, the client here is attempting to fit into the full_scan/sync pattern of the Esplora and Electrum clients. I'm not convinced this is a good approach yet; are there reasons to stick to that when the CBF workflow feels like it yearn for more reactive code that simply updates the wallet whenever it's ready to apply such an update? Again I have not spent a lot of time programming with the bitcoin core RPC interface and using that in wallets, so there might be considerations I'm not aware of here. Feel free to lay those out as I clearly need to sharpen my thinking around it.

[rustaceanrob]

Unfortunately I cannot speak very well for Kotlin developers, but I can walk through how my PR on the bindings works, which of course uses this crate. This crate, as well as the bindings, prioritizes the user that simply wants to use the Wallet struct, and I try to build my assumptions around this class of user. Other developers may reach for kyoto directly if they require something more detailed. As such, we can and do offer an opinionated way to construct the Node, which uses much of the information found in the Wallet. When using the builder module, you may pass a Wallet reference along with your Node configurations, and get an opinionated bdk_kyoto::Client. This Client may shutdown the node and broadcast transactions, as well as call a crucial update function. This function breaks events down into two parts, which I feel is appropriate for this fast-tracked Wallet set-up:

1. Noncritical but interesting events, which are handled by a trait NodeMessageHandler. This trait has some implementations provided by the library, but production applications will certainly use their own implementation. In my WIP branch of the BDKSwiftExampleWallet, I define one of these traits to make UI updates and print some messages:

class MessageHandler: ObservableObject, NodeMessageHandler {
    @Published var progress: Double = 20
    @Published var height: UInt32? = nil
    
    func blocksDisconnected(blocks: [UInt32]) {}
    
    func connectionsMet() {}
    
    func dialog(dialog: String) {
        print(dialog)
    }
    
    func stateChanged(state: BitcoinDevKit.NodeState) {
        DispatchQueue.main.async { [self] in
            switch state {
            case .behind:
                progress = 20
            case .headersSynced:
                progress = 40
            case .filterHeadersSynced:
                progress = 60
            case .filtersSynced:
                progress = 80
            case .transactionsSynced:
                progress = 100
            }
        }
    }
    
    func synced(tip: UInt32) {
        print("Synced to \(tip)")
    }
    
    func txFailed(txid: BitcoinDevKit.Txid) {}
    
    func txSent(txid: BitcoinDevKit.Txid) {}
    
    func warning(warning: BitcoinDevKit.Warning) {
        switch warning {
        case .notEnoughConnections:
            print("Searching for connections")
        case .peerTimedOut:
            print("A peer timed out")
        case .unsolicitedMessage:
            print("A peer sent an unsolicited message")
        case .couldNotConnect:
            print("The node reached out to a peer and could not connect")
        case .corruptedHeaders:
            print("The loaded headers do not link together")
        case .transactionRejected:
            print("A transaction was rejected")
        case .failedPersistance(warning: let warning):
            print(warning)
        case .evaluatingFork:
            print("Evaluating a potential fork")
        case .emptyPeerDatabase:
            print("The peer database is empty")
        case .unexpectedSyncError(warning: let warning):
            print(warning)
        case .noCompactFilters:
            print("A connected peer does not serve compact block filters")
        case .potentialStaleTip:
            print("The node has not seen a new block for a long duration")
        case .unlinkableAnchor:
            print("The configured recovery does not link to block headers stored in the database")
        }
    }
}

2. Any other events, particularly blocks, are handled by our Client. This is so we may construct a Wallet update. I think conforming to the FullScanResult here is correct, as we simply want to pass a single update to the Wallet every time we sync to the tip of the chain. In the WIP branch, I do this like so:

        Task {
            while true {
                try await bdkClient.sync(logger)
                self.walletSyncState = .synced
                self.getBalance()
                self.getTransactions()
            }
        }

With this design, I think we are preserving the event-based style of API, where the user must define the NodeMessageHandler (could use a rename perhaps). However, asking them to process the blocks themselves does not offer any clear benefit if they are just using the Wallet. Instead, we return an update when we have it, from Client::update. Each time we get a value here, we would want to update the UI as well.

So in summary:

1. Wallet updates are constructed by Client::update
2. Client::update must be called with a &dyn NodeMessageHandler, forcing the user to respond to noncritical events the node is issuing

As far as missing events, the channel receiver is popping events off in a loop, so the only way that would happen is to have more than 32 events yet to be processed. Since most of these are printing to the console or calling a simple method, its an extremely fringe and unlikely scenario to miss anything

[thunderbiscuit]

Thanks for the detailed response. I will implement Kyoto in the Android example wallet this week and will be able to provide more extensive feedback!

Some of those questions might pertain more to the way the API is wrapped in the bindings, not sure. Will likely post/comment on your bitcoindevkit/bdk-ffi#591 PR. For example, at first glance the LightClient.update() method's role is to return an Update which I assume can then be applied to the Wallet, but it also has the role of taking in this list of callbacks that the client will use for its other class of events. This group of callbacks is provided for each iteration of the loop in the sync logic

Task {
    while true {
        try await bdkClient.sync(logger)
}

instead of configured once on the client (but maybe there is a reason to change the callbacks we want applied over time? In which case it might be better to receive the event as an event and apply our logic there instead of the callback).

Anyway I'm about halfway done on the Android side, will keep digging!

[rustaceanrob]

Yeah, good observation. We had this designed as you described before, where the callback trait that was configured once. I found it very annoying when integrating with the Swift wallet because the UI component that is responding to minor changes in state must be passed all the way down to the app initialization code. I decided it may be easier for the app developer to simply pass a UI component at the callsite. I conceptualize update as "here is everything that happened to the wallet" and the argument passed as "here is everything that happened to the node along the way"

Also, on Wednesday and throughout the rest of the week I will be on a continental US timezone if you wanted my live feedback as you work on the Android wallet

[thunderbiscuit]

I implemented the client in this branch of the example Android wallet. It works! The node + UI pick up the new blocks as I mine them on regtest super fast. It's snappy and super cool in comparison to the electrum workflow. 🚀

A few quick thoughts (will come back to this tomorrow).

1. Updates are nullable? What does that mean? Indeed sometimes the returned value of client.udpate() is null. Is that related to when one of the callbacks is triggered, i.e. and event happened but no Update is produced?
2. Long times between updates because you just wait on blocks. Would be nice to fire something to let the app know the node is still running? Just a thought. In one case last night the node must have stopped for some reason but I waited 20ish minutes until a block was mined on Signet to then realize it wasn't picking it up and that the node must have died on me but I had no way to tell.
3. The bindings currently don't offer the option to set up the port for peers. Would be good to add.
4. The NodeMessageHandler is a long list of callbacks. "Message handler" led me down the wrong mental model initially (it's much more than that). Sorry I know naming is hard haha but I have to mention it 🤣
5. I'm not sure I can provide in the callbacks everything I need, at least for the UI. In any case it sort of meddles my UI-related logic with a very "inner" component (the Kyoto node). Need to test more. I found ways around this, but not sure how clean it is. My rough thought is that if you need a lot of domain logic, you want to define it yourself on events somewhere different than the initial callback (meaning you must decide what the behaviour is when creating the Node, as opposed to having it a bit more flexible user-side where they might change config mid-way through). This is still just a rough "feeling" however, because I'm still using it very "simply" and not production-ready. That's also why the callbacks work so well; if you just want to log those it's perfect for that.
6. Ran into an issue where the loop kept applying thousands of updates to the wallet until a runtime crash. This was after I had stopped my bitcoin core node and gave it a few hours, and then reloaded a pre-existing wallet. Never found what the issue was; it wasn't emitting warnings as far as I can tell, but all the udpates were null. I deleted the app and reinstalled a fresh wallet and it worked fine. Something to keep an eye on.

Overall, super work! I love to see it in action.

[rustaceanrob]

1. The update is a relationship to the last time you attempted to sync with the chain. If there are no new blocks found since the last time you called update, then the update will be null. This is the case where a user opens the application and syncs to the tip of the chain, closes the app, then re-opens it 5 minutes later with no changes to the blockchain. There will still be callbacks to inform the user of what the node is doing in this case, but there is no update for the wallet to use.
2. This sounds like basically implementing the same sort of ping and pong relationship between two nodes on the network, except it would be from node to user. I would have to make changes in kyoto for this however. If the node is running for 30 minutes and has not found a block, then the node should issue a warning and begin looking for new peers. I think that is pretty close to what we are going for here?
3. I think I tried to with the SocketAddr variant, but I am going to rework this in the bindings soon. (edit: updated with a more robust Peer type)
4. It has been renamed to NodeEventHandler. I think better?
5. Even if it was node configurable, the node would need to be shutdown and restarted when configurations change. In general this is a hard compromise to find between too many callbacks and trying to keep things uniform. The UI stuff is hard because I also think it would be nice to define the trait a single time at startup, but the node initialization code and UI code often live in very different parts of the project.
6. I haven't seen this before on the Rust side. If there are multiple Receiver<NodeMessage> out in the wild (used within Client::update) and one is not popping off the events (calling Receiver::recv), then the event will remain on the stack forever. In this case, every call to update would issue the same event over and over. That is my best guess as to what happened