Skip to content

Conversation

@ValuedMammal
Copy link
Collaborator

@ValuedMammal ValuedMammal commented Sep 15, 2024

The PR adds a bip158 module to the bdk_bitcoind_rpc crate along with a new type FilterIter that can be used for retrieving blocks from a full node which contain transactions relevant to a list of script pubkeys.

Notes to the reviewers

Changelog notice

bdk_bitcoind_rpc: Added bip158 module as a means of updating bdk_chain structures

Checklists

All Submissions:

  • I've signed all my commits
  • I followed the contribution guidelines
  • I ran cargo fmt and cargo clippy before committing

New Features:

  • I've added tests for the new feature
  • I've added docs for the new feature

@evanlinjin
Copy link
Member

This is what we've all been waiting for.

@LLFourn
Copy link
Collaborator

LLFourn commented Sep 30, 2024

ConceptACK. Don't have time for a full review now. @evanlinjin?

@notmandatory notmandatory added this to the 1.1.0 milestone Oct 15, 2024
@notmandatory notmandatory added the discussion There's still a discussion ongoing label Oct 15, 2024
@ValuedMammal
Copy link
Collaborator Author

Thanks for the review @oleonardolima

@ValuedMammal
Copy link
Collaborator Author

Rebased and addressed comments from @oleonardolima

@ValuedMammal ValuedMammal force-pushed the feat/filter-iter branch 2 times, most recently from 455c16c to 1074412 Compare October 23, 2024 14:22
@ValuedMammal
Copy link
Collaborator Author

In the last push

  • Removed unneeded next_filter field
  • Fixed up test checking for Error::NoScripts
  • Added test filter_iter_returns_matched_blocks

CI failure seems unrelated

Copy link
Contributor

@oleonardolima oleonardolima left a comment

Choose a reason for hiding this comment

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

cACK db65d96

@ValuedMammal ValuedMammal force-pushed the feat/filter-iter branch 2 times, most recently from 19a5926 to 7ad95d3 Compare January 18, 2025 00:02
@ValuedMammal
Copy link
Collaborator Author

Rebased

Copy link
Contributor

@nymius nymius left a comment

Choose a reason for hiding this comment

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

tACK 7ad95d3

@ValuedMammal
Copy link
Collaborator Author

Rebased

Copy link
Member

@notmandatory notmandatory left a comment

Choose a reason for hiding this comment

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

tACK a6364e2

This is cool stuff, I added some docs to make it easier for others to run.

@ValuedMammal
Copy link
Collaborator Author

Thanks @notmandatory

@notmandatory notmandatory merged commit 82a2423 into bitcoindevkit:master Jan 27, 2025
23 checks passed
@ValuedMammal ValuedMammal deleted the feat/filter-iter branch January 27, 2025 17:47
@ValuedMammal ValuedMammal mentioned this pull request Feb 4, 2025
41 tasks
@evanlinjin
Copy link
Member

We need to test this against reorgs. It seems like the architecture of this may result in inconsistent state of checkpoints during reorgs.

  1. Have a chain with blocks .. [100:A] [101:B] with relevant txs in both A and B.
  2. Sync up to 100 (calling .next).
  3. Reorg so that blocks from 100 are replaced (inclusive). .. [100:A'] [101:B'].
  4. Call .next again.
  5. Check that checkpoints end in .. [100:A'] [101:B'] and we have emitted relevant txs in A' and B'.

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

Labels

discussion There's still a discussion ongoing module-blockchain new feature New feature or request

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

6 participants