Rework snapshot/downloader interface

[anacrolix]

Currently stage_snapshotsync calls the downloader API for headers, then other stuff. The responsibilities for filtering and understanding the file types has been pushed down too close to the downloader. Additionally the API provides no way for callers to distinguish what files they are interested in: The downloader slurps down anything on the disk, and assumes "completion" relates to any torrents it knows about.

Anything to do with file name filters needs to be pushed up out of the downloader, and up into chain-specific code (beyond even the stage snapshot sync code). The downloader should be agnostic of any stage concepts.

Probably the logging of progress should be moved out of the downloader too. The API should probably be providing the ability to add, remove, and query the state of files from the downloader.

This might span more than just network/p2p. @mh0lt has a great vision for this.

[VBulikov]

VBulikov added Imp2 as Importance

[sudeepdino008]

and up into chain-specific code (beyond even the stage snapshot sync code)

I think stage_snapshot sync is code is perfectly fine to hold the "stage concepts". Do you see stage_snapshot sync as part of downloader or something which is using the downloader?

As such for me downloader is fundamentally the current erigon-lib/downloader module. It doesn't have any stage logic etc. The only think I don't like there is the downloader/snaptype module which is aware of entities and their files parsing etc. If we take this out, other modules would stop relying on erigon-lib/downloader/snaptype for file parsing.

Probably the logging of progress should be moved out of the downloader too. The API should probably be providing the ability to add, remove, and query the state of files from the downloader.

yes.

The downloader should be agnostic of any stage concepts.

agreed.

[anacrolix]

and up into chain-specific code (beyond even the stage snapshot sync code)

I think stage_snapshot sync is code is perfectly fine to hold the "stage concepts". Do you see stage_snapshot sync as part of downloader or something which is using the downloader?

Using the downloader. I guess it's not technically part of the interface, but the downloader does a few things to help facilitate the way the stage sync works that it should need to if the API was different

As such for me downloader is fundamentally the current erigon-lib/downloader module. It doesn't have any stage logic etc. The only think I don't like there is the downloader/snaptype module which is aware of entities and their files parsing etc. If we take this out, other modules would stop relying on erigon-lib/downloader/snaptype for file parsing.

I need to double check but I think the coupling between downloader and snaptype has been lessened now.

[anacrolix]

Downloader depends on snaptype for:

• Some webseed utilities that should be deprecated eventually.
• Various utilities for deciding if something should be seedable.
• Some snapshot upload stuff with rclone? Not sure this should be here.

If the decisions about what to pass to the downloader for various purposes is moved out of the downloader as discussed, I think that will make it possible to remove the downloaders knowledge of snaptype.

[AskAlexSharov]

before changing API - please add higher-level tests for calling Delete method: #16622
now: we have only too low-level unit-tests