Substrate: Sync imports same blocks repeatedly

[rphmeier]

This is a violation of the interface of the sync service and generally indicates that something is going wrong. Sanity checks in other components make this non-critical but generally should be addressed as it is a waste of bandwidth.

[rphmeier]

(footprint label seems relevant; it's often downloading the same block hundreds of times -- that's a big waste of network bandwidth)

[gavofyork]

footprint is meant for enhancements that reduce memory footprint.

this is a bug. perhaps its side-effect is also to reduce footprint but that's moot. we label with only the most severe and/or precise (i.e. the highest up the list).

other examples:

• all panics are bugs.
• bugs tend to be annoyances.
• optimisations are enhancements.

we only label with the most important one.

[gterzian]

I will take a look...

[gterzian]

@rphmeier Do you mean that sync sends a GenericMessage::BlockRequest for blocks it has already seen, or that is handles redundant incoming BlockResponse(or something else)?

[rphmeier]

@gterzian I'm not very familiar with the sync internals; All I know is that the GRANDPA block import worker often has the same blocks fed to it (often 10+ times in succession). The sync service should neither:

• download the same block more than once (wasted effort)
• feed the block to the import worker more than once

and I'm not sure which of these is happening here.

[gavofyork]

is this fixed yet @rphmeier @arkpar ?

[arkpar]

It should not be happening anymore, but it would be nice to verify that and add some additional checks of block status before handing blocks to GRANDPA verification.

[gavofyork]

ok - maybe one for the chaos testnet then. @gterzian @rphmeier