Page MenuHomePhabricator

[avalanche] Add conflicting proofs to the vote
Needs RevisionPublic

Authored by Fabien on Thu, Sep 2, 13:45.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Summary

This puts the conflicting proofs to votes on the avalanche network. It's currently limited to polling for proofs only, and hidden behind a flag to avoid bloating the network until the whole proof voting feature is complete.

Depends on D10026.

Ref T1634.

Test Plan
./test/functional/test_runner.py abc_p2p_avalanche_proof_voting

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_add_conflicting_proofs_to_vote
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 16527
Build 32916: Build Diffbuild-clang · build-debug · build-diff · build-clang-tidy · lint-circular-dependencies · build-without-wallet
Build 32915: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Thu, Sep 2, 13:45
deadalnix requested changes to this revision.Thu, Sep 2, 14:44
deadalnix added a subscriber: deadalnix.

There are obvious problems with this. For instance, one can completely disable avalanche by flooding the system with proofs. While it is not expected that all problems are solved right out of the bat, this is somewhat surprising that none of the limitations have been thought through and do not inspire confidence that this is moving in the right direction at all. Documenting known problems in the code, in addition of having the benefice that problems are documented in the code, is the only way a reviewer can asses if the person writing the code is actually moving somewhere sensible with it.

src/avalanche/peermanager.cpp
260

This is computed but not used anywhere.

267

This seems wasteful to compute all of this when none of it matters when accepted is false.

271

El famoso

if (condition) return true;
retrun false;

What about that instead?

return condition;
src/avalanche/peermanager.h
189

This is a change in the general locking model of the thread manager. Why?

src/avalanche/processor.cpp
486–499

This whole mess this to be a pretty string signal to me that this list belong in the Processor, just like blocks, not in the PeerManager.

This revision now requires changes to proceed.Thu, Sep 2, 14:44
Mengerian added a task: Restricted Maniphest Task.Tue, Sep 14, 16:49
Mengerian removed a task: Restricted Maniphest Task.