Add new filter type v0 for segwit only Scripts to blockfilterindex #18223

pull dangershony wants to merge 9 commits into bitcoin:master from dangershony:nutrino-p2wpkh-filters changing 4 files +115 −8
  1. dangershony commented at 2:00 pm on February 28, 2020: contributor

    Introduce a new BlockFilter type [v0] next to the existing basic filter type, which is described in BIP 158. https://github.com/bitcoin/bips/blob/master/bip-0158.mediawiki#block-filter

    Resolves #18222

    The BlockFilterIndex implementation allows for the addition of other filter types, this PR will add a new filter type for segwit script programs v0 types only. Light clients (especially new clients that deal with segwit addresses only) do not need any legacy script types in the filters, and with this PR can choose to only index a much smaller portion of the chain. That will greatly reduce the amount of data on disk and that is fetch over the network.

    The same GCS parameters where used as basic type.

    Total size of serialized filers as of block 619 361:

    • Basic = 4.76 GB
    • v0 = 252 MB

    Building the Index time

    • Basic = 3.5 hours
    • v0 = 3.5 hours

    Enabled in the config by:

    0blockfilterindex=p2wpkh
    

    Enabling more then one filter:

    0blockfilterindex=v0
    1blockfilterindex=basic
    
  2. Add nutrino filter for p2wpkh script types 42a93bba78
  3. Add possible values to cli description 19b9959a3c
  4. nopara73 changes_requested
  5. nopara73 commented at 2:07 pm on February 28, 2020: none
    Can you add some tests?
  6. DrahtBot added the label RPC/REST/ZMQ on Feb 28, 2020
  7. in src/blockfilter.h:91 in 19b9959a3c outdated
    87@@ -88,6 +88,8 @@ constexpr uint32_t BASIC_FILTER_M = 784931;
    88 enum class BlockFilterType : uint8_t
    89 {
    90     BASIC = 0,
    91+    // Floter 1 is reserved as option to include all filters
    


    molnard commented at 2:09 pm on February 28, 2020:
    0    // Filter 1 is reserved as an option to include all filters.
    
  8. in src/blockfilter.cpp:353 in 19b9959a3c outdated
    346@@ -309,6 +347,13 @@ bool BlockFilter::BuildParams(GCSFilter::Params& params) const
    347         params.m_P = BASIC_FILTER_P;
    348         params.m_M = BASIC_FILTER_M;
    349         return true;
    350+    case BlockFilterType::P2WPKH:
    351+        params.m_siphash_k0 = m_block_hash.GetUint64(0);
    352+        params.m_siphash_k1 = m_block_hash.GetUint64(1);
    353+        // using the same filter params as basic type
    


    molnard commented at 2:10 pm on February 28, 2020:
    0        // Using the same filter params as basic type.
    
  9. molnard changes_requested
  10. molnard changes_requested
  11. molnard commented at 2:13 pm on February 28, 2020: none
    PR name: filer => filter Add new filer type p2wpkh to blockfilterindex Add new filter type p2wpkh to blockfilterindex
  12. dangershony renamed this:
    Add new filer type p2wpkh to blockfilterindex
    Add new filter type p2wpkh to blockfilterindex
    on Feb 28, 2020
  13. nopara73 commented at 2:40 pm on February 28, 2020: none
    Note this PR and its accompanying issue (https://github.com/bitcoin/bitcoin/issues/18221) conforms to the original BIP157 filter extensibility proposal (https://github.com/bitcoin/bips/blob/master/bip-0157.mediawiki#filter-types) and to the proposal I submitted about filter combinations too: https://github.com/bitcoin/bitcoin/issues/18221
  14. Apply suggestions from code review
    Co-Authored-By: Dávid Molnár <molnardavid84@gmail.com>
    70448154c9
  15. Adding tests for p2wpkh filter type fcb3690581
  16. nopara73 commented at 5:40 pm on February 28, 2020: none
    For the record there is a discussion about that probably it should combine both P2WPKH and P2WSH scripts: #18222 (comment)
  17. fanquake added the label UTXO Db and Indexes on Feb 28, 2020
  18. Make the filter element for witness ver0 22b2ac6c38
  19. dangershony renamed this:
    Add new filter type p2wpkh to blockfilterindex
    Add new filter type v0 for segwit only Scripts to blockfilterindex
    on Mar 1, 2020
  20. nopara73 approved
  21. nopara73 commented at 2:05 pm on March 1, 2020: none
    tACK. Code LGTM. SegWit v0 filters’ size: 151MB.
  22. in src/blockfilter.cpp:340 in 22b2ac6c38 outdated
    336+        m_filter = GCSFilter(params, BasicFilterElements(block, block_undo));
    337+        break;
    338+    case BlockFilterType::V0:
    339+        m_filter = GCSFilter(params, V0FilterElements(block, block_undo));
    340+        break;
    341+    }
    


    Sjors commented at 3:58 pm on March 1, 2020:
    enumeration value 'INVALID' not handled in switch. I guess in this case default: assert(false) would be appropriate?

    dangershony commented at 11:44 pm on March 1, 2020:
    It’s handled in the code above so I left it out, but for best practice I agree so I’ll add an assert as suggested.
  23. in src/blockfilter.cpp:352 in 22b2ac6c38 outdated
    348@@ -309,6 +349,13 @@ bool BlockFilter::BuildParams(GCSFilter::Params& params) const
    349         params.m_P = BASIC_FILTER_P;
    350         params.m_M = BASIC_FILTER_M;
    351         return true;
    352+    case BlockFilterType::V0:
    


    Sjors commented at 4:12 pm on March 1, 2020:
    This case is identical to BlockFilterType::BASIC, so you can avoid the duplication.

    dangershony commented at 11:42 pm on March 1, 2020:
    ah good point, will fix.
  24. in src/blockfilter.cpp:295 in 22b2ac6c38 outdated
    290+        for (const CTxOut& txout : tx->vout) {
    291+            const CScript& scriptPubKey = txout.scriptPubKey;
    292+            int witnessversion;
    293+            std::vector<unsigned char> witnessprogram;
    294+            if (scriptPubKey.empty() || !scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) continue;
    295+            if (!(witnessversion == 0 && (witnessprogram.size() == WITNESS_V0_KEYHASH_SIZE || witnessprogram.size() == WITNESS_V0_SCRIPTHASH_SIZE)))  continue;
    


    Sjors commented at 4:16 pm on March 1, 2020:
    Instead of this new function, maybe just add these four lines to BasicFilterElements (after the OP_RETURN check) and give that two additional arguments bool only_segwit = true, int witness_version = 0

    dangershony commented at 11:53 pm on March 1, 2020:
    In that case the method name should change to BuildFilterElements no?

    dangershony commented at 12:14 pm on March 2, 2020:
    Any particular reason why defaulting segwit to true?

    dangershony commented at 1:30 pm on March 2, 2020:
    Done

    Sjors commented at 1:46 pm on March 2, 2020:
    No, it should be false I think (basic filter is the default)
  25. in src/blockfilter.cpp:306 in 22b2ac6c38 outdated
    301+        for (const Coin& prevout : tx_undo.vprevout) {
    302+            const CScript& scriptPubKey = prevout.out.scriptPubKey;
    303+            int witnessversion;
    304+            std::vector<unsigned char> witnessprogram;
    305+            if (scriptPubKey.empty() || !scriptPubKey.IsWitnessProgram(witnessversion, witnessprogram)) continue;
    306+            if (!(witnessversion == 0 && (witnessprogram.size() == WITNESS_V0_KEYHASH_SIZE || witnessprogram.size() == WITNESS_V0_SCRIPTHASH_SIZE))) continue;
    


    Sjors commented at 4:23 pm on March 1, 2020:
    The basic filter undo code doesn’t have a check for OP_RETURN. @jimpo was that because the difference on undo data is negligible? I suppose that’s not the case here though, but then I’d prefer adding OP_RETURN as well for readability.

    dangershony commented at 11:48 pm on March 1, 2020:
    Is opreturn not already filtered out with the witness check? Or you mean to add the opreturn check to the blockundo on the code of the basic filter type?

    Sjors commented at 8:26 am on March 2, 2020:
    If you combine this with the code from the basic filter then it’s probably more readable to OP_RETURN back for that as well (assuming that doesn’t break anything). For the v0 filter it indeed doesn’t matter.

    dangershony commented at 12:11 pm on March 2, 2020:
    Shall I add the OP_RETURN check also on the spent script then? I agree with you I see no reason why it’s omitted.

    dangershony commented at 1:30 pm on March 2, 2020:
    Done
  26. Sjors commented at 5:00 pm on March 1, 2020: member

    Travis is failing do to a whitespace linter issue.

    I left some code review comments; it’s generally not advisable to spend too much time improving code before a concept ACK. I’m happy to review, as I’m inclined to concept ACK this, but we don’t want to be stuck with an abandoned filter type (even though it looks pretty low maintance).

  27. Act on review dac4d5a74f
  28. Sjors commented at 8:31 am on March 2, 2020: member
    You may want to squash all this into 1 commit (perhaps a separate commit to add OP_RETURN to the basic undo filter).
  29. Move the filter logic to a single method named BuildFilterElements 3055ac89c1
  30. Change the BuildFilterElements default args dc1fe756fc
  31. dangershony commented at 9:41 pm on March 2, 2020: contributor
    Thank you @Sjors for your helpful review, I will squash the commits when there is a general ack.
  32. MarcoFalke commented at 11:25 am on March 3, 2020: member
    Tend to NACK. I fail to see a use case for this, and a missing use case does not justify the time spent on code review and the resulting complexity/user confusion and maintenance overhead when this is merged.
  33. nopara73 commented at 5:17 pm on March 3, 2020: none
    What do you mean no use case? It’s already being used and in production for 1.5 years now.
  34. luke-jr commented at 3:12 pm on March 4, 2020: member

    Concept NACK if I understand this correctly.

    If we assume everything will be Segwit eventually, this filter will just become redundant with the basic one?

    If the goal is to reduce the initial download, couldn’t the wallet just start with its birthdate block anyway?

    Plus, remotely-accessible filters are still a bad idea in the first place, which this seems to presuppose…

  35. MarcoFalke commented at 3:18 pm on March 4, 2020: member

    What do you mean no use case? It’s already being used and in production for 1.5 years now.

    If there is a use case internal to the Wasabi wallet, that is fine. However, for anyone else (and probably also Wasabi wallet), they are better off using the existing BIP 158 filters.

  36. in src/blockfilter.cpp:285 in 3055ac89c1 outdated
    283 
    284     for (const CTxUndo& tx_undo : block_undo.vtxundo) {
    285         for (const Coin& prevout : tx_undo.vprevout) {
    286             const CScript& script = prevout.out.scriptPubKey;
    287-            if (script.empty()) continue;
    288+            if (script.empty() || script[0] == OP_RETURN) continue;
    


    luke-jr commented at 3:20 pm on March 4, 2020:
    Is this a bugfix to the existing code? Please PR it separately if so.

    luke-jr commented at 7:10 pm on March 4, 2020:
    Actually, it’s it absolutely impossible to encounter an OP_RETURN here? These are spent pubkeys, right? You can’t spend an OP_RETURN

    dangershony commented at 8:38 pm on March 4, 2020:
    I agree but this is in response to this comment (for readability) #18223 (review)

    luke-jr commented at 8:55 pm on March 4, 2020:

    I don’t think this makes it any more readable…

    Prefer a comment simply stating that it is impossible.


    dangershony commented at 9:10 pm on March 4, 2020:
    Reverted.

    Sjors commented at 9:49 am on March 6, 2020:
    Aargh, I totally misread that code :-)
  37. in src/blockfilter.cpp:326 in dc1fe756fc outdated
    322+        m_filter = GCSFilter(params, BuildFilterElements(block, block_undo));
    323+        break;
    324+    case BlockFilterType::V0:
    325+        m_filter = GCSFilter(params, BuildFilterElements(block, block_undo, true));
    326+        break;
    327+    default: assert(false);
    


    luke-jr commented at 3:21 pm on March 4, 2020:
    Please use case BlockFilterType::INVALID here so new enum values warn at compile time.
  38. in src/blockfilter.cpp:334 in dc1fe756fc outdated
    330 
    331 bool BlockFilter::BuildParams(GCSFilter::Params& params) const
    332 {
    333     switch (m_filter_type) {
    334+    case BlockFilterType::V0:
    335     case BlockFilterType::BASIC:
    


    luke-jr commented at 3:32 pm on March 4, 2020:
    Let’s keep the order BASIC,V0 here?
  39. Act on review 5561e7a0c7
  40. nopara73 commented at 10:06 am on March 5, 2020: none
    @dangershony Let’s discuss alternative solutions. If I wouldn’t be so biased I think even I would NACK this with P2P in mind. (Not for RPC only though.)
  41. dangershony commented at 1:34 pm on March 5, 2020: contributor
    After internal discussions we decided to close this PR (thanks for providing review)
  42. dangershony closed this on Mar 5, 2020

  43. MaxHillebrand commented at 2:20 pm on March 5, 2020: none

    Thanks for the review everyone!

    Though I am sad about the concept NACK. I had high hopes that with customizable block filters served over RPC, we make a great step towards further separating the wallet functionality from Core, making it more compatible with dedicated wallet software. Wasabi is currently segwit v0 only, and it will be segwit v1 taproot only as soon as possible [similar to lightning wallets too]. Thus the custom filters have a superb use case, as I see it. Basic filters are unfortunately not feasible for Wasabi [even after segwit activation, basic filters are 4.8 GB; compared to 250MB for segwit v0 filters]. I do not know of another way to provide full verification while preserving the “light client feel” of Wasabi. This means, very unfortunately, that Wasabi will not have full verification for the foreseeable future, and the current Bitcoin Core integration is only a gimmicky bandwidth savings, not much more.

    Anyway, thank You all for making Core as beautiful as it is, and I am eager to continue the collaboration towards integrating it properly in Wasabi.

  44. luke-jr commented at 2:44 pm on March 5, 2020: member

    Neutrino is NOT full verification, and people falsely portraying it as such is a huge part of the problem with Neutrino. It is no safer than other SPV protocols, and less efficient.

    I don’t see why this means Wasabi will forego full validation. That doesn’t logically follow at all.

    (I suggest we find someplace to continue this topic off this GitHub PR)

  45. MaxHillebrand commented at 2:50 pm on March 5, 2020: none

    Yes @luke-jr, the current way BIP158 is used in Wasabi relies on the server to build and send valid filters - there is no verification, and it relies on trust in the server. [this is the problem that we intended to solve with this PR]. Wasabi uses block filters solely for the privacy benefits, of having a light wallet without leaking the xpub or address to any third party server.

    However, we intended to use SegWit v0 only [later SegWit v1 only] block filters generated in the Wasabi in-built bitcoind, thus every Wasabi user, who opts in to run bitcoind, can cross reference the server filters, from his own generated ones, thus having full verification.

  46. luke-jr referenced this in commit d8d243fdb9 on Mar 5, 2020
  47. luke-jr referenced this in commit 3207ef01dc on Mar 5, 2020
  48. luke-jr referenced this in commit 98ce5703f2 on Mar 5, 2020
  49. luke-jr referenced this in commit a14e1a1427 on Mar 5, 2020
  50. luke-jr referenced this in commit c60aec1ea2 on Mar 5, 2020
  51. luke-jr referenced this in commit cab88a8255 on Mar 5, 2020
  52. luke-jr referenced this in commit 7956ac6dae on Mar 5, 2020
  53. luke-jr referenced this in commit 853dafdda0 on Mar 5, 2020
  54. luke-jr referenced this in commit b0dd7e6411 on Mar 5, 2020
  55. MarcoFalke commented at 3:55 pm on March 5, 2020: member

    even after segwit activation, basic filters are 4.8 GB; compared to 250MB for segwit v0 filters

    Segwit usage is higher than 50% right now, and assuming that most wallets have a rather recent birthday, the difference should be a factor of 2 or so (clearly less than 10). Further, assuming that taproot is activated some day, it will likely show a similar or higher usage pattern. With that in mind, promising users storage requirements that can not hold over the long term anyway and converge to those of the all-including basic BIP158 filter, seems slightly misleading.

    Finally, network bandwidth shouldn’t matter when users feed filters from their local bitcoind instance.

  56. sipa commented at 4:08 pm on March 5, 2020: member
    If it’s for local RPC usage, why is a filter needed at all? You can just fetch the block, and check it for matching transactions like a pre-BIP37 wallet would. If that results in any transactions that were missing from the remotely-provided filter, you know the filter is missing things.
  57. luke-jr commented at 4:14 pm on March 5, 2020: member

    Yes @luke-jr, the current way BIP158 is used in Wasabi relies on the server to build and send valid filters - there is no verification, and trust in the server. [this is the problem that we intended to solve with this PR].

    This PR cannot solve it.

    Wasabi uses block filters solely for the privacy benefits, of having a light wallet without leaking the xpub or address to any third party server.

    There are no privacy benefits when you’re using your own node.

    However, we intended to use SegWit v0 only [later SegWit v1 only] block filters generated in the Wasabi in-built bitcoind, thus every Wasabi user, who opts in to run bitcoind, can cross reference the server filters, from his own generated ones, thus having full verification.

    There is no need for a third-party server filter. Just use your own.

  58. nopara73 commented at 4:48 pm on March 5, 2020: none

    Let me clarify a few things.

    Wasabi client uses P2WPKH only filters those are built on and fetched from the Wasabi backend. We want to make Wasabi a hybrid full node, which we’ve progressed towards gradually, to the point that only the validation function is needed to be implemented. The idea is to serve the exact same filters from the server to the clients, so the client can just verify the correct filters are provided by the server until its own local Bitcoin Core node catches up. Wasabi wallets aren’t running all the time, they are closed and reopened constantly, but ultimately what really matters is that eventually the validation happens, the when is less important, IMO.

    Porting our own filter building implementation to the client or going through every block for every wallet is no-go as it has poor performance, because going through every block with RPC is a slow process. You may take it as a given as you are working on Bitcoin Core, but the performance of Bitcoin Core is impossible to even come close to for any project.

    What just came to my mind, the most important limitation for most performance issues while communicating with Core is that we cannot just pull out the input value-script pars when getting a transaction from Core. Would that be something sensible to start working on? Like a getblock rpc request where more information on the inputs are added to the tx?

  59. luke-jr commented at 5:06 pm on March 5, 2020: member

    Like a getblock rpc request where more information on the inputs are added to the tx?

    Like #16083 ? (It’s already in Knots, so you could even use it immediately!)

  60. dangershony commented at 10:23 pm on March 5, 2020: contributor
    @luke-jr that’s great, I also noticed you merged the v0 filter commits to Knots, eta when this will go in Knots master?
  61. luke-jr commented at 11:48 pm on March 5, 2020: member
    Knots doesn’t have a master, just releases. Should be in v0.19.1 (but disabled unless specified explicitly via -blockfilterindex=v0).
  62. Sjors commented at 10:05 am on March 6, 2020: member
    It may be worth studying c-lightning; they scan all blocks for lightning transactions, no filters needed, and in my experience it’s fast. You could wrap some of their C code if it looks too magical :-)
  63. nopara73 commented at 1:08 pm on March 6, 2020: none

    Like #16083 ? (It’s already in Knots, so you could even use it immediately!)

    image

  64. harding commented at 6:51 pm on March 8, 2020: contributor

    @MaxHillebrand

    Basic filters are unfortunately not feasible for Wasabi [even after segwit activation, basic filters are 4.8 GB; compared to 250MB for segwit v0 filters].

    As @MarcoFalke noted above, this conclusion seems unlikely, so I tested the size of the current “basic” filters myself from segwit activation to present and I get a size of 2.6 GB.

    $ for i in `seq 481824 620831` ; do bitcoin-cli getblockhash $i ; done | while read hash ; do bitcoin-cli getblockfilter $hash ; done | jq -r .filter | xxd -r -p- | wc -c
    2628728815
    

    Beyond that, I’m confused by this disk space argument in the first place. If you’re getting the filter for block x from your local node, don’t you also expect to be getting the contents of block x from the same node? Blocks are ~33x larger than a full basic filter for that block, so the extra few kilobytes disk space per filter (for present usage patterns) seems particularly irrelevant. @nopara73

    The idea is to serve the exact same filters from the server to the clients, so the client can just verify the correct filters are provided by the server until its own local Bitcoin Core node catches up.

    I understand the idea is for the Wasabi application to get identical filters from either Wasabi’s server or the local Bitcoin Core node, but I want to make sure you’re aware that, as long as the filter encoding is the same, the Wasabi application should be able to use the basic filters produced by Core without any changes to Wasabi’s application logic. That’s because the application doesn’t care whether the filter was constructed out of just P2WPKH transactions or all transactions—it’s just looking for potential output/input scriptPubKey matches to its wallet.

    So, unless I’m misunderstanding something, the Wasabi application should be able to transition seamlessly between using the P2WPKH-only filters served by the Wasabi server to basic (everything) filters served by the local Bitcoin Core node after it has caught up.

  65. dangershony commented at 9:42 pm on March 8, 2020: contributor

    @harding yeah this was incorrect the 4.8GB refers to the entire basic filter data, still core has no way to store locally only basic filters form segwit activation. (also note that 2.6 GB is probably too much if we can avoid it)

    the Wasabi application should be able to use the basic filters produced by Core without any changes to Wasabi’s application logic.

    Wasabi compresses the script on the clients before trying to match it with a filter (or more correctly wasabi builds the filter with compressed scripts) so legacy clients won’t work with basic filters from core.

  66. harding commented at 10:17 pm on March 8, 2020: contributor

    2.6 GB is probably too much if we can avoid it

    2.6 is roughly 50% the minimum amount of data Bitcoin Core needs to store anyway (chainstate plus minimum prune depth) or the rough equivalent of 10 days worth of blocks and undo data at current utilization levels, so 2.6 GB seems to me to be a pretty small additional requirement on top of using Bitcoin Core in the first place.

    And, as mentioned elsewhere in this thread, you can use the wallet birthday to prune filters from blocks before the wallet was created (though I don’t think we currently support filter pruning).

    wasabi builds the filter with compressed scripts) so legacy clients won’t work with basic filters from core.

    I’m confused. What are “compressed scripts”? I skimmed this PR—it is a nice example of a minimalistic change, thanks!—and it seemed to me that it builds both the basic and v0 filters the same way, except that the v0 filter skips past any transactions with non-v0 parts:

    0if (!script.IsWitnessProgram(witnessversion, witnessprogram)) 
    1if (witnessversion != witness_version) 
    2if (!(witnessversion == 0 && (witnessprogram.size() == WITNESS_V0_KEYHASH_SIZE || witnessprogram.size() == WITNESS_V0_SCRIPTHASH_SIZE))) continue; // specific v0 checks
    

    Unless I’m missing something, that should mean that any Wasabi clients compatible with this PR would also automatically be compatible with basic filters with no changes needed (except maybe to deal with the break in the chain of filter headers).

  67. dangershony commented at 11:47 am on March 9, 2020: contributor

    You are right, this filter will sill not be compatible with wasabi (and had we gone down this path would mean we have to support two api endpoints after the upgrade) there are a few narratives in parallel here (fetch filters from local core node, fetch filters from wasabi server, maintain backwards compatibility, least disruption to current wasabi design etc…) this will explain the confusion.

    so 2.6 GB seems to me to be a pretty small additional requirement on top of using Bitcoin Core in the first place.

    Each client (not using a local core node) will fetch 2.6GB of data form the server (wasabi stores all filters locally) if we can prevent this its preferable.

    Right new it seems we probably take the approach of using Knots and constructing the same filters wasabi currently uses from a Knots node (that can serve the spent ScriptPubKey) https://github.com/zkSNACKs/WalletWasabi/issues/3208#issuecomment-595768764

    FYI compressed scripts to bytes https://github.com/zkSNACKs/WalletWasabi/blob/master/WalletWasabi/Blockchain/BlockFilters/IndexBuilderService.cs#L264

  68. harding commented at 4:32 pm on March 9, 2020: contributor

    Each client (not using a local core node) will fetch 2.6GB of data form the server (wasabi stores all filters locally) if we can prevent this its preferable.

    Yes, this can be prevented, that’s what I’ve been trying to say but I must not be communicating effectively here, so let me start from first principles (my appologies for repeating stuff you probably already know).

    The process of creating a BIP158-style filter starts with a block that’s full of transactions which each spend one or more UTXOs and which each create one or more new UTXOs. For BIP158, scriptPubKeys for all spent UTXOs and created UTXOs (excepting OP_RETURN outputs) are hashed and then put into a sorted list:

    00000000
    11111111
    22222222
    33333333
    

    The list is then golumb coded to minimize its size to produce the final BIP158 basic filter. The client then takes a list of its scriptPubKeys, hashes them the same way, and compares each of them to the entries in the filter to see if there’s any matches. E.g., the wallet matches on 22222222 and so goes and downloads the block to find the possibly-relevant transaction.

    Now, as proposed in this PR, you can create a filter using a subset of the original data (e.g. only P2WPKH):

    11111111
    22222222
    

    If you use the same hashing and golumb coding, then any client which can process the subset filter above can also process the original full set filter to find its scriptPubKey 22222222.

    This means Wasabi’s server can generate and serve the small subset filter (~250 MB currently you say) while it’s also possible for clients to use their local node’s full filter (~2.6 GB post-segwit activation). This is my point: you don’t have to choose between a 250 MB and a 2.6 GB filter—you can use them both in different contexts with no special logic in the client.

    Right new it seems we probably take the approach of using Knots and constructing the same filters wasabi currently uses from a Knots node (that can serve the spent ScriptPubKey)

    If you do indeed have a custom GCS algorithm, that does seem like it might be the simplest path forward for you, but it seems a shame to me that choosing that path would remove the possibility of compatibility with the most popular and best reviewed full node implementation (as well as all other software that’s implemented standard BIP158).

    FYI compressed scripts to bytes https://github.com/zkSNACKs/WalletWasabi/blob/master/WalletWasabi/Blockchain/BlockFilters/IndexBuilderService.cs#L264

    That looks like it’s just doing the GCS compression; I’m not sure how your code differs from the same operation as described in BIP158.

  69. luke-jr referenced this in commit 873ab17936 on Oct 10, 2021
  70. luke-jr referenced this in commit 265dece3f7 on Oct 10, 2021
  71. luke-jr referenced this in commit 88ac0ffbab on Oct 10, 2021
  72. luke-jr referenced this in commit 838b71866a on Oct 10, 2021
  73. luke-jr referenced this in commit d1b6f6c4f7 on Oct 10, 2021
  74. luke-jr referenced this in commit a59e5cc2d5 on Oct 10, 2021
  75. luke-jr referenced this in commit a1761392ea on Oct 10, 2021
  76. luke-jr referenced this in commit 98cd5efe38 on Oct 10, 2021
  77. luke-jr referenced this in commit fb3881b0b8 on Oct 10, 2021
  78. DrahtBot locked this on Feb 15, 2022

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-10-31 03:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me