multi: Add batched cfilter fetching messages #3211
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
matheusd
force-pushed
the
test-multi-cf
branch
from
3b94d59
to
5a7ccf2
Compare
matheusd
commented
Nov 8, 2023
matheusd
commented
Nov 8, 2023
matheusd
commented
Nov 8, 2023
matheusd
commented
Nov 8, 2023
matheusd
force-pushed
the
test-multi-cf
branch
from
5a7ccf2
to
ebac834
Compare
This was referenced Nov 17, 2023
davecgh
added
the
wire protocol change
davecgh
requested changes
May 12, 2024
There was a problem hiding this comment.
Sorry it took so long to review this. I totally let it fall through the cracks.
I've reviewed the first commit and this is looking super nice overall! I really appreciate the effort put into make it easy to review and also determining all the worst case scenarios and adding tests to exercise them.
I figured I'd push the review thus far so you can start addressing the various (minor) points.
Also, please rebase this to the latest master. I've done it locally for testing, but needs to be done on the PR too.
davecgh
requested changes
May 12, 2024
davecgh
requested changes
May 12, 2024
davecgh
reviewed
May 12, 2024
This will be helpful in determining the max number of cfilters to return in a future batched getcfilter message. This will also nail down the maximum size of the filter based on the constants used throughout the project, such that if any of them changes, causing the max cfilter size to change, a test will break indicating the need to review any code paths that assume a max cfilter size.
|
Thank you for the review! I believe I addressed all the review items. |
davecgh
reviewed
May 13, 2024
|
Reviewed all the updates and they look good. The only thing remaining is my latest comment and then I'll get this merged. 💯 |
This adds the getcfsv2 and cfiltersv2 messages. These messages are intended to allow nodes in the P2P network to request and receive a batch of version 2 committed filters that span multiple sequential blocks in a chain. These messages are intended to be used when syncing SPV clients, which require requesting all committed filters on the current main chain and currently use a large number of getcfilter/cfilter messages. One of the critical issues in the design for these messages is the MaxCFiltersV2PerBatch constant, which establishes an upper bound on the max number of cfilters requested and replied. The current value of this constant was decided based on the max block size for all of the currently deployed networks, the max possible filter size for transactions in such a block and the max P2P message size. A runtime check is added to ensure any changes to the constants that were involved in deciding this number trigger a panic so that this bound is verified and, if needed, a new protocol version is introduced to update it. This check is meant to guard against inadvertedly changing the assumptions that went into establishing this bound without reviewing it.
This function will be used to reply to the recently introduced getcfsv2 P2P message. The LocateCFiltersV2 function fetches a batch of cfilters from the database and encodes it in a wire.CFiltersV2 message, ready to be sent to remote peers.
This adds the appropriate processing to the peer and server structs to respond to the recently introduced getcfsv2 message. It also bumps the peer and server max supported protocol versions to version 10 (BatchedCFiltersV2Version). This message queries the chain for a batch of committed filters spanning a set of sequential blocks and will be used by SPV clients to fetch committed filters during their initial sync process.
davecgh
approved these changes
May 13, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Close #3206
This adds the getcfsv2/cfiltersv2 set of messages, intended to be used by SPV clients to fetch batches of version 2 committed filters during their initial sync process.
Summary of changes: