"Ignore block request below NODE_NETWORK_LIMITED threshold" is skipped for noban permission in a manner which actually sends to requested block.
We should probably change it to use download permission, or simply ignore the request.
Ignoring requests could cause existing peer software to stall/timeout in theory, but I don't think any existing software will make such requests from a NODE_NETWORK_LIMITED peer in the first place?
@MarcoFalke @luke-jr was this closed by #21235?
i doubt it. 21235 was refactoring
This seems like an easy-enough logic fix, but I feel like I must be missing why we would would permit NoBan peers to request blocks from our pruned node below the NODE_NETWORK_LIMITED threshold?
After re-thinking on this issue, in my opinion it's correct to send the requested block (if it exists) to a NoBan peer here.
The NoBan permission is designed to augment the Peer with:
// Can't be banned/disconnected/discouraged for misbehavior
... which I would expect to include the "misbehaviour" of requesting blocks past the NODE_NETWORK_LIMITED threshold.
Download permission would have minimal practical impact, as NoBan implies Download.NoBan peer should be avoided if possible, and I don't think this specific misbehaviour feels strong-enough to override that guarantee.The only remaining alternative is to ignore the request but not disconnect the NoBan peer. In this case the remote node will eventually consider us stalling (after a timeout) and disconnect from us, which again achieves the opposite of the intention of the NoBan permission. If we are going to to this we may as well save everyone the time and disconnect the NoBan peer right away.
My preference would be to leave this code as-is and close this issue out; what do you think @luke-jr ?