bitcoindevkit / bdk

A modern, lightweight, descriptor-based wallet library written in Rust!
Other
842 stars 304 forks source link

Remove batch_insert_unconfirmed from TxGraph and IndexedTxGraph #1260

Open LLFourn opened 8 months ago

LLFourn commented 8 months ago

We have:

We only need batch_insert_relevant which should have the API:

    pub fn batch_insert_relevant<'t>(
        &mut self,
        txs: impl IntoIterator<Item = &'t Transaction>,
    ) -> ChangeSet<A, I::ChangeSet> {

Whether they are confirmed or unconfirmed (have only anchors or last seen) is not important for doing this job. The caller can add anchors and last seen with the methods available for that. Also the documentation should make it clear why this method exists in the first place: because relevancy cannot be determined until all of the transactions have been first scanned by the indexer.

Also there is a batch_insert_unconfirmed in TxGraph that doesn't need to exist (can be achieved by a for loop).

LLFourn commented 8 months ago

It looks like this was already sort of noted but we forgot to fix it: https://github.com/bitcoindevkit/bdk/pull/1041#discussion_r1347427162

evanlinjin commented 8 months ago

The caller will have to iterate over the changeset returned from batch_insert_relevant to see if last_seen and/or anchors are worth adding. Wouldn't that be cumbersome?

LLFourn commented 8 months ago

Good point. I hadn't considered that. I think we can at least remove batch_insert_unconfirmed from TxGraph and IndexedTxGraph.