Gap Synced blocks are kept even though they are out of pruning window

[skunert]

While reviewing #5103 I was wondering how we handle gap synced blocks together with a block pruning setting.

After warp sync reached the top and we have downloaded the state, the tip of the chain continues with normal sync operations. In the meantime, the gap between genesis and the warp target block is being downloaded from the other peers.
We issue the peer gap block requests here:

polkadot-sdk/substrate/client/network/sync/src/strategy/chain_sync.rs

Lines 2042 to 2043 in 2bd187f

	fn peer_gap_block_request<B: BlockT>(
	id: &PeerId,

Attributes to fetch are defined here and show that we fetch bodies too:

polkadot-sdk/substrate/client/network/sync/src/strategy/chain_sync.rs

Lines 1203 to 1215 in 2bd187f

	fn required_block_attributes(&self) -> BlockAttributes {
	match self.mode {
	ChainSyncMode::Full =>
	BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY,
	ChainSyncMode::LightState { storage_chain_mode: false, .. } =>
	BlockAttributes::HEADER | BlockAttributes::JUSTIFICATION | BlockAttributes::BODY,
	ChainSyncMode::LightState { storage_chain_mode: true, .. } =>
	BlockAttributes::HEADER |
	BlockAttributes::JUSTIFICATION |
	BlockAttributes::INDEXED_BODY,
	}
	}

However the pruning of block bodies only happens by pruning the current finalized block - pruning window.

polkadot-sdk/substrate/client/db/src/lib.rs

Lines 1826 to 1827 in 2bd187f

	Ok(())
	}

So my suspicion is that the gap blocks are currently all kept (but should be verified). Another question for me currently is why we even download block bodies for gap sync.

[tmpolaczyk]

So my suspicion is that the gap blocks are currently all kept

That's correct.

Another question for me currently is why we even download block bodies for gap sync.

There is some related discussion in #2710, where I tried to add a flag to disable the block history download, but then I realized that simply enabling block prunning should already skip the block download. But that doesn't work.

I would be happy if this is fixed, warp sync would use much fewer resources if it didn't have to download the block history when it is not needed.

[lexnv]

cc #2738

[nazar-pc]

I'd love to see gap sync decoupled somehow as in our blockchain we don't even attempt to sync the gap at all and had to prevent gap sync with a hack: autonomys@799ca7c

Ideally gap sync would not be initiated at all, but right now Substrate assumes gap means warp sync, which is not necessarily true for projects that do much deeper customizations than parachains typically do.

[bkchr]

I'd love to see gap sync decoupled somehow as in our blockchain we don't even attempt to sync the gap

Yeah. We need to improve this. We probably need to keep the Gap in the db. However, we should be more smart when it comes to closing this Gap.