Differential D12014

[avalanche] Create a type alias for proof id sets
ClosedPublic
Actions

Authored by sdulfari on Sep 20 2022, 20:56.

Tags

None

Subscribers

Details

Reviewers

Group Reviewers

Restricted Project

Commits

rABC2c4dd9196603: [avalanche] Create a type alias for proof id sets

Summary

Passing around sets of proof ids is becoming more common with changes like D12013. Providing a type alias
for this just makes sense.

Test Plan

ninja check

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

sdulfari created this revision.Sep 20 2022, 20:56

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 20 2022, 20:56

sdulfari requested review of this revision.Sep 20 2022, 20:56

Harbormaster completed remote builds in B20090: Diff 35056.Sep 20 2022, 21:11

Fabien requested changes to this revision.Sep 21 2022, 07:49

Fabien added a subscriber: Fabien.

Fabien added inline comments.

src/avalanche/proofpool.cpp
9 ↗	(On Diff #35056)	IMO this indicates that ProofIdSet belongs to proofpool.h and not ProofId.h

This revision now requires changes to proceed.Sep 21 2022, 07:49

sdulfari requested review of this revision.Sep 21 2022, 16:13

sdulfari added inline comments.

src/avalanche/proofpool.cpp
9 ↗	(On Diff #35056)	If you want a ProofIdSet for some other purpose than using a pool, then why would you pull in proofpool.h? ProofPool is currently the primary usage but this is not guaranteed so I don't see a compelling reason to put it there.

Fabien added inline comments.Sep 21 2022, 18:43

src/avalanche/proofpool.cpp
9 ↗	(On Diff #35056)	For 2 reasons: There is currently no other use case in the current state of this diff, and I doubt there are many more waiting as the only module responsible of dealing with proofs is the peer manager The proofid module should not declare a type of a container of itself, when this is obvious that it will never be used in this compilation unit but only at the call sites.

Move ProofIdSet to to proofpool.h

Harbormaster completed remote builds in B20131: Diff 35115.Sep 22 2022, 17:11

Fabien accepted this revision.Sep 22 2022, 17:15

This revision is now accepted and ready to land.Sep 22 2022, 17:15

Closed by commit rABC2c4dd9196603: [avalanche] Create a type alias for proof id sets (authored by sdulfari). · Explain WhySep 22 2022, 17:18

This revision was automatically updated to reflect the committed changes.

sdulfari added a commit: rABC2c4dd9196603: [avalanche] Create a type alias for proof id sets.

Revision Contents
Changeset List

Path

Size

src/

avalanche/

3 lines

5 lines

7 lines

test/

proofpool_tests.cpp

2 lines

Diff 35117

src/avalanche/peermanager.h

Loading...

src/avalanche/proofpool.h

Loading...

src/avalanche/proofpool.cpp

Loading...

src/avalanche/test/proofpool_tests.cpp

Loading...