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

Add bdk_core blog post #100

Merged
merged 5 commits into from
Jun 5, 2022
Merged

Conversation

LLFourn
Copy link
Contributor

@LLFourn LLFourn commented May 10, 2022

Thanks in advance for reading and providing feedback. Don't mind keeping this as a draft for a while to get feedback before publishing.

@netlify
Copy link

netlify bot commented May 10, 2022

Deploy Preview for awesome-golick-685c88 ready!

Name Link
🔨 Latest commit 233e113
🔍 Latest deploy log https://app.netlify.com/sites/awesome-golick-685c88/deploys/6281e55799e48d0008cc754f
😎 Deploy Preview https://deploy-preview-100--awesome-golick-685c88.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link
Member

@afilini afilini left a comment

Choose a reason for hiding this comment

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

Just a couple of small things I noticed while reading this.

I think the concept sounds really cool, I'm almost convinced on most points but I'm still trying to wrap my head around it.

Also, I see you've added a checkpoints.jpg and checkpoints.png file, maybe you've accidentally committed one of the two

docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
@thunderbiscuit
Copy link
Member

thunderbiscuit commented May 10, 2022

Awesome post. Thanks for taking the time to write this. A few comments:

  1. Are you interested in getting typo feedback (I kept track of a few and can just add them as review comments if you'd like) or would you prefer only content comments for now?
  2. Loving the diagrams 🔥
  3. Do I understand correctly that the decoupling of Wallet and DescriptorTrackers could make it easy to build a wallet that used/tracked a whole bunch of descriptors? You'd basically "register" DescriptorTrackers with the wallet and it wouldn't care how many you have (maybe with the exception of designating one of them as the internal/change one)?
  4. The blog post helped me understand your thinking for restructuring some of the internals of bdk. The title, however, used the word bdk_core, which had me thinking that you'd propose breaking bdk into components (maybe a bdk_core and some other supporting crates), but the post doesn't address that. I was left thinking "Well what's in bdk_core after this?" Quoting your intro paragraph: My rather ambitious plan is to start developing a new bdk_core library. Maybe a section on what would change between bdk and bdk_core would be useful, including which parts of the new proposed architecture would go into bdk_core (Wallet + DescriptorTracker + persistent storage + Blockchain? Only some of those?)
  5. Loving those types of blog posts. This is how you help people become power users of a library; good explanations of the internals, diagrams, exploration of trade-off space and examples, etc. Kudos.

@notmandatory
Copy link
Member

Overall concept ACK and love direction this is going. These are my initial comments, will have more once I fully grok the details:

  1. I second @thunderbiscuit's comment that a little more discussion on how the bdk repo would be reorganized would be useful, we've discussed it but would be good to have it written so we're sure we're on the same page and to help explain to others.
  2. My understanding is within the bdk repo we'd have a workspace with a strip down core lib containing the primary BDK mechanisms and traits, I think we could keep the name bdk for this lib rather than introduce a new name like bdk_core. This would be similar to how the core lib of the tokio repo workspace is tokio.
  3. Also in this workspace would be the accessory libs that implement required bdk traits such as bdk_esplora or would we combine existing blockchain clients into a feature flagged bdk_blockchain crate?
  4. Similar for data storage implementations, do you envision a bdk_sqlite, etc. or bdk_datastore (bdk_persister?) or something like that?
  5. I'd also like to see a bdk_wallet crate that provides a simple two descriptor wallet to help transition existing projects that are using bdk and as an example for projects that want to create their own policies using the core bdk mechanisms. Doesn't need to be part of your original PR, something we as a team can build.

@LLFourn
Copy link
Contributor Author

LLFourn commented May 11, 2022

@thunderbiscuit wrote:

  1. Are you interested in getting typo feedback (I kept track of a few and can just add them as review comments if you'd like) or would you prefer only content comments for now?

Please give me typo feedback!

  1. Loving the diagrams fire
  2. Do I understand correctly that the decoupling of Wallet and DescriptorTrackers could make it easy to build a wallet that used/tracked a whole bunch of descriptors? You'd basically "register" DescriptorTrackers with the wallet and it wouldn't care how many you have (maybe with the exception of designating one of them as the internal/change one)?

Yes there should be a DescriptorTrackers type thing that have multiple and aggregate some of the data in them. I will put this in scope.

  1. The blog post helped me understand your thinking for restructuring some of the internals of bdk. The title, however, used the word bdk_core, which had me thinking that you'd propose breaking bdk into components (maybe a bdk_core and some other supporting crates), but the post doesn't address that. I was left thinking "Well what's in bdk_core after this?" Quoting your intro paragraph: My rather ambitious plan is to start developing a new bdk_core library. Maybe a section on what would change between bdk and bdk_core would be useful, including which parts of the new proposed architecture would go into bdk_core (Wallet + DescriptorTracker + persistent storage + Blockchain? Only some of those?)

I just a paragraph about this at the start:

The bdk_core idea is still "in the lab". We're not sure yet whether bdk_core will just be what's left of bdk once we spin off all the components that have extra dependencies into their own crates and refine it a bit. In that case bdk_core will just be called bdk v1.0.0 or something. It might be that bdk lives on with its current APIs and uses stuff bdk_core to implement it internally.

So the answer is I don't know yet but this new stuff should at least start off as a separate crate.

@notmandatory wrote:

Overall concept ACK and love direction this is going. These are my initial comments, will have more once I fully grok the details:

  1. I second @thunderbiscuit's comment that a little more discussion on how the bdk repo would be reorganized would be useful, we've discussed it but would be good to have it written so we're sure we're on the same page and to help explain to others.
  2. My understanding is within the bdk repo we'd have a workspace with a strip down core lib containing the primary BDK mechanisms and traits, I think we could keep the name bdk for this lib rather than introduce a new name like bdk_core. This would be similar to how the core lib of the tokio repo workspace is tokio.

See response to @thunderbiscuit above.

  1. Also in this workspace would be the accessory libs that implement required bdk traits such as bdk_esplora or would we combine existing blockchain clients into a feature flagged bdk_blockchain crate?

I would have separate crates for each blockchain.

  1. Similar for data storage implementations, do you envision a bdk_sqlite, etc. or bdk_datastore (bdk_persister?) or something like that?

same answer.

  1. I'd also like to see a bdk_wallet crate that provides a simple two descriptor wallet to help transition existing projects that are using bdk and as an example for projects that want to create their own policies using the core bdk mechanisms. Doesn't need to be part of your original PR, something we as a team can build.

Yeah for now bdk will continue to exist and I will slowly just just it to use bdk_core stuff under the hood.

Copy link
Member

@thunderbiscuit thunderbiscuit left a comment

Choose a reason for hiding this comment

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

Some of that is stylistic, take what you like and chuck the rest out!

draft: false
---

The Bitcoin Developer Kit (BDK) lets you do a lot of useful things through convenient high level
Copy link
Member

Choose a reason for hiding this comment

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

This just made me realize that I don't really know if "Dev" stands for Development or Developer. I always thought it was the "Bitcoin Development Kit". But I might have been wrong this whole time 🤣.

Choose a reason for hiding this comment

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

"Bitcoin Devkit"

Copy link
Member

Choose a reason for hiding this comment

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

BDK should be like SDK, and SDK generally means "Software Development Kit" ie. a kit for developing software, the term "Software Developer's Kit" isn't common. Likewise BDK is a kit for developing bitcoin software, so "Bitcoin Development Kit" or "Bitcoin Dev Kit" for short. :-)

https://trends.google.com/trends/explore?date=today%205-y&q=%22Software%20Development%20Kit%22,%22Software%20Developer%27s%20Kit%22

docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
docs/_blog/bdk_core_pt1.md Outdated Show resolved Hide resolved
gets in the way since it defines whether you do it synchronously or asynchrononusly. Applications
can control this through `bdk`'s `async-interface` feature flag which internally changes the trait
definition through macros. Another annoyance is that when using `async-interface` the future that
gets returned from `WalletSync` [cannot be `Send`](https://github.com/bitcoindevkit/bdk/issues/165)

Choose a reason for hiding this comment

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

[cannot be sent (`Send`)]

Maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cannot be Send implies it cannot be sent across threads. I think the sentence is ok the way it is.

descriptor a `DescriptorTracker`. Here's a diagram that communicates how I imagine it relates to the
other components.

![](./bdk_core_pt1/descriptor-tracker.jpg)

Choose a reason for hiding this comment

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

Where is the diagram source code?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it comes from miro.com so I don't have source.

LLFourn and others added 2 commits May 16, 2022 15:43
Co-authored-by: thunderbiscuit <thunderB@protonmail.com>
@LLFourn
Copy link
Contributor Author

LLFourn commented May 16, 2022

Thanks @thunderbiscuit and @katesalazar! I've applied the feedback.

@LLFourn
Copy link
Contributor Author

LLFourn commented May 27, 2022

This can be merged from my perspective. Thanks to everyone who reviewed.

Copy link
Contributor

@rajarshimaitra rajarshimaitra left a comment

Choose a reason for hiding this comment

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

Big Concept + Review ACK 233e113..

This is feels good to go from my eyes..

I also have a dev friend who wanted to do something niche in their wallet and was restricted by bdk API, so ended up making their own workaround..

Also Congrats on having the 100th post!!!

A deserving one for that spot.. :D

@notmandatory
Copy link
Member

reACK great start to re-organizing bdk mechanisms to support existing Wallet policies while opening the way for new ones!

@notmandatory notmandatory merged commit 2672725 into bitcoindevkit:master Jun 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants