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_peermaneger_pool_multi_index

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 17334
Build 34496: Build Diff	lint-circular-dependencies · build-without-wallet · build-diff · build-clang-tidy · build-debug · build-clang
Build 34495: arc lint + arc unit

Event Timeline

Fabien created this revision.Nov 24 2021, 16:14

Herald added a reviewer: Restricted Project. · View Herald TranscriptNov 24 2021, 16:14

Fabien requested review of this revision.Nov 24 2021, 16:14

Fabien added a child revision: D10522: [avalanche] Add wrappers to add and remove a proof from a proof pool.Nov 24 2021, 16:24

Harbormaster completed remote builds in B17334: Diff 30986.Nov 24 2021, 16:29

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	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	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.

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

Harbormaster completed remote builds in B17366: Diff 31025.Nov 25 2021, 14:50

deadalnix added inline comments.Nov 25 2021, 15:53

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

Harbormaster completed remote builds in B17374: Diff 31047.Nov 25 2021, 17:12

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.

Harbormaster completed remote builds in B17376: Diff 31050.Nov 25 2021, 20:22

deadalnix accepted this revision.Nov 25 2021, 22:06

deadalnix added inline comments.

src/avalanche/peermanager.cpp
285 ↗	(On Diff #31050)	Not that if we insert a duplicate, we will delete the original. This is probably a problem.
src/avalanche/peermanager.h
102 ↗	(On Diff #31050)	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

Closed by commit rABC98c4acad8c24: [avalanche] Introduce the ProofPool multi index structure (authored by Fabien). · Explain WhyNov 26 2021, 15:24

This revision was automatically updated to reflect the committed changes.

Fabien added a commit: rABC98c4acad8c24: [avalanche] Introduce the ProofPool multi index structure.

[avalanche] Introduce the ProofPool multi index structure
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 30986

src/avalanche/peermanager.h

src/avalanche/peermanager.cpp

[avalanche] Introduce the ProofPool multi index structureClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 30986

src/avalanche/peermanager.h

src/avalanche/peermanager.cpp

[avalanche] Introduce the ProofPool multi index structure
ClosedPublic
Actions

Revision Contents
Changeset List