bitcoindevkit / bdk

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

Research new design and test strategy for compact_filters module #680

Closed notmandatory closed 1 year ago

notmandatory commented 2 years ago

Before adding new multi-peer capabilities (see #81) we need to research how other implementations of compact block filters are designed and tested, in particular how to make testing complex parts of the protocol easier.

Other implementations to study are:

rajarshimaitra commented 2 years ago

I have made passes through both nakamoto and neutrino and so far this is what I feel..

nakamoto is overly verbose in its code abstractions. The whole crate is thousands of lines of code and sometimes unnecessary traits and struct wrappers for no good reason. And I think they have also tried to recreate the entire bitcoin p2p framework deep inside their p2p layers and then using it for CBF stuffs.. I don't know why that's necessary.. I didn't find the whole crate even useful as ref..

neutrino on the other hand is much more structured and looks more mature. They have opinionated design choices on memory and database, that are specific for CBF client, and their structure is much easy to follow.. They have mocking tests too almost for all modules. They have good structure around the p2p validation logic and chainstore management.

So I feel the way to go forward will be something like this

I have few thoughts on the architecture, referencing from neutrino, but those can be discussed in details on separate issues as they are more specific contextual stuffs.

rajarshimaitra commented 2 years ago

Upon further trials in bdk_core repo, it feels we can work with the nakamoto crate.. Which is working with multi peer environment in signet. Check this PR for initial trials.. https://github.com/LLFourn/bdk_core_staging/pull/14

In a recent discussion with @notmandatory it felt it might be prudent to use the existing nakamoto crate for CBF logic instead of trying to create our own.

If nakamoto can work for all bdk_core, bdk and ldk as they stand right now, that might be good straight forward path for getting BIP157 into bdk/ldk ecosystem quickly..

As a possible approach we discussed it might make sense to create a separate development branch in bdk to implement bdk's Blockchain trait on nakamoto structures.

Purpose is mostly for trying CBF out in bdk-ffi mobile apps and see how its working in mobile environment. nakamoto uses sled db, which is known to have caused problem in mobile environments previously. So getting it out for testing quickly can give us visibility if we need to modify nakamoto for bdk. As mobile is the most anticipated use environment for CBF.. And bdk can keep working with its own CBF module as it, in mean time..

If all goes okay, then we can think of more concrete integration way into bdk.. Two possible path

Please feel free to comment or suggest over above approach ideas.. :)

notmandatory commented 2 years ago

I think this work to do a trial integration with nakamoto can be done as a regular feature branch in someone's bdk fork, rather than as a new branch in the main bdk repo. Then if we can get Blockchain implemented with nakamoto we can and should certainly also test it out with bdk-cli and bdk-ffi and the mobile projects prior to merging. If we put it behind a nakamoto feature flag I could see releasing it as an experimental blockchain to start with.

notmandatory commented 1 year ago

Closing this issue, it's a big to nebulous an idea and testing will be different post bdk_core 1.0 changes.