Page MenuHomePhabricator

[avalanche] Remember rejected proofs
Needs RevisionPublic

Authored by PiRK on Thu, Apr 15, 13:43.


Group Reviewers
Restricted Project

Add a rolling bloom filter to remember rejected proofs and avoid verifying them multiple times.

Add an accessor method to test this filter, and add the corresponding unit test.

This is a piece of D9370 with improvements.

Test Plan

ninja && ninja check

Diff Detail

Event Timeline

PiRK requested review of this revision.Thu, Apr 15, 13:44

rebase onto master (this diff has no dependency)

Fabien requested changes to this revision.Mon, Apr 19, 09:31
Fabien added a subscriber: Fabien.

What makes them recent and why are they different from other rejected proofs ?

This revision now requires changes to proceed.Mon, Apr 19, 09:31

rename recentProofRejects -> rejectedProofs.
I used the recent word to mean that the filter has a limited size and will drop the oldest entries when full. But that is an implementation detail. A proof that enters this pool will be bad forever, there is no reset of the filter done on tip updates, like there is for transactions.

Rename the test method PeerManager::addRecentReject -> PeerManager::rejectProof

Also add a comment to explain the choice of parameters for the bloom filter (same params as for transactions).

rephrase the misleading comment.

deadalnix requested changes to this revision.Tue, Apr 20, 00:01
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.

Why is this checking for this specific state?


I'm not sure this makes sense. This check is probabilistic, it it shouldn't be there, but somewhere between the network on here.

this definitively raises the question: should this filter be in the PeerManager at all?


How does CRollingBloomFilter ends up being declared here?


This is the section you are in, this comment is redundant.

This revision now requires changes to proceed.Tue, Apr 20, 00:01
PiRK retitled this revision from [avalanche] Remember recently rejected proofs to [avalanche] Remember rejected proofs.Tue, Apr 20, 09:57

Because the orphan proofs will go into another pool.


Having the filter in the PeerManager is an easy way to fit it in the existing codebase and avoid having the network layer deal with proof verifications. The PeerManager is where proofs are currently verified and stored (if they are good).

I thought about putting it elsewhere, but it requires some clumsy forwarding of ProofValidationState, if we want to avoid checking a proof twice (this is a previous attempt at doing that :