Skip to content

BIP 329 "Wallet Labels Export Format" Support #168

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

Open
gabridome opened this issue Jan 18, 2023 · 25 comments
Open

BIP 329 "Wallet Labels Export Format" Support #168

gabridome opened this issue Jan 18, 2023 · 25 comments
Labels
new feature New feature or request

Comments

@gabridome
Copy link

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
Copy link
Member

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 notmandatory added the new feature New feature or request label Jan 31, 2023
@notmandatory
Copy link
Member

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.

@notmandatory notmandatory transferred this issue from bitcoindevkit/bdk-cli Jan 31, 2023
@notmandatory notmandatory changed the title BIP 329 Support BIP 329 "Wallet Labels Export Format" Support Mar 2, 2023
@xavierfiechter
Copy link

What about "Label Import" ?

@praveenperera
Copy link
Contributor

Can I help with this?

@notmandatory
Copy link
Member

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

@praveenperera
Copy link
Contributor

@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
Copy link
Member

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

@praveenperera
Copy link
Contributor

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

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

@storopoli
Copy link
Contributor

Yes, it does make sense. I can close bitcoindevkit/bdk#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 bitcoindevkit/bdk#1526.
I still have no idea how to integrate with the Wallet for the other label types from BIP 329.

@praveenperera
Copy link
Contributor

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

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

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

@praveenperera
Copy link
Contributor

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

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.

@notmandatory notmandatory moved this from Todo to Discussion in BDK Wallet Aug 22, 2024
@praveenperera
Copy link
Contributor

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

@storopoli
Copy link
Contributor

No, I'm not. All yours :)

@pedromvpg
Copy link

Hey @praveenperera! Did you get any progress on this?

@praveenperera
Copy link
Contributor

@pedromvpg nope sorry, should have updated the issue, I had to prioritize other things. But I think I should be able to get to it in 4 weeks if no one has done it by then.

@praveenperera
Copy link
Contributor

praveenperera commented Feb 9, 2025

Hey @storopoli I am finally working on this, I noticed that you added labels to LocalOutput (the UTXOs), but I was looking at how sparrow exports BIP329 labels and it looks like the labels are on the transaction, and the inputs and outputs, with the refs all being related to the transaction_id

{"type":"tx","ref":"2d37562f17d3976aee8a64e67125b0d6350bb10394c4d443d34b4c30b4c10b28","label":"last txn received","origin":"pkh([73c5da0a/44h/0h/0h])"}
{"type":"output","ref":"2d37562f17d3976aee8a64e67125b0d6350bb10394c4d443d34b4c30b4c10b28:0","label":"last txn received (received)"}
{"type":"tx","ref":"077bef534b56c2ceda41f4b86f9441fa43e4bd1b09094d8e04289077c63824f8","label":"last transaction sent","origin":"wpkh([73c5da0a/84h/0h/0h])"}
{"type":"input","ref":"077bef534b56c2ceda41f4b86f9441fa43e4bd1b09094d8e04289077c63824f8:0","label":"last transaction sent (input)"}

The ref for inputs and outputs is the tx_id suffixed with the index.

So I am wondering if you would agree that it makes more sense to add the labels to Transaction, TxIn and TxOut structs,

pub struct Transaction {
    /// The protocol version, is currently expected to be 1 or 2 (BIP 68).
    pub version: Version,
    /// Block height or timestamp. Transaction cannot be included in a block until this height/time.
    ///
    /// ### Relevant BIPs
    ///
    /// * [BIP-65 OP_CHECKLOCKTIMEVERIFY](https://github.com/bitcoin/bips/blob/master/bip-0065.mediawiki)
    /// * [BIP-113 Median time-past as endpoint for lock-time calculations](https://github.com/bitcoin/bips/blob/master/bip-0113.mediawiki)
    pub lock_time: absolute::LockTime,
    /// List of transaction inputs.
    pub input: Vec<TxIn>,
    /// List of transaction outputs.
    pub output: Vec<TxOut>,
    /// The label for this transaction according to the
    /// [BIP 329](https://github.com/bitcoin/bips/blob/master/bip-0329.mediawiki)
    /// format
    #[cfg(feature = "labels")]
    pub label: Option<Label>,
}

And then the labels for the UTXO can be found by looking at the transaction output?

But I know those come from the Bitcoin package, should i try adding them there instead? I made an issue asking if they would be open to me adding support: rust-bitcoin/rust-bitcoin#4027

Do you think theres any reason we should do this in BDK instead?

If they don't want it in there, maybe labels should be its own structure with refs to Transactions, Inputs and Outputs? That could be persisted in either sqlite or the file store?

NOTE: After thinking about it some more, I think Labels should be separate, and we should just be able to look up a label by the transaction id or address: rust-bitcoin/rust-bitcoin#4027 (comment). I think they should be completely seperate, and not be put on the UTXOs either.

@DanGould
Copy link

DanGould commented Feb 9, 2025

I think we want labels on both Coins (LocalUtxo, historic) (edges) and Transaction (nodes) parts of the graph. Perhaps I'm overlooking something, but a TxIn and a TxOut express the same information as far as a purpose / owner / agent label is concerned which is who (or what) controls it.

Edit for clarity: In my view coin (txin/txout, i'd probably just choose out for simplicity otherwise you could get conflicting labels) labels express what entity owns coins and a Transaction labels represent how that ownership came to be.

@praveenperera
Copy link
Contributor

@DanGould I am going off the sparrow implementation right now, using that as the source of truth (maybe I shouldn't not, sure)

But the way it works is you, it allows you to add a label to an address, transaction, or utxo, and lets you edit labels for a txn, inputs and outputs.

Usually the labels are either added to the address or the transaction. If I add a label to a sending transaction called transaction for bike, it will ALSO create a label for the input with type input called transaction for bike (input) and if theres change it will also create a label of type output called transaction for bike (change). So adding one labels creates 2-3

  1. transaction
  2. input,
  3. output *if change)

If you add a label on a received transaction lets say paid for painting, it will

  • Add a label for that transaction called paid for painting
  • Add a label of type output with the ref being the transaction_id:<index> called paid for painting (received)
  • Add a label for that address with type addr called paid for painting

Note: in sparrow if there was multiple transactions on one address, it will add this set of labels for all the transactions and outputs of each of those txns in the same address, i'm not sure we should follow the same method.

This is all to say that even tho the input of one transaction is the output of another it can be labeled twice, because the transaction is the root of the labeling. Right now at-least in sparrow there are conflicting labels.

It shows a different label for the output depending on the context. For example the first image shows the input label, but the second image is when you click into the transaction it shows the output label.

Image Image

I hope all that makes sense

exported bip329 from sparrow (with duplicate transactions in the same address removed)

{"type":"xpub","ref":"xpub6CatWdiZiodmUeTDp8LT5or8nmbKNcuyvz7WyksVFkKB4RHwCD3XyuvPEbvqAQY3rAPshWcMLoP2fMFMKHPJ4ZeZXYVUhLv1VMrjPC7PW6V","label":"BIP39"}
{"type":"tx","ref":"8a9cc9253407b3f5842a3a7bd1308f9ae84a79c51126af48abe5ec16284c8cdf","label":"l-received","origin":"wpkh([73c5da0a/84h/0h/0h])"}
{"type":"tx","ref":"077bef534b56c2ceda41f4b86f9441fa43e4bd1b09094d8e04289077c63824f8","label":"l-sent","origin":"wpkh([73c5da0a/84h/0h/0h])"}
{"type":"addr","ref":"bc1qcr8te4kr609gcawutmrza0j4xv80jy8z306fy"}
{"type":"output","ref":"8a9cc9253407b3f5842a3a7bd1308f9ae84a79c51126af48abe5ec16284c8cdf:6","label":"l-received (received)"}
{"type":"input","ref":"077bef534b56c2ceda41f4b86f9441fa43e4bd1b09094d8e04289077c63824f8:0","label":"l-sent (input)"}

@notmandatory notmandatory transferred this issue from bitcoindevkit/bdk Apr 7, 2025
@Musab1258
Copy link
Contributor

Hi everyone, I participated in a program organized by ChaincodeLabs, the BOSS Program. And when I got to the Proof of Concept (POC) stage, I picked this issue as my POC project since it was part of the listed projects.

I can see that a lot of discussion had happened on this issue and @storopoli has made a PR which he eventually closed. And although I have started working on this issue, I want to to ask if anyone is currently not working on this issue so I can keep on working on it and make a PR.

@notmandatory
Copy link
Member

This may only need an example using the library @praveenperera created here: https://github.com/bitcoinppl/bip329

And/or this would be a useful new feature to add to the bdk-cli example wallet which was recently updated to use bdk 1.0.

@Musab1258
Copy link
Contributor

@notmandatory, I made a PR for an example using the library here #222. Can you please help me review it.

@Musab1258
Copy link
Contributor

This may only need an example using the library @praveenperera created here: https://github.com/bitcoinppl/bip329

And/or this would be a useful new feature to add to the bdk-cli example wallet which was recently updated to use bdk 1.0.

@notmandatory To seek clarification, if I am to add this feature to bdk-cli based on your suggestion, I should also make use of the bip329 library right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
Status: Discussion
Development

Successfully merging a pull request may close this issue.

9 participants