Page MenuHomePhabricator

[avalanche] Introduce the ProofPool multi index structure
ClosedPublic

Authored by Fabien on Nov 24 2021, 16:14.

Details

Reviewers
deadalnix
Group Reviewers
Restricted Project
Maniphest Tasks
Restricted Maniphest Task
Commits
rABC98c4acad8c24: [avalanche] Introduce the ProofPool multi index structure
Summary

This diff adds a generic multi index container for storing the proofs, with an utxo based mapping. The proofs can then be retrieve by utxos but also by proofid as needed. This container will be used to replace the orphan pool in a follow-up, hence the name validProofPool by opposition to the coming orphanProofPool. There is no change in behavior.

Ref T1854.

Depends on D10520.

Test Plan
ninja all check-all

Diff Detail

Repository
rABC Bitcoin ABC
Branch
avalanche_peermanager_pool_multi_index
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 17376
Build 34580: Build Diffbuild-debug · build-without-wallet · build-diff · build-clang · lint-circular-dependencies · build-clang-tidy
Build 34579: arc lint + arc unit

Event Timeline

Fabien requested review of this revision.Nov 24 2021, 16:14
deadalnix requested changes to this revision.Nov 25 2021, 02:23
deadalnix added a subscriber: deadalnix.
deadalnix added inline comments.
src/avalanche/peermanager.h
94 ↗(On Diff #30986)

The outpoint is already in the proof, no need to copy it.

This revision now requires changes to proceed.Nov 25 2021, 02:23
Fabien requested review of this revision.Nov 25 2021, 09:50
Fabien added inline comments.
src/avalanche/peermanager.h
94 ↗(On Diff #30986)

I don't know how to avoid the copy here, if possible at all.
I want a container that use the outpoint as a key, and a proof has outpoints as a 1..n relation so it's not possible to extract an outpoint from a proof so that it is suitable as a key.
Note that it's not making things worst than the actual map which also copies the outpoint key.

Fabien planned changes to this revision.Nov 25 2021, 14:20

So apparently this is possible by using a wrapper function to return the reference, and instruct boost to use it as the source for the key.
Boost store a "pointer to member returning the key" internally, so that member cannot be a reference (pointer to ref), but can be a pointer to a member function that returns a reference.

Store a const ref to avoid the copy, and use a wrapper function as a key extractor to make boost happy

src/avalanche/peermanager.h
95 ↗(On Diff #31025)

It would be preferable to store an index or a pointer here instead of a reference if this is at all possible, which i think it is due to the accessor.

Use the index of the utxo as a key

deadalnix requested changes to this revision.Nov 25 2021, 18:41
deadalnix added inline comments.
src/avalanche/peermanager.cpp
271 ↗(On Diff #31047)

This clearly doesn't make a lot of sense. The utxo is fetched from the proof, and then there is a very complex process in the other hand to reverse that operation. That is useless work (and possibly a DoS vector is this work is not O(1) ).

src/avalanche/peermanager.h
112 ↗(On Diff #31047)

What is the complexity of this whole contraption?

This revision now requires changes to proceed.Nov 25 2021, 18:41

Remove the double loop madness. This makes the insert API a bit more tedious but much more efficient.

deadalnix added inline comments.
src/avalanche/peermanager.cpp
285

Not that if we insert a duplicate, we will delete the original. This is probably a problem.

src/avalanche/peermanager.h
102

I would have put them in the other order, but it's not a blocker.

This revision is now accepted and ready to land.Nov 25 2021, 22:06