[avalanche] Use sane default for the minimum number of nodes that sent compact proofs
ClosedPublic
Actions

Authored by Fabien on Sep 6 2022, 12:47.

Details

Reviewers

PiRK
sdulfari

Group Reviewers

Restricted Project

Commits

Summary

This param is used to determine when we are confident we have the right set of proofs in our quorum.
This is similar to D11917 and D11918.

Depends on D11918.

Test Plan

ninja all check-extended

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Fabien requested review of this revision.Sep 6 2022, 12:47

PiRK accepted this revision.Sep 6 2022, 14:57

This revision is now accepted and ready to land.Sep 6 2022, 14:57

sdulfari requested changes to this revision.Sep 6 2022, 16:23

sdulfari added a subscriber: sdulfari.

sdulfari added inline comments.

src/avalanche/test/processor_tests.cpp
112 ↗	(On Diff #34822)	Do any of the tests rely on this being 0? It should be safer to make this a number greater than 0 if all of the tests pass with it. This question is relevant to all of the tests, not just this line.

This revision now requires changes to proceed.Sep 6 2022, 16:23

Fabien requested review of this revision.Sep 6 2022, 19:01

Fabien added inline comments.

src/avalanche/test/processor_tests.cpp
112 ↗	(On Diff #34822)	yes, all the tests that use polling expect the quorum to be established once the 8 nodes are connected. Since we don't send network messages in the unit test, we don't want to set this. For functional tests it depends on the test. Most tests are not testing quorum establishment so there is no need to have this value set to anything apart from slowing down the tests, and some are overriding this to test the change in behavior. It can make sense to try to convert all or most of the tests to accommodate the default rather then overriding the param, but this can be done in separate diffs.