On-demand headers relay: error in the relay_mandatory_header_from_range

[svyatonik]

Right now on-demand relays are only used in practice to sync mandatory headers in Polkadot <> Kusama bridge - they are not actually on-demand relays anymore (because we use batch transactions there). However, there's a dumb condition in the code which is wrong when it is used like that. So here we check that the required_header_number >= mandatory_source_header_number and then we set it like that: *required_header_number = mandatory_source_header_number;.
So if relay finds a mandatory header 100, it first sets the required_header_number to 100 and in next iteration it checks if 100 >= 100 and does not relay next mandatory header. We should fix it, even though this relayer will hopefully be abandoned soon.

[svyatonik]

OTOH we are starting headers relay that may relay mandatory headers IIRC, so why it was not syncing? Needs additional investigation

[svyatonik]

OTOH we are starting headers relay that may relay mandatory headers IIRC, so why it was not syncing? Needs additional investigation

Ok, it also reads the best known source chain header from the same required_header_number => it can not proceed

[svyatonik]

Ok, let's not touch that code right now - in the future, I hope, we will drop support for complex relayers and right now we don't need the fix anymore. Ill leave the issue open, though - maybe we'll see the same issue again. The most obvious and the least intrusive fix is to simply change the type of required_header_number from simple Option<BlockNumber> to Option<enum { ActuallyRequired(BlockNumber), SelectedMandatory(BlockNumber) }>. The underlying relayer should see the value from ActuallyRequired and we internally (in the on-demand relay) may also use the ActuallyRequired variant when we want to sync mandatory headers.

UPD: why I think it'd be better not to touch it now is because affected relayer is only used in production bridge - Rococo<>Westend is already migrated to use separate relayers set. So it'd be hard to battle test it before it'll reach prod

[svyatonik]

Not actual with the #3002