Added support for MSG_FILTERED_WITNESS_BLOCK messages. #10350

1. 
   CodeShark commented at 0:59 am on May 7, 2017: contributor

   After much deliberation and a few attempts at other approaches to provide a workable near-term sync mechanism for thin clients that require witness data, I decided to just go with the simplest short-term path with the least amount of complications expecting BIP37 to be completely replaced eventually - hopefully in the not-very-distant future.

   I believe the approach of filtering blocks on the client is a simpler and superior approach for most use cases than requiring the server to perform the filtering. I believe @Roasbeef has something written up for this that he’s using for lnd. I would love to see that approach used in Bitcoin Core as well.

   But given the good likelihood of nearterm SegWit activation on the Bitcoin mainnet, I believe this solution will suffice for all essential use cases of BIP37 for now - and I don’t believe it’s worth the effort to try to make more complex additions to BIP37 since it will eventually be entirely replaced.

   Peers can request MSG_FILTERED_WITNESS_BLOCK and will receive a merkleblock structure with transactions serialized with witnesses. The merkle proof for the witnesses is not supplied. This means that the client cannot verify that the witness data is what’s actually in the block. However, the attack vectors here given the actual intended use cases seem extremely slim for several reasons:

   1. The witness data contains signatures which the client can still verify. Spoofing the witness would require supplying signatures that still redeem the output, meaning that only parties that can sign for the output could produce false witness data.

   2. In order to use BIP37 with any real degree of privacy and security, you need to connect to a trusted node. If this is your setup, adding merkle proofs for witnesses is an unnecessary complication.

   3. The foreseen intended use cases here are wallets that support multisignature scripts or scripts with multiple execution paths where you want to be able to check which signatures are provided or which execution path has been taken. In anticipated use cases, there is not much an attacker could gain from a transaction being signed in two different ways - and typically, the attacker would be easily identifiable.

2. 
   gmaxwell commented at 1:05 am on May 7, 2017: contributor

   Why trash privacy, create additional CPU attack vectors, dos attack vectors for the client, etc. to save at most 26kbit/sec? It doesn’t seem like a useful tradeoff.
3. 
   CodeShark commented at 1:08 am on May 7, 2017: contributor

   Bandwidth isn’t the main concern. Code complexity is. It’s only intended to be a very short-term solution until we can introduce clientside block filtering.
4. 
   CodeShark commented at 1:10 am on May 7, 2017: contributor

   I should add that using BIP37 with untrusted nodes is already quite insecure.
5. 
   TheBlueMatt commented at 1:19 am on May 7, 2017: member

   I also dont see the rush…SegWit isn’t gonna activate any time soon, why extend the current crap isntead of replacing it properly when there is no rush?
6. 
   CodeShark commented at 1:27 am on May 7, 2017: contributor

   @TheBlueMatt It’s already active on testnet as well as on several alts which seek to minimize the code diff.
7. 
   TheBlueMatt commented at 1:48 am on May 7, 2017: member

   @CodeShark testnet is for testing Bitcoin, I dont generally consider it a target for features? As you point out this is a rather simple patch, if some alts have a need for it, they can easily take and maintain this patch.
8. 
   CodeShark commented at 3:25 am on May 8, 2017: contributor

   My entire stack currently relies on BIP37 for synching with a node, including a number of testing tools I use. The alternatives of either maintaining a custom server (one more thick dependency) or using RPC (excruciatingly and impractically slow and inefficient) aren’t really options for me at this point. So until we do something like clientside block filtering I’m left with either using this patch or having to hack something up that’s really ugly on my end.

   I understand that there’s a desire to not pollute the codebase with stuff that most people probably will not use - so I fully understand if other devs are reluctant to want to merge this. But I do not believe it is particularly intrusive - it doesn’t really get in the way of anyone else and is easy to review and test for correctness. And it would save at least some people a nontrivial amount of additional effort in having to maintain separate branches and patches.

   I’m eager to contribute to the effort of finding a good long-term solution to the thin client sync issue and helping to implement it. But right now I need to make some important software releases, and the more I need to worry about my own software’s short-term compatibility issues the less I can focus on the bigger picture.

   Anyhow, I appreciate the consideration.

9. fanquake added the label P2P on May 8, 2017
10. Handle MSG_FILTERED_WITNESS_BLOCK messages b75d60a025
11. Protocol version bump for MSG_FILTERED_WITNESS_BLOCK fcf7d60071
12. CodeShark force-pushed on Aug 12, 2017
13. MarcoFalke added the label Needs rebase on Jun 6, 2018
14. 
    MarcoFalke commented at 4:32 pm on August 25, 2018: member

    Closing for now. Let me know when you want to continue working on this
15. MarcoFalke closed this on Aug 25, 2018

    ---

16. laanwj removed the label Needs rebase on Oct 24, 2019
17. fanquake referenced this in commit f2d1b9881f on Aug 18, 2020
18. sidhujag referenced this in commit 61201a625c on Aug 18, 2020
19. MarcoFalke locked this on Dec 16, 2021

Contributors
CodeShark gmaxwell TheBlueMatt MarcoFalke

Labels
P2P