polkadot-sdk icon indicating copy to clipboard operation
polkadot-sdk copied to clipboard

Remove the prospective-parachains subsystem from collators

Open alindima opened this issue 9 months ago • 2 comments

Implements https://github.com/paritytech/polkadot-sdk/issues/4429

Collators only need to maintain the implicit view for the paraid they are collating on. In this case, bypass prospective-parachains entirely. It's still useful to use the GetMinimumRelayParents message from prospective-parachains for validators, because the data is already present there.

This enables us to entirely remove the subsystem from collators, which consumed resources needlessly

Aims to resolve https://github.com/paritytech/polkadot-sdk/issues/4167

TODO:

  • [ ] fix unit tests

alindima avatar May 15 '24 14:05 alindima

I reran the original experiments from #4167 and this PR indeed fixes the issue!

skunert avatar May 16 '24 19:05 skunert

It looks like we only use the min relay parent for sanity checking before advertising collations, I would propose to remove this check instead and rely on validators rejecting it and disconnecting the collator.

There are two situations when this would happen:

  • somebody writes a bad collator that authors on ancient relay parents which is a bug, and having a strong signal as getting disconnected/banned is better than relying on this sanity check as the effect is the same - parachain doesn't make progress at all
  • the collator is so far behind in syncing the relay chain such that this check would not actually fail at all, and it will still be the validator rejecting the collation

As discussed offline, we need to keep the implicit view on the collator side regardless of whether we query it before advertising. We still need to query the allowed ancestry so that collators are able to build and advertise collations that are built on older relay parents and for pruning these collations when the relay parents go out of scope.

So we could at most remove one line of code, the one that re-checks the ancestry before advertising. But I don't think it's worth it. It's a sanity check that'll save us from advertising outdated collations

alindima avatar May 20 '24 07:05 alindima

The CI pipeline was cancelled due to failure one of the required jobs. Job name: test-linux-stable 3/3 Logs: https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6245613

paritytech-cicd-pr avatar May 20 '24 12:05 paritytech-cicd-pr