Page MenuHomePhabricator

[avalanche] Don't consider our quorum valid if we don't have enough nodes connected
ClosedPublic

Authored by Fabien on Jul 19 2022, 13:36.

Details

Summary

The quorum is considered valid if we got connected to enough nodes and
got some specific messages from them, but if the count drops (due to
node disconnect) we might fall below the minimum number required for
getting a convergent vote, and keep polling. This diff makes the quorum
invalid if the node has less than 8 avalanche nodes connected.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_processor_no_poll_8_nodes
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 19669
Build 39057: Build Difflint-circular-dependencies · build-without-wallet · build-diff · build-debug · build-clang-tidy · build-clang
Build 39056: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Jul 19 2022, 13:36
sdulfari added inline comments.
src/avalanche/test/processor_tests.cpp
1355 ↗(On Diff #34413)

No more latching

1375 ↗(On Diff #34413)

This isn't true anymore, right?

src/avalanche/test/processor_tests.cpp
1355 ↗(On Diff #34413)

This parameter is still latched, if you were connected to 80% of the stakes and get a new proof that you don't know the node yet you don't want to have a bad quorum and get flip-floping

sdulfari requested changes to this revision.Jul 20 2022, 18:36

The code changes are fine, but the comments (or lack of) confuse the intent.

src/avalanche/processor.cpp
678 ↗(On Diff #34413)

This needs a comment why we short circuit before checking the latch.

679 ↗(On Diff #34413)

This comment conflates this function call with polling. While mostly true, it's also used in periodic networking.

This revision now requires changes to proceed.Jul 20 2022, 18:36

Rebase and add the requested comment

This revision is now accepted and ready to land.Jul 23 2022, 05:24