eth/downloader: fix error aggregator

[rjl493456442]

This PR fixes an incorrect error aggregation in downloader.

Downloader has a few components used to sync up chain. Whenever an error occurs in any of
these components, the sync procedure needs to be aborted and the error is expected to be
bubbled up. This error is an important marker, one use case is as an indicator if transactions
from network should be accepted or not.

However this error aggregation is wrong. Previously the aggregated error is always overwritten
by the returned value from components, but the fact is some components(e.g. processFullSyncContent)
won't return error in some cases.

This PR fixes this error, and I believe it can prevent accepting transactions when syncing.

[holiman]

To clarify.. if the sequence of errors is errCanceled, errCanceled, nil, then the original code would return nil, and the new code would return errCanceled.

The outer handling looks like this:

		// If the downloader fails, report an error as in beacon chain mode there
		// should be no errors as long as the chain we're syncing to is valid.
		if err := b.downloader.synchronise("", common.Hash{}, nil, nil, mode, true, b.started); err != nil {
			log.Error("Beacon backfilling failed", "err", err)
			return
		}
		// Synchronization succeeded. Since this happens async, notify the outer
		// context to disable snap syncing and enable transaction propagation.
		if b.success != nil {
			b.success()
		}

So the original code would flag this as succeeded, whereas this PR would (correctly) mark it as backfilling failed.

[holiman]

LGTM

[holiman]

@karalabe ptal

[karalabe]

The original code was somewhat the same as the one being corrected to now: 7641bbe#diff-80cc409bf7f1e4a3d19174ed878e4dcb3d40b19b6e0580bd3816a174c57e0fb2L558

[karalabe]

Is there a reason to have this special case of checking for cancel and doing a break if not, or could we just break out even in case of a cancel (i.e. the original code). Feels a bit convoluted why cancel is special cased.

[rjl493456442]

Because whenever error occurs, all components will be aborted with errCancel returned. We special handle the errCancel in order to dig out the real error message.

[rjl493456442]

The original code was somewhat the same as the one being corrected to now: 7641bbe#diff-80cc409bf7f1e4a3d19174ed878e4dcb3d40b19b6e0580bd3816a174c57e0fb2L558

Yep

[karalabe]

SGTM?