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

Expose GRFilter's methods that accept a cached stream reader #1186

Merged
merged 2 commits into from
Sep 20, 2023

Conversation

lontivero
Copy link
Contributor

This allows to use call match the same filter against a different set of elements without requiring to re-read the filter data, which it really expensive.

This allows to use call match the same filter against a different set of elements without requiring to re-read the filter data, which it really expensive.
/// <param name="key">Key used for hashing the data elements.</param>
/// <param name="reader">Golomb-Rice stream reader.</param>
/// <returns>true if at least one of the elements is in the filter, otherwise false.</returns>
public bool MatchAny(IEnumerable<byte[]> data, byte[] key, GRCodedStreamReader reader)
Copy link
Collaborator

@NicolasDorier NicolasDorier Sep 16, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be static internal method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this method can be very useful in many scenarios. Just one example:

public void ProcessFilter(GolombRiceFilter filter, uint256 blockId, IEnumerable<Wallet> openedWallets)
{
   var reader = filter.GetGRStreamReader();
   foreach (var wallet in openedWallets)
   {
      if (filter.MatchAny(wallet.GetAllScriptPubKeys(), filterKey, reader))
      {
           Console.WriteLine($"The block {blockId} contains transactions relevant for wallet {wallet.Name}");
      }
   }
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be static internal method?

There are P, N, and M referenced in InternalMatchAny. So it would require additional refactoring.

Copy link
Contributor

@kiminuo kiminuo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few comments.

NBitcoin/BIP158/BitStream.cs Outdated Show resolved Hide resolved
NBitcoin/BIP158/BitStream.cs Outdated Show resolved Hide resolved
NBitcoin/BIP158/BitStream.cs Show resolved Hide resolved
/// <returns>A new cached Golomb-Rice stream reader instance</returns>
public CachedGRCodedStreamReader GetGRStreamReader()
{
return new CachedGRCodedStreamReader(new BitStream(Data), P, 0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stream data is a sorted set of ulong numbers so, it seems that caching on the fly is better than caching everything immediately. Is that the idea here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A GR filter is set of "compressed" data items. Uncompressing an item is an expensive operation. The matching algorithm uncompress items as it needs it and it stops immediately after a match is found so, it doesn't uncompress items unnecessarily (all items are uncompressed only in the worst scenario where not matching element is found in the filter)

NBitcoin/BIP158/GolombRiceFilter.cs Outdated Show resolved Hide resolved
/// <param name="key">Key used for hashing the data elements.</param>
/// <param name="reader">Golomb-Rice stream reader.</param>
/// <returns>true if at least one of the elements is in the filter, otherwise false.</returns>
public bool MatchAny(IEnumerable<byte[]> data, byte[] key, GRCodedStreamReader reader)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this can be static internal method?

There are P, N, and M referenced in InternalMatchAny. So it would require additional refactoring.

@NicolasDorier NicolasDorier merged commit ffec394 into MetacoSA:master Sep 20, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants