I don't think this design with a config is a plus. A ton of code just got significantly more convoluted.

I don't think we need a different message from avahello. You are creating a rube goldberg machine here.

Is there a reason not to pass the chain manager here, instead of passing down various values extracted from it?

You are creating special cases where none is necessary. Just handle the zero proof case like any other case is handled.

The rationale makes no sense. It is possible to balloon the pool in 30mins, so this is insufficient. So you need another criteria to evict from the pool, but it you have that other criteria, it is more likely than not that this one becomes useless.

No this does not help, because you'll have to get rid of proofs for which there is no node regardless. In fact, not only do you need to, but this becomes significantly more important to once you limit how many proofs you can accept.

Maybe we should just evict proof for which we have no peer after some time instead. We kind of have to do it anyways, and this makes this patch moot as far as I can tell.

I'm not app at all with ho this is going.

I don't understand what's the purpose of this. At no point we should expect a node to send the whole set of proof without being requested. I don't think solves any problem.

I had a quick look. Didn't review everything in there as it is quite big, but the little I review is quite worrying. This cealry doesn't match the kind of quality we want to put out there.

These number do not make sense to me.

Why do we want to compute the id of a proof that isn't initialized, and then recompute the correct one later? That doesn't make sense.

These thresholds seems REALLY low. How did you pick them?

There is no need to break things before fixing them.

So one successful round and w can stale forever? Seriously, I don't even know what to say, this is obviously not good.

This needs to happen for GetAvalancheVoteForBlock too.

I see, it is about to use a member. LGTM.

It doesn't looks like it is using any state of the object, so why should this be a member method of that object? That doesn't make sense.

This count cannot be simply retrieved using size() because it is a multimap like structure.

I don't understand what problem this is solving. First, this is using low free gizmo that seem completely unecessary - aren't VoteReccord owned by locks to begin with?