Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove batch_insert_unconfirmed from TxGraph and IndexedTxGraph #1260

Open
LLFourn opened this issue Jan 7, 2024 · 3 comments
Open

Remove batch_insert_unconfirmed from TxGraph and IndexedTxGraph #1260

LLFourn opened this issue Jan 7, 2024 · 3 comments
Labels
api A breaking API change module-blockchain

Comments

@LLFourn
Copy link
Contributor

LLFourn commented Jan 7, 2024

We have:

  • batch_insert_unconfirmed
  • batch_insert_relevant_unconfirmed
  • batch_insert_relevant

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
Copy link
Contributor Author

LLFourn commented Jan 7, 2024

It looks like this was already sort of noted but we forgot to fix it: #1041 (comment)

@evanlinjin
Copy link
Member

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
Copy link
Contributor Author

LLFourn commented Jan 12, 2024

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

@LLFourn LLFourn changed the title IndexedTxGraph::batch_insert_* methods are overwraught Remove batch_insert_unconfirmed from TxGraph and IndexedTxGraph Jan 12, 2024
@notmandatory notmandatory added module-blockchain api A breaking API change labels Mar 18, 2024
@notmandatory notmandatory added this to BDK Mar 18, 2024
@notmandatory notmandatory moved this to Todo in BDK Mar 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api A breaking API change module-blockchain
Projects
Status: Todo
Development

No branches or pull requests

3 participants