Page MenuHomePhabricator

[avalanche] Orphan the conflicting proofs rather than ignoring them
ClosedPublic

Authored by Fabien on Oct 21 2021, 15:48.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC515d4fa81950: [avalanche] Orphan the conflicting proofs rather than ignoring them
Summary

A new proof with conflicting utxos is currently discarded entirely. This might be an otherwise valid proof, and it might become valid in the future if the peers containing the conflicting utxos are invalidated.

This diff adds such conflicting proofs to the orphan pool. As a consequence:

  • It makes it easy for the processor to determine if a proof has been accepted (in the proof pool), rejected (in the orphan pool), or invalidated (in neither of these).
  • It makes it possible to retrieve a conflicting proof very easily from its proof id using the a simple PeerManager::getProof() call, that can be used to fix the actual O(n²) that occurs on the proof vote. This will be done in a follow-up.

Ref T1854.

Depends on D10383.

Test Plan
ninja all check-all

Diff Detail

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

Event Timeline

Fabien requested review of this revision.Oct 21 2021, 15:48
deadalnix requested changes to this revision.Oct 21 2021, 20:16
deadalnix added a subscriber: deadalnix.

I think this is probably worth splitting in two, that would make review significantly easier.

src/avalanche/peermanager.cpp
201 ↗(On Diff #30628)

Can't this be a patch on its own? This seems generally useful, regardless of the rest of this diff, and would be easier to review.

This revision now requires changes to proceed.Oct 21 2021, 20:16
deadalnix added inline comments.
src/avalanche/peermanager.cpp
298 ↗(On Diff #30646)

This makes me think the PeerManager could be a CCoinView.

This revision is now accepted and ready to land.Oct 26 2021, 21:07
src/avalanche/peermanager.cpp
298 ↗(On Diff #30646)

It's an interesting idea