Page MenuHomePhabricator

[avalanche] Do not poll avalanche invalidated blocks more than once
ClosedPublic

Authored by Fabien on Mar 1 2023, 00:21.

Details

Summary

Although rare it is possible to trigger polling of a block after avalanche has
concluded that it is invalidated. This is already not the case for finalized
blocks so make the behavior the same for invalidated blocks.

Test Plan
ninja check check-functional

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

sdulfari requested review of this revision.Mar 1 2023, 00:21
Fabien requested changes to this revision.Mar 1 2023, 09:20
Fabien added a subscriber: Fabien.

There is already a similar mechanism for the proof, and we will also need it for the txs, so we should try to factor the feature or at least use the same mechanism so it can be efficiently refactored later.

src/avalanche/processor.cpp
604 ↗(On Diff #38148)

Don't use a vector and you don't need that either. This is time consuming for the sake of avoiding the vector from growing unbound.

1000 ↗(On Diff #38148)

dito it should be random access

src/avalanche/processor.h
305 ↗(On Diff #38148)

Don't use a vector when all you want is to check if the block belongs to the container

This revision now requires changes to proceed.Mar 1 2023, 09:20

Use a bloom filter instead

D13219 is a draft to give an idea what the followup can look like to generalize this. Still needs updates to tests and renaming pending acceptance of this diff.

Fabien requested changes to this revision.Mar 2 2023, 09:04
Fabien added inline comments.
src/avalanche/processor.cpp
583 ↗(On Diff #38216)
src/avalanche/processor.h
307 ↗(On Diff #38216)

The chosen values for the filter make the structure ~90kB. This is OK but suboptimal: we don't need a capacity of 10k blocks, but we need a low false positive rate. I suggest to divide the capacity and the false positive rate by x100 (so the structure size remains the same).
Having only 100 blocks here is no big deal: if they are removed over time they will be considered again for polling, but since they will be deeply buried down the chain it doesn't matter.

This revision now requires changes to proceed.Mar 2 2023, 09:04
Fabien edited reviewers, added: sdulfari; removed: Fabien.

Update the filter parameters and add a comment

This revision is now accepted and ready to land.Mar 2 2023, 22:07