Get rid of polling in WarpSync

[dmitry-markin]

This PR is part of Sync 2.0 preparation work and a step towards ChainSync as a pure state machine.

Refs #534

[altonen]

I think this is fine for now but in the future there should be some kind of syncing strategy selection like:

enum Strategy {
    Warp(WarpSync),
    Full(FullSync),
    Whatever(...)
}

enum Action {
    SendRequest(Vec<u8>),
    SendResponse(Vec<u8>),
    ImportBlock(Block),
}

enum SyncEvent {
    ResponseReceived(Vec<u8>),
}

trait Strategy {
     fn register_event(&mut self, event: SyncEvent);
     fn next_action(&mut self) -> Option<SyncAction>;
}

impl SyncingEngine {
    async fn run(mut self) {
        loop {
            tokio::select! {
                response = self.pending_responses.select_next_some() => {
                    self.strategy.register_event(SyncEvent::ResponseReceived(response));
                }
            }

            if let Some(action) = self.strategy.next_action() {
                // TODO: perform action
            }
        }
    }
}

Details of the syncing algorithm would be opaque to SyncingEngine and it would only deal with I/O and dispatch the events to the correct syncing strategy which would deal with the logic of that particular strategy and emit I/O-related events to SyncingEngine. How feasible this is to remains to be seen but worth looking into.

[bkchr]

Yeah that is a good idea. Goes into the direction of: #532

[dmitry-markin]

@altonen Thanks for the snipped provided! My idea was to gradually move all polling to the upper levels (SyncingEngine) first, otherwise it's difficult to do everything in one go. And WarpSync requiring polling was quite an annoying part, so I started from it.

[altonen]

Yes I agree this shouldn't be done in this PR and I think your approach of first gradually removing stuff away from ChainSync is good but I just wanted to highlight what the ultimate plan is.

[altonen]

LGTM. Did you verify yet that both relay and parachain warp sync work?

[dmitry-markin]

LGTM. Did you verify yet that both relay and parachain warp sync work?

I've only checked that warp sync tests are passing (both regular and "target block"). I'll manually check polkadot warp sync too. To be honest, I don't know how to test parachain warp sync. Do we have some "default" parachain full node in cumulus for such tests?

[dmitry-markin]

@altonen both relay- and parachain warp sync works (tested with polkadot & polkadot-parachain).

[altonen]

Same here, any reason this isn't merged?

[dmitry-markin]

Same here, any reason this isn't merged?

Also due to a failing zombienet test.

[dmitry-markin]

bot merge

[command-bot]

@dmitry-markin Unknown command "merge". Refer to help docs and/or source code.