A peer could try to waste our resources by sending us unrequested blocks with low work (eg to fill up our disk). Since e265200 we no longer request blocks until we know we’re on a chain with more than nMinimumChainWork (our anti-DoS threshold), but we would still process unrequested blocks that had more work than our tip (which generally has low-work during IBD), even though we may not yet have found a headers chain with sufficient work.
Fix this and add a test.
This is only useful if tip chain work is less than minimum chain work, which I think only happens in IBD?
Edit: ready more carefully the PR description.
Maybe we should just ignore unrequested blocks if tip chain work < minimum chain work? This way this test could be moved before AcceptBlockHeader?
Concept ACK, though fForceProcessing/fRequested doesn’t do the thing you think it does, see, eg #9352 (review). @TheBlueMatt Is there a mistake here? I know we use the fForceProcessing/fRequested in a couple ways but I believe this is correct – for instance the unusual use of it in compact block processing shouldn’t be an issue, because we don’t try to reconstruct compact blocks when our chain tip is far behind. Did I miss a case?
@sdaftuar Indeed. You were right, I was confused, but this does leave the logic being added here super brittle to future changes. For those following along at home, the reason compact blocks always passing fRequested as true doesn’t break this is that it does a CanDirectFetch first, which only looks at the timestamp of our chainActive.Tip(). Thus, this logic would be broken if there were any way for an attacker to cheaply feed us a single best-chain block which had a timestamp close to today, though there should be no way to do so as other ways to get the block onto disk (and, thus, to become our chainActive.Tip()) without meeting the nMinimumChainWork test in FindNextBlocksToDownload. This anti-DoS-logic-stradling-validation-and-net_processing stuff irritates me endlessly, but I think this is fine for now (and should likely be backported in 0.15.0.2 if we do one as a part of #11531).
utACK (didn’t review test changes, which will conflict with #11531) f0940e462cb435f10ef6f052dae609655202f245
A peer could try to waste our resources by sending us unrequested blocks with
low work, eg to fill up our disk. Since
e2652002b6011f793185d473f87f1730c625593b we no longer request blocks until we
know we're on a chain with more than nMinimumChainWork (our anti-DoS
threshold), but we would still process unrequested blocks that had more work
than our tip. This commit fixes that behavior.
utACK 01b52ce.
Nit, I would squash the test with the commit adding the feature.
