Page MenuHomePhabricator

[avalanche] Introduce the ProofPool class
AbandonedPublic

Authored by Fabien on Nov 10 2021, 23:30.

Details

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

This class will be responsible for storing the proofs associated with each utxo. This is designed to work with any number of proof per utxo but the default is 2: the valid proof and the best replacement candidate. They are sorted using the conflicting proof comparator so the preferred proof appears first, and the proof are moved/evicted as others are attached.

This will be used in the peermanager to handle the conflicting proofs, and will replace the orphan pool entirely.

Ref T1854.

Depends on D10439.

Test Plan
ninja check-avalanche-utxostore_tests

Diff Detail

Event Timeline

Fabien requested review of this revision.Nov 10 2021, 23:30

Remove an unused (for now) template accessor.

deadalnix requested changes to this revision.Nov 17 2021, 17:43
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/avalanche/utxostore.h
24 ↗(On Diff #30846)

This is a pretty bad name, as this doesn't store UTXOs.

This revision now requires changes to proceed.Nov 17 2021, 17:43

Rename UtxoStore => ProofPool
At the end of the day, this is intended to store all the proofs we want to remind, including the conflicting proofs and orphan, so this really is our proof pool.

Fabien retitled this revision from [avalanche] Introduce the Utxo Store to [avalanche] Introduce the ProofPool class.Nov 18 2021, 12:14
Fabien planned changes to this revision.Nov 18 2021, 12:22

More renaming for consistency

Put addProof and removeProof next to the other

This comment was removed by Fabien.
deadalnix requested changes to this revision.Nov 24 2021, 01:38
deadalnix added inline comments.
src/avalanche/proofpool.cpp
16

Using proof and proofs doesn't help comprehend what this does.

18

I don't think you need emplace here. Just insert.

37

This whole thing is an order of magnitude more complex than it needs to.

Just use an array, and do something akin to

for (auto &p : proofs) {

if (proof.isbeterthan(p)) {
    swap(p, proof);
}

}

if p(roof != nullptr) removeProof(proof);

But the more I read this, the more I question the whole approach. The complexity is clearly not in proprotions to what this is doing - (effectivel a multimap with some prunning).

src/avalanche/proofpool.h
28

This isn't a map of utxo, this is a map from utxo to proofs. So it is a map of proofs, but considering this is in the proof pool, this isn't super informative.

35

Do we really want to keep more than 1? And if the size is 2, do w really want to crate all these sets? Just store two pointers in there, no?

38

shifted?

This revision now requires changes to proceed.Nov 24 2021, 01:38