<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process. A summary of reviews will appear here.
I think this would be a good thing to do if our current targets would be fuzzing the logic that these params are influencing (afaict this is not the case right now) but without that it unfortunately just invalidates our input corpora.
prefer_evict is only used in CConnman::AttemptToEvictConnection which is currently not being fuzzed.recv_flood_size is used to set the value of CNode::fPauseRecv which is used to determine if we should stop reading from the socket in CConnman::GenerateWaitSockets but this is also not being fuzzed.Feel free to double check: https://marcofalke.github.io/b-c-cov/fuzz.coverage/src/net.cpp.gcov.html
So I would prefer not to do this, unless we are also adding targets that fuzz the desired logic.
So I would prefer not to do this, unless we are also adding targets that fuzz the desired logic.
I noticed they were missing while working on net fuzzing improvement, so would be better to add recv_flood_size and prefer_evict just when adding coverage for functions that use them? I thought spliting could be better, but I can open a PR adding the targets or update this one.
so would be better to add them just when adding coverage for functions that use them?
Yes I think that would be better, otherwise there is no need to add them.
Yes I think that would be better, otherwise there is no need to add them.
Ok, cool. I will change this PR to draft.
I think it can just be closed for now? Can be re-opened when there are relevant changes.
@dergoegge I just removed perfer_evict from ConsumeNode, kept recv_flood_size and added target for MarkReceivedMsgsForProcessing.
MarkReceivedMsgsForProcessing uses m_recv_flood_size and plays with vRecvMsg which may be filled by ReceiveMsgBytes.
If it makes sense for a PR scope, I can update the description and we can go ahead this way here. Otherwise, it's ok for me to close it for now and add recv_flood_size and prefer_evict alongside with more appropriate net targets.
MarkReceivedMsgsForProcessing uses m_recv_flood_size and plays with vRecvMsg which may be filled by ReceiveMsgBytes.
This still doesn't do anything besides setting fPauseRecv. You'd also want to fuzz the logic that uses fPauseRecv (i.e. CConnman::SocketHandler).
it's ok for me to close it for now and add recv_flood_size and prefer_evict alongside with more appropriate net targets.
That would be better. Fwiw we already have a target for fuzzing the eviction logic in isolation called node_eviction, so prefer_evict is probably less interesting.