ouroboros-network
ouroboros-network copied to clipboard
ChainSync rate-limiting
Following #423, this PR implements a rate-limiting functionality that can be expressed as:
When a peer does a rollback from a block with block number N and then in less that one second it does another rollback from a block with block number =< N, this second rollback is delayed until one second has passed since the first rollback.
Note that the first commit is merely documentation whereas the second one has the actual code changes.
We're still doing plenty of design discussion, so I've only looked through this diff quickly.
First: great job updating the comments; I like it.
Second: adding a common wrapper to the state seems reasonable. I don wonder if it might be clearer/simpler to instead add a common field to the three existing state types we already have (Known
, Unknown
, and ()
). (Maybe I'm misremembering, but I think it's only ever those three.) That might keep the types a bit tidier.
Third: I'm having trouble shaking the thought that we maybe don't need to retain the rollback history when we drain the pipe and go through the find intersection path. Do you have any clarity there as to why we definitely would need to?
Hmm, maybe my discomfort is actually about the drainThePipe
part; maybe we need to be using those messages to update the rollback history (and possibly punish) instead of simply discarding them?
I think this PR is no longer relevant after pipelining work? @amesgen @nfrisby And perhaps even #423 can be closed too?
Yeah, I think the Google doc you wrote will be the more useful resource when we eventually swing our attention back here. Thanks!