search

bitcoindevkit / bdk

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

BIP 329 "Wallet Labels Export Format" Support #848

Open gabridome opened 1 year ago

gabridome commented 1 year ago

The spec is new but it seems not invasive. I find particularly interesting the outputs's labels. https://github.com/bitcoin/bips/blob/master/bip-0329.mediawiki

thunderbiscuit commented 1 year ago

I like the labelling idea a lot. But in this case this might be information that would be stored in the BD itself (unless the cli kept a separate persistence of some sort), and so I think this issue is better suited for BDK itself rather than bdk-cli.

notmandatory commented 1 year ago

I agree this looks like a good spec to add support for but the feature should be added in the bdk repo, once there we can enable it in the bdk-cli tool. Moving this issue to the bdk repo for now.

xavierfiechter commented 1 year ago

What about "Label Import" ?

praveenperera commented 5 months ago

Can I help with this?

notmandatory commented 5 months ago

Hi @praveenperera yes I don't know of anyone else looking into this issue.

praveenperera commented 5 months ago

@notmandatory sounds good, the implementation looks relatively simple, I just have to familiarize myself with BDK a bit more to figure out the place to add it.

I’ll probably need this in a few weeks, so assuming no one else has picked it up by then, I’ll work on it at that time.

If someone else wants to take it before then, feel free.

notmandatory commented 5 months ago

Feel free to ask questions in our discord text or weekly voice chats too.

praveenperera commented 4 months ago

Made a standalone crate, maybe it makes sense to incorporate it into bdk?

https://github.com/bitcoinppl/bip329?tab=readme-ov-file

storopoli commented 4 months ago

Yes, it does make sense. I can close #1526 in favor of your bip329 crate. We just need to integrate it to the BDK's Wallet. I did for the input and output label types. You can check it at #1526. I still have no idea how to integrate with the Wallet for the other label types from BIP 329.

praveenperera commented 4 months ago

@storopoli oh didn’t know you were working on this today too. What are the odds we both started this on the same day haha.

I’m not sure the best way to integrate it yet either.

storopoli commented 4 months ago

Check my implementation, steal the best parts, and make a PR. I'll help review it. What do you think?

What are the odds we both started this on the same day

We're pretty much in-sync now that things are stable (beta) with the storage it would be very opportunistic to implement BIP 329 labels.

xavierfiechter commented 4 months ago

@storopoli @praveenperera love to see this coming! Thanks for your time and effort.

praveenperera commented 4 months ago

@storopoli do you want to continue on the integration work since you’ve already started this PR? Let me know if you need any changes made in the library.

storopoli commented 4 months ago

Alright. I might get your tests. One thing that I was thinking yesterday is that the wallet needs to be fully synced for any import labels. This will make much more easier and efficient in the storage side to fetch the Outpoints and Addresses in order to store the label. I'll keep adding stuff to the PR. Please feel free to review the draft and add suggestions.

praveenperera commented 2 months ago

@storopoli hey are you working on this, if not I think I can take it on next week.

storopoli commented 2 months ago

No, I'm not. All yours :)