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

spv: Move initial startup sync from per-peer to syncer #2291

Closed
wants to merge 11 commits into from

Conversation

matheusd
Copy link
Member

This PR refactors the initial sync process of the spv syncer to being global instead of per-peer.

This removes a lot of duplicated effort that the wallet performed due to the prior sync process being per peer. In particular, fetching headers and cfilters is now done only once per batch of headers instead of being repeated per peer.

Future work will fix some lingering edge cases in the syncing process and will add further performance improvements.

Part of #2289

This makes it easier to debug SPV sync issues by allowing more debugging
info for the sync process itself, as opposed to all peer messages
exchanged.
This will make it easier to perform future refactors that affect the
peer startup procedure.

The peer running loop in the syncer is slighlty refactored to remove the
need for additional contexts and wait channels.
This moves the "fetching missing cfilters" stage of syncing from
individual peer startup to the syncer startup.

This makes it possible to use multiple peers to fetch cfilters from
(which is not yet implemented) and reduces overall load by ensuring this
stage is only done once instead of being done for each new peer.
This moves the fetch headers and cfilters from the peer startup to the
syncer startup.

This allows multiple peers to be used to fetch headers and cfilters and
avoids duplicate effor in fetching them.
This moves the disconnection logic for peers with too old chains ealier
in the stack. This avoids having to perform additional work for a peer
that is not usable and makes it looking for a valid peer faster.
This moves the address discovery and rescan procedures from each
individual peer startup to the global syncer. This allows using multiple
peers for the rescan and removes the need for having a syncer global
lock, since it is now guaranteed that there is a single execution of the
rescan procedure.
This makes the remote peers configure to send block announcements via
sendheaders only after the initial sync is completed. This ensures the
initial sync process can fetch all the needed headers from any and all
peers before they are configured to send header annoucements.
The locators are now only needed during the initial getHeaders sync
stage, therefore they are not needed in the global syncer struct.

Since they are accessed only from a single goroutine now, the mutex is
also unneeded.
This will help in determining the syncing time during the refactoring.
This moves the code that initializes a batch of wallet.BlockNodes to
outside the cfilter fetching code during initial sync.

This will make it easier to move around code in later refactorings.
This modifies the SPV syncing logic to check all peers for any that have
an advertised height larger than the current wallet tip height to use as
a syncing peer.

This ensures that all peers are checked for headers during initial sync,
instead of a single one.

It also adds a new function to disconnect from peers that have fallen
behind the wallet's mainchain tip height during the initial sync,
ensuring the wallet does not hold onto a peer that can't possibly help
the syncing process.
@matheusd matheusd marked this pull request as draft November 9, 2023 14:10
@matheusd
Copy link
Member Author

matheusd commented Nov 9, 2023

Temporarily converted to draft PR as I'm splitting off some work into separate, smaller PRs so that this final one is easier to reason about.

@matheusd
Copy link
Member Author

This has been superseded by various merged PRs.

@matheusd matheusd closed this Nov 20, 2023
@matheusd matheusd deleted the refactor-fetch-cf branch November 20, 2023 14:51
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.

1 participant