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

Feature Request: BIP47 payment codes for private donations (PayNym) #549

Open
satrinity402 opened this issue Feb 17, 2022 · 33 comments
Open
Assignees
Labels
new feature New feature or request

Comments

@satrinity402
Copy link

satrinity402 commented Feb 17, 2022

In the current environment, it would be extremely beneficial if bitcoin wallet developers using BDK could have an easy way to integrate BIP47 payment codes into their wallets for receiving payments privately (especially donations) without needing to setup a server to generate fresh addresses for each payment. Would need functionality for sending to other BIP47 payment codes as well.

Both Samourai Wallet and Sparrow Wallet are FOSS, and have integrated this functionality into their wallets.

@notmandatory notmandatory added the new feature New feature or request label Feb 18, 2022
@artdesignbySF
Copy link

I am for this enhancement and would love to see it adopted by all major wallets.

@BitcoinQnA
Copy link

Another +1 here

@satrinity402
Copy link
Author

Looks like there is already a BIP47 implementation for Rust: https://github.com/rust-bitcoin/rust-bip47

@modl21
Copy link

modl21 commented Feb 18, 2022

I am for this enhancement and would love to see it adopted by all major wallets.

I echo this sentiment.

@luke-jr
Copy link

luke-jr commented Feb 19, 2022

The goal of a reusable donation address is great, but BIP47 is a bad way to do it. Recommend against implementing.

@rottenwheel
Copy link

rottenwheel commented Feb 19, 2022

@luke-jr Yeah, that's why we should always post our legacy BTC addresses to receive donations; everyone can snoop in for eternity, right? No SegWit, that must be a bad way to do it too. We need to advise against BIP47 and SegWit, all the way.

A toast for Bitcoin's efficiency and privacy!

@ElmoOutlaw
Copy link

Let's not get ahead of ourselves here!
We have spent years trying to ignore BIP47 and have spent a huge deal of energy and time trying to push the narrative against the Red Wall that we just can't possibly start to implement it into our wallets.
Don't get me wrong it's nothing to do with privacy, it's more of a save face kinda thang.

Just HODL thru it y'all.

@BitcoinQnA
Copy link

The goal of a reusable donation address is great, but BIP47 is a bad way to do it. Recommend against implementing.

@luke-jr It would be useful if you could explain the technicals behind why you think this. Saying 'it's bad' doesn't help.

@apemithrandir
Copy link

Are we doing a bounty for this one?
Some of the peeps were sceptical of a pr merge into Bluewallet
(BlueWallet/BlueWallet#2883).
I could offer a first to implement bounty on my 500k sats from Bluewallet.
Not sure if that is acceptable within the gentleman's agreements.

@afilini
Copy link
Member

afilini commented Feb 21, 2022

I'm gonna try to work on this myself since there's so much demand for it, I might have to fix a few things along the way but it shouldn't take too long

The goal of a reusable donation address is great, but BIP47 is a bad way to do it. Recommend against implementing.

@luke-jr with BDK we are trying to support as much as we can in terms of existing standards, so I think it makes sense to implement it even if it might not be the best option for everyone. We had a similar discussion in #534 about BIP69, and in the end we decided to just add a warning and avoid removing the implementation entirely, in case somebody needs it for compatibility with older protocols.

If you can provide a technical explanation we could consider doing the same for BIP47 and provide a link to this discussion in the warning message.

@artdesignbySF
Copy link

The goal of a reusable donation address is great, but BIP47 is a bad way to do it. Recommend against implementing.

@luke-jr It would be useful if you could explain the technicals behind why you think this. Saying 'it's bad' doesn't help.

I am always interested to learn why things are bad for bitcoin. Please explain.

@notmandatory
Copy link
Member

notmandatory commented Mar 2, 2022

@luke-jr Yeah, that's why we should always post our legacy BTC addresses to receive donations; everyone can snoop in for eternity, right? No SegWit, that must be a bad way to do it too. We need to advise against BIP47 and SegWit, all the way.

@rottenstonks there is no need to get snarky or ad hominem in this discussion.

@notmandatory
Copy link
Member

notmandatory commented Mar 3, 2022

I've done a little googling and not found any good analysis about BIP47. There may be other issues but I suspect one of the concerns is with the on-chain "Notification Transactions", in particular:

  1. on-chain transactions are used for messaging (to communicate payment codes) and data storage (to recover payment codes from an HD seed);
  2. they must be created for every pair of senders/receivers so N^2 transactions for N users, which isn't great scaling.

An interesting, related proposal for "Reusable Taproot Addresses" looks to reduce the "Notification Transaction" overhead of BIP47.

EDIT: BIP47v3 appears to use a similar approach as "Reusable Taproot Addresses" of using TX outputs to reduce on-chain footprint of transmitting payment codes.

@satrinity402
Copy link
Author

While BIP47 isn’t perfect, it’s currently the only way to receive bitcoin funds privately on chain from a static text string/QR code. Adding it to BDK will at least give wallet devs the option of integrating it into their wallets.

Sparrow Wallet just implemented BIP47 today: https://github.com/sparrowwallet/sparrow/releases/tag/1.6.0

@afilini
Copy link
Member

afilini commented Mar 4, 2022

I'm having some issues with BIP47, it doesn't really integrate well with our descriptors-everywhere model.

Since I see a few Sparrow/Samourai people in here, do you think there's any room for changes to the BIP, since it's still in draft? I would still implement the current thing, but I'd like to see it disappear going forward...

Mainly I would change the way the shared secret is applied to a payment code: rather than deriving the nth key first and then applying some tweaking, I would tweak the xpub directly and then derive children on that, which would make it possible to express the entire alice-bob wallet in a single descriptor as pkh(<tweaked_xpub>/*)

@satrinity402
Copy link
Author

I'm having some issues with BIP47, it doesn't really integrate well with our descriptors-everywhere model.

Since I see a few Sparrow/Samourai people in here, do you think there's any room for changes to the BIP, since it's still in draft? I would still implement the current thing, but I'd like to see it disappear going forward...

Mainly I would change the way the shared secret is applied to a payment code: rather than deriving the nth key first and then applying some tweaking, I would tweak the xpub directly and then derive children on that, which would make it possible to express the entire alice-bob wallet in a single descriptor as pkh(<tweaked_xpub>/*)

I posed the question to a few guys on twitter that have implemented v1 BIP47. TDev responded with a tweet thread that may provide some useful info:

https://twitter.com/samouraidev/status/1499796801733419009?s=21

@decentralizedb
Copy link

I am for this enhancement and would love to see it adopted by all major wallets.

+1 I would love to see this implementation adopted by more wallets.

afilini added a commit to afilini/bdk that referenced this issue Mar 15, 2022
Add three new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `StatefulBlockchain` is the opposite of `StatelessBlockchain`: it
  provides a method to "clone" a `Blockchain` with an updated internal
  state (a new wallet checksum and, optionally, a different number of
  blocks to skip from genesis). Potentially this also allows reusing the
  underlying connection on `Blockchain` types that support it.
- `MultiBlockchain` is a generalization of this concept: it's
  implemented automatically for every type that implements
  `StatefulBlockchain` and for every `Arc<T>` where `T` is a
  `StatelessBlockchain`. This allows a piece of code that deals with
  multiple sub-wallets to just get a `&B: MultiBlockchain` without having
  to deal with stateful and statless blockchains individually.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter stateful). It hasn't been
implemented on the CBF blockchain, because I don't think it would work
in its current form (it throws away old block filters, so it's hard to
go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 15, 2022
Add three new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `StatefulBlockchain` is the opposite of `StatelessBlockchain`: it
  provides a method to "clone" a `Blockchain` with an updated internal
  state (a new wallet checksum and, optionally, a different number of
  blocks to skip from genesis). Potentially this also allows reusing the
  underlying connection on `Blockchain` types that support it.
- `MultiBlockchain` is a generalization of this concept: it's
  implemented automatically for every type that implements
  `StatefulBlockchain` and for every `Arc<T>` where `T` is a
  `StatelessBlockchain`. This allows a piece of code that deals with
  multiple sub-wallets to just get a `&B: MultiBlockchain` without having
  to deal with stateful and statless blockchains individually.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter stateful). It hasn't been
implemented on the CBF blockchain, because I don't think it would work
in its current form (it throws away old block filters, so it's hard to
go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
@GBKS
Copy link

GBKS commented Mar 16, 2022

Just want to post a clarification because "PayNym" is mentioned in the title, and I think some people think it's the same as BIP 47. "PayNym" is a custom implementation by Samourai which adds user-friendly handles ("+christoph") via a centralized directory. BIP 47 by itself does not give you nice handles, if that's what anyone is expecting.

@artdesignbySF
Copy link

I'm having some issues with BIP47, it doesn't really integrate well with our descriptors-everywhere model.

Since I see a few Sparrow/Samourai people in here, do you think there's any room for changes to the BIP, since it's still in draft? I would still implement the current thing, but I'd like to see it disappear going forward...

Mainly I would change the way the shared secret is applied to a payment code: rather than deriving the nth key first and then applying some tweaking, I would tweak the xpub directly and then derive children on that, which would make it possible to express the entire alice-bob wallet in a single descriptor as pkh(<tweaked_xpub>/*)

I don't understand much of this.. but I am interested to lean if descriptors and bip47 can work without hitch together.

@afilini
Copy link
Member

afilini commented Mar 16, 2022

Yeah I guess I should clarify this: I'm not working on an integration with "paynyms", because that's essentially a centralized directory maintained by Samourai (and I assume it's fairly trivial to connect and upload/download payment codes anyway if you anybody wants that).

What I'm working on is a way to send a payment from a BDK wallet to a BIP47 payment code.

but I am interested to lean if descriptors and bip47 can work without hitch together.

The answer is: kinda. There are a few things that are not very descriptor-friendly, or "light-wallet" friendly in general. But it can work, I'm almost done with the implementation and I'm working on upstreaming patches progressively.

@satrinity402
Copy link
Author

Yeah I guess I should clarify this: I'm not working on an integration with "paynyms", because that's essentially a centralized directory maintained by Samourai (and I assume it's fairly trivial to connect and upload/download payment codes anyway if you anybody wants that).

What I'm working on is a way to send a payment from a BDK wallet to a BIP47 payment code.

but I am interested to lean if descriptors and bip47 can work without hitch together.

The answer is: kinda. There are a few things that are not very descriptor-friendly, or "light-wallet" friendly in general. But it can work, I'm almost done with the implementation and I'm working on upstreaming patches progressively.

Will people also be able to receive bitcoin via their own generated BIP47 payment code? Or will only sends work in this first iteration?

@afilini
Copy link
Member

afilini commented Mar 17, 2022

Yes sorry, I meant that I'm not working on sending to paynym but only sending to payment code.

But receiving also works, and if you want to be integrated with the directory you could generate your own payment code (which would be handled by BDK) and upload it to the paynym directory (which would have to be implemented externally)

afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Mar 23, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
@ghost
Copy link

ghost commented Mar 23, 2022

Is there any reason to implement this in-house as opposed to using https://github.com/rust-bitcoin/rust-bip47?

@afilini
Copy link
Member

afilini commented Mar 24, 2022

See my answer here.

In short, this does a lot more compared to rust-bip47, which can mainly be used to generate addresses. Addresses are not useful to us, we need descriptors, so it seemed wasteful to add that extra dependency just to use a couple of structs.

@LLFourn
Copy link
Contributor

LLFourn commented Mar 30, 2022

Attempting to keep discussion of merits of BIP47 away from the PR #574. In there @afilini mentioned:

Also, my personal opinion is that BIP47 is pretty cool: it works with zero interaction between sender and receiver, it's relatively easy to get it working on light clients, while some of the other protocols that try to achieve the same goal are totally impossible to implement.

Why is this impossible to implement? There are several reasons I can see why it might be tricky atm like it's a WIP and needs TR support in BDK but is there something else?

@afilini
Copy link
Member

afilini commented Apr 2, 2022

I think I meant to say that it's impossible to implement on light-clients, but since BDK is mainly designed for clients that are not constantly online it also applies here: basically you need to scan every tx, from every block (plus mempool if you wanna see unconfirmed txs) to look for transactions paying to you.

It's heavy in terms of bandwidth and computation, which are both very scarce resources in light clients.

@LLFourn
Copy link
Contributor

LLFourn commented Apr 4, 2022

Ah ok bandwidth and computation are good points. I'd say this is a far cry away from impossible though. It's helpful to weigh the extra computational and bandwidth cost against the poor privacy offered by BIP47:

  1. You can use chain analysis to link a notification payment to the sender's address cluster.
  2. It is highly likely that some people will make payment from the same cluster they did the notification payment from.
  3. Now you look for outputs from clusters that made notification payments that are spent together and you have identified the receiver's wallet.

In other words, in practice, BIP47 does little for privacy unless you can make sure everyone paying you doesn't link the notification tx to their payments. You must also avoid spending the coins you receive together. In order for BIP47 to be have practical benefit it has to be linked with a coinjoin or other mixing service on the sender and receiver end or both. This is probably why the only existing BIP47 implementations are coupled with wallets that do coinjoins.

On the other hand silent payments can't be identified from passive chain analysis and do not have sophisticated behaviour requirements for senders or receiver to be private against passive blockchain observers. It is true that they take more bandwidth -- but this bandwidth does not necessarily need to be consumed on a resource constrained device. The proposal makes possible a "scanning key" which can outsource the computation. Also note that in the worst case BIP47 requires you do download every block too at least in the period of time of the payments. I speculate that during the trucker thing (which is what inspired this issue) they would have to have downloaded almost every block for a time since they were getting so many donations.

afilini added a commit to afilini/bdk that referenced this issue Apr 13, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 13, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 13, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 13, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 13, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 15, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 15, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 15, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 15, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue Apr 15, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
@afilini
Copy link
Member

afilini commented Apr 19, 2022

Yes it's true that I may have been a bit too harsh on them, it's not necessarily impossible but definitely challenging.

We can try implementing silent payments as well in BDK once the spec is more or less finalized. We may have to make changes to the blockchain traits though, or at least add a new trait to extend them and get block notifications or something (cc #527 #306 )

I agree that notification transactions can easily dox you and your network, so here's what I'm planning to do: we can add a deprecation warning on the API to send payments and notification transactions: in that way, if you just use our implementation to generate a payment code and receive donations you don't get any scary warnings, but as soon as you start sending payments to others you'll get a message telling you to be careful with coin selection.

For completeness, I have a couple of counter-arguments to your comment:

You must also avoid spending the coins you receive together.

This is true for every wallet in general, even for silent payments.

I speculate that during the trucker thing (which is what inspired this issue) they would have to have downloaded almost every block for a time since they were getting so many donations.

Downloading blocks because you've received a donation is great! I think we would all be happy to trade 1MB of data for ad on-chain donation, which i guess would be easily at least 5$. The problem is when you constantly download blocks and never receive anything.. In a way we could say that BIP47 has a linear relationship between bandwidth usage and number of payments received, while silent payments have a very large constant usage, which is essentially the worst case for BIP47 (every block contains a payment for you).

@LLFourn
Copy link
Contributor

LLFourn commented Apr 19, 2022

I agree with the above except for:

You must also avoid spending the coins you receive together

This is true for every wallet in general, even for silent payments.

To be clear my claims are with respect to passive observers. Spending silent payments together provides no information to a passive observer. It's the same as if you had a BTC pay server giving out unique addresses and spending some of them together (actually it's better since there's no IP traffic going to your particular pay server). On the other hand spending BIP47 payments together can easily identify the wallet to a passive observer if any of the senders had a signal tx linked to the payment tx. This is an important difference in privacy guarantees.

afilini added a commit to afilini/bdk that referenced this issue May 9, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
afilini added a commit to afilini/bdk that referenced this issue May 9, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
notmandatory added a commit that referenced this issue May 11, 2022
…tiple wallets

8795da4 wallet: Move `wallet_name_from_descriptor` above the tests (Alekos Filini)
9c405e9 [blockchain] Add traits to reuse `Blockchain`s across multiple wallets (Alekos Filini)
2d83af4 Move testutils macro module before the others (Alekos Filini)

Pull request description:

  ### Description

  Add three new traits:
  - `StatelessBlockchain` is used to tag `Blockchain`s that don't have any wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
  - `StatefulBlockchain` is the opposite of `StatelessBlockchain`: it provides a method to "clone" a `Blockchain` with an updated internal state (a new wallet checksum and, optionally, a different number of blocks to skip from genesis). Potentially this also allows reusing the underlying connection on `Blockchain` types that support it.
  - `MultiBlockchain` is a generalization of this concept: it's implemented automatically for every type that implements `StatefulBlockchain` and for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a piece of code that deals with multiple sub-wallets to just get a `&B: MultiBlockchain` without having to deal with stateful and statless blockchains individually.

  These new traits have been implemented for Electrum, Esplora and RPC (the first two being stateless and the latter stateful). It hasn't been implemented on the CBF blockchain, because I don't think it would work in its current form (it throws away old block filters, so it's hard to go back and rescan).

  This is the first step for #549, as BIP47 needs to sync many different descriptors internally.

  It's also very useful for #486.

  ### Notes to the reviewers

  This is still a draft because:
  - I'm still wondering if these traits should "inherit" from `Blockchain` instead of the less-restrictive `WalletSync` + `GetHeight` which is the bare minimum to sync a wallet
  - I need to write tests, at least for rpc which is stateful
  - I need to add examples

  ### Checklists

  #### All Submissions:

  * [x] I've signed all my commits
  * [x] I followed the [contribution guidelines](https://github.com/bitcoindevkit/bdk/blob/master/CONTRIBUTING.md)
  * [x] I ran `cargo fmt` and `cargo clippy` before committing

  #### New Features:

  * [x] I've added tests for the new feature
  * [x] I've added docs for the new feature
  * [x] I've updated `CHANGELOG.md`

ACKs for top commit:
  rajarshimaitra:
    tACK 8795da4

Tree-SHA512: 03f3b63d51199b26a20d58cf64929fd690e2530f873120291a7ffea14a6237334845ceb37bff20d6c5466fca961699460af42134d561935d77b830e2e131df9d
atalw pushed a commit to atalw/bdk that referenced this issue May 16, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
william-devinci pushed a commit to william-devinci/bdk that referenced this issue Nov 16, 2022
Add two new traits:
- `StatelessBlockchain` is used to tag `Blockchain`s that don't have any
  wallet-specic state, i.e. they can be used as-is to sync multiple wallets.
- `BlockchainFactory` is a trait for objects that can build multiple
  blockchains for different descriptors. It's implemented automatically
  for every `Arc<T>` where `T` is a `StatelessBlockchain`. This allows a
  piece of code that deals with multiple sub-wallets to just get a
  `&B: BlockchainFactory` to sync all of them.

These new traits have been implemented for Electrum, Esplora and RPC
(the first two being stateless and the latter having a dedicated
`RpcBlockchainFactory` struct). It hasn't been implemented on the CBF
blockchain, because I don't think it would work in its current form
(it throws away old block filters, so it's hard to go back and rescan).

This is the first step for bitcoindevkit#549, as BIP47 needs to sync many different
descriptors internally.

It's also very useful for bitcoindevkit#486.
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: No status
Development

Successfully merging a pull request may close this issue.